GNOME.org
 

[evolution-patches] Re: Prettify folder property dialog

Am Mi, den 02.06.2004, 12:26 Uhr -0400 schrieb Rodney Dawes:
> On M�, 2004-06-02 at 16:22 +0200, Christian Neumair wrote:
> > Rodney Dawes schrieb: 
> > 
> > > Some comments below. 
> > > 
> > > On M�, 2004-06-02 at 12:55 +0200, Christian Neumair wrote:
> > >> The attached patch pretifies the folder property dialog and moves it
> > >> towards HIG compliance.
> > >> Any comments/suggestions?
> > > 
> > >> @@ -56,7 +56,7 @@
> > >>         CamelArgV *argv = prop_data->argv;
> > >>         int i;
> > >>         
> > >> -       if (response != GTK_RESPONSE_OK) {
> > >> +       if (response != GTK_RESPONSE_CLOSE) {
> > >>                 gtk_widget_destroy (dialog);
> > >>                 return;
> > >>         }
> > > 
> > > Shouldn't the response always be GTK_RESPONSE_CLOSE? Why would we check
> > > for !=? We're just going to close it all the time no matter what we get
> > > here, aren't we?
> > 
> > There is also a destroy reponse. 
> 
> And it presumably also destroys the window. So, I don't see why we need
> to handle them differently, unless we are doing something when we click
> the "Close" button.

You're right. I've removed that check.

> > >> +       gtk_window_set_resizable (GTK_WINDOW (dialog), FALSE);
> > > 
> > > All dialogs should be resizeable really. Is there any reason not to
> > > allow resizing it?
> > 
> > Is there any reason why "(a)ll dialogs should be resizeable"? It simply 
> > makes no sense to resize this dialog. 
> 
> And most people won't resize it, so why should we enforce it not being
> resized? And, resizing the dialog is a good test of whether the contents
> are packed properly or not.

I still think size changes are nonsense. Anyway, the attached patch re-
allows this now.

> > >> +       gtk_container_set_border_width (GTK_CONTAINER (dialog), 5);
> > >> +       gtk_box_set_spacing (GTK_BOX (GTK_DIALOG (dialog)->vbox), 2);
> > > 
> > > These are not HIG-compliant values. You should realize the dialog
> > > instead, and set the border width for the child of dialog->vbox to 12,
> > > the border width of dialog->vbox to 0, and the border width of
> > > dialog->action_area to 12.
> > 
> > They are. People keep thinking that. There is an additional border of 2 for 
> > GtkDialogs which is a style property. Hacking around with style properties 
> > is even more evil than doing what I did. I've written down more than once 
> > how to achieve HIG compliance as long as GtkDialog is broken. We have to be 
> > pragmatic about that. Gtk 3 will have it fixed for sure. 
> 
> Setting the correct values outside of the style properties is in no way
> "hacking around" with style properties. It is a way to enforce the HIG.
> Setting odd border widths looks weird when you go through the code. The
> way it has been getting done in Evolution, is as I stated, which is to
> set the border widths properly to values of 12. Your method does not get
> the correct spacing between the dialog contents and the buttons, which
> should be 24 pixels. What GTK+ 3.x will and won't do is not of concern
> here. It doesn't exist. What is of concern, is what GTK+ 2.x does do.
> 
> > >> +       gtk_container_set_border_width (GTK_CONTAINER (table), 5);
> > > 
> > > Again. This is not a HIG-compliant value. This should probably be 12.
> > > 5 + 5 != 12, which is what the HIG states should be the border between
> > > the contents of the dialog, and the window's border.
> > 
> > See above. I know the HIG pretty well. 
> 
> So does everyone else. Everyone knows it differently, because the HIG is
> a self-contradictory document in many places. This makes it difficult to
> come up with ONE way to do things. Your way is not the only one that
> leads to a path of HIG-compliance. And, I believe using proper values
> that are stated in the HIG make it easier for people to find HIG-related
> issues when going through the code. It also seems to enforce the HIG in
> a slightly better manner.

OK, the problem is that I used to design glade dialogs from within the
glade editor. glade silently adds these infamous 2 px without asking. I
was conveying this to the code. This patch now does it in a clean
fashion.

regs,
 Chris

Index: mail/em-folder-properties.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/em-folder-properties.c,v
retrieving revision 1.2
diff -u -r1.2 em-folder-properties.c
--- mail/em-folder-properties.c	6 Feb 2004 06:35:47 -0000	1.2
+++ mail/em-folder-properties.c	3 Jun 2004 10:10:21 -0000
@@ -56,11 +56,6 @@
 	CamelArgV *argv = prop_data->argv;
 	int i;
 	
-	if (response != GTK_RESPONSE_OK) {
-		gtk_widget_destroy (dialog);
-		return;
-	}
-	
 	for (i = 0; i < argv->argc; i++) {
 		CamelArg *arg = &argv->argv[i];
 		
@@ -107,7 +102,7 @@
 	CamelArgV *argv;
 	GSList *list, *l;
 	gint32 count, i;
-	char *name;
+	char *name, *title;
 	char countstr[16];
 	int row = 0, total=0, unread=0;
 	
@@ -117,54 +112,58 @@
 	camel_object_get (folder, NULL, CAMEL_FOLDER_PROPERTIES, &list, CAMEL_FOLDER_NAME, &name,
 			  CAMEL_FOLDER_TOTAL, &total, CAMEL_FOLDER_UNREAD, &unread, NULL);
 	
-	dialog = gtk_dialog_new_with_buttons (_("Folder properties"), NULL,
-					      GTK_DIALOG_DESTROY_WITH_PARENT,
-					      GTK_STOCK_OK, GTK_RESPONSE_OK,
+	title = g_strdup_printf (_("Properties of %s"), name);
+	dialog = gtk_dialog_new_with_buttons (title, NULL,
+					      GTK_DIALOG_DESTROY_WITH_PARENT | GTK_DIALOG_NO_SEPARATOR,
+					      GTK_STOCK_CLOSE, GTK_RESPONSE_CLOSE,
 					      NULL);
-	
+	g_free (title);
+	gtk_widget_ensure_style (dialog);
+	gtk_container_set_border_width (GTK_CONTAINER (GTK_DIALOG (dialog)->vbox), 0);
+	gtk_container_set_border_width (GTK_CONTAINER (GTK_DIALOG (dialog)->action_area), 12);
+
 	/* TODO: maybe we want some basic properties here, like message counts/approximate size/etc */
-	w = gtk_frame_new (_("Properties"));
-	gtk_widget_show (w);
-	gtk_box_pack_start ((GtkBox *) ((GtkDialog *) dialog)->vbox, w, TRUE, TRUE, 6);
-	
 	table = gtk_table_new (g_slist_length (list) + 3, 2, FALSE);
+	gtk_table_set_row_spacings (GTK_TABLE (table), 6);
+	gtk_table_set_col_spacings (GTK_TABLE (table), 12);
+	gtk_container_set_border_width (GTK_CONTAINER (table), 12);
 	gtk_widget_show (table);
-	gtk_container_add ((GtkContainer *) w, table);
+	gtk_container_add (GTK_CONTAINER (GTK_DIALOG (dialog)->vbox), table);
 
 	/* TODO: can this be done in a loop? */
-	label = gtk_label_new (_("Folder Name"));
+	label = gtk_label_new (_("Folder name:"));
 	gtk_widget_show (label);
-	gtk_misc_set_alignment ((GtkMisc *) label, 1.0, 0.5);
-	gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row+1, GTK_FILL | GTK_EXPAND, 0, 3, 0);
+	gtk_misc_set_alignment ((GtkMisc *) label, 0.0, 0.5);
+	gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row+1, GTK_FILL, 0, 0, 0);
 	
 	label = gtk_label_new (name);
 	gtk_widget_show (label);
 	gtk_misc_set_alignment ((GtkMisc *) label, 0.0, 0.5);
-	gtk_table_attach ((GtkTable *) table, label, 1, 2, row, row+1, GTK_FILL | GTK_EXPAND, 0, 3, 0);
+	gtk_table_attach ((GtkTable *) table, label, 1, 2, row, row+1, GTK_FILL | GTK_EXPAND, 0, 0, 0);
 	row++;
 
-	label = gtk_label_new (_("Total messages"));
+	label = gtk_label_new (ngettext ("Total message:", "Total messages:", total));
 	gtk_widget_show (label);
-	gtk_misc_set_alignment ((GtkMisc *) label, 1.0, 0.5);
-	gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row+1, GTK_FILL | GTK_EXPAND, 0, 3, 0);
+	gtk_misc_set_alignment ((GtkMisc *) label, 0.0, 0.5);
+	gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row+1, GTK_FILL, 0, 0, 0);
 	
 	sprintf(countstr, "%d", total);
 	label = gtk_label_new (countstr);
 	gtk_widget_show (label);
 	gtk_misc_set_alignment ((GtkMisc *) label, 0.0, 0.5);
