GNOME Bugzilla – Bug 619541
split out PanelMenu, port AppDisplay to use it
Last modified: 2010-06-10 19:06:58 UTC
see patches the spacing/style may be slightly off in the scaled-down AppDisplay version of the menu. (Jeremy said we want to keep the smaller fonts there.) I made it work by scaling everything in terms of the font-size, but it might be better to just override each spacing/padding value explicitly, to get exactly the spacing we want.
Created attachment 161882 [details] [review] [boxPointer] implement the other arrowSide values
Created attachment 161883 [details] [review] [popupMenu] split this out from panel.js We want to use this menu style in other places as well
Created attachment 161884 [details] [review] [appDisplay] port from ShellMenu to PopupMenu This fixes the style, and also makes it keyboard navigable
Created attachment 161885 [details] [review] [ShellMenu] remove; no longer used
Review of attachment 161882 [details] [review]: Looks good overall - I wonder if there should be support for swapping left/right automatically for RTL locales. One comment: ::: js/ui/boxpointer.js @@ +92,3 @@ + break; + case St.Side.BOTTOM: + childBox.y2 -= - rise; -(-rise) looks wrong - are you sure you didn't mean y2 -= rise ?
Created attachment 162645 [details] [review] [clockButton] port right-click menu to popupMenu I just committed a patch for bug 600276 which conflicts with this series - if you agree to that fix, feel free to squash it into the popupMenu patch. There is one small regression in regard to the committed patch(*) - I didn't find a non-intrusive way of fixing it, so I decided to leave it in (assuming that the right-click menu is just temporary anyway) (*) I was thinking of buying beer for whoever finds it first, but well, here you are: when opening a menu with the calendar expanded, two clicks are required to close the calendar. Duh.
Comment on attachment 161882 [details] [review] [boxPointer] implement the other arrowSide values pushed the part Florian reviewed (with the bugfix) Attachment 161882 [details] pushed as 702f596 - [boxPointer] implement the other arrowSide values
Review of attachment 161885 [details] [review]: Obviously fine once the other patches are merged
Review of attachment 161883 [details] [review]: Looks good - the code in panel.js regarding the clock button has changed though, so the patch has to be adjusted. ::: data/theme/gnome-shell.css @@ +115,3 @@ + */ + * override .popup-menu.font-size, everything else will scale with it. +/* The remaining popup-menu sizing is all done in ems, so that if you Nice. ::: js/ui/popupMenu.js @@ +108,3 @@ + let pattern = new Cairo.LinearGradient(margin, gradientOffset, width - margin, gradientOffset + gradientHeight); + pattern.addColorStopRGBA(0, startColor.red / 255, startColor.green / 255, startColor.blue / 255, startColor.alpha / 255); + pattern.addColorStopRGBA(0.5, endColor.red / 255, endColor.green / 255, endColor.blue / 255, endColor.alpha / 0xFF); Don't mix decimal and hex.
Review of attachment 161884 [details] [review]: Nice cleanup - both code-wise and visually! ::: js/ui/appDisplay.js @@ +401,2 @@ this._menu = null; + this._menuManager = new PopupMenu.PopupMenuManager(this); It is slightly odd to have a menu manager for a single menu - one for each icon. On the other hand, using the menu only would only result in the relevant code copied over from PopupMenuManager ...
I squashed in your clock patch and made the other suggested fixes and committed it. Attachment 161883 [details] pushed as 7fbf8ae - [popupMenu] split this out from panel.js Attachment 161884 [details] pushed as 748739e - [appDisplay] port from ShellMenu to PopupMenu Attachment 161885 [details] pushed as 016ab1a - [ShellMenu] remove; no longer used