GNOME Bugzilla – Bug 741904
Finish up support for traditional menubar
Last modified: 2016-01-15 13:59:10 UTC
+++ This bug was initially created as a clone of Bug #741610 +++ This is the "0.1" part of the promised future where each app would only need 2.1 menus. This adds support for hiding a few select items on Mac OS in order to avoid having to create a completely separate menu structure there. The initial bug report contains a patch porting gedit to this feature, though it is unfinished.
Created attachment 295295 [details] [review] Use the OSX menubar layout for XFCE, Unity, etc. Update Ryan's patch to make it apply and compile again. Also change window_menu to gear_menu to properly differentiate between the two modes when deciding whether to hide the menu button.
Review of attachment 295295 [details] [review]: Added some minor comments, I have not tested it yet ::: gedit/gedit-app.c @@ +78,3 @@ GSettings *window_settings; + GMenuModel *gear_menu; I am not totally happy with the name gear menu since it is not a gear anymore... on the other hand I do not have any great alternative name in mind and we already use gear in the .ui... @@ +1793,3 @@ + /* First look in the gear or window menu */ + if (app->priv->gear_menu) please use tabs to indent and always use {} also for one liners ::: gedit/gedit-window.c @@ +2803,3 @@ + else + { + gtk_menu_button_set_menu_model (window->priv->gear_button, gear_menu); is hide enough, maybe you should set them no-show-all ?
Created attachment 295366 [details] [review] Use the OSX menubar layout for XFCE, Unity, etc. Thanks for the review! I dislike 'gear menu' as well. I changed it to make it consistent (it's called gear menu in ui files and lots of places in gedit-window). We could rename it in a separate patch. You're right about no-show-all. It is needed for at least the fullscreen menu button. I've added it for both. Sorry about the whitespace.
Is the patch against master? It does not apply here... Jesse: when you have time can you test it on OSX?
Created attachment 295459 [details] [review] Use the OSX menubar layout for XFCE, Unity, etc. Small update: remove the old menu resource files from gedit/Makefile.am.
Created attachment 310096 [details] [review] Use the OSX menubar layout for XFCE, Unity, etc. Update for master. Is this patch blocked on anything?
Review of attachment 310096 [details] [review]: See minor comment ::: gedit/gedit-app.h @@ -90,3 @@ GdkEvent *event); -G_END_DECLS why removing this?
Created attachment 310165 [details] [review] Use the OSX menubar layout for XFCE, Unity, etc. > why removing this? Oops. Thanks for the catch!
Mmm... I thought we concluded that the result was not really satisactory for now? Do I misremember?
IIRC, there was a discussion with seb128, probably on IRC but I don't see it in my backlog.
The menubar is desired for e.g Unity, we are looking again that updating gedit for Ubuntu but that's one of the blocking issues, what's the status of those changes? Is that something you are wanting to include upstream? (the other issue is still CSD, especially when maximizing you get decorations in the unity bar and in the gedit toolbar which doesn't look great)
Review of attachment 310165 [details] [review]: Looks good, if it works. We can merge this I think, after testing that it works fine on different environments (GNOME, Unity and Xfce at least).
Created attachment 319099 [details] [review] Use the OSX menubar layout for XFCE, Unity, etc. Two small updates to this patch: Add underscores to toplevel menu items and fix an action name (win.redo). Sorry for letting this sit so long.
Created attachment 319100 [details] [review] menus-traditional.ui: add keyboard shortcuts The accelerators that are set with gtk_application_set_accels_for_action() are not exported on D-Bus. Annotate the menu xml until they are, so that desktops like Unity can show them in the global menu bar.
Review of attachment 319100 [details] [review]: While the patch looks good to me, I'd rather keep in sync the normal menus. Any chance you can add this to the normal ones too if it is not added yet?
(In reply to Ignacio Casal Quinteiro (nacho) from comment #15) > While the patch looks good to me, I'd rather keep in sync the normal menus. > Any chance you can add this to the normal ones too if it is not added yet? I think the other menus don't need those because they're inside the window and can thus pull accel information out of the application object themselves.
OK, go for it then
Thanks! Pushed as 24e4e79 and c3fdf13.