-	gtk_table_attach ((GtkTable *) table, label, 1, 2, row, row+1, GTK_FILL | GTK_EXPAND, 0, 3, 0);
+	gtk_table_attach ((GtkTable *) table, label, 1, 2, row, row+1, GTK_FILL | GTK_EXPAND, 0, 0, 0);
 	row++;
 
-	label = gtk_label_new (_("Unread messages"));
+	label = gtk_label_new (ngettext ("Unread message:", "Unread messages:", total));
 	gtk_widget_show (label);
-	gtk_misc_set_alignment ((GtkMisc *) label, 1.0, 0.5);
-	gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row+1, GTK_FILL | GTK_EXPAND, 0, 3, 0);
+	gtk_misc_set_alignment ((GtkMisc *) label, 0.0, 0.5);
+	gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row+1, GTK_FILL, 0, 0, 0);
 	
 	sprintf(countstr, "%d", unread);
 	label = gtk_label_new (countstr);
 	gtk_widget_show (label);
 	gtk_misc_set_alignment ((GtkMisc *) label, 0.0, 0.5);
-	gtk_table_attach ((GtkTable *) table, label, 1, 2, row, row+1, GTK_FILL | GTK_EXPAND, 0, 3, 0);
+	gtk_table_attach ((GtkTable *) table, label, 1, 2, row, row+1, GTK_FILL | GTK_EXPAND, 0, 0, 0);
 	row++;
 
 	/* build an arggetv/argv to retrieve/store the results */
