[Slider] Replace SliderUnstyled with useSlider hook
#35770
Conversation
ZeeshanTamboli
added
component: slider
@material-ui/core: parsed: -0.17% 😍, gzip: -0.19% 😍 |
Signed-off-by: Zeeshan Tamboli <zeeshan.tamboli@gmail.com>
| }, | ||
| ownerState: { | ||
| ...ownerState, | ||
| ...rootSlotProps?.ownerState, |
There was a problem hiding this comment.
I wonder what could be the use case for providing ownerState in slotProps.*. I can't think of any scenarios that would require overriding the state, so if you have anything in mind, please share.
There was a problem hiding this comment.
I don't have anything in mind or a use case in mind. I had wondered the same while adding this. I later thought that I didn't want to introduce any breaking changes, so let's keep it.
I don't quite know the context of why it was added. We can remove it in v6.
|
@michaldudak Ready for further review. |
ZeeshanTamboli
deleted the
slider/replace-SliderUnstyled-with-useSlider-hook
branch
| > | ||
| <ThumbSlot | ||
| data-index={index} | ||
| data-focusvisible={focusedThumbIndex === index} |
There was a problem hiding this comment.
What is this data-focusvisible attribute for? It doesn't seem used anywhere. I see that it was added in https://github.com/mui/material-ui/pull/33132/files#diff-975fc00fa1f3f65ac56ee8a55c1b67cb95fc72bf7dcfa6669ffd3f3dc4ef6b74R269.
I believe that we already have a class name for this problem .Mui-focusVisible, so better maximum the learning transferability between components.
| data-focusvisible={focusedThumbIndex === index} |
There was a problem hiding this comment.
Nice catch. Looks unnecessarily added. Created PR: #36091
Motivation:
See #35398.
Move the logic of
valueLabelDisplayprop to Material UI. To do this, we migrate the Material UI Slider component to use useSlider hook to get access to the states. This has the advantage of how the Slider's component structure is rendered by Material UI. We are able to have thevalueLabelDisplaylogic controlled which is not needed in MUI Base and does not work as expected there. We also create the SliderValueLabel component in Material UI for value label.We also did this since Material UI's Slider was dependent on SliderUnstyled and there are changes (improvements) in SliderUnstyled in ZeeshanTamboli#2.
The JSX structure is kept intact like the
SliderUnstyled.There should be no breaking changes.
Previously created PR: #35479 where we decided to split it in two PRs.
Follow up PR: ZeeshanTamboli#2