Re: [evolution-patches] Seeking review for bug #47751 & #47752

[Jeffrey Stedfast <fejj ximian com>]

On Wed, 2003-08-20 at 02:56, Maxx Cao wrote:
> Hi,
> 
> This patch fixes bug #47751 (New/Edit filter windows do not close on
> Esc) and #47752(New/Edit vfolder windows do not close on Esc).
> Now when user presses ESC on the rule editor, he/she will be asked
> whether he/she wants to close the dialog.
> 
> Please review. Thanks.
> 
> -- 
> Best Regards
> Maxx
> 
> ______________________________________________________________________
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/filter/ChangeLog,v
> retrieving revision 1.373
> diff -u -r1.373 ChangeLog
> --- ChangeLog	5 Aug 2003 21:12:14 -0000	1.373
> +++ ChangeLog	20 Aug 2003 06:49:51 -0000
> @@ -1,3 +1,12 @@
> +2003-08-20  Maxx Cao  <maxx cao sun com>
> +
> +	** For bug #47751 & #47752 
> +
> +	* rule-editor.c (rule_editor_key_press): Use Esc Key to close the
> +	editor.
> +	(rule_add): connect "key_press_event" of the "Add Rule" dialog.
> +	(rule_edit): and the "Edit Rule" dialog.
> +
>  2003-08-05  Not Zed  <NotZed Ximian com>
>  
>  	** See bug #42636.
> Index: rule-editor.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/filter/rule-editor.c,v
> retrieving revision 1.54
> diff -u -r1.54 rule-editor.c
> --- rule-editor.c	29 Apr 2003 17:08:40 -0000	1.54
> +++ rule-editor.c	20 Aug 2003 06:49:52 -0000
> @@ -32,6 +32,8 @@
>  
>  #include "rule-editor.h"
>  
> +#include <gdk/gdkkeysyms.h>
> +
>  static int enable_undo = 0;
>  
>  void rule_editor_add_undo (RuleEditor *re, int type, FilterRule *rule, int rank, int newrank);
> @@ -270,6 +272,27 @@
>  	gtk_widget_destroy (dialog);
>  }
>  
> +static gboolean
> +rule_editor_key_press (GtkWidget *widget, GdkEventKey *event, gpointer data)
> +{
> +	if (event->keyval == GDK_Escape){

please add a space before the {

> +		GtkWidget *confirm_dialog;
> +		int response;
> +
> +		confirm_dialog = gtk_message_dialog_new (widget, GTK_DIALOG_MODAL,
> +							 GTK_MESSAGE_QUESTION, GTK_BUTTONS_OK_CANCEL,
> +							 "Are you sure to quit now?");
> +

the message text needs to be marked for translation and also I suggest a
better wording, such as:

"You are about to cancel editing this rule. Are you sure you want to
cancel?"

your message is too ambiguous - am I quitting the application? the rule
editor? or what?

but perhaps the message text should warn the user that cancelling will
discard all changes?

probably also want to change OK_CANCEL to YES_NO.

> +		response = gtk_dialog_run (GTK_DIALOG (confirm_dialog));
> +		gtk_widget_destroy (confirm_dialog);
> +
> +		if (response == GTK_RESPONSE_OK)
> +			gtk_dialog_response (widget, GTK_RESPONSE_CANCEL);

I'd prefer an empty line here.

> +		return TRUE;
> +	}

and an empty line here too.

> +	return FALSE;
> +}
> +
>  static void
>  rule_add (GtkWidget *widget, RuleEditor *re)
>  {
> @@ -297,6 +320,7 @@
>  	gtk_box_pack_start (GTK_BOX (GTK_DIALOG (re->dialog)->vbox), rules, TRUE, TRUE, 3);
>  	
>  	g_signal_connect (re->dialog, "response", G_CALLBACK (add_editor_response), re);
> +	g_signal_connect (re->dialog, "key_press_event", G_CALLBACK (rule_editor_key_press), NULL);
>  	g_object_weak_ref ((GObject *) re->dialog, (GWeakNotify) editor_destroy, re);
>  	
>  	gtk_widget_set_sensitive (GTK_WIDGET (re), FALSE);
> @@ -379,6 +403,7 @@
>  	gtk_box_pack_start (GTK_BOX (GTK_DIALOG (re->dialog)->vbox), rules, TRUE, TRUE, 3);
>  	
>  	g_signal_connect (re->dialog, "response", G_CALLBACK (edit_editor_response), re);
> +	g_signal_connect (re->dialog, "key_press_event", G_CALLBACK (rule_editor_key_press), NULL);
>  	g_object_weak_ref ((GObject *) re->dialog, (GWeakNotify) editor_destroy, re);
>  	
>  	gtk_widget_set_sensitive (GTK_WIDGET (re), FALSE);



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