There's not much in this widget that remains the same, compared to the current zoom widget in the menu panel: * There's a static label shown at the start of the menu item. * The middle section consists of a spinner form control, which shows the zoom level of the currently visible page (tab/ browser) - just like the indicator in the location bar. ** The design shows a dropdown control to adjust the zoom level, but this is impossible to implement inside a panel. Use a spinner control instead. ** The zoom level can be adjusted by entering a new value, followed by <ENTER> in the control, or by using the mouse on the spinner buttons. * An 'Enter Full Screen' button is shown at the end of the menu item, which shows it's caption in a tooltip when hovered. A separator is shown below this menu item.

(In reply to Mike de Boer [:mikedeboer] from comment #0) > ** The design shows a dropdown control to adjust the zoom level, but this is > impossible to implement inside a panel. Use a spinner control instead. Is this really true? Based on bug 964944, I kind of thought it was possible... might need thorough testing and/or a further 'experiment' bug though!

OK, I don't know, TBH. I think it's a good idea to ask Bryan which control he'd prefer. Bryan. for the zoom control widget inside the static menu panel/ hamburger menu, whatever, would you prefer a select dropdown or a spinner control (which has +/- buttons)? If you prefer a select dropdown, what will the value of each option be?

Bug to consider: bug #1363679 - Zooming with: +/- Zoom Controls buttons in hamburger menu, Zoom In/Out buttons in Menu Bar in View menu in Zoom submenu, Ctrl + +/- keyboard shortcuts; - are inconsistent with zooming with Mouse Scroll, so when users don't have mouse with scroll, they loose massive amount of zoom control.

Mock-up: https://mozilla.invisionapp.com/share/H3BIW4C9U#/230516723_Application_Menu

Comment on attachment 8873075 [details] Bug 1354105 - Add zoom controls - zoom in, zoom reset, zoom out and fullscreen - buttons to the Photon app menu. https://reviewboard.mozilla.org/r/144526/#review148410 I think this misses the full screen button? Sorry if it's not obvious - but that's part of the same <toolbaritem> in the spec and I didn't file a separate bug. It *is* also mentioned in comment #0... :-) If you want to push it to a followup, that's also fine, up to you. From a quick glance I'm a little confused about the styling changes - based on https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/230516723 the distance between the edit and zoom buttons is different, in order for them to line up vertically. It looks like your CSS changes just reduce the spacing for both sets of buttons?

(In reply to :Gijs from comment #6) > I think this misses the full screen button? Sorry if it's not obvious - but > that's part of the same <toolbaritem> in the spec and I didn't file a > separate bug. It *is* also mentioned in comment #0... :-) Argh! Forgot about that one :/ I'll add it in the next push. > From a quick glance I'm a little confused about the styling changes - based > on https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/230516723 the > distance between the edit and zoom buttons is different, in order for them > to line up vertically. It looks like your CSS changes just reduce the > spacing for both sets of buttons? Yeah, that's a bad on my part. I didn't compare both items good enough.

Comment on attachment 8873075 [details] Bug 1354105 - Add zoom controls - zoom in, zoom reset, zoom out and fullscreen - buttons to the Photon app menu. https://reviewboard.mozilla.org/r/144526/#review148762 Most of my comments about that ruddy vertical separator. ;-) But this looks good, and I trust you to deal with that, so r=me after the below issues are fixed. Don't need to see it again unless you think I do. Thanks! :-) ::: browser/themes/shared/customizableui/panelUI.inc.css:1345 (Diff revision 2) > +photonpanelmultiview .PanelUI-subView .toolbaritem-combined-buttons > > + .subviewbutton-iconic > .toolbarbutton-text, Is the `.PanelUI-subView` selector really required here? ::: browser/themes/shared/customizableui/panelUI.inc.css:1347 (Diff revision 2) > +photonpanelmultiview .PanelUI-subView .toolbaritem-combined-buttons > > + .subviewbutton:not(.subviewbutton-iconic) > .toolbarbutton-icon { I wonder if this second selector should just be #appMenu-zoomReset-button > .toolbarbutton-icon ... there aren't any other items, are there? We don't want it to apply to buttons outside the combined-buttons set, even if they don't have icons, so I think that would be OK, plus it makes the selector more efficient and easier to read. :-) ::: browser/themes/shared/customizableui/panelUI.inc.css:1361 (Diff revision 2) > + border: 1px solid #e1e1e1; > + border-radius: 10000px; > + padding: 1px 8px; > +} > + > +photonpanelmultiview .PanelUI-subView .toolbaritem-combined-buttons > Nit: trailing space ::: browser/themes/shared/customizableui/panelUI.inc.css:1496 (Diff revision 2) > +.PanelUI-subView toolbarseparator[orient="vertical"] { > + height: 24px; This surprises me, for 2 reasons. First, it's not in a photon block, and the selector looks like it'd apply to non-photon stuff, which is a bit scary. Second, I already added a similar set of code for the sync separator. The selector looks like this: #appMenu-fxa-container[fxastatus="signedin"] > toolbarseparator and it lives in this file. Judging by the code comment, I guess you might need to override the background-color for this to look good on OS X? Or perhaps I just did a shoddy job of the other separator and I could have done it more concisely, somehow? Regardless, can we unify these and make them the same color/size/mechanism (background vs. border)? (Funny thing, we also seem to use a different colour...) It looks like the margins might need to be separate, which is fine - but we should try to share the rest of the styles, I think.

Pushed by mdeboer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cb9964c00980 Add zoom controls - zoom in, zoom reset, zoom out and fullscreen - buttons to the Photon app menu. r=Gijs

On Windows: When I hit the full screen toggle control via the hamburger menu, the photon hamburger menu stays. On Ubuntu/Mac: When I hit the full screen toggle control via the hamburger menu, the photon hamburger menu disappears. Is this expected functionality for the operating systems? If not, which scenario is the intended one? Thanks!

Hi Grover, thanks for checking! The intended behavior is that the menu should disappear on all operating systems. If that doesn't happen on Windows, then that's odd and a bug that'd block this one. For tracking purposes I think it'd be best to have a new bug for it and not re-opening this one.

Verified on Windows, Mac, and Ubuntu.