Closed Bug 1354116 Opened 8 years ago Closed 8 years ago

Add 'Open File...', 'Save Page As...', 'Page Setup...' and 'Print' buttons to static hamburger menu

Categories

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

enhancement

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.4 - May 1
Tracking Status
firefox55 --- verified

People

(Reporter: mikedeboer, Assigned: Gijs)

References

()

Details

(Whiteboard: [photon-structure])

Attachments

(1 file)

These new buttons are copied from the main 'File' menu. The icon will be smaller; the size to be the same as all other menu item icons. In order of appearance: * The 'Open File...' button is a new widget that will show the 'Open File' dialog window. There's no icon shown. * The 'Save Page As...' button is a new widget that will show the 'Save Page As' dialog window. There's no icon shown. * The 'Page Setup...' button is a new widget that will show the 'Page Setup' dialog window. There's no icon shown. * The 'Print' menu is the same as the one from the current menu panel. Commands are the same as the menu items in the 'File' menu. Please take a (possible future) hotkey assignment into account. (no pun intended.) A separator is shown below the 'Print' menu item.
Blocks: 1354119
Blocks: 1354120
No longer blocks: 1354119
No longer depends on: 1354113
Flags: qe-verify+
Priority: -- → P2
QA Contact: gwimberly
Whiteboard: [photon] → [photon-structure]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 55.4 - May 1
Priority: P2 → P1
Will you add tests for these?
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8860090 [details] Bug 1354116 - add open, save, page setup, print buttons to main hamburger panel, https://reviewboard.mozilla.org/r/132112/#review135254 The tooltip for the Print icon on OSX says 'undefined'... I think you need to take a look at the dynamic tooltip thingy
Attachment #8860090 - Flags: review?(mdeboer)
(In reply to Mike de Boer [:mikedeboer] from comment #2) > Will you add tests for these? I wasn't planning on it - the other ones (new (private) window) also don't, and both "open" and "save file" (and on some OSes, "print" and "page setup" as well!) open native dialogs, so they're kind of painful to test... (In reply to Mike de Boer [:mikedeboer] from comment #3) > Comment on attachment 8860090 [details] > Bug 1354116 - add open, save, page setup, print buttons to main hamburger > panel, > > https://reviewboard.mozilla.org/r/132112/#review135254 > > The tooltip for the Print icon on OSX says 'undefined'... I think you need > to take a look at the dynamic tooltip thingy Huh, well-spotted. TBH, that's the only item that I'm giving a tooltip so far, I think, so I'm somewhat inclined to just get rid of the tooltip. Though maybe instead I should ask UX whether they want *all* of these items to have tooltips... We should be consistent either way. Aaron, do you want tooltips similar to the items in the current hamburger panel, with shortcuts where appropriate (try e.g. the history entry in the current panel, or "print" on OS X, or "find", or "save page", ...)
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(abenson)
I'd be inclined to add a keyboard shortcut at the end of the item whenever available; we'll be needing that functionality later as well.
And _no_ tooltips.
(In reply to Mike de Boer [:mikedeboer] from comment #5) > I'd be inclined to add a keyboard shortcut at the end of the item whenever > available; we'll be needing that functionality later as well. We have existing code and style code for this, and I actually had a WIP version of this for bug 1354095 and then I realized the mock didn't have it, so I left it out.
(In reply to :Gijs from comment #7) > We have existing code and style code for this, and I actually had a WIP > version of this for bug 1354095 and then I realized the mock didn't have it, > so I left it out. Yes, but Bryan (or Aaron) did mention that it's preferred to have a keyboard shortcut shown when available - even if it's not in the mockup.
Items in the Application Menu are pretty self-descriptive and don't need tooltips. They may have had them before because the items were presented in a grid of buttons, but I'm just speculating. We should also show keyboard shortcuts whenever possible.
Flags: needinfo?(abenson)
The indenting of items with/without icons is a bit terrible with this patch, but I expect that will mostly get fixed by the restyle anyway. If not, we can fix it separately - it seems premature to focus on it right now. :-)
(In reply to :Gijs from comment #11) > The indenting of items with/without icons is a bit terrible with this patch, > but I expect that will mostly get fixed by the restyle anyway. If not, we > can fix it separately - it seems premature to focus on it right now. :-) Absolutely!
Comment on attachment 8860090 [details] Bug 1354116 - add open, save, page setup, print buttons to main hamburger panel, https://reviewboard.mozilla.org/r/132112/#review136150 Very nice!! ::: browser/components/customizableui/content/panelUI.js:153 (Diff revision 2) > * > * @param aEvent the event (if any) that triggers showing the menu. > */ > show(aEvent) { > + if (gPhotonStructure) { > + this._ensureShortcutsShown(); Hmm, this will have to move to become smarter and more generic when we introduce subviews that contain items with keyboard shortcuts. But that'll have to happen after the structure changes. I totally agree with keeping the markup as-is.
Attachment #8860090 - Flags: review?(mdeboer) → review+
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/56f5c11857cd add open, save, page setup, print buttons to main hamburger panel, r=mikedeboer
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Verified on Windows 10, Ubuntu, and Mac.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Blocks: 1387512
Depends on: 1387846
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: