-
Notifications
You must be signed in to change notification settings - Fork 781
[core] Added min, max and step parameters for slider widget #6777
Conversation
Like for the setpoint, the min and max value for a slider can differ based on the underlying thing that is being controlled, so it shouldn't be hardcoded and a user should be able to change these values. The default values, if the parameters are not given, are 0 (minValue) and 100 (maxValue), like they're currently, so that users shouldn't see a difference when they're not changing their configurations. Fixes eclipse-archived#6777 Signed-off-by: Florian <[email protected]>
Like for the setpoint, the min and max value for a slider can differ based on the underlying thing that is being controlled, so it shouldn't be hardcoded and a user should be able to change these values. The default values, if the parameters are not given, are 0 (minValue) and 100 (maxValue), like they're currently, so that users shouldn't see a difference when they're not changing their configurations. Fixes eclipse-archived#6776 Signed-off-by: Florian <[email protected]>
Signed-off-by: Florian <[email protected]>
Could you please adjust Classic UI too ? |
The slider widget was originally only introduced for Dimmer items and no others. |
I looked at it, too, however, I've two questions here:
E.g. the temperature (like in the openHAB example) or, in the actual use case, controlling the brightness of an item, which expects the brightness as a range from 0 to 255 instead of 0 to 100. Currently it would need to be "translated" in a rule to work with a slider.
Hmm, I don't think so. The |
For a slider you need to know the minimum step size, too. Or how do you want decide which values the slider position should be assigned to? |
Why? The assignment of the values doesn't change when you make the min and Max values configurable (the slider had min and Max values before, too, they were just hardcoded). The slider had a specific width and based on that width the range between the min andax values are assigned to the slider relatively. Let's say, that the slider width is 100%, then each step of the slider position is 1%, it doesn't really matter what the absolute values are then, they are calculated by the relative position of the slider. |
I did not have a look at the code so I could be wrong and if you already checked it... |
So, for now the slider has a default step size (in basic UI only), which is 1 (see the mdl documentation), so it would be possible to add a step parameter, too. I really do not care so much about this part of it, so I'm ok with adding a step parameter, too (which should default to 1 I think). |
The default is 1, if not set, otherwise a user might set another value to make the step of the slider bigger or shorter. Signed-off-by: Florian <[email protected]>
The latest commit changes the slider to have another new parameter (step) and made changes to the basic UI to take this into account. I didn't changed the classic UI so far because I want to check with you guys first about one thing: My solution here would be to use the same widget for a slider and a setpoint in the classic UI, is that ok? Any other opinions? :) |
I am not a user of the basic or classic UI, so hopefully another one can comment. |
@FlorianSW I do not think that anything should be changed in the Classic UI, because the widget just works fine and min/max do not have any impact on it. Completely changing its behavior to not send INCREASE/DECREASE anymore would rather be a breaking change that might annoy some users. |
...me.ui.basic/src/main/java/org/eclipse/smarthome/ui/basic/internal/render/SliderRenderer.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Florian <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
May I ask you to also do an update to https://www.openhab.org/docs/configuration/sitemaps.html#element-type-slider? |
Of course, I'll submit a PR :) May I ask if there's already a plan when (not exactly of course, just an estimation) this will be included in an ESH release, as well as an openHAB release? |
Will be in the latest openHAB distro snapshots later this week, so the docs can be adapted right away! |
Just jumping in here to thank you for not forgetting about step ;-)
That's all right, but still, step is absolutely needed in order to have a defined granularity. Say the 0..100% slider is 100 pixels wide: the minimum increment then would be 1%. If the slider grows to 500 pixels (presentation is up to the UI after all), the minimum increment suddenly becomes 0.2%. Since the UI doesn't know whether the underlying number should be integer or floating point, what increment between data points should it use? Having 'step' solves that uncertaincy, so thanks for not making my life hard when implementing this in the Android app 👍 |
Like for the setpoint, the min and max value for a slider can differ based
on the underlying thing that is being controlled, so it shouldn't be
hardcoded and a user should be able to change these values.
The default values, if the parameters are not given, are 0 (minValue) and
100 (maxValue), like they're currently, so that users shouldn't see a
difference when they're not changing their configurations.
Fixes #6776
Signed-off-by: Florian [email protected]