Closed Bug 1290174 Opened 5 years ago Closed 5 years ago

The font size is too small on the permission sections of the control center panel

Categories

(Firefox :: General, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 51
Tracking Status
firefox51 --- verified

People

(Reporter: florian, Assigned: jkt)

References

Details

(Whiteboard: [fxprivacy])

Attachments

(3 files)

Attached image Screenshot and mockup
The text on the lines where we have 'x' icons is small (smaller than on the mockup), and I'm afraid they may be unreadable to some people.

See attached screenshot, with the mockup on the left side and the actual implementation on the right side.
Whiteboard: [privacy] [triage]
The text seems to be the same size as the tab titles, tooltips, and other system messages, so arguably if we have a readability issue here we also have one with the whole default theme.

Other labels in the Control Center have larger text though, so it might be useful to get some consistency.

Using larger text makes it more likely for the permission line to wrap, and maybe this was taken into account when the list of permissions was implemented in the Control Center.

The text size hasn't changed in bug 1203292, removing the dependency to clarify.
No longer blocks: 1203292
Whiteboard: [privacy] [triage] → [fxprivacy][triage]
Assignee: nobody → jkt
Flags: qe-verify+
Priority: -- → P2
Whiteboard: [fxprivacy][triage] → [fxprivacy]
Component: General → Layout: Text
Product: Firefox → Core
Moving this back to Firefox:General.  AFAICS this isn't a core layout bug, it's a Firefox UI implementation issue.
Component: Layout: Text → General
Product: Core → Firefox
Attachment #8777809 - Flags: review?(jhofmann)
Attachment #8777809 - Flags: review?(jhofmann) → review-
Comment on attachment 8777809 [details]
Bug 1290174 - Increasing text size of permission prompts and spacing around icons.

https://reviewboard.mozilla.org/r/69282/#review66398

::: browser/themes/shared/controlcenter/panel.inc.css:381
(Diff revision 1)
>  }
>  
>  .identity-popup-permission-icon {
>    width: 16px;
>    height: 16px;
> +  margin-inline-end: 18px;

Not sure how it looks on your machine, but on my Mac 18px on each seems a bit much, especially for the state labels on the right. (I'll attach a screenshot). After playing around with it 10px and 6px feel right to me.

::: browser/themes/shared/controlcenter/panel.inc.css:385
(Diff revision 1)
>    height: 16px;
> +  margin-inline-end: 18px;
>  }
>  
>  .identity-popup-permission-label {
>    margin-inline-start: 1em;

I think this rule is being overridden and can be removed now.
Attached image WIP Screenshot
(In reply to Johann Hofmann [:johannh] from comment #4)
> Not sure how it looks on your machine, but on my Mac 18px on each seems a
> bit much, especially for the state labels on the right. (I'll attach a
> screenshot). After playing around with it 10px and 6px feel right to me.

As far as I can tell, the permission manes should still be aligned with the "Permissions" header.

This is what the margin of 1em on ".identity-popup-permission-label" was doing, the same 1em start margin can probably be used as the end margin on the icon to achieve the same effect.
For the margin between the button and the text, we can definitely increase that but be aware that the less space we give to the text, the more chances we have that it wraps on multiple lines for some locales.
(In reply to :Paolo Amadini from comment #6)
> (In reply to Johann Hofmann [:johannh] from comment #4)
> > Not sure how it looks on your machine, but on my Mac 18px on each seems a
> > bit much, especially for the state labels on the right. (I'll attach a
> > screenshot). After playing around with it 10px and 6px feel right to me.
> 
> As far as I can tell, the permission manes should still be aligned with the
> "Permissions" header.
> 
> This is what the margin of 1em on ".identity-popup-permission-label" was
> doing, the same 1em start margin can probably be used as the end margin on
> the icon to achieve the same effect.

Yes, good point. 1em might be the way to go for that. Thanks!

We should still decrease the right label distance to the x.

I tried how it looks with wrapping and I think it looks acceptable if it's not happening too often.
Comment on attachment 8777809 [details]
Bug 1290174 - Increasing text size of permission prompts and spacing around icons.

https://reviewboard.mozilla.org/r/69282/#review67176

Looks good now, thanks!

::: browser/themes/shared/controlcenter/panel.inc.css:389
(Diff revision 4)
>  .identity-popup-permission-label {
>    margin-inline-start: 1em;
>  }
>  
>  .identity-popup-permission-state-label {
> +  margin-inline-end: 3px;

Note that the state labels on Windows are a bit too close to the (x) icons for my personal taste. I would suggest adding 1 or 2 px here.
Attachment #8777809 - Flags: review?(jhofmann) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/8bdde675e429d79fc707ec2327de05d3fae31f65
Bug 1290174 - Increasing text size of permission prompts and spacing around icons. r=johannh
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/8bdde675e429
Increasing text size of permission prompts and spacing around icons. r=johannh
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8bdde675e429
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
This fix caused a misalignment on Windows between Allow/Block and the X button - http://imgur.com/a/TZD6s
Depends on: 1296322
Verified fixed FX 51.0a1 (2016-08-17) Win 7, Ubuntu 14.04, OS X 10.12
Status: RESOLVED → VERIFIED
Depends on: 1299101
You need to log in before you can comment on or make changes to this bug.