Initial creation of the range slider and testbed#114
Initial creation of the range slider and testbed#114Tiomat85 wants to merge 9 commits intonion-software:masterfrom
Conversation
KRLango
left a comment
KRLango
left a comment
There was a problem hiding this comment.
Minor formatting issue - but otherwise looks good to me.
|
After reviewing this I started reviewing nion-software/nionswift#1582 and realised the range slider is currently hard-coded to be 0-1. This means all usages will have to handle the conversion to min/max themselves. Wouldn't that be better in this class so it is done in a single place? |
You are correct, this is to match the behaviour to that of the existing SliderCanvasItem. I would not be opposed to adding an extra block of work migrating both of those over to a dynamic min/max and roll some of that boiler plate code down. |
Let's push that to a future PR, if at all. |
cmeyer
left a comment
cmeyer
left a comment
There was a problem hiding this comment.
Not a full review - just some initial comments:
- the "handles" are different (wider) from the slider canvas item. why?
- the handles seem to work fundamentally differently from the slider - is the range defined by the center of the handle or the left/right edges? if the center, then it is impossible to get the interval to be 0. if the edges, then can they be shaped like triangles to be more like bookends?
- do we want to define/enforce an optional minimum interval? IIRC this is required for one of the display item situations
- I'm going back and forth whether we want to make this part of
nionuiyet or just a custom canvas item innionswift. there are still some usage details to work out and it may be better to try this first on thenionswiftside. I know I'm the one who suggested putting it innionuibut it's more intrusive (i.e. modifying theUserInterfaceclass) than I was expecting.
|
Some odd behaviour that I observed while testing the range slider:
Similar things happen if you move the minimum bar towards the maximum as well. |
A collection of fixes for slider motion. One fix of making sure that the same maths is used for conversion from min/max to center/width. Multiple fixes for making sure event propagation did not cause undesired additional changes, mostly around the inclusion of the __suppress_updates flag which limits the updates to only a single event driven update.This removed most of the 'wobbly' effect where events triggered as part of the partial update caused other side-effects. Also changed the handle style to be a trial of a bookend style handle to see whether that works better.
|
Note for testing: This will not cause any range sliders to show up. The test case for this is in nion-software/nionswift#1582 |
Tighten range calculation. Adjusting the way the min/max are adjusted together to maintain absolute width rather than applying the delta independantly which could lead to odd rounding behaviour.
|
As discussed in meeting, can we put this code at the point of use in |
5 participants
Creation of a new Range Slider control and widget.
A range slider has two 'handles' which are adjustable as a minimum and maximum of the value. It should also provide the ability to drag them both together.