Closed Bug 907708 Opened 8 years ago Closed 8 years ago

[HD][Settings] App Permission Styling and Layout

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect, P1)

All
Other
defect

Tracking

(blocking-b2g:hd+, b2g-v1.1hd fixed, b2g-v1.2 fixed)

VERIFIED FIXED
blocking-b2g hd+
Tracking Status
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed

People

(Reporter: epang, Assigned: pivanov)

References

Details

(Keywords: verifyme, Whiteboard: helix vsd, HD)

Attachments

(8 files)

Pavel, can you update the app permission screens?  They seem to be really outdated.

1. The drop down should be full length and under the text.

2. Update the styling of the uninstall button.

Thanks!
Whiteboard: helix vsd → helix vsd, HD
HD+ layout
blocking-b2g: hd? → hd+
Attached file patch for Gaia/master
Attachment #793590 - Flags: review?(sjochimek)
(In reply to Pavel Ivanov [:ivanovpavel] from comment #2)
> Created attachment 793590 [details]
> patch for Gaia/master

Pavel, can you attach screens? Thanks!
Comment on attachment 793590 [details]
patch for Gaia/master

Nice one, but too much javascript to not asking one of the Setting's owners/peers.
Attachment #793590 - Flags: review?(sjochimek) → review?(ehung)
Attached image After patch screenshot
Attachment #794111 - Flags: feedback?(epang)
Comment on attachment 794111 [details]
After patch screenshot

Much better!  And will be perfect when the dropdown has 15px margins from the other bug. Thanks!
Attachment #794111 - Flags: feedback?(epang) → feedback+
Comment on attachment 793590 [details]
patch for Gaia/master

r=me, everything looks good except the text displayed on the button. I think we should display option text instead of selected value. In this case, when I select "Grant" but the button display "allow", it's weird to me. 
We can fix this by updating strings to let displayed text and option value are identical, or using select.options[select.selectedIndex].textContent to get displayed option text.
Attachment #793590 - Flags: review?(ehung) → review+
Pavel, can we land this now?
Flags: needinfo?(pivanov)
Blocks: 892558
Hey Eric,
are you OK with Evelyn's suggestion?
Flags: needinfo?(pivanov)
(In reply to Pavel Ivanov [:ivanovpavel] from comment #9)
> Hey Eric,
> are you OK with Evelyn's suggestion?

Hey Pavel,

Yes, i wasn't aware the selected text was different then the list for selection.  There should be three values in the list:
1. Ask
2. Deny
3. Grant

When selection the button should display the same text.  Thanks! :)
Flags: needinfo?(pivanov)
Hey Evelyn,
I change this line https://github.com/mozilla-b2g/gaia/pull/11669/files#L0R317
Is it OK?
Flags: needinfo?(pivanov) → needinfo?(ehung)
yes, it's good. Thanks.
Flags: needinfo?(ehung)
Landed on master:
https://github.com/mozilla-b2g/gaia/commit/cc27e4c43b7675f43a6078a9219e5d08e70fff26

Landed on v1.1.0hd: 
327aadd086f2c377c9e6b6010656c7731d532d6d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Landed on v1.1.0hd: 
327aadd086f2c377c9e6b6010656c7731d532d6d

===> Page not found. Did I misunderstand anything?

-------------------------------------------------------
Hi, Pavel,

Could I have your help?
I cannot find the patch on V1.1.0 hd branch.
Did you uplift it?
Thank you.
Flags: needinfo?(pivanov)
Hey William,

commit 327aadd086f2c377c9e6b6010656c7731d532d6d
Author: Pavel Ivanov <pivanov@mozilla.com>
Date:   Tue Sep 10 08:48:42 2013 -0700

    Merge pull request #11669 from pivanov/bug-907708

    Bug 907708 - [HD][Settings] App Permission Styling and Layout
Flags: needinfo?(pivanov)
Hi, Pavel,

I still can reproduce this problem on V1.1.0 hd.
Have we merged this patch to Gaia-V1.1.0hd branch?
I cannot see any commit relate to this problem on V1.1.0 hd branch.
Attaching the screenshot.
Thank you.

* Test build:
- Gaia:     8059864bd278645ab5d2598b5ed595154136c253
- Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/710e8b760ccb
- BuildID   20130915042201
- Version   18.0
Attached image Permission list
Attached image Github Commit
Still can reproduce this bug on V1.1.0 hd

* Test Build:
 - Gaia:     7859b08b221463e0c5559034ab151747af6eb038
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/8c326236a729
 - BuildID   20130924042203
 - Version   18.0

Attaching the screenshot.
Hi, Pavel,

I still cannot see the latest change on V1.1.0 hd branch.
Any thought?
Flags: needinfo?(pivanov)
The code still not land yet.

[2013/10/21 Helix Testing]
Gaia:     c829a2042594b6c3a4899ee27979799a0f301534
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/f7c657f6d019
BuildID   20131015042201
Version   18.0
Flags: needinfo?(ehung)
accidentally touched the flag. put it back to avoid misunderstanding
I can't find the commit number in v1.1.0 either.
uplift again: https://github.com/mozilla-b2g/gaia/commit/7e51a774b3a48fb93870a25c0b9291e1d04260e3
Flags: needinfo?(ehung)
Will come back to verify the bug later.
Keywords: verifyme
QA Contact: atsai
Not yet uplifted to V1.1.0hd.
Still can reproduce this bug.

* Test Build:
 - Gaia:     6c21c9fdfc9f75ad02fc0859ba68f700d8737666
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/082ba0ff768b
 - BuildID   20131025042341
 - Version   18.0
--------- Quote the Eric's comment --------- < Comment 10 >
Yes, i wasn't aware the selected text was different then the list for selection.  There should be three values in the list:
1. Ask
2. Deny
3. Grant

When selection the button should display the same text.  Thanks! :)
--------------------------------------------

