GNOME Bugzilla – Bug 87944
Review Menu API
Last modified: 2004-12-22 21:47:04 UTC
Menu API (Andreas Holzmann's comments): * The interface for creating changeable accelerators is bad. We need a way to create unique accelerator paths automatically to improve this. * Menu items that are added after its container (MenuBar, OptionMenu, etc) have been realized don't get its accelerators initialized automatically at the moment. You have to call accelerate() manually. * Accelerators that are in menu items of a popup menu must be initialized manually with accelerate().
There are now clean examples in examples/book/menus/ that test this stuff. Can someone remind me why we can't just accelerate() the menu items as soon as they are created?
I'm working on it. I've done some cleanup in menu_elems.[cc|h] and I'm playing around with different menu accelerator initializations. The next 12 days I'm traveling and I don't have online access. But I have a laptop and I hope I've the time to do more work on the menus. In the meanwhile some more thoughts. Menu accelerators The main problem of menu accelerators is that it needs an AccelGroup that has been initialized with a window. Currently the Window class has an AccelGroup member that gets initialized on first access. The accelerate() method connects the menu items with the AccelGroup. The difficult part is that the objects can be initialized in different order: 1) Generate MenuBar, attach menu items, attach menubar to window. 2) Generate MenuBar, attach menubar to window, attach menu items. (this does not work correctly with the current gtkmm version, accelerators aren't initialized) There are several possibilities for creating accelerated menus. 1) Keep it like it is now and fix the bug mentioned above. 2) Initialize AccelKeys with AccelGroup manually. Each time an AccelKey is created you would have to give an AccelGroup parameter. Also for stock menu items because they take accel keys from the stock list. e.g. menuitems.push_back(MenuElem("_Close", AccelKey(accel_group, "<control>q"), slot (&on_quit_function))); menuitems.push_back(StockMenuElem(Gtk::Stock::Open, accel_group, slot (&on_open_function))); 3) Initialize MenuBars and popup menus manually with an AccelGroup. After building the menu you would have to call accelerate(accel_group). e.g. menuitems.push_back(MenuElem("_Close", AccelKey("<control>q"), slot (&on_quit_function))); menuitems.push_back(StockMenuElem(Gtk::Stock::Open, slot (&on_open_function))); menubar.accelerate(accel_group); 2+3 wouldn't require Gtk::Window to have an AccelGroup member. The user can take care of this himself. Changeable accelerators I can't think of an easy way of generating the unique id that are necessary for this automatically. Just "some" id would not be good, because the id should be the same if the source changes and a new menu item is added there. We cannot use the menu name, because usually they used like this: _("Open") and therefore changes if the user changes the language.
I don't think that coders should have to specify an AccelGroup. Maybe they should have to specify a Gtk::Window, or we can find the Gtk::Window by recursively calling get_parent().
Created attachment 10180 [details] [review] My modifications to the menu implementation
Created attachment 10181 [details] New file gtk/gtkmm/accelmap.cc
Created attachment 10182 [details] New file gtk/gtkmm/accelmap.h
OK, please go ahead an commit, but: 1. Please add some doxygen comments to the popup() method, telling people _when_ to call accelerate(). Your email explained this well. 2. Couldn't we have accelerate(Window&) and accelerate(Widget&) overrides, like I had for popup()? Thanks for clearing all this up.
1. accelerate documentation updated 2. accelerate(Widget&) added 3. commited