1374339 - Door hanger menus has visual inconsistencies

Status: RESOLVED WORKSFORME

(Firefox :: Menus, defect, P1)

Description

[Amy Lee [:amylee] UX]

Attachment: Door Hanger Panels Spec.png

There are some visual inconsistencies with door hanger panels in the toolbar. Please see attached spec for details.

Comment 1

[Daniela Arcese [:daniela]]

Attachment: Control panel door hanger

:amylee is this what you were thinking for the font size? The titles in the downloads panel are slightly larger because they are calculated from a variable - should these match?
This font size also brings up some alignment questions: should the 'Permissions' heading be vertically center aligned to the icon or aligned to top? Also, should the extra space at the bottom of the panel be removed?

Comment 2

[Amy Lee [:amylee] UX]

(In reply to darcese from comment #1)
> Created attachment 8883963 [details]
> Control panel door hanger
> 
> :amylee is this what you were thinking for the font size? The titles in the
> downloads panel are slightly larger because they are calculated from a
> variable - should these match?

The font sizes should match the Photon hamburger door-hanger text size.

> This font size also brings up some alignment questions: should the
> 'Permissions' heading be vertically center aligned to the icon or aligned to
> top? Also, should the extra space at the bottom of the panel be removed?

Permissions should be vertically center aligned and yes please remove the extra space at the bottom.

Let me know if you have any more questions :-)

Comment 3

[Daniela Arcese [:daniela]]

Attachment: Bug 1374339 - Fix door hangers visual inconsistencies.

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

Comment 4

[:Paolo Amadini]

Comment on attachment 8887081 [details]
Bug 1374339 - Fix door hangers visual inconsistencies.

Thank you very much for the contribution!

Please file a new bug for each of the issues in the screenshot, even if the patch for each issue turns out to be quite simple. There are Bugzilla components for the Downloads Panel and the Site Identity area.

We discussed the font size issue a few times already, but since Amy brought it up again and it keeps returning, maybe we should align it with the rest. Johann might be a better reviewer for the Site Identity portions, while I can look at Downloads Panel patches.

Bug 1304304 is already on file for centering the return arrow, but the solution in this patch is not enough because we don't want to change the size of the buttons. We'll need to use a feature of PanelMultiView that allows specifying an anchor element when opening the subview.

Comment 5

[Johann Hofmann [:johannh]]

I don't think attachment 8883963 [details] delivers a good user experience. The headlines should be more prominent, especially the site origin. I would like to know in what context or as part of what project these recommendations were created.

Amy, if you intend to do a re-design of the identity popup, can you please provide us with a full spec for that?

I'm happy to review minor changes such as removing the gradient for the button, but major visual changes should have some sort of justification, such as being part of a bigger concept or clearing some pressing UX problem.

Comment 6

[Amy Lee [:amylee] UX]

