GNOME Bugzilla – Bug 516108
Allow themes to define a minimum size and aspect ratio for GtkArrow.
Last modified: 2012-01-26 04:51:09 UTC
With regard to devices with a small screen, it is sometimes desirable to adjust the minimum size and aspect ratio of GtkArrow. Someimtes the arrow is unreadable on the screen. According style properties have done the job in the case of Maemo, so the natural next step is to have this feature upstream.
Created attachment 105088 [details] [review] Introduce "min-size and "aspect-ratio" style properties.
Hm, I like the idea. A few things I noticed: * defining PROP_MIN_SIZE and PROP_ASPECT_RATIO should not happen for style properties * I think it would make sense to take the arrow-type into account. This means that a left/right arrow needs different handling in gtk_arrow_size_request. This is probably not that interesting for this particular bug. But I noticed that there are already several style properties that related to arrow sizes. IE. the scroll-arrow-[hv]length (GtkWidget), arrow-size (GtkComboBox).
Created attachment 110020 [details] [review] Proposed Patch Updated the patch to take comments into account.
I can understand the case for min-size, but aspect-ratio looks a bit suspicious to me. Wouldn't that have to be flipped when changing the arrow orientation ? Is there any example of aspect-ratio != 1 actually being used ?
Created attachment 112839 [details] [review] Take orientation into consideration Correctly spotted, the formula ought to be flipped depending on the arrow type/ orientation. The attached patch does this.
Any input on the updated patch?
(In reply to comment #4) > I can understand the case for min-size, but aspect-ratio looks a bit suspicious > to me. Wouldn't that have to be flipped when changing the arrow orientation ? > > Is there any example of aspect-ratio != 1 actually being used ? Off head, displays with xdpi!=ydpi, not sure how common those are though, particularly in the embedded device space.
Quick comment from the middle of another review: gtk_arrow_size_request (GtkWidget *widget, GtkRequisition *requisition) This must be: gtk_arrow_size_request (GtkWidget *widget, GtkRequisition *requisition) More later...
Some more comments: - the addition of a size-request implementation is needed regardless of the addition of the two style properties, since GtkArrow completely ignores changes of GtkMisc' padding properties. - the aspect-radio needs to be honored in size-allocate too, not only in size-request, since the widget can always be allocated more space than it requested. It shouldn't fill its entire allocation with the arrow then but use the set aspect-ratio to center the arrow into the available space.
Created attachment 119568 [details] [review] Style fix and updated to trunk
Mitch, any chance you could have another look?
Yep: - size_allocate() is only declared and not implemented - Since: 2.20 now - + default: + ; should probably say break; just for the sake of it Other than that, I suppose you tested the behavior to work correctly, because I didn't :)
Created attachment 144899 [details] [review] Update for GTK+ 2.20, removed size_allocate
Minimum size seems to work nicely, but I understand what aspect-ratio is supposed to do.
Looking at this again, I don't think GtkArrow needs any new features