After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 163317 - HIGfication of addto dialog
HIGfication of addto dialog
Status: RESOLVED FIXED
Product: gnome-panel
Classification: Other
Component: panel
git master
Other Linux
: Normal normal
: ---
Assigned To: Panel Maintainers
Panel Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-01-08 10:44 UTC by Luca Ferretti
Modified: 2005-01-12 19:58 UTC
See Also:
GNOME target: ---
GNOME version: 2.9/2.10


Attachments
The proposed patch (2.87 KB, patch)
2005-01-08 10:45 UTC, Luca Ferretti
committed Details | Review
The current version of dialog. (71.61 KB, image/png)
2005-01-11 04:16 UTC, Dennis Cranston
  Details
The previous version of the dialog. (75.01 KB, image/png)
2005-01-11 04:17 UTC, Dennis Cranston
  Details
Zoomed screenshot using Traditional theme (12.16 KB, image/png)
2005-01-11 09:31 UTC, Luca Ferretti
  Details
A patch against GTK+ (3.78 KB, patch)
2005-01-11 15:47 UTC, Luca Ferretti
none Details | Review
A patch against gnome-panel (739 bytes, patch)
2005-01-11 15:48 UTC, Luca Ferretti
none Details | Review

Description Luca Ferretti 2005-01-08 10:44:44 UTC
Changes, based on HIG requests:

* Use "Add to Panel" window title 'cause the menu entry to call this dialog is
"Add to Panel..." (see Windows->Dialog section)

* Don't use bold for the list header label (see Controls->Lists)

* Invert CANCEL and RESPONSE_BACK buttons order (see
Windows->Alerts->AlertsButtons and/or Windows->Dialogs->AdditionalButtons) 

* Fix spacing/border sizes.

--Not fixed--
* When invoked by "Add to Drawer..." pop-up menu entry the title is "Add to
Panel" and the label, when the drawer has no name is "Select an _item to add to
the panel:"
Comment 1 Luca Ferretti 2005-01-08 10:45:30 UTC
Created attachment 35656 [details] [review]
The proposed patch
Comment 2 Vincent Untz 2005-01-09 21:48:36 UTC
Thanks Luca. I committed a slightly modified version of your patch and fixed the
drawer problem.
Comment 3 Dennis Cranston 2005-01-10 00:35:52 UTC
This commit uses incorrect widget spacing.  See the original fix from bug 144857.

This part of the patch should be reverted:
-	gtk_container_set_border_width (GTK_CONTAINER (dialog->addto_dialog), 5);
-	gtk_box_set_spacing (GTK_BOX (GTK_DIALOG (dialog->addto_dialog)->vbox), 2);
+	gtk_container_set_border_width (GTK_CONTAINER (dialog->addto_dialog), 6);
+	gtk_box_set_spacing (GTK_BOX (GTK_DIALOG (dialog->addto_dialog)->vbox), 12);
 	g_signal_connect (G_OBJECT (dialog->addto_dialog), "response",
 			  G_CALLBACK (panel_addto_dialog_response), dialog);
 	g_signal_connect (dialog->addto_dialog, "destroy",
 			  G_CALLBACK (panel_addto_dialog_destroy), dialog);
 
 	vbox = gtk_vbox_new (FALSE, 12);
-	gtk_container_set_border_width (GTK_CONTAINER (vbox), 5);
+	gtk_container_set_border_width (GTK_CONTAINER (vbox), 6);
Comment 4 Vincent Untz 2005-01-10 07:30:58 UTC
Luca, Dennis: ok, one of you should explain why the spacing should be (5,2) or
(6,12). I looked quickly at
http://developer.gnome.org/projects/gup/hig/2.0/design-window.html#window-layout-spacing
and it seems it should be (6,12)...
Comment 5 Luca Ferretti 2005-01-10 16:58:31 UTC
The HIG page you linked says "Leave a 12-pixel border between the edge of the
window and the nearest controls"(1). Moreover in
http://developer.gnome.org/projects/gup/hig/2.0/windows-alert.html#alert-spacing
you can read "Add one line break at the standard font size below both the
primary and secondary text, or 24 pixels if you are using Glade"(2).

So IMHO
gtk_container_set_border_width (GTK_CONTAINER (dialog), 6) AND
gtk_container_set_border_width (GTK_CONTAINER (vbox), 6); are used to respect
request #1, and gtk_box_set_spacing (GTK_BOX (GTK_DIALOG (dialog)->vbox), 12);
is use to respect request #2.

Try do do the same in glade and count pixels :-)
Comment 6 Vincent Untz 2005-01-10 17:21:37 UTC
Dennis: do you agree? If so, can we close the bug?
Comment 7 Dennis Cranston 2005-01-11 04:15:40 UTC
Here are two screenshots measuring the distance in pixels from the border of the
dialog to the action area and the vbox, as well as the distance in pixels
between the dialog's action area and vbox. 
Comment 8 Dennis Cranston 2005-01-11 04:16:41 UTC
Created attachment 35800 [details]
The current version of dialog.
Comment 9 Dennis Cranston 2005-01-11 04:17:08 UTC
Created attachment 35801 [details]
The previous version of the dialog.
Comment 10 Dennis Cranston 2005-01-11 04:24:44 UTC
The current version in cvs has 18 pixels between the dialog's vbox and the
window border.  It has 17 pixels between the dialog's action area and the window
border.  And, the distance between the action area and vbox is 23 pixels.

