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 96236 - Draw menus better
Draw menus better
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.1.x
Other other
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2002-10-19 11:30 UTC by Soren Sandmann Pedersen
Modified: 2011-02-04 16:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The patch (23.57 KB, patch)
2002-10-19 12:17 UTC, Soren Sandmann Pedersen
none Details | Review
Patch to center GtkSeparatorMenuItems (478 bytes, patch)
2003-01-19 20:01 UTC, Rodney Dawes
none Details | Review
Updated patch (25.51 KB, patch)
2003-07-01 01:57 UTC, Soren Sandmann Pedersen
none Details | Review
New patch (27.82 KB, patch)
2003-07-04 15:04 UTC, Soren Sandmann Pedersen
none Details | Review

Description Soren Sandmann Pedersen 2002-10-19 11:30:26 UTC
The patch I'll attach shortly does

        - use a black border around menus
        - increase spacing around separator menu items and center the
          separator line in its allocation
        - increase spacing around toggles and icons in menus
        - add a bit of vertical padding around menus
        - makes arrows a bit bigger

Before and after shots at

        http://www.daimi.au.dk/~sandmann/pictures/nautilus-before.png
        http://www.daimi.au.dk/~sandmann/pictures/nautilus-after.png

The black border and the bigger arrows look slightly weird with the bevels
on selected menu items. With the flat menus from #80691, the black border
makes it easier to see where one menu stops and another begins. The extra
spacing works well with both the current and the suggested color scheme.
Comment 1 Soren Sandmann Pedersen 2002-10-19 12:16:51 UTC
I should add that the patch creates four new style properties:

        - GtkMenu::vertical_padding        
        - GtkMenuItem::horizontal_padding
        - GtkMenuItem::icon_spacing
        - GtkMenuItem::arrow_spacing

Comment 2 Soren Sandmann Pedersen 2002-10-19 12:17:51 UTC
Created attachment 11686 [details] [review]
The patch
Comment 3 Soren Sandmann Pedersen 2002-10-19 12:25:58 UTC
I am putting this on the 2.2.0 milestone, because I think it should be
looked at together with the color scheme thing (#80691). Ie., if this 
is 2.4, then the color scheme should also be 2.4.
Comment 4 Owen Taylor 2002-10-19 20:06:47 UTC
Unmilestoning until I get a chance to update my bug lists.
Comment 5 Owen Taylor 2002-10-20 18:41:54 UTC
Putting on 2.4 ... too much other stuff to deal with for 2.2.
I'm not sure that the black line is consisistent with the rest
of the style of the current default theme... also if we do go 
with a black line here, there are some other places in GTK+,
like the GtkHandleBox tearoff window that would get one as well.
Comment 6 Havoc Pennington 2002-10-20 20:34:58 UTC
I think it looks significantly nicer with these changes. Cc'ing
some artists.
Comment 7 Rodney Dawes 2003-01-19 03:27:12 UTC
I'd like to have a better look at this patch. Can you update it to
apply cleanly to gtk+ 2.2.0? I just created a somewhat similar patch
to GTK+,
which centers SeparatorMenuItem, and adds a 1px border to all menuitems,
so that there is some spacing between icons. Thanks.
Comment 8 Tuomas Kuosmanen 2003-01-19 12:16:16 UTC
I think this is an improvement, the current menus get too squeezed
together especially with small fonts, so it looks better this way.

I agree with the "could use this on other places as well" -thing.
Comment 9 Rodney Dawes 2003-01-19 20:01:16 UTC
Created attachment 13678 [details] [review]
Patch to center GtkSeparatorMenuItems
Comment 10 Rodney Dawes 2003-01-19 20:04:14 UTC
I just attached a patch to center the Separator menuitems. Can we get
at least this part in a 2.2.x release? It will make the themes look a
LOT better than they do now.

As for the black border... does it always exist, or is it disableable?
It is a nice thing to have, but it will make engines that have totally
flat menus look worse, I think, weird at least anyway. I'd really like
to try the main patch on 2.2 and check it out for bugs or other weird
issues if possible.
Comment 11 Soren Sandmann Pedersen 2003-07-01 01:47:24 UTC
Rodney: the black borders are part of the default theme engine, so
themes that use their own engine will not be affected.

I am going to upload a new patch shortly. The new patch has as its
main features that 

     - it applies cleanly
     - it tweaks the vertical padding a bit (from 3 to 0)

It does not actually deal with handleboxes, because

     - handleboxes do not request any space for borders, and making
       them do so would make it look weird when the child is a 
       menu bar or another widget that draws its own frame

     - handleboxes are basically unused (applications always use
       BonoboDock).

     - handleboxes will be deprecated when a GtkDock arrives
 
     - it doesn't look terrible the way it is

For these reasons I don't think it's worth dealing with handleboxes
for this cosmetic issue.
Comment 12 Soren Sandmann Pedersen 2003-07-01 01:51:42 UTC
That should be "from 3 to _1_"
Comment 13 Soren Sandmann Pedersen 2003-07-01 01:57:32 UTC
Created attachment 17948 [details] [review]
Updated patch
Comment 14 Soren Sandmann Pedersen 2003-07-04 15:04:59 UTC
Created attachment 18036 [details] [review]
New patch
Comment 15 Soren Sandmann Pedersen 2003-07-04 15:06:14 UTC
New patch attached that fixes image items in RTL mode

To give a better idea of how it looks, here is a new screenshot:

http://www.daimi.au.dk/~sandmann/pictures/pretty-menus.png
Comment 16 Owen Taylor 2003-07-04 16:14:19 UTC
Why don't you just go ahead and commit this stuff to HEAD -
I'm never going to have the time (or, really, interest) to
go through these changes line by line, pixel by pixel.

Just a few general comments:

 * It would be nice to have an entry in widget-geometry.txt
   for any widget where you are adding style properties
   for padding values.

 * With the relatively heavy menu border, the overlap
   and offset of the submenus really looks odder
   than currently. Might be good to look at
   bug 96557.

 * Having "icon_spacing" in GtkMenuItem and having it
   affect the indicator is a bit odd. I'd go with
   "toggle_spacing", which while also weird, at
   least matches ::toggle_size_request().

 * +#include "gtkhandlebox.h" is a leftover.
Comment 17 Soren Sandmann Pedersen 2003-07-06 15:34:27 UTC
Sun Jul  6 17:21:23 2003  Soeren Sandmann  <sandmann@daimi.au.dk>

	* docs/widget_geometry.txt: better drawing of GtkMenuItem
	* docs/widget_geometry.txt: add notes about GtkMenu
	* gtk/gtkstyle.c: remove leftover "#include "gtkhandlebox.h""

Sat Jul  5 10:34:00 2003  Soeren Sandmann  <sandmann@daimi.au.dk>

	* gtk/gtkmenu.c: add vertical_padding style property.
	
	* gtk/gtkmenuitem.c: add style properties toggle_spacing,
	arrow_spacing and horizontal_padding. Also center separators and
	make them a bit taller.

	* gtk/*menuitem.c: use new style properties.
	
	* docs/widget_geometry.txt: Add note about GtkMenuItem

	* gtk/gtkstyle.c 
	(gtk_default_draw_vline, gtk_default_draw_hline):
	fix +/-1 errors. 

	(gtk_default_draw_shadow): draw a black border around menus.

	* gtk/gtkvseparator, gtk/gtkhseparator.c, gtk/gtkmenuitem.c: fix
	calls to gtk_paint_hline() and gtk_paint_vline() (they take x1,
	x2 and y1, y2 respectively, not x, width and y, height).