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 459997 - Size toggle cell renderers based on font size
Size toggle cell renderers based on font size
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Widget: GtkTreeView
2.11.x
Other Linux
: Normal normal
: ---
Assigned To: gtktreeview-bugs
gtktreeview-bugs
Depends on:
Blocks:
 
 
Reported: 2007-07-24 18:55 UTC by Ross Burton
Modified: 2018-04-15 00:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Prototype Patch (1.85 KB, patch)
2007-07-24 18:55 UTC, Ross Burton
none Details | Review
Revised patch (1.86 KB, patch)
2007-07-24 19:05 UTC, Ross Burton
none Details | Review
Another patch (2.75 KB, patch)
2007-07-24 20:02 UTC, Ross Burton
none Details | Review
Final patch? (3.00 KB, patch)
2007-07-25 10:17 UTC, Ross Burton
none Details | Review
Handle replacing the widget (3.25 KB, patch)
2007-07-26 09:55 UTC, Ross Burton
reviewed Details | Review

Description Ross Burton 2007-07-24 18:55:03 UTC
Hard-coding the size of the toggle cell renderer to 12 pixels is a bit silly when devices like the N800 have 200-dpi screens.   Maemo has a nice hack which changes the size of the check box to 26 pixels, but that isn't really a scalable solution.

Attaching a prototype patch which makes the size of the check box the same as the font size.  There are some issues but I'd like some feedback:

* Check box size.  My initial patch used just the size of the ascent, but the attached patch uses ascent+descent.  This results in check boxes which seem a bit  big to me, but I'm sure I'll get used to it.

* You cannot override the size via the indicator-size property yet.

* This function is called *very* frequently, I guess it should cache the widget so it doesn't recalculate all of the time.

Apart from that, this works well for me.
Comment 1 Ross Burton 2007-07-24 18:55:38 UTC
Created attachment 92297 [details] [review]
Prototype Patch
Comment 2 Ross Burton 2007-07-24 18:59:21 UTC
Another idea for the indicator size: use something magic like 80% of the font size.
Comment 3 Ross Burton 2007-07-24 19:05:00 UTC
Created attachment 92298 [details] [review]
Revised patch

This patch uses 85% of the total font height.  On both a laptop and a PDA, this produces a pleasing size check box.
Comment 4 Ross Burton 2007-07-24 20:02:39 UTC
Created attachment 92303 [details] [review]
Another patch

This patch cleans up some of the code, and lets the user get and set the size-indicator property as they'd expect.  It defaults to the font size, but if the property is set then that value is used.
Comment 5 Ross Burton 2007-07-25 10:17:00 UTC
Created attachment 92343 [details] [review]
Final patch?

This patch caches the widget the renderer is drawing too, so that the size isn't recalculated if the widget doesn't change.  It also connects to style-set on that widget so that the cached size is updated if the theme is changed.

I think this is a final patch now, and works well for me.
Comment 6 Ross Burton 2007-07-26 09:55:26 UTC
Created attachment 92445 [details] [review]
Handle replacing the widget

This patch cleans up some compile warnings, and removes the weak pointer/disconnects the signal when the widget changes.
Comment 7 Ross Burton 2007-07-31 09:41:22 UTC
http://www.pokylinux.org/images/sato/large-fonts.png is a screenshot of this patch in action.
Comment 8 Thomas Wood 2007-08-02 14:23:12 UTC
This would probably also help with accessibility themes that suggest large fonts. Do the checkboxes get resized when the font setting is changed?
Comment 9 Ross Burton 2007-08-02 14:28:57 UTC
Yes, they do.
Comment 10 Matthias Clasen 2007-11-11 00:46:27 UTC
In similar situations in GtkTextTag, we use a pattern of 
property + boolean set-property. I think it would be good to do the same here.

I think the idea of tying indicator size to font size is good, but I think
the patch would be more compelling if it also worked for check/radio buttons/menuitems. Of course, that is quite a bit more work...
Comment 11 Ross Burton 2007-11-11 15:17:37 UTC
At the moment the font size is used unless the indicator-size property is set, and reading the indicator-size property returns the size being used (either the calculated size or the overridden size), which matches the existing semantics.

Wouldn't adding indicator-size-set confuse things, as existing code which sets indicator-size would no longer work unless it defaulted to TRUE, in which case this change would be pointless.

I'll work on buttons and menu items, but as there are style properties for those its less important.
Comment 12 Kristian Rietveld 2007-11-11 15:41:43 UTC
I'd say that sizing the toggle boxes is really up to the theme, just as far check/radio/menuitems...  The bigger problem here is that GtkCellRenderers obviously can't have style properties since they aren't GtkWidgets.  Solving that would solve this issue and some more too ... (see 308428).

Comment 13 Ross Burton 2007-11-11 16:01:26 UTC
Kris, I disagree.  Just because I'm using (say) the Darkilouche GTK+ theme doesn't mean that I want 12px check boxes.  The theme designer cannot know what the DPI of  my screen is, so they cannot know what is a reasonable size for widgets.

My long-term plan is for everything which at the moment is sized in pixels to default to values based on the font size, so that it scales correctly based on the current font size/DPI being used.  Having to hack GTK+ (see Maemo) to change the size of the tree view check boxes, and then hard-code the size of various other check boxes in the theme, is not really a usable solution
Comment 14 Matthias Clasen 2018-02-10 05:19:30 UTC
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
Comment 15 Matthias Clasen 2018-04-15 00:19:04 UTC
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla.

If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab:

https://gitlab.gnome.org/GNOME/gtk/issues/new