GNOME Bugzilla – Bug 442042
GtkScaleButton is too limited
Last modified: 2014-08-03 15:19:51 UTC
GtkScaleButton has several shortcomings, actually it's not really good for anything than being a base class of GtkVolumeButton. Missing stuff is: - no configurable orientation (the only scales that are usually vertical are volume controls, it's bad ui style for anything else) - popup widget not themeable (please gtk_widget_set_name() a reasonable name on the popup) - it always covers the button, which is bad once it can be shown horizontally. (in gimp, we have a scale+spinbutton widget that we would like to replace with a spinbutton+popup_scale widget: old: [=====XX===] [spin] new: [spin] [scalebutton] in this scenario, the popup scale would cover the spinbutton, making entering exact values with the scale impossible. Please make the popup widget positionable on each side of the button, depending on scale orientation) - i don't really see what the +/- buttons are good for except for the volume button that seems to be modeled after the panel's volume applet. (please make showing of these buttons optional)
Additionally, in an application like gimp where you have many many configuration options in places like the tool options, it would be really useful to be able to plug a "reset" button into the popup. But that's maybe overkill ;) Please at least add some way to get to the popup widget so we can add the reset button ourselves.
Created attachment 113493 [details] [review] Patch implementing a "orientation" CONSTRUCT_ONLY property
Rock, thanks for fixing this.
Is there a specific reason the property is construct-only? I imagine cases where you could want this to be changed dynamically for example when dragging a panel from the top to the side and it changes orientation.
Because it's considerably easier to implement because it doesn't require re-creating the scale when the property changes. But the use case you present is entirely reasobanle indeed. I would however prefer to do this patch on-top of this one. Any other comments on the patch? Bastien?
Looks fine to me.
Committed with the addition of unrefing the adjustment in finalize(): 2008-06-27 Michael Natterer <mitch@imendio.com> Bug 442042 – GtkScaleButton is too limited * gtk/gtkscalebutton.c: add "orientation" property. Make sure the stuff that is part of the public API continues to be created in init() to stay compatible. Move creating of the popup scale to constructor(). Add an internal HScale class. Changed popup positioning for horizontal scales accordingly.
Jikes, i just found that the patch is incomplete wrt creating the box that contains the scale and buttons. It needs to be a hbox for horizontal scales of course. Will fix that along with changing the property into a non-construct-only one.
Created attachment 113535 [details] [review] Patch fixing above mentioned issues and turning the new property into a normal one
Another issue addressed: 2008-06-27 Michael Natterer <mitch@imendio.com> Bug 442042 – GtkScaleButton is too limited * gtk/gtkscalebutton.c (gtk_scale_button_init): set the name "gtk-scalebutton-popup-window" on the popup window so it is properly themeable.
(In reply to comment #9) > Created an attachment (id=113535) [edit] > Patch fixing above mentioned issues and turning the new property into a normal > one This one applies but fails for me, possibly because the default orientation is not set. This fails: sb = gtk.ScaleButton(gtk.ICON_SIZE_INVALID, 0, 100, 2) assert sb.get_property('orientation') == gtk.ORIENTATION_VERTICAL
Created attachment 113576 [details] [review] Indeed, this patch fixes the default value.
Yeah, another case where having obox in gtk would be handy. The comment I actually wanted to make here is that the property needs a doc comment with a Since: tag.
Um, the doc comment has a Since ;) And what is an obox?
obox is a reorientable box thats used e.g in many applets: http://svn.gnome.org/viewvc/gnome-panel/trunk/applets/clock/obox.h?revision=10532&view=markup
Bastien, does this patch look ok?
As Matthias mentions, using obox rather than adding hacks to GtkScaleButton would be good.
The "box" part of the hack is about a few percent. The fact of having these problems with switching orientation is a general one and as you can see much worse when you need to subclass. Read: the hack and effort of having to toggle the scale's orientation is by orders of magnitude larger than s/hbox/vbox/ I don't think this patch depends on obox availibility but once there it could be ported to using obox in half an hour.
Maybe there should be a 3.0 goal of getting rid of all these GtkHFoo and GtkVFoo and simply have an orientation api in the base class.
(In reply to comment #19) > Maybe there should be a 3.0 goal of getting rid of all these GtkHFoo and > GtkVFoo and simply have an orientation api in the base class. Please file a new bug to that effect, and feel free to commit that patch if you're happy with it then :)
Committed: 2008-07-01 Michael Natterer <mitch@imendio.com> Bug 442042 – GtkScaleButton is too limited * gtk/gtkscalebutton.[ch]: turn "orientation" into a normal property that can be changed at any time after widget creation. Add public API for it. * gtk/gtk.symbols: add gtk_scale_button_get/set_orientation.
Created attachment 113748 [details] [review] Patch to use GtkBox' "orientation" property from bug #541009
Forgot to commit the testing code: 2008-07-21 Michael Natterer <mitch@imendio.com> Bug 442042 – GtkScaleButton is too limited * tests/testvolumebutton.c: forgot to commit code that tests toggling the button's orientation. Also fixes the file's coding style.
I guess we can commit this now, Mitch
Not really, I updated the patch locally to also use flippable scales, will attach a new one.
I want to absolutely second the hiding of the +/- buttons in the default apperance. There is no way to actually tell the exact position, and thus it is rather irritating to have buttons for a single step. Combined with this point about the exact position I'd also argue that a value should be shown. A volume knob on a stereo is analog indeed (or at least sometimes these days), but this isn't an analog knob, and I don't think we should pretend that it should be one because that would be IMHO a little over the head of a standard volume control. Right now it is already sort of analog, because there are these 100 little steps on the scale which a human can not easily discern. The ScaleButton slider is distressing me every time and led me to replace GtkVolumeButton with a self constructed volume control, which looks essentially the same, but with these changes: 1) No +/- buttons 2) Value is displayed at the top of the slider window 3) The button itself accepts the same controls as the slider, that is mwheelup/mwheeldown, key up/down pgup/pgdown, and key+ctrl for a step of 10 instead of 1 4) The window has a pretty sensible logic built in that auto-hides it when it is no longer in use. I made a small video demonstrating this functionality: http://futurepast.free.fr/vslider.ogg Out of these 4 things, I'd propose 1-3 to be incorporated into ScaleButton and it would be great if someone else would think that #4 might be also useful.
Committed a somewhat different patch that got forgotten on my disk: 2009-02-19 Michael Natterer <mitch@imendio.com> * gtk/gtkscalebutton.c: make the orientation flipping much simpler by using the GtkOrientable features of the involved widgets: (gtk_scale_button_init): create the frame, box and scale here, they never need to be recreated because they implement GtkOrientable. (gtk_scale_button_constructor): remove their construction here. (gtk_scale_button_set_orientation): don't destroy and re-create anything. Instead, simply set the orientation of the above created widgets and fiddle a bit with the "plus" and "minus" buttons' packing and the scale's "inverted" state. Remove separate internal GtkScaleButtonHScale and GtkScaleButtonVScale subclasses and simply have a GtkScaleButtonScale directly inherited from GtkScale.