(In reply to Johann Hofmann [:johannh] from comment #5)
> I don't think attachment 8883963 [details] delivers a good user experience.
> The headlines should be more prominent, especially the site origin. I would
> like to know in what context or as part of what project these
> recommendations were created.
> 
> Amy, if you intend to do a re-design of the identity popup, can you please
> provide us with a full spec for that?
> 
> I'm happy to review minor changes such as removing the gradient for the
> button, but major visual changes should have some sort of justification,
> such as being part of a bigger concept or clearing some pressing UX problem.

Hi Johann,

These recommendations were based on the new Photon style guide. The style guide is used by the UX team to unify our UI components so there is visual consistency across the browser. My recommendation was based on the style guide for text styles for door-hanger titles (http://design.firefox.com/photon/visual/typography.html#styles – Scroll to "Titles" for reference image of the library door-hanger). NI Amin to see if he has an opinion on the text recommendation change.

Comment 7

[Johann Hofmann [:johannh]]

(In reply to Amy Lee [:amylee] UX from comment #6)
> (In reply to Johann Hofmann [:johannh] from comment #5)
> > I don't think attachment 8883963 [details] delivers a good user experience.
> > The headlines should be more prominent, especially the site origin. I would
> > like to know in what context or as part of what project these
> > recommendations were created.
> > 
> > Amy, if you intend to do a re-design of the identity popup, can you please
> > provide us with a full spec for that?
> > 
> > I'm happy to review minor changes such as removing the gradient for the
> > button, but major visual changes should have some sort of justification,
> > such as being part of a bigger concept or clearing some pressing UX problem.
> 
> Hi Johann,
> 
> These recommendations were based on the new Photon style guide. The style
> guide is used by the UX team to unify our UI components so there is visual
> consistency across the browser. My recommendation was based on the style
> guide for text styles for door-hanger titles
> (http://design.firefox.com/photon/visual/typography.html#styles – Scroll to
> "Titles" for reference image of the library door-hanger). NI Amin to see if
> he has an opinion on the text recommendation change.

If you want to change the title font please do it in a way that is consistent with the other elements in the identity popup. I don't think it's any use blindly applying numbers from the style guide in components that were not designed with that style guide in mind (but with a different, coherent, concept). I think what I'm missing is some mockup of how the new identity popup is supposed to look like.

Comment 8

[Amy Lee [:amylee] UX]

(In reply to Johann Hofmann [:johannh] from comment #7)
> (In reply to Amy Lee [:amylee] UX from comment #6)
> > (In reply to Johann Hofmann [:johannh] from comment #5)
> > > I don't think attachment 8883963 [details] delivers a good user experience.
> > > The headlines should be more prominent, especially the site origin. I would
> > > like to know in what context or as part of what project these
> > > recommendations were created.
> > > 
> > > Amy, if you intend to do a re-design of the identity popup, can you please
> > > provide us with a full spec for that?
> > > 
> > > I'm happy to review minor changes such as removing the gradient for the
> > > button, but major visual changes should have some sort of justification,
> > > such as being part of a bigger concept or clearing some pressing UX problem.
> > 
> > Hi Johann,
> > 
> > These recommendations were based on the new Photon style guide. The style
> > guide is used by the UX team to unify our UI components so there is visual
> > consistency across the browser. My recommendation was based on the style
> > guide for text styles for door-hanger titles
> > (http://design.firefox.com/photon/visual/typography.html#styles – Scroll to
> > "Titles" for reference image of the library door-hanger). NI Amin to see if
> > he has an opinion on the text recommendation change.
> 
> If you want to change the title font please do it in a way that is
> consistent with the other elements in the identity popup. I don't think it's
> any use blindly applying numbers from the style guide in components that
> were not designed with that style guide in mind (but with a different,
> coherent, concept). I think what I'm missing is some mockup of how the new
> identity popup is supposed to look like.

Hi Johann,

I can provide a spec. Will attach when ready.

Comment 9

[Amin Al Hazwani [:amin] (Firefox UX)]

Hey everyone,

a mockup could be the best way to assess if the photon design system is mature enough to cover the scope of the identity popup, which is very large :) This component features a lot of views/configurations (certificate, permissions, TP). Once we understand the context in which the decisions were made for this component we could try to use the photon grid, typography, colors and icons. Amy, can I be of any help on this bug? I am based in the same office as Johann.

Comment 10

[Amy Lee [:amylee] UX]

Hi, 

I've attached a spec for updating the sub menu navigation to be aligned with Photon menus (see link below). We are going to hold off on changing any of the font sizes currently in the door-hanger.

Please let me know if you have any questions about the spec. 

UX Spec: https://mozilla.invisionapp.com/share/URCPD936K

Comment 11

[:Paolo Amadini]

(In reply to Amy Lee [:amylee] UX from comment #10)
> Please let me know if you have any questions about the spec. 

In Photon panels the main view does not slide out, but the subview slides over it. I guess we want to do the same here for consistency. In fact, most of the new styles and behavior can likely be applied using the <photonpanelmultiview> element.

Comment 12

[Amy Lee [:amylee] UX]

Hi Paolo, 

There is work going on to update door-hanger sub menu transitions. Please see Bug 1374749. All door hangers with sub menus should follow the same pattern. Thanks!

Comment 13

[:Paolo Amadini]

Nice, I didn't know of that work. Applying the <photonpanelmultiview> here will use the new transition when it lands. Thanks!

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

darcese, are you still working on this?

Comment 15

[Erica Wright [:ewright]]

Since :darcese is back in school I'll answer for her. Each part of this bug was broken into smaller bugs and I believe she fixed one or two of them, and the rest have been left open for further work/discussion. I think this bug can be closed.

Comment 16

[Marco Mucci [:MarcoM]]

(In reply to Erica Wright [:ewright] from comment #15)
> Since :darcese is back in school I'll answer for her. Each part of this bug
> was broken into smaller bugs and I believe she fixed one or two of them, and
> the rest have been left open for further work/discussion. I think this bug
> can be closed.

Marking as fixed per Comment 15.

Comment 17

[u428464]

AFAIK the identity subview still doesn't use Photon style as shown on the mockup here: https://mozilla.invisionapp.com/share/URCPD936K#/screens

Comment 18

[Johann Hofmann [:johannh]]

Amy, we still want that to happen, right? Should we file a follow-up bug (focused on Photonifying the identity popup) since this one has become a little noisy?

Comment 19

[Amin Al Hazwani [:amin] (Firefox UX)]

Yeah, afaik we still wanted to make this happen but in a following/second phase.

Comment 20

[Amy Lee [:amylee] UX]

(In reply to Johann Hofmann [:johannh] from comment #18)
> Amy, we still want that to happen, right? Should we file a follow-up bug
> (focused on Photonifying the identity popup) since this one has become a
> little noisy?

Yes, we still want this to happen. A follow-up bug would be great.