Review Request: Allow slider to be exponential to the spin box on KDoubleNumInput

[Cyrille Berger]

> On 2009-04-13 16:46:16, Aaron Seigo wrote:
> > coool! i've long wanted this for various places in the code. nice to see it actually coming! some comments follow, though nothing fatal imho.

Great :) I agree with the singleStep as well, will make a few renaming of the variable and adjust the API documentation (when possible) for the final patch. Once a decision for setSliderEnabled has been taken, I guess it will be ok for commit ?


> On 2009-04-13 16:46:16, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/kdeui/widgets/knuminput.h, line 556
> > <http://reviewboard.kde.org/r/582/diff/1/?file=5274#file5274line556>
> >
> >     default arg is odd here since it requires one to look at the apidox when its used in code to see the defaults:
> >     
> >     setSliderEnabled();
> >     
> >     vs
> >     
> >     setSliderEnabled(true);
> >     setSliderEanbled(false);
> >     
> >     it's not worth the 4-5 extra characters and also in line with the rest of our api :)
> >     
> >     also, a getter: bool isSliderEnbled() const?

The thing is KIntegerNumInput has the default argument (and it's too late, since it's in 4.2), so while I agree it is silly, it will make discrepancies between the two widgets. So not sure, what should win there... 


> On 2009-04-13 16:46:16, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/kdeui/widgets/knuminput.cpp, line 773
> > <http://reviewboard.kde.org/r/582/diff/1/?file=5275#file5275line773>
> >
> >     should check to make sure d->exponent is > 0 here to avoid possible div by zero? either that or else check for zero passed into the setter for d->exponent

And assert ? Or emit a message with kError and reject the value ?


- Cyrille


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/582/#review914
-----------------------------------------------------------


On 2009-04-13 12:26:29, Cyrille Berger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/582/
> -----------------------------------------------------------
> 
> (Updated 2009-04-13 12:26:29)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Can't seem to be able to update patches on reviewboard :( so creating a new request.
> 
> The main point if this patch is to allow to have an "exponential" slider for KDoubleNumInput. It's usefull when you have an input where the main interesting range is small values, while supporting a big range. For instance, in Krita, when we set the radius of the brush we have a range of 0.0 to 9999.0, but most of the time the user work in the range of 0.0-100.0, but since the slider is linear to 0.0 to 9999.0, the most usefull range is only 10% of the slider which makes the slider very unusuable, hence the idea of an exponential slider, where we could make sure the range 0.0-100.0 use half the slider.
> 
> 
> Extra goodies, add a few Q_PROPERTY for enabling slider in designer. And it also bring KDoubleNumInput API closer to KIntegerNumInput (unfortunately I can't go further to have the same setRange function as in KIntegerNumInput since it would break ABI)
> 
> In this new patch, I have also added step/setStep (and Q_PROPERTY) to change the steps without calling setRange. In the Qt API, it's called setSingleStep, but in knuminput it was refered as "step", not sure if I shouldn't call the new function (and its property) singleStep (and possibly adjust the naming in the other function parameters, when possible) to be closer to Qt's API.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/widgets/knuminput.h 952354 
>   trunk/KDE/kdelibs/kdeui/widgets/knuminput.cpp 952354 
> 
> Diff: http://reviewboard.kde.org/r/582/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Cyrille
> 
>