Add value prop for convenience #16
Conversation
Is a numeric value instead of a one-element array
|
oh hey @zqianem ! This is kind of what I had running originally as a workaround (as I described in #12) but I was unable to achieve what you have here (I didn't think about two-way binding the values to each other)... I haven't had time to check it out and play with it yet but I can see the logic and it seems sound. I just worry about recursion or looping, or passing both props simultaneously. I'll get around to testing it thoroughly and determine the impact it has in due course... but for now thanks a lot for the contribution !! 🙇 |
| export let vertical = false; | ||
| export let float = false; | ||
| export let hover = true; | ||
|
|
||
| // keep value and values in sync with each other | ||
| const updateValues = () => { if (value !== values[0]) values = [value] }; | ||
| const updateValue = () => { if (value !== values[0]) value = values[0] }; |
There was a problem hiding this comment.
Ran into a situation where I wanted to have value to be a continuous value while having relatively large discrete step sizes for the RangeSlider, so I changed the update check in my local patch from value !== values[0] to alignValueToStep(value) !== values[0]. That way, I could update value elsewhere in my app without it being aligned to the RangeSlider step.
Would you want to have this behavior by default? I'm okay either way as I can maintain my patch if not.
There was a problem hiding this comment.
can you do a REPL with and without the behavior as an example, @zqianem ?
I had this patch checked out locally for testing the other day, but didn't get review completed, I don't 100% follow this comment/change though... are you saying that because this change reacts to value and then eventually goes through alignValueToStep() that eventually propogates back and changes the input value to align ?
There was a problem hiding this comment.
https://svelte.dev/repl/4fe353de6b1d4248bce140c61903a367?version=3.31.2
Here's a REPL showing the behaviour before your PR, with the values[] property instead ... I think this is the desired behaviour, it does feel a bit weird when the value can change from elsewhere, but equally it would be very strange to step a slider but allow the bound value to not adhere to the rules of it, don't you think?
I'm willing to hear opinions on this
There was a problem hiding this comment.
Here's a REPL comparing the implementations: https://svelte.dev/repl/061bbed31e2c4529a1a267aa63d7e5dd?version=3.31.2
I think the middle example (New Behavior (value prop)) is the ideal, where the slider with the step size of 1 changes the value by ones, while the slider with the step size of 10 can only change the value by tens. The first and last examples are a bit odd in that the opposite behavior occurs; not sure how to best address this as I think we want the value and values props to behave as similarly as possible.
More concretely, here is the project where I was using this behavior: https://5ff673a5b316db24c45db7c2--loving-morse-6b791c.netlify.app/. The globe can be rotated by either dragging its surface or opening the "Perspective" menu and using the sliders. Before the alignValueToStep(value) !== values[0 patch, the slider would cause the dragging of the globe to be snapped to the step size of the slider, but afterwards, it is free to move smoothly. (Here is the globe before the patch: https://5ff0b9f53aa2402db335c823--loving-morse-6b791c.netlify.app/; notice how the dragging of the globe is snapped and not smooth.)
Is a numeric value instead of a one-element array.
This is a bit of hack, admittedly. Addresses #12. Not sure how you wanted to update the docs for this.