Closed Bug 727650 (australis-navbar) Opened 13 years ago Closed 6 years ago

[meta] Implement the new Australis styling for the navbar

Categories

(Firefox :: Theme, defect)

All
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jaws, Unassigned)

References

()

Details

(Keywords: meta)

Attachments

(9 obsolete files)

No description provided.
The specs for the navbar can be found on the webpage linked to in the URL field.
Attached patch wip1 (obsolete) (deleted) — Splinter Review
I added an additional tab image for selected because it needs to overlap the nav-bar to cover the box-shadow. Should I change the tabs at all with this bug or should we leave that to another bug?
Attached image Screenshot of wip1 (obsolete) (deleted) —
(In reply to Joshua M from comment #3) > Should I change the tabs at all with this > bug or should we leave that to another bug? Let's push as much work with tabs as possible to another bug.
(In reply to Joshua M from comment #3) > I added an additional tab image for selected because it needs to overlap the > nav-bar to cover the box-shadow. Should I change the tabs at all with this > bug or should we leave that to another bug? Thanks for making this patch! The tabs should be left for a separate bug.
Side-note: It is quite awesome that those two comments (#5, #6) were left at the same time :)
Attached patch wip2 (obsolete) (deleted) — Splinter Review
Reverted tab related changes.
Attachment #597676 - Attachment is obsolete: true
Attachment #597677 - Attachment is obsolete: true
Would it be acceptable if I fix Bug 726259 here? There is just a typo in a few of the selectors in browser-aero.css.
(In reply to Joshua M from comment #9) > Would it be acceptable if I fix Bug 726259 here? There is just a typo in a > few of the selectors in browser-aero.css. Yeah that's fine.
Blocks: 726529
Blocks: 726259
No longer blocks: 726529
Attached patch wip3 (obsolete) (deleted) — Splinter Review
Fixed Bug 726259 by fixing some typos in browser-aero.css.
Attachment #597683 - Attachment is obsolete: true
Is anyone available to give me feedback on my patch? Also the resulting gradient isn't the same as the one in the mockups even though the values match the design specs. It could be related to the noise added to the toolbar, will we be adding that?
Oh, I didn't know you were looking for feedback. On your next patch, can you add the "feedback?" flag and request feedback from either myself, Frank Yan, or shorlander? I'll try to get you some feedback either today or tomorrow since I'm traveling right now.
(In reply to Joshua M from comment #12) > Is anyone available to give me feedback on my patch? I am! The best way to get feedback is set the feedback flag to '?' and to ask specific questions that you'd like answers for. :) One specific thing that I would like to see done in this bug is to make the back button retain a border color that matches the border of the URL bar even when the back button is disabled. It looks ugly when disabled in the current styling, IMHO. > Also the resulting gradient isn't the same as the one in the mockups even > though the values match the design specs. It could be related to the noise > added to the toolbar, will we be adding that? According to the stylesheet Stephen applied to his spec, he's using the following: background-color: hsla(0,0%,100%,.7); border: 1px solid transparent; border-color: hsla(210,54%,20%,.25) hsla(210,54%,20%,.27) hsla(210,54%,20%,.3); border-radius: 2px; By default, background-clip is border-box, so the translucent white background-color extending underneath the border is making the border lighter. https://developer.mozilla.org/en/CSS/background-clip Stephen, was that intentional? We have background-clip: padding-box; set on the URL bar. https://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser.css#1218 If you change it to border-box, it will hopefully match the spec without causing adverse side effects.
(In reply to Frank Yan (:fryn) from comment #14) > (In reply to Joshua M from comment #12) > > Is anyone available to give me feedback on my patch? > > I am! The best way to get feedback is set the feedback flag to '?' and to > ask specific questions that you'd like answers for. :) > I wasn't quite sure who to request feedback from, thanks. > One specific thing that I would like to see done in this bug is to make the > back button retain a border color that matches the border of the URL bar > even when the back button is disabled. It looks ugly when disabled in the > current styling, IMHO. Currently when the buttons are disabled the entire button has a lower opacity, rather than just the icon. The back button icon is skinned rather than the button it self. I could either include a separate toolbar image for disabled, or possibly lower the opacity of all icons except back button, then give back button a special icon for disabled (just lower opacity). Do either of these sound like a possible solution? Just to clarify, should I attempt to style the urlbar and toolbar buttons as well?
> Currently when the buttons are disabled the entire button has a lower > opacity, rather than just the icon. The back button icon is skinned rather > than the button it self. I could either include a separate toolbar image for > disabled, or possibly lower the opacity of all icons except back button, > then give back button a special icon for disabled (just lower opacity). Do > either of these sound like a possible solution? Perhaps, rather than decreasing the opacity of the entire buttons when disabled, we could just decrease the opacity of the icons and the opacity of the background-image and background-color (by modifying the alpha channel values). I'd prefer this solution since it would maintain a consistent appearance between the back button and other toolbar buttons. The border of the toolbar buttons will be lighter with the new theming, so not changing their border color when disabled shouldn't be a problem. > Just to clarify, should I attempt to style the urlbar and toolbar buttons as > well? Sure! I don't know if Stephen has official PNG files ready for the new icons yet. He's busy this week, but I can ping him next week for them. Let me know if you have more questions. :)
Attached patch wip4 (obsolete) (deleted) — Splinter Review
Implemented most of the new button and urlbar style.
Attachment #597698 - Attachment is obsolete: true
Attachment #598164 - Flags: feedback?(fryn)
Joshua, could you please rebase your patch on top of that one in bug 702225? Thanks. By the way, small targeted patches are often preferable. If you have a patch for bug 726259, please get it up for review there.
Depends on: 702225
OS: All → Windows 7
(In reply to Dão Gottwald [:dao] from comment #18) > Joshua, could you please rebase your patch on top of that one in bug 702225? > Thanks. > Sure. Would you like me to work on the buttons here or on bug 702225? > By the way, small targeted patches are often preferable. If you have a patch > for bug 726259, please get it up for review there. Done.
(In reply to Joshua M from comment #19) > (In reply to Dão Gottwald [:dao] from comment #18) > > Joshua, could you please rebase your patch on top of that one in bug 702225? > > Thanks. > > > > Sure. Would you like me to work on the buttons here or on bug 702225? I'd like to land bug 702225 as-is as an incremental step and you can keep updating the style here. > > By the way, small targeted patches are often preferable. If you have a patch > > for bug 726259, please get it up for review there. > > Done. Thanks.
No longer blocks: 726259
Can I just drive by and tell you all that you rock, and that I love seeing that this is happening? :)
Thanks for updating your patch, Joshua. I'm out of town this weekend, but I'll give you feedback when I return early next week.
You should also reduce the border radius to 2.5px in this: #main-window[sizemode=normal][tabsontop=false] #PersonalToolbar:not(:-moz-lwtheme) { border-top-left-radius: 3.5px; border-top-right-radius: 3.5px; }
Depends on: 625742
No longer depends on: 625742
The font size in the URL bar and search box is too small in Firefox. Making it bigger would improve readability, specially on large monitors. See Google Chrome for a comparison.
(In reply to Siddhartha Dugar (sdrocking) from comment #24) > The font size in the URL bar and search box is too small in Firefox. Making > it bigger would improve readability, specially on large monitors. See Google > Chrome for a comparison. Please don't take to spamming your agenda. There is an Accessibility Team that you can request to take a look at your bug using the appropriate keyword, the same goes for the UX Team. The fact that no one marked your bug WONTFIX is disappointing and unfortunate given that the UX Team undoubtedly have had a look at font-sizing, nonetheless that doesn't mean you must spam your bug elsewhere.
(In reply to Frank Yan (:fryn) from comment #14) > By default, background-clip is border-box, so the translucent white > background-color extending underneath the border is making the border > lighter. > https://developer.mozilla.org/en/CSS/background-clip > > Stephen, was that intentional? It should be padding-box. In the CSS on my mockup I applied it to the hover state and not the normal state for some reason :) (In reply to Frank Yan (:fryn) from comment #16) > Sure! I don't know if Stephen has official PNG files ready for the new icons > yet. He's busy this week, but I can ping him next week for them. I will attach a new toolbar.png here. (In reply to Alex Limi (:limi) — Firefox UX Team from comment #21) > Can I just drive by and tell you all that you rock, and that I love seeing > that this is happening? :) This, definitely. Thank you!
Comment on attachment 598164 [details] [diff] [review] wip4 >- border-top-left-radius: 3.5px; >- border-top-right-radius: 3.5px; >+ border-top-left-radius: 2.5px; >+ border-top-right-radius: 2.5px; If you file a separate bug on this, we can get it reviewed and landed today.
(In reply to Frank Yan (:fryn) from comment #14) > One specific thing that I would like to see done in this bug is to make the > back button retain a border color that matches the border of the URL bar > even when the back button is disabled. It looks ugly when disabled in the > current styling, IMHO. Please file a separate bug on this. Users used to complain about the button state not being easily identifiable at a glance when we primarily faded the glyph and made the border only slightly fainter in the disabled state.
(In reply to Dão Gottwald [:dao] from comment #27) > Comment on attachment 598164 [details] [diff] [review] > wip4 > > >- border-top-left-radius: 3.5px; > >- border-top-right-radius: 3.5px; > >+ border-top-left-radius: 2.5px; > >+ border-top-right-radius: 2.5px; > > If you file a separate bug on this, we can get it reviewed and landed today. Filed Bug 729293.
Depends on: 729293
Stephen or Dão, can you please clarify if the buttons should be border-less for the default state?
(In reply to Joshua M from comment #30) > Stephen or Dão, can you please clarify if the buttons should be border-less > for the default state? Yes border-less for the default state.
Attached patch Wip5 (obsolete) (deleted) — Splinter Review
Attachment #598164 - Attachment is obsolete: true
Attachment #600257 - Flags: feedback?(jwein)
Attachment #598164 - Flags: feedback?(fryn)
Comment on attachment 600257 [details] [diff] [review] Wip5 Review of attachment 600257 [details] [diff] [review]: ----------------------------------------------------------------- Looks good so far. For some reason the new navbar looks more blue to me than the current navbar. What's left to implement? I see that the toolbarbuttons will need to have less left and right margins (1px on each side instead of the current 3px) and the toolbar looks too tall to me, although I see that the design calls for it to be actually 2px taller. ::: browser/themes/winstripe/browser.css @@ +698,3 @@ > -moz-appearance: none; > padding: 1px 5px; > + background: 0 padding-box; What is the goal of setting background-position here? @@ +766,5 @@ > +} > + > +@navbarLargeIcons@ .toolbarbutton-1[type="menu-button"]:not(:hover):not([open="true"]) { > + background-position: -moz-calc(100% - 16px) center, > + -moz-calc(100% - 15px) -moz-calc(100% - 2px), Four-value syntax of background-position has just been implemented, so you might be able to use it in place of doing -moz-calc(100% - 2px), etc. https://bugzilla.mozilla.org/show_bug.cgi?id=522607
Attachment #600257 - Flags: feedback?(jwein) → feedback+
(In reply to Jared Wein [:jaws] from comment #33) > Comment on attachment 600257 [details] [diff] [review] > Wip5 > > Review of attachment 600257 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good so far. For some reason the new navbar looks more blue to me than > the current navbar. > I took the values for the navbar gradient from the Australis design spec page. > What's left to implement? Mainly just including the new toolbar icon set and tweaking the values to be more accurate. I'm not too sure how much work should be done on this bug, or if we should split it up. > I see that the toolbarbuttons will need to have less left and right margins (1px on each side instead of the current 3px) > and the toolbar looks too tall to me, although I see that the design calls > for it to be actually 2px taller. > I didn't notice the margin changes, I'll include that in my next patch. I did increase the padding on the navbar to match the design specs. > ::: browser/themes/winstripe/browser.css > @@ +698,3 @@ > > -moz-appearance: none; > > padding: 1px 5px; > > + background: 0 padding-box; > > What is the goal of setting background-position here? > The 0 was just there to clear the background, I'll use "none" instead. > @@ +766,5 @@ > > +} > > + > > +@navbarLargeIcons@ .toolbarbutton-1[type="menu-button"]:not(:hover):not([open="true"]) { > > + background-position: -moz-calc(100% - 16px) center, > > + -moz-calc(100% - 15px) -moz-calc(100% - 2px), > > Four-value syntax of background-position has just been implemented, so you > might be able to use it in place of doing -moz-calc(100% - 2px), etc. > https://bugzilla.mozilla.org/show_bug.cgi?id=522607 I'll see if I can use that in place of moz-calc.
Attached patch Updated Toolbar Icons (obsolete) (deleted) — Splinter Review
Patch with updated: - Toolbar.png - Toolbar-inverted.png - tabbrowser/newtab.png - tabview/tabview.png
Comment on attachment 601990 [details] [diff] [review] Updated Toolbar Icons Does this make sense on its own or should this wait for CSS changes?
(In reply to Dão Gottwald [:dao] from comment #36) > Comment on attachment 601990 [details] [diff] [review] > Updated Toolbar Icons > > Does this make sense on its own or should this wait for CSS changes? Good point :) It's just slight updates to the overall styling/shapes so they could stand on their own independent of the CSS changes. I will make a new bug.
Attachment #601990 - Attachment is obsolete: true
Depends on: 732093
Attached patch Wip6 (obsolete) (deleted) — Splinter Review
Attachment #600257 - Attachment is obsolete: true
Attachment #602038 - Flags: feedback?(shorlander)
Attachment #602038 - Flags: feedback?(jwein)
Comment on attachment 602038 [details] [diff] [review] Wip6 Review of attachment 602038 [details] [diff] [review]: ----------------------------------------------------------------- There are places in browser.css where the descendent selector is used, but the diff shows that they were there before. Normally these selectors are discouraged, but their previous presence makes me think that they were OK'd before, so I'm fine with them staying. Also, we'll need to make the same changes to pinstripe and gnomestripe. ::: browser/themes/winstripe/browser-aero.css @@ +42,5 @@ > #browser-bottombox:not(:-moz-lwtheme) { > background-color: @customToolbarColor@; > } > > + #main-window:not([disablechrome]) #navigator-toolbox:not(:-moz-lwtheme)::after { We shouldn't use the descendent selector here. Can you use the child selector here instead?
Attachment #602038 - Flags: feedback?(jwein) → feedback+
Sorry, it appears that this bug is only for winstripe. Please disregard my previous comment about pinstripe and gnomestripe.
This image shows a comparison between the current trunk and the applied patch. There is an increased spacing between the dropdown and the refresh icon, that we'll probably want to fix.
(In reply to Jared Wein [:jaws] from comment #39) > > + #main-window:not([disablechrome]) #navigator-toolbox:not(:-moz-lwtheme)::after { > > We shouldn't use the descendent selector here. Can you use the child > selector here instead? It doesn't really matter here since #main-window is the root element.
(In reply to Jared Wein [:jaws] from comment #41) > Created attachment 602047 [details] > increased spacing between awesomebar dropdown and refresh icon > > This image shows a comparison between the current trunk and the applied > patch. There is an increased spacing between the dropdown and the refresh > icon, that we'll probably want to fix. I increased the padding because the design specs also had increased padding. After working on the urlbar buttons for a bit it may be best to push that to another bug. There is the case where the reload/stop/go buttons are not combined with the urlbar and the go button would need to be styled differently. Unless of course there is a design for that case then I could easily get that in.
(In reply to Joshua M from comment #43) > (In reply to Jared Wein [:jaws] from comment #41) > > Created attachment 602047 [details] > > increased spacing between awesomebar dropdown and refresh icon > > > > This image shows a comparison between the current trunk and the applied > > patch. There is an increased spacing between the dropdown and the refresh > > icon, that we'll probably want to fix. > > I increased the padding because the design specs also had increased padding. > > After working on the urlbar buttons for a bit it may be best to push that to > another bug. There is the case where the reload/stop/go buttons are not > combined with the urlbar and the go button would need to be styled > differently. Unless of course there is a design for that case then I could > easily get that in. OK, yeah that makes sense. I would see about pushing that work to a different bug then so that this work doesn't get held up.
Depends on: 732915
(In reply to Jared Wein [:jaws] from comment #44) > OK, yeah that makes sense. I would see about pushing that work to a > different bug then so that this work doesn't get held up. Yeah separating that out makes sense. Looking at the rest of the patch now.
Comment on attachment 602038 [details] [diff] [review] Wip6 Review of attachment 602038 [details] [diff] [review]: ----------------------------------------------------------------- • Inactive Back Button: Changing the icon opacity is good but the button shape feels a little too washed out. A slightly more translucent border+background would make it more obvious without making it too weak. - Updated on the design spec: http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-designSpecs-windows7-mainWindow.html#sectionNavBarButtonsInactive • Split Buttons (e.g. zoom controls): Can we change the z-index of the currently hovered buttons so that it is always over the separator? • Toolbar Fields: - Focused Field: Even though I know this matches the spec it feels a little too light in practice. Changing it to border-color: hsla(206,100%,60%,.65) hsla(206,100%,55%,.65) hsla(206,100%,50%,.65); feels more solid - Field Hover Effect: To be consistent can we add a slight hover effect to the field border? Just bumping the border opacity on hover to: hsla(210,54%,20%,.35) hsla(210,54%,20%,.37) hsla(210,54%,20%,.4); • Tabs-on-Bottom: - We should use Toolbar-inverted.png for icons that end up directly on glass. Looks great, thank you!
Attachment #602038 - Flags: feedback?(shorlander) → feedback+
I've pushed these changes to the UX branch so others can see the work being done here and give feedback. I'll update the UX branch going forward with new versions of the patch as they are made available. https://hg.mozilla.org/projects/ux/rev/35f72e808c17
I think bug 712602 is related to what is being done here.
In the latest UX builds, the nav-bar appears to be taller than that of Nightlies, but there is one concern, why is the new Home Icon not there in UX builds ? The new Home icon landed in mc a few days back, but this patch seems to have the old Home Icon only. If by design Australis would have the same Home Icon as in the latest UX builds, then why was the new Home Icon pushed to mc ?
(In reply to Girish Sharma from comment #49) > In the latest UX builds, the nav-bar appears to be taller than that of > Nightlies, but there is one concern, why is the new Home Icon not there in > UX builds ? The new Home icon landed in mc a few days back, but this patch > seems to have the old Home Icon only. > If by design Australis would have the same Home Icon as in the latest UX > builds, then why was the new Home Icon pushed to mc ? This patch doesn't contain the icons. So m-c probably needs to be merged to UX to pick them up.
Does this bug cover the new approach with the star outside the location bar ?
(In reply to Guillaume C. [:GE3K0S] from comment #51) > Does this bug cover the new approach with the star outside the location bar ? No, that will be a separate bug.
Thanks for the answer. I notice a bug with the reload/go/stop : the glyph is too bright and is almost unnoticeable on hover.
(In reply to Stephen Horlander from comment #46) > Comment on attachment 602038 [details] [diff] [review] > Wip6 > > Review of attachment 602038 [details] [diff] [review]: > ----------------------------------------------------------------- > > • Inactive Back Button: Changing the icon opacity is good but the button > shape feels a little too washed out. A slightly more translucent > border+background would make it more obvious without making it too weak. > - Updated on the design spec: > http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis- > designSpecs-windows7-mainWindow.html#sectionNavBarButtonsInactive > Working on this. > • Split Buttons (e.g. zoom controls): Can we change the z-index of the > currently hovered buttons so that it is always over the separator? > The separator isn't visible on hover, do you mean the right button left border color? > • Toolbar Fields: > - Focused Field: Even though I know this matches the spec it feels a > little too light in practice. Changing it to border-color: > hsla(206,100%,60%,.65) hsla(206,100%,55%,.65) hsla(206,100%,50%,.65); feels > more solid > > - Field Hover Effect: To be consistent can we add a slight hover effect to > the field border? Just bumping the border opacity on hover to: > hsla(210,54%,20%,.35) hsla(210,54%,20%,.37) hsla(210,54%,20%,.4); > Done. > • Tabs-on-Bottom: > - We should use Toolbar-inverted.png for icons that end up directly on > glass. > Should the icons be the normal ones on hover and pressed? > Looks great, thank you! (In reply to Jared Wein [:jaws] from comment #47) > I've pushed these changes to the UX branch so others can see the work being > done here and give feedback. I'll update the UX branch going forward with > new versions of the patch as they are made available. > > https://hg.mozilla.org/projects/ux/rev/35f72e808c17 Thank you Jared. I'll have another patch out today.
Nice work. The new design of the Stop/Reload/Go button looks ugly - does it necessarily have to be changed from what we had previously (from Bug 684450)? The spacing between the borderless buttons looks too much. This can be reduced to save some space like we have in small icon mode.
(In reply to Siddhartha Dugar (sdrocking) from comment #55) > Nice work. > > The new design of the Stop/Reload/Go button looks ugly - does it necessarily > have to be changed from what we had previously (from Bug 684450)? I'm surprised to see it in this patch. Please file separate bugs for stuff like that. Nothing is won by merging strictly separate changes into one mega patch. I'm going to mark this a meta bug now.
Assignee: soapyhamhocks → nobody
Status: ASSIGNED → NEW
Keywords: meta
I think that would be great if the unified back and forward buttons had the same border color of the urlbar since they are attached
Depends on: 734371
Depends on: 734373
Depends on: 734374
(In reply to Joshua M from comment #54) > The separator isn't visible on hover, do you mean the right button left > border color? Sorry, that probably wasn't clear :) I mean the segment of the button that is currently being hovered should have its border and box-shadow overlapping the button next to it. For example it appears that the button on the left is overlapping the shadow and border of the button currently hovered button on the right: http://cl.ly/0z2W0g2s1H0r1A110l1m > Should the icons be the normal ones on hover and pressed? Leaving the inverted icons would be less jarring than switching the icons out. It might be even better to reduce the background opacity of the hover/pressed/toggled state: http://cl.ly/1g0o1W121v0C2z1L3M3K Thanks!
No longer depends on: 702225
Depends on: 736954
Depends on: 742419
Depends on: 748894
Depends on: 750122
No longer depends on: 750122
Attachment #602047 - Attachment is obsolete: true
Attachment #602038 - Attachment is obsolete: true
Depends on: 764755
Depends on: 764968
Depends on: 767321
No longer blocks: australis-buttons
Depends on: australis-buttons
Please check that it works with windows classic desktop. It didn't when implemented in Thunderbird. See bug 813638.
Depends on: 868807
Other bugs are taking care of this functionality, rendering this one superfluous.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
No longer depends on: 764968
There is still nev-bar work to do.
Blocks: australis
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Depends on: 764968
Alias: australis-navbar
Status: REOPENED → NEW
Bug 868622 is part of this.
Depends on: 873626
Depends on: 873657
No longer depends on: 767321
Depends on: 873662
According to the mockups we want bug 712602 resolved for this. But it could be just for mockup convenience.
Depends on: 875819
Depends on: 868622
Depends on: 879604
Depends on: 879609
Whiteboard: [Australis:M?]
Whiteboard: [Australis:M?]
Depends on: 895563
Depends on: 907992
Depends on: 916953
Depends on: 924204
Depends on: 933162
Depends on: 925690
Depends on: 932344
Depends on: 931343
*eating popcorn* :)
Restrict Comments: true
Depends on: 936786
Depends on: 937734
Depends on: 940320
Depends on: 940251
Depends on: 940448
Depends on: 931108
Depends on: 960518
Depends on: 969376
Depends on: 969904
Depends on: 972949
Depends on: 975808
Depends on: 984188
Blocks: 984714
No longer blocks: 984714
Depends on: 985674
Depends on: 985509
Depends on: 985918
No longer depends on: 985918
Depends on: 986087
Depends on: 986324
Depends on: 998157
Depends on: 998687
No longer depends on: 967663
\o/
Status: NEW → RESOLVED
Closed: 12 years ago6 years ago
Resolution: --- → FIXED
Summary: Implement the new Australis styling for the navbar → [meta] Implement the new Australis styling for the navbar
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: