Can't tell when multiple thumbs have moved because of pearling

[axelboc]

Description

This follows #212: on a slider with multiple thumbs and pearling enabled, more than one thumb can move at once, so the change handlers should probably receive an array of thumb indices instead of a single thumb index.

Ideally, if the slider has multiple thumbs, index should always be an array, as this would simplify looking for a thumb that changed: index.includes(0). In terms of types, this would mean something like this:

onChange?: (value: T, index: T) => void;

CodeSandbox

https://codesandbox.io/s/amazing-bogdan-cxbgs?file=/src/App.js

[stonebk]

Yeah, I think the type of the index param should match that of values param.

[stonebk]

@axelboc thanks for the additional PR!

What's the use-case for knowing exactly which thumbs moved due to pearling more than just knowing the thumb that was grabbed?

I ask, because I'm looking at the PR and it adds a fair amount of complexity for what seems might be a pretty far edge case.

[axelboc]

My use case involves floats. My slider has two thumbs and its min/max range and thumb values can be any floating-point numbers, with any precision supported by JS. react-slider is rightfully not meant to be used with floats of any precision (it rounds values to 5 decimals and expects the step to be a multiple of the min/max range), so instead, I render a slider with a min of 0, a max of 100 and a step of 1. I then use a D3 scale to convert the values back and forth.

The resulting implementation works very well, but I need the slider to be controlled (i.e. onChange), and I need to do something after the user has finished interacting (i.e. onAfterChange). In onChange, I want to update only the value(s) that changed to avoid any loss of accuracy from unnecessary D3 scale conversions. In onAfterChange, I also want to update only the values that changed for reasons too complex to explain.

In onChange, this can be achieved with a simple equality check between the input controlled value and the value provided in the handler, but the same cannot be done in onAfterChange, as the values end up being equal. The workaround I found is to save the initial value of the slider in onBeforeChange and do an equality check against this value in onAfterChange.

My PR is indeed quite complex given the specificity of my use case. I honestly thought it would be easier to expose the indices of the thumbs that move in the on-change handlers. 😅

Perhaps my onBeforeChange workaround would make for a much simpler implementation, but I was concerned about the rounding behaviour of react-slider when dealing with floats. I don't personally use react-slider with floats thanks to the D3 scale technique, which is why it works great for me.

I still think this would be a convenient feature for other, simpler use cases than mine, but I won't take offence, if you'd rather close my PR and mark this issue as wontfix. 😉

[stonebk]

Haha, thanks for the thorough explanation @axelboc!

Let me think on this one a little bit more...

[axelboc]

Since PR #232 has gotten stale, I thought I'd suggest an alternative API solution that is perhaps cleaner than the one in the PR:

// API in the PR
onChange={(value, index, pushed) => {
  // `value`: the new slider value (`number | array`)
  // `index`: the index of the thumb, with which the user has interacted (`number`)
  // `pushed`: an array listing the indices of the thumbs that moved due to pearling (`number[]`)
}}

// Alternative API
onChange={(value, thumbs) => {
  // `value`: the new slider value (`number | array`)
  // `thumbs`: an array containing one object for each thumb, of the form `{ moved: boolean, interacted?: boolean }`:
  //   - `moved`: indicates whether the thumb at this index has moved
  //   - `interacted`: indicates whether the thumb at this index is the one with which the user has interacted
  //   => so any thumb object with `{ moved: true }` (i.e. no `interacted` property) indicates a thumb that moved due to pearling
}}

Pros:

• Still backwards compatible (for users who don't care about this feature)
• No information left out
• Kinda self-explanatory (at least more than the previous API)
• All information in one place (we could even repeat the value of each thumb so people don't have to loop over two separate arrays).

Cons:

• Breaking change
• Same amount of complexity as in #232, since the React state of ReactSlider still has to keep track of the same amount of new information. Note however, that I believe we could move this information to a mutable instance property since it doesn't need to/shouldn't trigger re-renders. This might simplify things a bit.

[stonebk]

Sorry, I have been quite busy lately.

The alternative API is not really backwards compatible since you are changing the type of the thumbs array values from an integer to an object.

[axelboc]

Sorry, I have been quite busy lately.

No rush 😉 -- and again, if you don't think this issue is worth resolving, don't hesitate to close it.

The alternative API is not really backwards compatible since you are changing the type of the thumbs array values from an integer to an object.

Haha yeah of course, I meant more for the people who aren't concerned by this feature and only care about the first parameter (i.e. the vast majority of people). Just like adding the index parameter did not affect them, replacing the index parameter with an object won't affect them either. That being said, I get that this would require a major version bump regardless.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.