Created attachment 64643 [details] [review]
uimgr-toolbox.diff

Prove of concept patch making the toolbox use GtkAction stuff.
What's missing
  - The "recent files" entries don't have tooltips and accels
  - Not tested --with-python

Maintainers, moving the diagram menu over will involve changes in quite a few places, would you prefer patches like that or do it in a branch?

Created attachment 64972 [details] [review]
uimgr-toolbox.diff

Fix usage of action-based UI from python plugins.

http://mail.gnome.org/archives/dia-list/2006-May/msg00032.html

Created attachment 65459 [details] [review]
dia-uimgr.diff

GtkAction use in toolbox and display window.
Missing:
  + Move plugins to the new API and make sure they work
  + Check for memleaks (most objects share DIAs lifecycle though)
  + Coding style (patch almost done when Hans commented)
That's only peanuts, the hard work should be done.

Created attachment 65543 [details] [review]
dia-uimgr-02.diff

Hans' suggestions implemented (except multi file split) plus all plugins working. New translations of some menu strings might be needed.
With this patch plugin-registered menu items show up in both, the popup menu and the menubar.

Created attachment 65613 [details] [review]
dia-uimgr-03.diff

Considering patch done, please review.

can I vote for that patch to be comitted? Dia needs a lot of UI love and bringing good infrastructure is the first step.

This is not a matter of voting. Just of someone with commit access doing the review, some testing and commit. I've continued that process right now.

Just commited, thanks a lot Robert! Detailed review of some
missing stuff following ;-)

2006-05-20  Hans Breuer  <hans@breuer.org>

	Patch from Robert Staudinger to base dia menus on GtkAction framework
	instead on deprecated GtkItemFactory. Tracked in bug #340352. 
	En passant fixes bug #
	* configure.in : placementof ui files added data/Makefile to AC_OUTPUT
	* Makefile.am : added data dir
	* app/Makefile.am : added $(uidatadir)
	* app/commands.c app/commands.h app/dia_embedd.c app/diagram.c
	  app/diagram_tree_window.c app/diagram_tree_window.h
	  app/disp_callbacks.c app/display.c app/display.h
	  app/interface.c app/menus.c app/menus.h app/recent_files.c :
	changed a bunch of callback signatures to take GtkAction
	* app/pixmaps/Makefile.am 
	* data/Makefile.am data/display-ui.xml data/toolbox-ui.xml
	  data/popup-ui.xml : new files containing the menu structure
	* lib/filter.c lib/filter.h lib/plug-ins.h : callback filter
	  registration change to be more appropriate for actions
	* lib/libdia.def : updated externals
	* plug-ins/python/debug_objects.py plug-ins/python/diamodule.c
	  plug-ins/python/group_props.py plug-ins/python/gtkcons.py 
	  plug-ins/python/otypes.py plug-ins/python/pydiadoc.py 
	  plug-ins/python/scascale.py plug-ins/python/select_by.py :
	adapted to API change dia.register_callback to dia.register.action

	* app/authors.h : added Robert to the authors
	* app/commands.c : got rid of compiler warnings

The promised review notes:

* if (ddisp->menu_bar == NULL)
If I understood the action stuff correctly all this differentiation
should not be necessary anymore. The action should be the same no 
matter if it is in the menu_bar menu or the context menu?

* menus_initialize_updatable_items() looks a little odd in 
conjunction with the GtkAction framework. Would it be possible
to streamline by creating some or action groups or something?
And completely get rid of UpdatbleMenuItems?

* e.g. <Toolbox>/Self Doc/Object Types
Apparently the submenu somehow got lost during the transition

* hardcoding of pathes like UIDATADIR"/toolbox-ui.xml"
is not properly working for win32. The file placement needs to
be resolved at runtime via e.g. dia_get_data_directory(). I'll
fix this when building on win32 later.

* (Python) callback menu entries have changed from
  'Select/By/Text Color' to
  'Select/Select By/Select By Text Color'
  at least I liked the older scheme more. Was the change 
  intentionally or by accident?
  