The selected text was different then the list. Could I have your help to align these texts?
Attaching the screenshot. Many thanks!

Reopen it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached image App_Permission.png
Depends on: 929840
Flags: needinfo?(pivanov)
Who can uplift this here?
Keywords: checkin-needed
I think Kevin can do this ... Kevin?
Flags: needinfo?(kgrandon)
Since the fix we're talking about is in bug 929840, I think we should track uplifting in there. I went and nominated it.

After nominating it I noticed a slew of conflicts when trying to merge. We'd either have to uplift a lot of recent settings patches, or completely redo it. I'll try to update the dependency chain a bit to reflect this.
Flags: needinfo?(kgrandon)
Assuming that this is fixed on v1.2 based on when it hit master. Please change status-b2g-v1.2 (and uplift the fix!) if that's not the case. Also note that in situations where master is fixed and the branch isn't, the status flags should be used to track progress rather than re-opening the bug.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Removing checkin-needed per comment 31 since it appears nothing is being checked in here.
Keywords: checkin-needed
Please uplift this patch to V1.1.0 hd.
I still can reproduce bug I mentioned (Comment 27) on V1.1.0hd build.
The selected text was different then the list.

* Test Build:
 - Gaia:     355be5eebe5bb80f01d57076742c6e3beb8f8d0d
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/1d8b95908208
 - BuildID   20131107042200
 - Version   18.0
Depends on: 938262
Bug 929840 has been HD+'d
After discussing with Evelyn and Arthur, bug 929840 seems to have a lot of conflicts incurring. Let's take this one for v1.1HD along with bug 938262 instead.

A check-in needed here for v1.1HD.
Keywords: checkin-needed
John, can you please assist? :)
Flags: needinfo?(jhford)
I'm not entirely sure what needs to happen here based on the comments in this bug.  Wayne, I don't manage the v1.1.0hd branch, could you please uplift to it?
Flags: needinfo?(jhford) → needinfo?(wchang)
Thanks John, Tim helps out with v1.1HD uplifts I'll speak to him in person today on this one.
Flags: needinfo?(wchang)
The code have already reached v1.1.0hd in comment 24.

I've just uplifted bug 938262 to v1.1.0hd so we could try to reproduce this bug now.
Hi, Tim,

Thanks so much!
Verified it. Attach the screenshot.

* Test Build:
 - Gaia:     efbbbd0b4cada3e1dbdc0afbbb49bcdc79adbfb2
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/59d274520709
 - BuildID   20131205042203
 - Version   18.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.