[evolution-patches] Re: Prettify folder property dialog



On Mër , 2004-06-02 at 16:22 +0200, Christian Neumair wrote:
> 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. 

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.

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

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

-- dobey



Attachment: signature.asc
Description: This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]