Add footer to overflow panel that opens customize mode

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Toolbars and Customization
P1
normal
VERIFIED FIXED
5 months ago
14 days ago

People

(Reporter: Gijs, Assigned: bwinton)

Tracking

(Blocks: 2 bugs)

53 Branch
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed, firefox57 verified)

Details

(Whiteboard: [photon-structure])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
This should be straightforward, but we should add a conventionally-styled footer to the overflow panel that opens customize mode.

Updated

4 months ago
Flags: qe-verify+
Priority: -- → P2
QA Contact: gwimberly
Whiteboard: [photon] → [photon-structure]
Moving to reserve backlog for now.
Priority: P2 → P3

Updated

3 months ago
Whiteboard: [photon-structure] → [reserve-photon-structure]

Updated

2 months ago
Priority: P3 → P2
Whiteboard: [reserve-photon-structure] → [photon-structure]
(Assignee)

Comment 2

a month ago
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

Updated

a month ago
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment hidden (mozreview-request)
(Assignee)

Updated

a month ago
Attachment #8887061 - Flags: review?(shorlander)
(Reporter)

Comment 4

a month ago
This duplicates a bunch of stuff from bug 1354086 and probably will run into the same test failures.
(Assignee)

Comment 5

a month ago
Oh, fantastic!  I'll remove all of that stuff and let someone else worry about it.  ;)
Comment hidden (mozreview-request)
(Assignee)

Updated

a month ago
Attachment #8887061 - Flags: review?(shorlander)
(Reporter)

Comment 7

a month ago
mozreview-review
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 hidden (mozreview-request)
(Reporter)

Comment 9

29 days ago
mozreview-review
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 hidden (mozreview-request)
(Reporter)

Comment 11

25 days ago
mozreview-review
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+

Comment 12

24 days ago
Re: the center-aligned text .. it should be left-aligned now. The spec is outdated and needs to be updated.
Comment hidden (mozreview-request)

Comment 14

24 days ago
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.

Comment 15

24 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/67be9d4a6c2c
Status: ASSIGNED → RESOLVED
Last Resolved: 24 days ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Updated

24 days ago
Iteration: --- → 56.4 - Aug 1
Blocks: 1387512
Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
status-firefox57: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.