GNOME Bugzilla – Bug 342339
GtkRange::stepper-spacing style property not implementd correctly
Last modified: 2006-10-23 11:40:45 UTC
Set GtkRange::slider-spacing to something > 0 and look at tests/testgtk -> Range Control, it looks pretty horrible. - The stepper spacing is applied unconditionally, even if there are no steppers. - coordinate-to-adjustment value translation is broken because it is not aware of stepper spacing (the slider jumps when the mose drags starts in an end position).
Created attachment 65826 [details] [review] Patch fixing the bug The patch could be much easier if the stepper-spacing area between the steppers and the sliding area would not have to be sensitive to mouse clicks. Unfortunately, the trough is drawn all the way from one end to the other, even below the steppers. So the gap looks clickable and thus must me clickable. Maemo-gtk has a patch that turns stepper-spacing into something useful, by adding an additional trough-under-steppers style property which, when FALSE, draws the trough only in the sliding area, not under the steppers. I wonder if in this case the gap should not be clickable, since it doesn't look clickable any more?
Patch looks fine to me. Do you have any idea if stepper-spacing is even used in any themes ? Do we know when and why it was added ? In its current form, it seems pretty useless to me.
Created attachment 65839 [details] [review] Shows how stepper-spacing can be useful Attached patch implements said GtkRange::trough-under-steppers style property. Note that it's not finished, since the steppers need to be drawn slightly larger and without the offset that is normally reserved for the trough border. The patch also contains the patch attached to http://bugzilla.gnome.org/show_bug.cgi?id=342249 because they share a lot of common code and were extracted from the same maemo-gtk tree.
applying the patches and testing them, i have to say that trough-under-steppers=TRUE doesn't make sense for stepper-spacing>0. it just looks confusing and behaves counter-intuitive: there's a slider area which you can't slide to. as a result, i think there should be no trough-under-steppers properties, instead, that should be just be an internal boolean which is set from stepper-spacing==0. the fix in attachment #1 [details] (id=65826) looks good and is neccessary though, and can go in eitehr way. please commit.
Created attachment 66051 [details] [review] Updated patch Combined patch that: - fixes stepper-spacing differently - adds trough-under-steppers (which also gets enabled by stepper-spacing > 0) - adds trough-side-details, since it shares lots of code with trough-under-steppers.
I forgot, the patch also separates the internal logic for the focus padding and the trhough-border, since they are not any more next to each other if trough-under-steppers is FALSE.
I have to admit that I did not follow all the logic changes in the patch. But I assume you tested serveral combinations of style properties to verify them...we really need a theme test program. Out of interest, do you have any screenshots showing the new possibilities ? I want to collect some screenshots of the new theming stuff for the 2.10 announcement. It think the docs for the new style properties should mention the interaction between stepper-spacing an trough-under-steppers.
Created attachment 66128 [details] range-default.png
Created attachment 66129 [details] range-trough-under-steppers=FALSE.png
Created attachment 66130 [details] range-stepper-spacing=10.png
Created attachment 66131 [details] range-trough-border=5.png
Created attachment 66132 [details] range-stepper-spacing=10-trough-border=5.png
I didn't attach a screenshot for trough-side-details=TRUE because it makes no sense without an engine that supports it. It simply draws the two parts of the trough with different details, nothing else.
*** Bug 344028 has been marked as a duplicate of this bug. ***
ok, upon reconsidering the issue, i think there's no merit in holding off application of this patch. the patch looks pretty ok to me as far as i can tell, as do the screenshots. so please apply Mitch, but let's keep this report open until the test program of dependent bug #342257 has been implemented and we can give the changes proper theme testing.
Fixed in CVS. Leaving open for the reason in comment #15 2006-06-09 Michael Natterer <mitch@imendio.com> * gtk/gtkrange.c: added new style properties "trough-side-details" and "draw-trough-under-steppers" and fixed the "stepper-spacing" style property. Fixes bugs #342339 and #342249. "draw-trough-under-steppers", when set to FALSE, starts trough drawing next to the steppers instead of drawing the trough "below" (around) the steppers. If "stepper-spacing" is set to any value larger than zero, "draw-trough-under-steppers" is set to FALSE automatically to avoid rendering an area that looks clickable but isn't. (gtk_range_calc_layout): honor draw-trough-under-steppers. Don't reserve stepper-spacing on sides of the range that don't have steppers. (gtk_range_expose): changed drawing accordingly. Implement "trough-side-details" which enables drawing of the upper and lower parts of the trough with different details. This is useful for theme engines which want to draw these parts differently. (coord_to_value): correctly take all rendering options into account. stepper-spacing > 0 caused jumping and otherwise strangely behaving ranges before. (other functions): changed accordingly.
(In reply to comment #16) > Fixed in CVS. Leaving open for the reason in comment #15 changed my mind on keeping this bug open just for the sake of bug #342257. that bug has an open state itself and keeping this one open doesn't really help in any way i can see, the patch is applied, the basic issue fixed, so this can be closed.