Re: [evolution-patches] move e-charset-picker to GtkDialog

[Larry Ewing <lewing ximian com>]

On Sat, 2003-05-17 at 23:46, Jeffrey Stedfast wrote:
> A few isues...
> 
> 
> +       window = gtk_widget_get_toplevel (menu);
> +       if (!GTK_WIDGET_TOPLEVEL (menu))
> +               window = gtk_widget_get_ancestor (item,
> GTK_TYPE_WINDOW);
> 
> shouldn't this be:
> 
> +       if (!GTK_WIDGET_TOPLEVEL (window))
> 

yeah that was a typo, the if logic is probably pointless anyway but I
figured I'd be paranoid and make sure I couldn't possibly make things
worse than before.

> otherwise there's no point to the first get_toplevel() call.
> 
> +       g_signal_connect (G_OBJECT (entry), "activate",
> 
> you don't need the G_OBJECT() macro there, since g_signal_connect takes
> a gpointer argument.
> 

yup

> +       g_object_ref (dialog);
> +       if (gtk_dialog_run (dialog) == GTK_RESPONSE_OK) {
> 
> there's no reason to g_object_ref() there as far as I can tell. Also,
> you'll end up leaking the dialog if the response is OK and the inner
> if-statements also pass.
> 
> same g_object_ref/unref combo doesn't need to be there in the next
> function either.
> 

As I mentioned on irc I did this because it is marked
GTK_DIALOG_DESTROY_WITH_PARENT and I'm not certain gtk will block all
possible ways of destroying the parent even with it being modal.  I
figured the ref was harmless and might avoid problem.  Of course the ref
is only harmless if you unref it everywhere ;)

I left in the reffing (with the fix) but I'll happily take it out if
someone objects.  Also I made up the padding, can someone who knows what
we they are doing tell me what should be done HIG wise?

Thanks for catching all these problems Jeff, and happy belated birthday.

--Larry

Index: widgets/misc/e-charset-picker.c
===================================================================
RCS file: /cvs/gnome/evolution/widgets/misc/e-charset-picker.c,v
retrieving revision 1.14
diff -u -p -r1.14 e-charset-picker.c
--- widgets/misc/e-charset-picker.c	25 Mar 2003 15:48:09 -0000	1.14
+++ widgets/misc/e-charset-picker.c	18 May 2003 05:53:27 -0000
@@ -29,13 +29,13 @@
 #include <iconv.h>
 
 #include <gtk/gtkvbox.h>
+#include <gtk/gtkentry.h>
+#include <gtk/gtkstock.h>
 #include <gtk/gtklabel.h>
 #include <gtk/gtkmenuitem.h>
 #include <gtk/gtkoptionmenu.h>
 #include <gtk/gtksignal.h>
 
-#include <libgnomeui/gnome-dialog.h>
-#include <libgnomeui/gnome-dialog-util.h>
 #include <libgnome/gnome-i18n.h>
 
 #include <bonobo/bonobo-ui-node.h>
@@ -209,32 +209,64 @@ add_other_charset (GtkWidget *menu, GtkW
 }
 
 static void
-other_charset_callback (char *new_charset, gpointer data)
+activate_entry (GtkWidget *entry, GtkDialog *dialog)
 {
-	char **out = data;
-
-	*out = new_charset;
+	gtk_dialog_response (dialog, GTK_RESPONSE_OK);
 }
 
 static void
 activate_other (GtkWidget *item, gpointer menu)
 {
-	GtkWidget *window, *dialog;
+	GtkWidget *window, *entry, *label;
 	char *old_charset, *new_charset;
+	GtkDialog *dialog;
+
+	window = gtk_widget_get_toplevel (menu);
+	if (!GTK_WIDGET_TOPLEVEL (window))
+		window = gtk_widget_get_ancestor (item, GTK_TYPE_WINDOW);
+
+	old_charset = g_object_get_data(menu, "other_charset");
+
+	dialog = GTK_DIALOG (gtk_dialog_new_with_buttons (_("Character Encoding"),
+							  GTK_WINDOW (window),
+							  GTK_DIALOG_MODAL | GTK_DIALOG_DESTROY_WITH_PARENT,
+							  GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
+							  GTK_STOCK_OK, GTK_RESPONSE_OK,
+							  NULL));
+
+	gtk_dialog_set_default_response (dialog, GTK_RESPONSE_OK);
+
+	label = gtk_label_new (_("Enter the character set to use"));
+	gtk_label_set_line_wrap (GTK_LABEL (label), TRUE);
+	gtk_misc_set_alignment (GTK_MISC (label), 0.0, 0.0);
+	gtk_widget_show (label);
+
+	entry = gtk_entry_new ();
+	if (old_charset)
+		gtk_entry_set_text (GTK_ENTRY (entry), old_charset);
+	g_signal_connect (entry, "activate",
+			  G_CALLBACK (activate_entry), dialog);
+			  
+	gtk_container_set_border_width (GTK_CONTAINER (dialog->vbox), 6);
+	gtk_box_pack_start (GTK_BOX (dialog->vbox), label, FALSE, FALSE, 6);
+	gtk_box_pack_start (GTK_BOX (dialog->vbox), entry, FALSE, FALSE, 6);
 
-	window = gtk_widget_get_ancestor (item, GTK_TYPE_WINDOW);
-	old_charset = g_object_get_data(G_OBJECT(menu), "other_charset");
-	dialog = gnome_request_dialog (FALSE,
-				       _("Enter the character set to use"),
-				       old_charset, 0, other_charset_callback,
-				       &new_charset, GTK_WINDOW (window));
-	gnome_dialog_run_and_close (GNOME_DIALOG (dialog));
-
-	if (new_charset) {
-		if (add_other_charset (menu, item, new_charset))
-			return;
-		g_free (new_charset);
+	gtk_widget_show_all (GTK_WIDGET (dialog));
+
+	g_object_ref (dialog);
+	if (gtk_dialog_run (dialog) == GTK_RESPONSE_OK) {
+		new_charset = (char *)gtk_entry_get_text (GTK_ENTRY (entry));
+
+	       	if (*new_charset) {
+			if (add_other_charset (menu, item, new_charset)) {
+				gtk_widget_destroy (GTK_WIDGET (dialog));
+				g_object_unref (dialog);
+				return;
+			}
+		}
 	}
+	gtk_widget_destroy (GTK_WIDGET (dialog));
+	g_object_unref (dialog);
 
 	/* Revert to previous selection */
 	select_item (GTK_MENU_SHELL (menu), g_object_get_data(G_OBJECT(menu), "activated_item"));
@@ -348,36 +380,40 @@ char *
 e_charset_picker_dialog (const char *title, const char *prompt,
 			 const char *default_charset, GtkWindow *parent)
 {
-	GnomeDialog *dialog;
+	GtkDialog *dialog;
 	GtkWidget *label, *omenu, *picker;
-	int button;
-	char *charset;
+	char *charset = NULL;
 
-	dialog = GNOME_DIALOG (gnome_dialog_new (title, GNOME_STOCK_BUTTON_OK,
-						 GNOME_STOCK_BUTTON_CANCEL,
-						 NULL));
-	if (parent)
-		gnome_dialog_set_parent (dialog, parent);
+	dialog = GTK_DIALOG (gtk_dialog_new_with_buttons (title,
+							  parent,
+							  GTK_DIALOG_MODAL | GTK_DIALOG_DESTROY_WITH_PARENT,
+							  GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
+							  GTK_STOCK_OK, GTK_RESPONSE_OK,
+							  NULL));
+
+	gtk_dialog_set_default_response (dialog, GTK_RESPONSE_OK);
 
 	label = gtk_label_new (prompt);
 	gtk_label_set_line_wrap (GTK_LABEL (label), TRUE);
 	gtk_misc_set_alignment (GTK_MISC (label), 0.0, 0.0);
+
 	picker = e_charset_picker_new (default_charset);
 	omenu = gtk_option_menu_new ();
 	gtk_option_menu_set_menu (GTK_OPTION_MENU (omenu), picker);
 
-	gtk_container_set_border_width (GTK_CONTAINER (dialog->vbox), 4);
-	gtk_box_pack_start (GTK_BOX (dialog->vbox), label, FALSE, FALSE, 4);
-	gtk_box_pack_start (GTK_BOX (dialog->vbox), omenu, FALSE, FALSE, 4);
+	gtk_container_set_border_width (GTK_CONTAINER (dialog->vbox), 6);
+	gtk_box_pack_start (GTK_BOX (dialog->vbox), label, FALSE, FALSE, 6);
+	gtk_box_pack_start (GTK_BOX (dialog->vbox), omenu, FALSE, FALSE, 6);
 
 	gtk_widget_show_all (GTK_WIDGET (dialog));
-	button = gnome_dialog_run (dialog);
 
-	if (button == 0)
+	g_object_ref (dialog);
+
+	if (gtk_dialog_run (dialog) == GTK_RESPONSE_OK)
 		charset = e_charset_picker_get_charset (picker);
-	else
-		charset = NULL;
-	gnome_dialog_close (dialog);
+
+	gtk_widget_destroy (GTK_WIDGET (dialog));
+	g_object_unref (dialog);
 
 	return charset;
 }