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 537591 - Don't hardcode minimum width of menuitems
Don't hardcode minimum width of menuitems
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2008-06-10 13:30 UTC by Christian Dywan
Modified: 2008-07-02 12:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Introduce min-char-width (1.97 KB, patch)
2008-06-11 10:07 UTC, Christian Dywan
needs-work Details | Review
Minimum-width causing overly wide menu. (9.10 KB, image/png)
2008-07-01 09:46 UTC, Tim Janik
  Details
Introduce min-char-width, updated Since tag, removed unused variable (1.97 KB, patch)
2008-07-01 10:23 UTC, Christian Dywan
none Details | Review
Introduce width-chars (1.92 KB, patch)
2008-07-01 11:09 UTC, Christian Dywan
none Details | Review
Introduce width-chars, default to 12 (1.92 KB, patch)
2008-07-02 11:06 UTC, Christian Dywan
committed Details | Review

Description Christian Dywan 2008-06-10 13:30:47 UTC
The minimum width of menu items is hardcoded to the following:

return PANGO_PIXELS (7 * height);

This should be adjustable via a style property. For instance, it is too big for Maemo. Currently there is no way to change this in the theme.

Patch following.
Comment 1 Christian Dywan 2008-06-11 10:07:50 UTC
Created attachment 112531 [details] [review]
Introduce min-char-width

The current implementation is based on the height of a character. I propose to change this and instead base it on the normal width of a character, i.e. approximate_char_width in Pango's jargon.

The patch adds a new style property GtkMenuItem::min-char-width that defaults to 16 characters, which on my system yields almost the same minimum width as before, 104 instead of 105, 1 pixel smaller to be exact.

For comparison, a value of 6 characters would match the lower value used in the gtk fork used for Maemo and yields 90 on my system.
Comment 2 Tim Janik 2008-07-01 09:46:20 UTC
Created attachment 113768 [details]
Minimum-width causing overly wide menu.

The patch looks good to me, however, I'd vote to reduce the default minimum width to e.g. 12 or maybe even 8 characters, to avoid overly wide menus as in the attached screenshot (screenshot taken from Gtk+ without the patch). Also, I'd like Mitch to take a look at it as well.
Comment 3 Michael Natterer 2008-07-01 10:00:41 UTC
Some comments:

- Since: 2.14

- unused variable "height"

- perhaps "minimum-char-width" instead of "min-char-width" ? not sure here.

- i agree with tim's default value comment, it should be smaller.
Comment 4 Michael Natterer 2008-07-01 10:04:12 UTC
Hm actually there are more instances of "min-foo-bar" than "minimum-foo-bar"
in gtk and since it's so common we can also stick with "min".
Comment 5 Christian Dywan 2008-07-01 10:05:14 UTC
(In reply to comment #2)
> Created an attachment (id=113768) [edit]
> Minimum-width causing overly wide menu.
> 
> The patch looks good to me, however, I'd vote to reduce the default minimum
> width to e.g. 12 or maybe even 8 characters, to avoid overly wide menus as in
> the attached screenshot (screenshot taken from Gtk+ without the patch). Also,
> I'd like Mitch to take a look at it as well.

I don't personally think the current default leads to "overly wide menus". In fact I find it still too narrow for my taste.
The good thing is, with this patch you can decrease the minimum width locally as I will probably increase it. ^_^
Comment 6 Christian Dywan 2008-07-01 10:23:50 UTC
Created attachment 113769 [details] [review]
Introduce min-char-width, updated Since tag, removed unused variable

I fixed the Since tag and removed the unused variable.

The default is unchanged for now, I'm not sure if a 2:1 stomach decision warrants changing it. For what I want, I can open a follow-up bug and leave it to usability people to discuss the default value.
Comment 7 Tim Janik 2008-07-01 10:40:17 UTC
(In reply to comment #6)
> Created an attachment (id=113769) [edit]
> Introduce min-char-width, updated Since tag, removed unused variable

"min-char-width" actually sounds like a pixel value that'd control how wide a character is considered to be at minimum. Looking at GtkEntry::width-chars suggests a better name. Please adapt the patch to:
- use "min-width-chars" as property name
- use a default of 12 characters.

Comment 8 Michael Natterer 2008-07-01 10:54:09 UTC
Nitpick: in GtkEntry, the "width-chars" property also determines the
*minimum* width. Perhaps we should stick with "width-chars" to
be consistent?
Comment 9 Christian Dywan 2008-07-01 11:09:14 UTC
Created attachment 113775 [details] [review]
Introduce width-chars

This patch does the same as the previous one but renames it to width-chars, which is used in GtkLabel, GtkEntry and GtkFileChooserButton with the same meaning already. (Even though it is not properly documented in these preceding cases)
Comment 10 Christian Dywan 2008-07-02 11:06:39 UTC
Created attachment 113845 [details] [review]
Introduce width-chars, default to 12

This is the same patch, but defaulting to 12. After some discussion and testing with different menus and values, I think we agree that that's enough.
Comment 11 Tim Janik 2008-07-02 11:47:03 UTC
(In reply to comment #10)
> Created an attachment (id=113845) [edit]
> Introduce width-chars, default to 12
> 
> This is the same patch, but defaulting to 12. After some discussion and testing
> with different menus and values, I think we agree that that's enough.

Thanks a lot, please commit with appropriate changelog.
Comment 12 Michael Natterer 2008-07-02 12:06:04 UTC
Committed:

2008-07-02  Michael Natterer  <mitch@imendio.com>

	Bug 537591 – Don't hardcode minimum width of menuitems

	* gtk/gtkmenuitem.c: applied patch from Christian Dywan which
	introduces a "width-chars" style property which replaces the
	hardcoded minimum width of menuitems with submenu. Patch
	extracted from Maemo-GTK+.