* top-level menu entries are not translated anymore e.g.
"File/Quit" is "File/Beenden" instead of "Datei/Beenden"

(In reply to comment #10)
> The promised review notes:
> 
> * if (ddisp->menu_bar == NULL)
> If I understood the action stuff correctly all this differentiation
> should not be necessary anymore. The action should be the same no 
> matter if it is in the menu_bar menu or the context menu?

Distinction might still be needed in some cases where it's relevant in which mode dia is running. With visible menubars each diagram has its own GtkUIManager instance as it's not possible to have multiple toplevel widgets created for UI definitions.

> * menus_initialize_updatable_items() looks a little odd in 
> conjunction with the GtkAction framework. Would it be possible
> to streamline by creating some or action groups or something?
> And completely get rid of UpdatbleMenuItems?

The alternative is to query the actions by name each time.

> * e.g. <Toolbox>/Self Doc/Object Types
> Apparently the submenu somehow got lost during the transition

Sorry, forgot to mention that. I moved them into the Help menu (take that as a proposal please).

> * hardcoding of pathes like UIDATADIR"/toolbox-ui.xml"
> is not properly working for win32. The file placement needs to
> be resolved at runtime via e.g. dia_get_data_directory(). I'll
> fix this when building on win32 later.
> 
> * (Python) callback menu entries have changed from
>   'Select/By/Text Color' to
>   'Select/Select By/Select By Text Color'
>   at least I liked the older scheme more. Was the change 
>   intentionally or by accident?

A bit of both i guess, had to copy/past around some stuff ...

> * top-level menu entries are not translated anymore e.g.
> "File/Quit" is "File/Beenden" instead of "Datei/Beenden"

I'm pretty sure that the translation needs to be updated for that, because with GtkItemFactory the whole "item paths" where translated e.g. "<Toolbox>/File".

Some more issues:
* underscores in the recent file menu are no longer
  properly escaped
  
* the delete key does not work anymore for deleting objects
  instead it works in text editing. But that requires to use
  the menu for object deleting.

* the diagram menu (not menubar) has lost all it's accelerators
  and thus accelerators are not working w/o menubar

* apparently some are not working with menubar either. But these
  apparently can be brought back by adding them to GtkActionEntry
  lists again (just started to do that)

(In reply to comment #12)
> * the diagram menu (not menubar) has lost all it's accelerators
>   and thus accelerators are not working w/o menubar

For me they work, but are not displayed in the popup. That could have to do with the HIG that recommends not to show them in popups for space efficiency.

Some accelerators were lost in translation (e.g. Delete).
But for the displaying you are right. See http://bugzilla.gnome.org/show_bug.cgi?id=143007
The Gimp people already needed to work around that,
so maybe this should just be switchable in gtk?

BTW: do you plan to work on some of the other issues?

*** Bug 402116 has been marked as a duplicate of this bug. ***

Additional issue:

The diagram tree context menu is not GtkActionized yet, neither is DiaDynamicMenu. For some reason, this makes them non-translated.

although it probably got broken due to the action patch, I think it can be fixed w/o proting the other menus to GtkAction. I think we hust need to add gtk_item_factory_set_translate_func() again to make GtkItemFactory pickup Dia specific translations again.

Some of the menus are still hand-crafted, but I think I got them all.

There's something funny with the recent_files stuff, see bug #452984.  I would like an explanation (as comments) of recent_merge_ids, and why it is considered a critical failure if it is null when menus_clear_recent is called.  Seems like a perfectly valid situation to me.

my recent commit should have fixed the remainign issues:

2008-04-05  Hans Breuer  <hans@breuer.org>

	* app/menus.c(ensure_menu_path) : allow plug-ins to create a menu 
	path again. Does not yet work fopr the integrated ui, see bug #526146
	(_ui_manager_connect_proxy) : resurrection of menuitem tooltips,
	especially for recent file menu. They were lost in trnaslation to
	the GtkAction framework: bug #340352