Re: [Evolution-hackers] Thinking about menus

[Not Zed <notzed ximian com>]

Patches should go to evolution-patches, and they should contain ChangeLog entries, if they are intended to be actual patches and not just examples.

I'm reviewing it assumg it was actually intended to go in as is, and not just a prototype, and it can't, it doesn't even work.

1. you add a bunch of whitespace in random places.  Try reading the patch guidelines.
2. you rename a bunch of menu paths for seemingly no reason whatsoever, please do not do this (e.g. the label ones, the flag ones).
3. we're in feature freeze, 'view source' is a new function, it also a rarely used one and doesn't belong on a context menu
4. vfolder on mailto links are another new feature, similarly rarely used
5. there is unecessary code duplication, emfv_popup_selection and emfv_popup_message are both the same.  they also both don't work, e_popup_add_items is passing nothing as the list of menus.  They also duplicate stuff in emfv_popup.  Everything should be done in one function, based on the context.  Almost everything should be able to be done by playing with the visibility bits in the menu items (vs the enable bits), even if you need another context mask bit to do it.
6. you're mixing/abusing the popup context with the html editing functions, the popup context is on a list (including singleton) of messages, not a html view.
7. the whole 'Quoted Reply' thing is redundant, and doesn't even do what it says it does, since it just does the same as the normal reply functions
8. copy email address is already handled by a plugin

9.
-	{ E_POPUP_ITEM, "00.uri.10", N_("Se_nd message to..."), emp_uri_popup_address_send, NULL, NULL, EM_POPUP_URI_MAILTO },
+	{ E_POPUP_ITEM, "00.uri.10", N_("_Send New Message To..."), emp_uri_popup_address_send, NULL, NULL, EM_POPUP_URI_MAILTO },

Seems pointless changing the accelerator for this, as well as the text.

There are probably more issues, but since it wont even work i wont bother looking harder.


On Fri, 2005-01-21 at 15:22 -0500, Benjamin Kahn wrote:
> Attached is the latest version of the context menus patch ONLY.  It
> should be very similar to the last patch I sent to this list, and it
> compiles for me.
>
> It appears that the full menu reorganization isn't going to happen for
> Evolution 2.2, so I've put up the latest version of the full menu patch
> at:
>
> http://primates.ximian.com/~xkahn/evolution-context-and-mailer-menus.patch
>
> note that this patch won't apply well when the context menu stuff is
> applied.  :^)  But I'll try to update it when that happens.
>
> On Mon, 2005-01-10 at 12:25 -0500, Benjamin Kahn wrote:
> > With Evolution 2.2 fast approaching with a new plugin architecture, I
> > (along with Rodney and a bunch of other people) have been looking at the
> > Evolution menus.  
> > 
> > We've started with with the mailer context menus and the main mailer
> > menus.  This is needed because the current menus are a discouraging mess
> > and with menu merging from plugins, things are only going to get
> > worse.  
> > 
> > The goals are:
> > 
> >       * Get rid of the Actions menu.
> >       * Make it obvious where to find things.  (Eliminate seeking as
> >         much as possible)
> >       * Make common operations available without using submenus.
> >       * Move closer to the HIG recommendations.
> >         ( http://developer.gnome.org/projects/gup/hig/2.0/menus-standard.html )
> >       * Provide a good structure for plugins to add new menu items.
> >       * Umm...  And simplicity.  
> > 
> > I've posted screenshots of the effort to the Evolution Blog:
> > http://codeblogs.ximian.com/blogs/evolution/archives/000441.html
> > 
> > The latest version of the glade mockup is here: (I'll try to keep this
> > current)
> > http://primates.ximian.com/~xkahn/evo-menus/evo-menus.tar.gz
> > NOTE: Evolution does not use glade to create menus.  This is simply a
> > mockup.  It will never be the official version.)
> > 
> > I am generating a patch for Evolution that uses the new menus.  I'm
> > about 50% done.
> > 
> > And, obviously, a similar layout needs to be designed for the other
> > components.  
> > 
> > _______________________________________________
> > evolution-hackers maillist  -  evolution-hackers lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-hackers
> >