Closed Bug 1354126 Opened 8 years ago Closed 8 years ago

Restyle customize mode footer

Categories

(Firefox :: Toolbars and Customization, enhancement, P1)

Product:
Firefox
Component:
Toolbars and Customization
Version:
53 Branch
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 55
Iteration:
55.6 - May 29
Tracking Flags:
Tracking Status
firefox55 --- fixed
firefox56 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-structure])

Attachments

(3 files)

Bug 1354126 - allow footer to wrap underneath the panel and add a 'done' button,
59 bytes, text/x-review-board-request
mikedeboer
: review+
Details
Bug 1354126 - use checkbox for titlebar toggle in customize mode,
59 bytes, text/x-review-board-request
mikedeboer
: review+
Details
Bug 1354126 - update labeling and styling of footer buttons,
59 bytes, text/x-review-board-request
mikedeboer
: review+
Details
See https://mozilla.invisionapp.com/share/5ZAEYEW8M#/screens/225203398 . We should restyle the footer to match the spec. This includes: - extending it underneath the overflow panel (hamburger panel will be removed) - changing the background/border colours of the footer - updating the titlebar switch imagery / code - adding a 'done' button' next to 'restore defaults' that closes customize mode - ensuring the design works well on narrow window sizes (I'll file a UX bug for this).
Depends on: 1354140
Flags: qe-verify+
Priority: -- → P2
QA Contact: gwimberly
Whiteboard: [photon] → [photon-structure]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 55.6 - May 29
Priority: P2 → P1
Comment on attachment 8870133 [details] Bug 1354126 - allow footer to wrap underneath the panel and add a 'done' button, https://reviewboard.mozilla.org/r/141590/#review145574 You left me no nits :'(
Attachment #8870133 - Flags: review?(mdeboer) → review+
Comment on attachment 8870134 [details] Bug 1354126 - use checkbox for titlebar toggle in customize mode, https://reviewboard.mozilla.org/r/141592/#review145580 Nice cleanup is nice.
Attachment #8870134 - Flags: review?(mdeboer) → review+
Comment on attachment 8870135 [details] Bug 1354126 - update labeling and styling of footer buttons, https://reviewboard.mozilla.org/r/141594/#review145584 Oh, that looks so much nicer! The `:not([type=menu])` type of buttons have more horizontal padding. Can you add that? I also think the font-weight of the 'Done' caption is more like `700`, not `bold`. You chose not to include the toolbar background color change in this patch, why? Can we ask Aaron for the new icon of the Themes button? Would be nice to just call this Done. ;-) ::: browser/themes/shared/customizableui/customizeMode.inc.css:190 (Diff revision 1) > + list-style-image: url(chrome://browser/skin/arrow-dropdown.svg); > +} > + > +.customizationmode-button:focus:not([disabled]), > +.customizationmode-button:active:not([disabled]), > +.customizationmode-button:hover:not([disabled]), or `.customizationmode-button:-moz-any(:focus,:active,:hover):not([disabled])`? ::: browser/themes/shared/customizableui/customizeMode.inc.css:197 (Diff revision 1) > + background-color: #e1e1e5; > } > > -.customizationmode-button:hover:active:not([disabled]), > -.customizationmode-button[open], > -.customizationmode-button[checked] { > +#customization-done-button:focus:not([disabled]), > +#customization-done-button:active:not([disabled]), > +#customization-done-button:hover:not([disabled]) { ...which you can use here. Or is that more expensive or something?
Attachment #8870135 - Flags: review?(mdeboer) → review-
Comment on attachment 8870135 [details] Bug 1354126 - update labeling and styling of footer buttons, https://reviewboard.mozilla.org/r/141594/#review145962 ::: browser/themes/shared/customizableui/customizeMode.inc.css:197 (Diff revision 1) > + background-color: #e1e1e5; > } > > -.customizationmode-button:hover:active:not([disabled]), > -.customizationmode-button[open], > -.customizationmode-button[checked] { > +#customization-done-button:focus:not([disabled]), > +#customization-done-button:active:not([disabled]), > +#customization-done-button:hover:not([disabled]) { Maybe: https://developer.mozilla.org/en/docs/Web/CSS/:any#Issues_with_performance_and_specificity It's hard to know from that text whether the perf concern still applies given they're modifiers on an #id selector... and we already use similar -moz-any stuff on other items in panelUI.css, so I'm tempted to think we should just simplify this, too.
(In reply to Mike de Boer [:mikedeboer] from comment #7) > The `:not([type=menu])` type of buttons have more horizontal padding. Can > you add that? Per our discussion, I added a min-width for the buttons at the end - I think it's mostly to make the 'undo' and 'done' buttons not look silly. > I also think the font-weight of the 'Done' caption is more > like `700`, not `bold`. Fixed. > You chose not to include the toolbar background > color change in this patch, why? Well, I just missed there was any change. Looking at it again, it looks like we're using the same color as for the toolbar, which is what we'll want to keep doing, so I've kept it like this for now. > Can we ask Aaron for the new icon of the Themes button? Would be nice to > just call this Done. ;-) The icon in the mock is actually the old icon! We'll update the theme icons when shipping the photon theme, I think - it's just using the icons we use for the add-on manager, too, and it looks like we'll be renaming compact light/dark for Photon. It can probably just be part of that. I'll file some bugs.
Comment on attachment 8870135 [details] Bug 1354126 - update labeling and styling of footer buttons, https://reviewboard.mozilla.org/r/141594/#review145996 Looking good, thanks! I suspect there's going to be a follow-up to change the toolbar background color on OSX after all (since they create their things on OSX usually), but later is OK too.
Attachment #8870135 - Flags: review?(mdeboer) → review+
FYI: the patches seem to have bitrotted a bit in jar.inc.mn.
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c0f69399f6c6 allow footer to wrap underneath the panel and add a 'done' button, r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/30628e2f9a9e use checkbox for titlebar toggle in customize mode, r=mikedeboer https://hg.mozilla.org/integration/autoland/rev/542bbb51c412 update labeling and styling of footer buttons, r=mikedeboer
(In reply to Mike de Boer [:mikedeboer] from comment #11) > Comment on attachment 8870135 [details] > Bug 1354126 - update labeling and styling of footer buttons, > > https://reviewboard.mozilla.org/r/141594/#review145996 > > Looking good, thanks! I suspect there's going to be a follow-up to change > the toolbar background color on OSX after all (since they create their > things on OSX usually), but later is OK too. Filed bug 1367439. (In reply to Mike de Boer [:mikedeboer] from comment #12) > FYI: the patches seem to have bitrotted a bit in jar.inc.mn. hg rebase autoresolved for me \o/
https://hg.mozilla.org/mozilla-central/rev/c0f69399f6c6 https://hg.mozilla.org/mozilla-central/rev/30628e2f9a9e https://hg.mozilla.org/mozilla-central/rev/542bbb51c412
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I see a small talos improvement from this code: == Change summary for alert #6855 (as of May 24 2017 14:54 UTC) == Improvements: 2% cart summary windows8-64 opt e10s 29.26 -> 28.68 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6855
Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
status-firefox56: --- → verified
Flags: qe-verify+
Blocks: 1387512
Depends on: 1391007
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: