[HD][Settings] App Permission Styling and Layout

VERIFIED FIXED

Status

Firefox OS
Gaia::Settings
P1
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: epang, Assigned: ivanovpavel)

Tracking

({verifyme})

unspecified
All
Other
verifyme
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: helix vsd, HD)

Attachments

(8 attachments)

(Reporter)

Description

5 years ago
Created attachment 793470 [details]
Systems App Permission.png

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!
(Reporter)

Updated

5 years ago
Whiteboard: helix vsd → helix vsd, HD
HD+ layout
blocking-b2g: hd? → hd+
Created attachment 793590 [details]
patch for Gaia/master
Attachment #793590 - Flags: review?(sjochimek)
(Reporter)

Comment 3

5 years ago
(In reply to Pavel Ivanov [:ivanovpavel] from comment #2)
> Created attachment 793590 [details]
> patch for Gaia/master

Pavel, can you attach screens? Thanks!

Comment 4

5 years ago
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)
Created attachment 794111 [details]
After patch screenshot
Attachment #794111 - Flags: feedback?(epang)
(Reporter)

Comment 6

5 years ago
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 7

5 years ago
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+
(Reporter)

Comment 8

4 years ago
Pavel, can we land this now?
Flags: needinfo?(pivanov)
(Reporter)

Updated

4 years ago
Blocks: 892558
Hey Eric,
are you OK with Evelyn's suggestion?
Flags: needinfo?(pivanov)
(Reporter)

Comment 10

4 years ago
(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)

Comment 12

4 years ago
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
Last Resolved: 4 years ago
status-b2g-v1.1hd: ? → fixed
Resolution: --- → FIXED

Comment 14

4 years ago
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.

Updated

4 years ago
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)

Comment 16

4 years ago
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

Comment 17

4 years ago
Created attachment 805300 [details]
Permission list

Comment 18

4 years ago
Created attachment 805302 [details]
Github Commit

Comment 19

4 years ago
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.

Comment 20

4 years ago
Created attachment 811166 [details]
App Permission Styling and Layout

Comment 21

4 years ago
Hi, Pavel,

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

Comment 22

4 years ago
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)

Updated

4 years ago
status-b2g-v1.1hd: fixed → affected

Comment 23

4 years ago
accidentally touched the flag. put it back to avoid misunderstanding
status-b2g-v1.1hd: affected → fixed

Comment 24

4 years ago
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)

Comment 25

4 years ago
Will come back to verify the bug later.
Keywords: verifyme
QA Contact: atsai

Comment 26

4 years ago
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

Comment 27

4 years ago
--------- 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 → ---

Comment 28

4 years ago
Created attachment 824446 [details]
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
Last Resolved: 4 years ago4 years ago
status-b2g-v1.1hd: fixed → affected
status-b2g-v1.2: --- → fixed
Resolution: --- → FIXED
Removing checkin-needed per comment 31 since it appears nothing is being checked in here.
Keywords: checkin-needed

Comment 34

4 years ago
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

Updated

4 years ago
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.
status-b2g-v1.1hd: affected → fixed
Keywords: checkin-needed

Comment 41

4 years ago
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

Comment 42

4 years ago
Created attachment 8343090 [details]
App_Permission (Fixed).png
You need to log in before you can comment on or make changes to this bug.