Closed Bug 1354097 Opened 8 years ago Closed 7 years ago

Add footer to overflow panel that opens customize mode

Categories

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

53 Branch
enhancement

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.4 - Aug 1
Tracking Status
firefox56 --- fixed
firefox57 --- verified

People

(Reporter: Gijs, Assigned: bwinton)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-structure])

Attachments

(1 file)

This should be straightforward, but we should add a conventionally-styled footer to the overflow panel that opens customize mode.
Flags: qe-verify+
Priority: -- → P2
QA Contact: gwimberly
Whiteboard: [photon] → [photon-structure]
Moving to reserve backlog for now.
Priority: P2 → P3
Whiteboard: [photon-structure] → [reserve-photon-structure]
Priority: P3 → P2
Whiteboard: [reserve-photon-structure] → [photon-structure]
I've got a patch in progress, so I'll grab this one so that no-one else starts working on it… :)
Assignee: nobody → bwinton
Status: NEW → ASSIGNED
Priority: P2 → P1
Attachment #8887061 - Flags: review?(shorlander)
This duplicates a bunch of stuff from bug 1354086 and probably will run into the same test failures.
Oh, fantastic! I'll remove all of that stuff and let someone else worry about it. ;)
Attachment #8887061 - Flags: review?(shorlander)
Comment on attachment 8887061 [details] Bug 1354097 - Style and add a customize button to the overflow panel. , ui-r=shorlander. https://reviewboard.mozilla.org/r/157804/#review163408 ::: browser/components/customizableui/content/panelUI.inc.xul:420 (Diff revision 2) > overflowfortoolbar="nav-bar"/> > <toolbarseparator id="widget-overflow-fixed-separator" hidden="true"/> > <vbox id="widget-overflow-fixed-list" class="widget-overflow-list" hidden="true" > emptylabel="&customizeMode.emptyOverflowList.description;"/> > </vbox> > + <menuitem command="cmd_CustomizeToolbars" This should be a toolbarbutton. Is there a styling reason to use a menuitem? :-)
Attachment #8887061 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8887061 [details] Bug 1354097 - Style and add a customize button to the overflow panel. , ui-r=shorlander. https://reviewboard.mozilla.org/r/157804/#review164696 This is very tidy - thanks! Still some comments, because what would life be without comments? ;-) ::: browser/components/customizableui/content/panelUI.inc.xul:426 (Diff revision 3) > #ifdef MOZ_PHOTON_THEME > + <toolbarbutton command="cmd_CustomizeToolbars" > + id="overflowMenu-customize-button" > + class="subviewbutton panel-subview-footer" > + accesskey="&viewCustomizeToolbar.accesskey;" > + label="&viewCustomizeToolbar.label;"/> Interestingly, the mocks say "Customize Toolbar..." . This says "Customize..." . Was that a deliberate change? More generally, we probably need a new string and access key anyway, because in the view menu context is different so different access keys may or may not be available. Finally, in the mocks ( https://mozilla.invisionapp.com/share/BXBFSCU6P#/screens/229252091_Customization ) the footer text is centered. Not sure how tricky that is, but I think it would make sense to do that here.
Attachment #8887061 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8887061 [details] Bug 1354097 - Style and add a customize button to the overflow panel. , ui-r=shorlander. https://reviewboard.mozilla.org/r/157804/#review165936 Huh, so I am a bit of an idiot, because I didn't realize literally every. other. subview. footer. doesn't center text. Which makes me wonder if the centering here is intentional or not... So, um, r=me for this version, but please check with Bryan / Aaron / Stephen if this is correct, or if the footer here should just be left-aligned like the other ones, and then you can remove the rule I just made you add. Sorry for the hassle!
Attachment #8887061 - Flags: review?(gijskruitbosch+bugs) → review+
Re: the center-aligned text .. it should be left-aligned now. The spec is outdated and needs to be updated.
Pushed by bwinton@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/67be9d4a6c2c Style and add a customize button to the overflow panel. r=Gijs, ui-r=shorlander.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Iteration: --- → 56.4 - Aug 1
Blocks: 1387512
Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: