-
-
Notifications
You must be signed in to change notification settings - Fork 91
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
[Slider] Fix thumb positioning #1411
[Slider] Fix thumb positioning #1411
Conversation
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
3d84498
to
9b9e937
Compare
9b9e937
to
1ac42ee
Compare
5ccd245
to
c419c5e
Compare
event: React.KeyboardEvent | React.ChangeEvent, | ||
) => void; | ||
/** | ||
* Callback fired when drag ends and invokes onValueCommitted. |
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.
nit: it's really up to the consumer to decide when this function is really called (as useSliderRoot doesn't call it itself as far as I can see). IMO a better description would be "Function to be called when drag ends. Invokes onValueCommitted
"
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.
Nice, bugs seem to be fixed. I have a minor remark, but it's good to go anyway.
Fixes #1360
Fixes #1406
Demo: https://stackblitz.com/edit/vitejs-vite-zsatgbgk?file=src%2FApp.tsx
The demo shows both fixes:
when setting the value with the NumberField, the corresponding slider should reposition correctly
when dragging a thumb to match the same value set by a NumberField (of an adjacent row) the thumbs should line up exactly and have the same CSS
width
valueI have followed (at least) the PR section of the contributing guide.