The previous version has 16 pixels between the dialog's vbox and the window
border.  It has 16 pixels between the dialog's action area and the window
border.  And, the distance between the action area and vbox is 12 pixels.
Comment 11 Luca Ferretti 2005-01-11 09:31:15 UTC
Created attachment 35818 [details]
Zoomed screenshot using Traditional theme
Comment 12 Luca Ferretti 2005-01-11 09:42:28 UTC
I count 13 pixels between button's right side and window border inner side
(maybe the metacity theme adds 1 pixel? Or GTK+?)

The scroller has an additional pixel (see it's not aligned with right side of
button). Maybe added by GTK+ and/or theme? Maybe we have to suppose the same
pixel is added at the bottom side of the scroller (this could meand that 23->22)

Moreover the default buttons spacing is not 6 px, but 10 px.

But WHERE is the problem? It's in GTK+ code, in themes rc files or in metacity
theme descriptions?


<quote source="gtk/gtkdialog.c">
gtk_widget_class_install_style_property (widget_class,
                                           g_param_spec_int ("content_area_border",
                                                             P_("Content area
border"),
                                                             P_("Width of border
around the main dialog area"),
                                                             0,
                                                             G_MAXINT,
                                                             2,
                                                             G_PARAM_READABLE));
  gtk_widget_class_install_style_property (widget_class,
                                           g_param_spec_int ("button_spacing",
                                                             P_("Button spacing"),
                                                             P_("Spacing between
buttons"),
                                                             0,
                                                             G_MAXINT,
                                                             10,
                                                             G_PARAM_READABLE));

  gtk_widget_class_install_style_property (widget_class,
                                           g_param_spec_int ("action_area_border",
                                                             P_("Action area
border"),
                                                             P_("Width of border
around the button area at the bottom of the dialog"),
                                                             0,
                                                             G_MAXINT,
                                                             5,
                                                             G_PARAM_READABLE));
</quote>

IMHO a lot of problems come direcly from GTK+. Someone will have to fix all
those non-HIG spacing/border sizes.
Comment 13 Vincent Untz 2005-01-11 09:46:28 UTC
So... Do you both agree that this part of the patch should be reverted?

(I agree that some day, we'll need to fix GTK+)
Comment 14 Luca Ferretti 2005-01-11 09:53:58 UTC
Or a fine tuning :-)

I'll try something later.
Comment 15 Luca Ferretti 2005-01-11 15:47:03 UTC
Created attachment 35833 [details] [review]
A patch against GTK+

A patch against GTK+ to make the default GtkDialog a little more HIG compliant.
Comment 16 Luca Ferretti 2005-01-11 15:48:55 UTC
Created attachment 35834 [details] [review]
A patch against gnome-panel

A patch against gnome-panel if/when the proposed patch above will be applied.
The only needed spacing tuning are 6 pixels around the topmost container added
in the GtkDialog.
Comment 17 Luca Ferretti 2005-01-11 15:50:39 UTC
Gosh... fix it inside gnome-panel is a mess. And it's stupid.

So my proposal is
 1. fix defaul values in GTK+ for GtkDialog (see attachement )
 2. apply proposed patch to current gnome-panel in attachment 

I did not open any bug against Gtk+. Vincent, can yo do it?

Issues.
- Will GTK+ maintainers accept this patch _now_?

- If the GTK+ patch will be applied in 2.6.2, a lot of fine-tuned-by-hand
dialogs will appear wrong. The proposed GTK+ patch manage all spacing/border
directly in GTK+. A developer just have to insert a container in the dialog and
set the spacing of this container to 6 pixels. (see the propose changes to
current panel code in 

- The GTK+ proposed patch is sub-optimal. Ideally and IMHO a GtkDialog should be
something like:

GtkWindow +
          |
          +-> GtkVBox+ (border=12, spacing=24)
                     |
                     +-> GtkButtonBox+ (pack=end, expand=false, spacing=6)
                     |               |
                     |               +buttons...
                     |
                     +container/widget...

(try it with glade)
So developer will just have to insert the proper container in the dialog, and
they will not have to set the (additional) border size. Unfortunately this will
break APIs, so I'll propose for GTK3.
Comment 18 Dennis Cranston 2005-01-12 01:12:41 UTC
If gtk+ is patched then you will have to remove/adjust all the:
  gtk_container_set_border_width (GTK_CONTAINER (dialog->addto_dialog), 5);
  gtk_box_set_spacing (GTK_BOX (GTK_DIALOG (dialog->addto_dialog)->vbox), 2);

This padding is currently used in gnome-utils, gnome-media,
gnome-control-center, gnome-applets, procman, gconf-editor, and probably others.
Comment 19 Dennis Cranston 2005-01-12 06:43:06 UTC
Lucia,  I agree that we should try to get the spacing fixed in gtk+.  The gtk+
maintainers will probably shy away from make your proposed spacing adjustments
in the 2.x series, but they should seriously consider it for version 3.

When using the traditional theme the previous version of the dialog has 12
pixels between the dialog's vbox and the window border and 12 pixels between the
dialog's action area and the window border.  Also, the distance between the
action area and vbox was 12 pixels.  So, for the time being we revert your
spacing adjustments.
Comment 20 Vincent Untz 2005-01-12 08:33:25 UTC
I reverted the changes.
Dennis, Luca: this should really be fixed in GTK+. I'm pretty sure there's
already a bug against GTK+ for this but I can't find it...
Comment 21 Dennis Cranston 2005-01-12 19:55:09 UTC
Luca, Vincent,  I opened bug 163850 to get address the gtk+ issues.  Thanks.
Comment 22 Dennis Cranston 2005-01-12 19:58:44 UTC
Luca, I see you have made similar changes to a couple dialogs in procman.  You
might want to correct those changes for the time being.  Thanks.