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

Status: VERIFIED FIXED

(Firefox :: General, defect, P2)

Description

[Florian Quèze [:florian]]

Attachment: 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.

Comment 1

[:Paolo Amadini]

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.

Comment 2

[Jonathan Kew [:jfkthame]]

Moving this back to Firefox:General.  AFAICS this isn't a core layout bug, it's a Firefox UI implementation issue.

Comment 3

[Jonathan Kingston [:jkt] he/him]

Attachment: Bug 1290174 - Increasing text size of permission prompts and spacing around icons.

Review commit: https://reviewboard.mozilla.org/r/69282/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69282/

Comment 4

[Johann Hofmann [:johannh]]

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.

Comment 5

[Johann Hofmann [:johannh]]

Attachment: WIP Screenshot

Comment 6

[:Paolo Amadini]

(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.

Comment 7

[:Paolo Amadini]

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.

Comment 8

[Johann Hofmann [:johannh]]

(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 9

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8777809 [details]
Bug 1290174 - Increasing text size of permission prompts and spacing around icons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69282/diff/1-2/

Comment 10

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8777809 [details]
Bug 1290174 - Increasing text size of permission prompts and spacing around icons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69282/diff/2-3/

Comment 11

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8777809 [details]
Bug 1290174 - Increasing text size of permission prompts and spacing around icons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69282/diff/3-4/

Comment 12

[Johann Hofmann [:johannh]]

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.

Comment 13

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8777809 [details]
Bug 1290174 - Increasing text size of permission prompts and spacing around icons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69282/diff/4-5/

Comment 14

[Johann Hofmann [:johannh]]

https://hg.mozilla.org/integration/fx-team/rev/8bdde675e429d79fc707ec2327de05d3fae31f65
Bug 1290174 - Increasing text size of permission prompts and spacing around icons. r=johannh

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/8bdde675e429
Increasing text size of permission prompts and spacing around icons. r=johannh

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8bdde675e429

Comment 17

[Paul Silaghi, QA [:pauly]]

This fix caused a misalignment on Windows between Allow/Block and the X button - http://imgur.com/a/TZD6s

Comment 18

[Paul Silaghi, QA [:pauly]]

Verified fixed FX 51.0a1 (2016-08-17) Win 7, Ubuntu 14.04, OS X 10.12