Re: [evolution-patches] [UI] Fix for Bug 246230: Add Filter Rule dialog - label suggestions

[Jeffrey Stedfast <fejj novell com>]

see comments toward the end

On Mon, 2005-11-21 at 14:04 +0530, Johnny Jacob wrote:
> Hi
> Worked on bug 246230. please review this patch.
> 
> Johnny
> 
> 
> Index: mail/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
> retrieving revision 1.3725
> diff -u -p -r1.3725 ChangeLog
> --- mail/ChangeLog	18 Nov 2005 09:25:25 -0000	1.3725
> +++ mail/ChangeLog	21 Nov 2005 08:18:48 -0000
> @@ -1,3 +1,11 @@
> +2005-11-21  Johnny Jacob <johnnyjacob gmail com>
> +
> +	** See bug #246230
> +	* filtertypes.xml :
> +	* em-filter-rule.c :
> +	* em-folder-selection-button.c :
> +	Changed the labels.
> +
>  2005-11-11  Boby Wang <boby wang sun com>
> 
>  	** See bug 321214.
> Index: mail/filtertypes.xml
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/filtertypes.xml,v
> retrieving revision 1.3
> diff -u -p -r1.3 filtertypes.xml
> --- mail/filtertypes.xml	21 Jun 2005 05:33:47 -0000	1.3
> +++ mail/filtertypes.xml	21 Nov 2005 08:21:33 -0000
> @@ -646,12 +646,12 @@
> 
>  <actionset>
>   <part name="move-to-folder">
> -  <title>Move to Folder</title>
> +  <title>Move to folder</title>
>    <code>(move-to ${folder})</code>
>    <input type="folder" name="folder"/>
>   </part>
>   <part name="copy-to-folder">
> -  <title>Copy to Folder</title>
> +  <title>Copy to folder</title>
>    <code>(copy-to ${folder})</code>
>    <input type="folder" name="folder"/>
>   </part>
> @@ -660,26 +660,26 @@
>    <code>(delete)</code>
>   </part>
>   <part name="stop">
> -  <title>Stop Processing</title>
> +  <title>Stop processing</title>
>    <code>(stop)</code>
>   </part>
>   <part name="colour">
> -  <title>Assign Color</title>
> +  <title>Assign color</title>
>    <code>(set-colour ${colour})</code>
>    <input type="colour" name="colour"/>
>   </part>
>   <part name="score">
> -  <title>Assign Score</title>
> +  <title>Assign score</title>
>    <code>(set-score ${score})</code>
>    <input type="score" name="score"/>
>   </part>
>   <part name="adj-score">
> -  <title>Adjust Score</title>
> +  <title>Adjust score</title>
>    <code>(adjust-score ${score})</code>
>    <input type="score" name="score"/>
>   </part>
>   <part name="set-status">
> -  <title>Set Status</title>
> +  <title>Set status</title>
>    <code>
>     (set-system-flag ${flag})
>    </code>
> @@ -705,7 +705,7 @@
>    </input>
>   </part>
>   <part name="unset-status">
> -  <title>Unset Status</title>
> +  <title>Unset status</title>
>    <code>
>     (unset-system-flag ${flag})
>    </code>
> @@ -735,17 +735,17 @@
>    <code>(beep)</code>
>   </part>
>   <part name="play-sound">
> -  <title>Play Sound</title>
> +  <title>Play sound</title>
>    <code>(play-sound ${sound})</code>
>    <input type="file" name="sound"/>
>   </part>
>   <part name="shell">
> -  <title>Run Program</title>
> +  <title>Run program</title>
>    <code>(shell "/bin/sh" "-c" ${command})</code>
>    <input type="command" name="command"/>
>   </part>
>   <part name="pipe">
> -  <title>Pipe to Program</title>
> +  <title>Pipe to program</title>
>    <code>(pipe-message "/bin/sh" "-c" ${command})</code>
>    <input type="command" name="command"/>
>   </part>
> Index: mail/em-filter-rule.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-filter-rule.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 em-filter-rule.c
> --- mail/em-filter-rule.c	17 Jun 2004 07:34:49 -0000	1.1
> +++ mail/em-filter-rule.c	21 Nov 2005 08:23:16 -0000
> @@ -485,7 +485,7 @@ get_widget(FilterRule *fr, RuleContext *
>          widget = FILTER_RULE_CLASS(parent_class)->get_widget(fr, rc);
>  	
>  	/* and now for the action area */
> -	label = gtk_label_new(_("<b>Then</b>"));
> +	label = gtk_label_new(_("<b>Actions</b>"));
>  	gtk_label_set_use_markup(GTK_LABEL(label), TRUE);
>  	gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
>  	gtk_box_pack_start(GTK_BOX(widget), label, FALSE, FALSE, 0);
> Index: mail/em-folder-selection-button.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-folder-selection-button.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 em-folder-selection-button.c
> --- mail/em-folder-selection-button.c	17 Jun 2005 15:20:29 -0000	1.13
> +++ mail/em-folder-selection-button.c	21 Nov 2005 08:23:45 -0000
> @@ -120,7 +120,7 @@ static void
>  set_contents_unselected (EMFolderSelectionButton *button)
>  {	
>  	gtk_image_set_from_pixbuf (GTK_IMAGE (button->priv->icon), NULL);
> -	gtk_label_set_text (GTK_LABEL (button->priv->label), _("<click here
> to select a folder>"));
> +	gtk_label_set_text (GTK_LABEL (button->priv->label), _("Select Folder"));
>  }
> 
>  static void
> Index: filter/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/filter/ChangeLog,v
> retrieving revision 1.427
> diff -u -p -r1.427 ChangeLog
> --- filter/ChangeLog	24 Aug 2005 03:04:19 -0000	1.427
> +++ filter/ChangeLog	21 Nov 2005 08:24:23 -0000
> @@ -1,3 +1,9 @@
> +2005-11-21  Johnny Jacob <johnnyjacob gmail com>
> +	
> +	** See bug 246230
> +	
> +	* filter-rule.c (get_widget) : Added labels for the criteria table
> +
>  2005-08-23  Not Zed  <NotZed Ximian com>
> 
>  	* filter-rule.c (get_widget): cast.
> Index: filter/filter-rule.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/filter/filter-rule.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 filter-rule.c
> --- filter/filter-rule.c	24 Aug 2005 03:04:19 -0000	1.69
> +++ filter/filter-rule.c	21 Nov 2005 08:24:56 -0000
> @@ -788,7 +788,7 @@ get_widget (FilterRule *fr, struct _Rule
>  	   rule parts need to be a vbox */
>  	vbox = gtk_vbox_new (FALSE, 6);
> 
> -	label = gtk_label_new_with_mnemonic (_("_Rule name:"));
> +	label = gtk_label_new_with_mnemonic (_("_Filter name:"));
>  	name = gtk_entry_new ();
>  	gtk_label_set_mnemonic_widget ((GtkLabel *)label, name);
> 
> @@ -816,7 +816,7 @@ get_widget (FilterRule *fr, struct _Rule
>  	gtk_box_pack_start (GTK_BOX (vbox), hbox, FALSE, FALSE, 0);
>  	gtk_widget_show (hbox);
> 
> -	text = g_strdup_printf("<b>%s</b>", _("If"));
> +	text = g_strdup_printf("<b>%s</b>", _("Perform actions on items that
> meet the following criteria"));
>  	label = gtk_label_new (text);
>  	gtk_label_set_use_markup (GTK_LABEL (label), TRUE);
>  	gtk_misc_set_alignment (GTK_MISC (label), 0, 0.5);
> @@ -855,9 +855,9 @@ get_widget (FilterRule *fr, struct _Rule
>  	gtk_box_pack_start (GTK_BOX (hbox), add, FALSE, FALSE, 0);
> 
>  	if (f->flags & RULE_CONTEXT_GROUPING) {
> -		const char *thread_types[] = { N_("if all criteria are met"),
> N_("if any criteria are met") };
> +		const char *thread_types[] = { N_("If all criteria are met"),
> N_("If any criteria are met") };
> 
> -		label = gtk_label_new (_("Execute actions"));
> +		label = gtk_label_new (_("Perform actions:"));
>  		menu = gtk_menu_new ();
>  	
>  		for (i=0;i<2;i++) {
> @@ -901,6 +901,22 @@ get_widget (FilterRule *fr, struct _Rule
>  	}
>  	
>  	gtk_box_pack_start (GTK_BOX (inframe), hbox, FALSE, FALSE, 3);
> +
> +	hbox=gtk_hbox_new(FALSE,35);
> +        gtk_box_pack_start(GTK_BOX(inframe),hbox,FALSE,FALSE,0);
> +        gtk_widget_show(hbox);
> +
> +        label=gtk_label_new("Field");
> +        gtk_box_pack_start(GTK_BOX(hbox),label,FALSE,FALSE,50);
> +        gtk_widget_show(label);
> +
> +        label=gtk_label_new("Condition");
> +        gtk_box_pack_start(GTK_BOX(hbox),label,FALSE,FALSE,25);
> +        gtk_widget_show(label);
> +
> +        label=gtk_label_new("Value");
> +        gtk_box_pack_start(GTK_BOX(hbox),label,FALSE,FALSE,40);
> +        gtk_widget_show(label);

please use identical indenting here. Also, spaces are a Good Thing(tm).

hbox<SPACE>=<SPACE>gtk_hbox_new<SPACE>(GTK_BOX<SPACE>(inframe),<SPACE>...

copy the spacing used in the surrounding code

You should also realise that using an hbox to pack labels into for the
column headers won't work because they won't always align with the
action items (you'd have noticed this if you tested your patch).


Also, I think the "table" label idea was shot down because it's not an
actual table with columns, some have more or less than the 3 columns.

see here: http://bugzilla.gnome.org/show_bug.cgi?id=246227



That said, I don't see any comments from our UI people accepting these
label changes as good ideas (they may or may not be, but until I see a
blessing from the UI dept. I personally would not accept the changes
into CVS - altho I'm no longer a developer on Evolution, so I can't
accept/reject - only offer suggestions)

Jeff

-- 
Jeffrey Stedfast
Evolution Hacker - Novell, Inc.
fejj ximian com  - www.novell.com