@@ -205,14 +204,14 @@
 			w = gtk_check_button_new_with_label (prop->description);
 			gtk_toggle_button_set_active ((GtkToggleButton *) w, argv->argv[i].ca_int != 0);
 			gtk_widget_show (w);
-			gtk_table_attach ((GtkTable *) table, w, 0, 2, row, row + 1, 0, 0, 3, 3);
+			gtk_table_attach ((GtkTable *) table, w, 0, 2, row, row + 1, GTK_FILL | GTK_EXPAND, 0, 0, 0);
 			prop_data->widgets[i] = w;
 			break;
 		case CAMEL_ARG_STR:
 			label = gtk_label_new (prop->description);
-			gtk_misc_set_alignment ((GtkMisc *) label, 1.0, 0.5);
+			gtk_misc_set_alignment ((GtkMisc *) label, 0.0, 0.5);
 			gtk_widget_show (label);
-			gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row + 1, GTK_FILL | GTK_EXPAND, 0, 3, 3);
+			gtk_table_attach ((GtkTable *) table, label, 0, 1, row, row + 1, GTK_FILL, 0, 0, 0);
 			
 			w = gtk_entry_new ();
 			gtk_widget_show (w);
@@ -221,7 +220,7 @@
 				camel_object_free (folder, argv->argv[i].tag, argv->argv[i].ca_str);
 				argv->argv[i].ca_str = NULL;
 			}
-			gtk_table_attach ((GtkTable *) table, w, 1, 2, row, row + 1, GTK_FILL, 0, 3, 3);
+			gtk_table_attach ((GtkTable *) table, w, 1, 2, row, row + 1, GTK_FILL | GTK_EXPAND, 0, 0, 0);
 			prop_data->widgets[i] = w;
 			break;
 		default:
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]