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

[Maxx Cao <maxx cao sun com>]

Hi Jeff,

Thanks a lot for your rapid comments!

As you suggested, I've changed the warning message to

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

I think it's clear enough and not too long. Users can understand what canceling
means, don't they?

I've attached the new patch here. Please review, thanks.

Jeffrey Stedfast wrote:

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);
    

-- 
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 09:14:13 -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 09:14:15 -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,30 @@
 	gtk_widget_destroy (dialog);
 }
 
+static gboolean
+rule_editor_key_press (GtkWidget *widget, GdkEventKey *event, gpointer data)
+{
+	if (event->keyval == GDK_Escape) {
+		GtkWidget *confirm_dialog;
+		int response;
+
+		confirm_dialog = gtk_message_dialog_new (widget, GTK_DIALOG_MODAL,
+							 GTK_MESSAGE_QUESTION, GTK_BUTTONS_YES_NO,
+							 _("You are about to cancel editing this rule. Are you sure you want to
+cancel?"));
+
+		response = gtk_dialog_run (GTK_DIALOG (confirm_dialog));
+		gtk_widget_destroy (confirm_dialog);
+
+		if (response == GTK_RESPONSE_OK)
+			gtk_dialog_response (widget, GTK_RESPONSE_YES);
+
+		return TRUE;
+	}
+
+	return FALSE;
+}
+
 static void
 rule_add (GtkWidget *widget, RuleEditor *re)
 {
@@ -297,6 +323,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 +406,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);