[evolution-patches] Re: Prettify folder property dialog

["Christian Neumair" <chris gnome-de org>]

Rodney Dawes schrieb: 

> Some comments below. 
>
> On Mër , 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. 

> > +       title = g_strdup_printf ("Properties of %s", name);
>
> As Michael said, this needs to be translated.

Indeed. 

> > +       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. 

> > +       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. 

> > +       g_free (title);
>
> You should probably position this directly after using "title", or at
> the end of the function, when we are definately done using the variables
> in the scope of the function.

Hm, ok. 

> > +       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. 

regs,
Chris