[SliderUnstyled] Add useSlider hook and polish #30094

mnajdova · 2021-12-07T12:35:35Z

Adds useSlider hook and updates the SliderUnstyled component's logic to be compatible with the other unstyled components. I don't expect there to be breaking changes in the @mui/material's Slider component. One change that I needed to make is to handle the component prop in the Slider component, as it differs to how it is implemented in the SliderUnstyled (the component prop just changes the tag, the components.Root replaces the whole root element, for example the styles won't be applied).

In a follow up PR I plan to address the TODOs that came up while converting the code to the hook which is written in TypeScript.

mui-pr-bot · 2021-12-07T12:37:53Z

Details of bundle changes

@material-ui/core: parsed: +0.03% , gzip: +0.08%
@material-ui/unstyled: parsed: +1.17% , gzip: +1.38%

Generated by 🚫 dangerJS against d556c51

mnajdova · 2021-12-08T12:14:49Z

packages/mui-material/src/Slider/Slider.js

@@ -457,25 +459,26 @@ const Slider = React.forwardRef(function Slider(inputProps, ref) {
        ...componentsProps,
        root: {
          ...componentsProps.root,
-          ...(shouldSpreadOwnerState(components.Root) && {
+          ...(shouldSpreadAdditionalProps(components.Root) && {
+            as: component,


This was previously handled in the SliderUnstyled. The logic was moved here now.

test/utils/describeConformanceUnstyled.tsx

…dova/material-ui into base/update-slider-unstyled

michaldudak · 2021-12-09T15:30:27Z

packages/mui-base/src/SliderUnstyled/useSlider.tsx

+  scale?: (value: number) => number;
+  step?: number | null;
+  tabIndex?: number;
+  value?: number | number[];


Technically, the value is number | [number, number] as arrays of size != 2 don't make sense in this context. But I'm afraid this could be a breaking change.

In general not being able to experiment and introduce breaking changes on Slider and Badge may be a bit limiting. We'll have to discuss it in the future.

Actually no, I believe there can be more values, for example, if more than two thumbs are used. See https://www.w3.org/TR/wai-aria-practices/#slidertwothumb.

In general not being able to experiment and introduce breaking changes on Slider and Badge may be a bit limiting. We'll have to discuss it in the future.

Agree, let's see how far we can get, and if there are blockers we can create duplicates components (not ideal, but I don't have any other idea)

michaldudak · 2021-12-09T15:42:24Z

packages/mui-base/src/SliderUnstyled/useSlider.tsx

+
+  const getTrackProps = () => {
+    return {
+      style: { ...trackStyle, ...componentsProps.track?.style },


How about instead of setting style directly in the hook we return the offset and leap values and let the caller set the styles? This way the hook would truly be "unstyled" (except for setting the styles of the hidden input, but I suppose that one is ok).

Yeah, let me try to refactor and see where we'll land

This would create a problem I think. I would expect that the unstyled version should handle this. If we let this be a responsibility of the users of the hook/unstyled component, we would need to document how each developer should set it, as it is related to the functionality of the component.

I will however move it to the SliderUnstyled as it is already setting style on some places for the thumbs, valueLabel etc, so that at least those will be in one place (in the component).

packages/mui-base/src/SliderUnstyled/useSlider.tsx

packages/mui-base/src/SliderUnstyled/SliderUnstyled.js

michaldudak · 2021-12-21T07:55:03Z

docs/pages/api-docs/slider.json

@@ -11,7 +11,6 @@
      },
      "default": "'primary'"
    },
-    "component": { "type": { "name": "elementType" } },


The component prop should remain in the docs, shouldn't it?

I think it's removed because the prop is available in the unstyled component. Will double check

michaldudak · 2021-12-21T08:01:55Z

packages/mui-base/src/SliderUnstyled/SliderUnstyled.js

  const classes = useUtilityClasses(ownerState);

  return (
    <Root
-      ref={handleRef}
-      onMouseDown={handleMouseDown}
+      {...getRootProps()}


Pass in the external event handlers, so that ours onMouseDown is not overridden by the caller accidentally.

It is handled in the hook itself, it was removed from here.

I meant that if someone adds an onMouseDown prop on SliderUnstyled, it'll shadow our internal handler defined in useSlider. To prevent this, pass in other to getRootProps.
Then, ideally, the createHandleMouseDown function in useSlider should check if defaultPrevented was set and skip its execution if it was.

It is destructured from the props, and the hook already handles this. Let me refactor to explicitly send it in the hook.

Addressed with - afac9ca

packages/mui-base/src/SliderUnstyled/SliderUnstyled.test.js

packages/mui-base/src/SliderUnstyled/useSlider.ts

michaldudak

👍

Note that #30362 has been completed, so you'll have to merge these changes.
Aside from that, it looks great.

mnajdova added 3 commits December 7, 2021 11:39

[SliderUnstyled] Update components & componentsProps

6814667

[useSlider] Add hook (WIP)

96c3dff

proptypes & docs:api

f47b01c

mnajdova added 3 commits December 8, 2021 09:41

Improve logic for forwarding props

086aa86

Various fixes

6f925a1

prettier

d3c0033

mnajdova marked this pull request as ready for review December 8, 2021 12:14

mnajdova requested a review from a team December 8, 2021 12:14

mnajdova commented Dec 8, 2021

View reviewed changes

test/utils/describeConformanceUnstyled.tsx Outdated Show resolved Hide resolved

Update test/utils/describeConformanceUnstyled.tsx

26287cc

mnajdova commented Dec 8, 2021

View reviewed changes

test/utils/describeConformanceUnstyled.tsx Outdated Show resolved Hide resolved

Run prettier

bd62bb2

mnajdova requested a review from michaldudak December 9, 2021 14:59

mnajdova added 4 commits December 10, 2021 11:40

Merge branch 'master' into base/update-slider-unstyled

57ff661

add handlers in the callback for the props

d7607ed

Merge branch 'base/update-slider-unstyled' of https://github.com/mnaj…

625e62d

…dova/material-ui into base/update-slider-unstyled

Remove unused imports

d81b1c8

michaldudak reviewed Dec 13, 2021

View reviewed changes

mnajdova added 5 commits December 17, 2021 06:53

Michal's review

662f1cd

Merge branch 'master' into base/update-slider-unstyled

cc5e511

Update the conformance test, remove getTrackProps usages

80fd507

Spread trackStyle

967978a

Various fixes

3cf779a

mnajdova requested a review from michaldudak December 20, 2021 09:02

oliviertassinari added package: base-ui Specific to @mui/base component: slider This is the name of the generic UI component, not the React module! labels Dec 20, 2021

michaldudak reviewed Dec 21, 2021

View reviewed changes

Merge branch 'master' into base/update-slider-unstyled

97ca246

Address review comments

52e5a7c

mnajdova requested a review from michaldudak December 21, 2021 10:22

Improve onMouseDown handling

afac9ca

mnajdova mentioned this pull request Dec 22, 2021

[Slider] Add input slot to components and componentsProps #30362

Merged

michaldudak reviewed Dec 27, 2021

View reviewed changes

packages/mui-base/src/SliderUnstyled/useSlider.ts Show resolved Hide resolved

packages/mui-base/src/SliderUnstyled/useSlider.ts Outdated Show resolved Hide resolved

Refactor event handlers

9f83569

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 10, 2022

michaldudak approved these changes Jan 10, 2022

View reviewed changes

mnajdova added 2 commits January 11, 2022 13:17

Merge branch 'master' into base/update-slider-unstyled

ebab422

Fixes after merge

d556c51

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 11, 2022

mnajdova merged commit 98212a3 into mui:master Jan 11, 2022

wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022

[SliderUnstyled] Add useSlider hook and polish (mui#30094)

dce16a0

michaldudak mentioned this pull request Feb 27, 2024

Unstyled components progress mui/base-ui#10

Open

35 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SliderUnstyled] Add useSlider hook and polish #30094

[SliderUnstyled] Add useSlider hook and polish #30094

mnajdova commented Dec 7, 2021 •

edited

mui-pr-bot commented Dec 7, 2021 •

edited

mnajdova Dec 8, 2021

michaldudak Dec 9, 2021

mnajdova Dec 17, 2021

michaldudak Dec 9, 2021

mnajdova Dec 14, 2021

mnajdova Dec 17, 2021 •

edited

michaldudak Dec 21, 2021

mnajdova Dec 21, 2021

michaldudak Dec 21, 2021

mnajdova Dec 21, 2021

michaldudak Dec 21, 2021

mnajdova Dec 22, 2021

mnajdova Dec 22, 2021

michaldudak left a comment •

edited

[SliderUnstyled] Add useSlider hook and polish #30094

[SliderUnstyled] Add useSlider hook and polish #30094

Conversation

mnajdova commented Dec 7, 2021 • edited

mui-pr-bot commented Dec 7, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova Dec 17, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaldudak left a comment • edited

Choose a reason for hiding this comment

mnajdova commented Dec 7, 2021 •

edited

mui-pr-bot commented Dec 7, 2021 •

edited

mnajdova Dec 17, 2021 •

edited

michaldudak left a comment •

edited