Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[react-aria-components] slider is not clamped between min and max during render #4978

Closed
meesvandongen opened this issue Aug 26, 2023 · 3 comments · Fixed by #5543
Closed
Labels
enhancement New feature or request RAC

Comments

@meesvandongen
Copy link

meesvandongen commented Aug 26, 2023

Provide a general summary of the issue here

The thumb of the slider is located outside of the max value when the current value exceeds the max value. e.g. when we have value=200, and the range is [0,100], and each 1 value is 1 pixel, the slider would be at 200px. In my opinion this does not make sense, as it does not follow what input type=range does.****

🤔 Expected Behavior?

The value of getThumbPercent is clamped between 0 and 1.

😯 Current Behavior

The value of getThumbPercent is not clamped

💁 Possible Solution

clamp the value of getThumbPercent between 0 and 1

🔦 Context

The value of some of the slider I use on a website can get values from multiple sources; while the min / max makes sense from a logical point of view, sometimes the actual value may be outside the range.

🖥️ Steps to Reproduce

https://codesandbox.io/s/react-tailwind-starter-forked-7qzhym?file=/src/App.js

Version

react-aria-components@1.0.0-alpha.6

What browsers are you seeing the problem on?

Chrome

What operating system are you using?

Windows

@snowystinger
Copy link
Member

Hey, thanks for the issue.

I don't think a Slider is the best thing to use to show the value if it can be outside the range. Either the range should be big enough to fit all values or a different control should be used. For instance, if a user adjusts the slider while it's at 200, it'll jump to be within the range immediately, and the user will never be able to get 200 back.

If you're actually clamping the value to the Slider, then you should have no problem with this as you can clean up the value before sending it to the Slider. That would then accurately reflect what has happened in the system. Instead of relying on us to implicitly handle it. For instance, if the user never adjusts the slider, you may still have the original value stored and save that instead of the clamped value.

@meesvandongen
Copy link
Author

meesvandongen commented Aug 28, 2023

I understand your position, but I disagree. Sometimes the world is chaotic and an unexpected overflow or underflow may happen. I can account for it in my application, because it is expected; but others may not. As such I believe the library should account for the edge case. In that case, clamping looks more coherent and adheres to the spec, while not doing so does not provide any advantages: the scenario where the user would adjust the slider at 200 will function the same in either implementation.

@snowystinger
Copy link
Member

Thanks for the link to the spec. I'm fine considering it.
We'd also probably want to consider it for NumberField, which can also have a min/max. I've just checked it and we don't clamp/sanitize that one either.
https://codesandbox.io/s/react-tailwind-starter-forked-rgx924?file=/src/App.js There may be others as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request RAC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants