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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Slider] Clamp thumbs position within range #1988

Merged
merged 4 commits into from Mar 17, 2023

Conversation

kombucha
Copy link
Contributor

@kombucha kombucha commented Feb 26, 2023

Description

It is possible to pass values that are outside of the defined min and max, in which case the positioning of the ui elements currently behaves unexpectedly. This change should bring the behaviour closer to what the native range input does.

  • Clamp thumb and range position percentages between 0 and 100 to avoid stretching UI outside of its natural boundaries
  • Added 2 cases in the Slider's Chromatic story to prevent regressions when dealing with out of bound values 馃И

In the before, the slider is stretched beyond its natural width (200px) 馃憖

Before After
Screenshot 2023-02-26 at 21 51 58 Screenshot 2023-02-26 at 21 53 28

@kombucha
Copy link
Contributor Author

Hey @benoitgrelard (sorry for the direct ping) 馃憢

Does the team have the bandwidth to review external contribution these days?
I see activity on the repo, but I also see a few PRs that are left hanging 馃槄

I understand if you don't, just need to know if I should start addressing this bug in user-land instead of waiting for upstream.

Thank you!

Copy link
Collaborator

@benoitgrelard benoitgrelard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution @kombucha, I've left a few comments.

packages/react/slider/src/Slider.tsx Outdated Show resolved Hide resolved
packages/react/slider/src/Slider.test.tsx Outdated Show resolved Hide resolved
@benoitgrelard benoitgrelard merged commit 7b197e5 into radix-ui:main Mar 17, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants