I think NotZed should really be the one to review this as he's more
familiar with it.
I did notice a few things tho.
+ if (selected)
+ for
(i=0;i<sizeof(emfv_popup_selection_items)/sizeof(emfv_popup_selection_items[0]);i++)
+ menus = g_slist_prepend(menus,
&emfv_popup_selection_items[i]);
+ else {
should really be:
if (selected) {
...
} else {
also, why are some menu items that do the same thing labeled
differently?
e.g.:
+static EPopupItem emfv_popup_message_items[] = {
+ {E_POPUP_ITEM, "10.emfv.00", N_("Reply to _All"),
emfv_popup_reply_all, NULL, "stock_mail-reply-to-all",
EM_POPUP_SELECT_ONE },
+ { E_POPUP_ITEM, "10.emfv.01", N_("_Reply to Sender"),
emfv_popup_reply_sender, NULL, "stock_mail-reply",
EM_POPUP_SELECT_ONE },
+ { E_POPUP_ITEM, "10.emfv.02", N_("_Forward"),
emfv_popup_forward, NULL, "stock_mail-forward", EM_POPUP_SELECT_MANY },
and then later:
+ { E_POPUP_ITEM, "10.emfv.00", N_("Quoted Reply to _All"),
emfv_popup_reply_all, NULL, "stock_mail-reply-to-all",
EM_POPUP_SELECT_ONE },
+ { E_POPUP_ITEM, "10.emfv.01", N_("Quoted _Reply to Sender"),
emfv_popup_reply_sender, NULL, "stock_mail-reply",
EM_POPUP_SELECT_ONE },
+ { E_POPUP_ITEM, "10.emfv.02", N_("Quoted _Forward"),
emfv_popup_forward, NULL, "stock_mail-forward", EM_POPUP_SELECT_MANY },
Is there any reason we need "Quoted " in front of each? And if so,
should the first group of menu items be changed to be worded the same?
Jeff
On Tue, 2005-01-25 at 17:59 -0500, Rodney Dawes wrote:
> Here is an updated version of the mailer context menus patch. It
> compresses a bit of the code into the emfv_popup routine, and
> pulls out a few "features", namely "View Source" and "Select All".
> This applies against HEAD, builds without warnings, and everything
> works.
>
> I will be pushing for UI freeze exception for this patch, but it
> is best we work out all the internal disagreements here first, and
> get to some final agreement on what it should be.
>
> -- dobey
>
--
Jeffrey Stedfast
Evolution Hacker - Novell, Inc.
fejj ximian com - www.novell.com
Attachment:
smime.p7s
Description: S/MIME cryptographic signature