[Slider] New Slider components and hook by mj12albert · Pull Request #373 · mui/base-ui

mj12albert · 2024-04-29T10:38:27Z

Closes #216
Closes #76
Closes #77

Preview

https://deploy-preview-373--base-ui.netlify.app/base-ui/react-slider/

Major changes

Thumbs can no longer swap positions/be dragged past each other. The related disableSwap prop has been dropped as well.
A new Control subcomponent is added to represent the click/touch area of the slider. Previously this was combined with the root, but Slider.Root in the new API is expected to contain the Output element and a label as well
The rail slot/component previously referred to the full length of the slider. This is now renamed to instead be the Track
The previous track component referred to the filled portion of the slider "bar". This is now renamed to instead be the Indicator
The API does not expose the thumb's input element anymore, it's automatically wired to the corresponding thumb (a span by default), and allows for styling the thumb directly with :focus-visible
The Mark component/slot, and the marks prop will be dropped. Marks will be reintroduced with a new API in [slider] Support for marks #462
The step prop defaults to 1 and can no longer be null.
The ValueLabel slot/component and related features have been dropped. This can be implemented using Tooltip as in this demo
The restricted values feature will be redesigned, and decoupled from any props related to the visual marks: [slider] Arbitrarily restrict the possible values of the slider #412
The isRtl prop is replaced with direction: 'ltr' | 'rtl' for consistency
A new minStepsBetweenValues prop (default 0) is added to prevent thumb overlap
The scale prop is deprecated

Summary

All relevant tests from material-ui have been ported over, a few new ones were added
The render prop for the Thumb component is a special case that does not use standard utils, because we need to render a "hidden" <input> element. As a function it provides an additional inputProps argument, and it also supports the shorthand syntax. Documented here.
On the final location of the input - it's kept as a child of the thumb (for now) because we could solve [slider] Issues with adding focus-visible styles to the thumb slot #77 without actually having to move the input

Demos

The first 4 are also in the docs

Basic/Range: https://deploy-preview-373--base-ui.netlify.app/experiments/slider/
With DIY <label> & aria-labelledby: https://deploy-preview-373--base-ui.netlify.app/experiments/slider/#with-labels
Vertical: https://deploy-preview-373--base-ui.netlify.app/experiments/slider/#vertical
RTL: https://deploy-preview-373--base-ui.netlify.app/experiments/slider/#rtl
Old landing page temperature slider with new API: https://deploy-preview-373--base-ui.netlify.app/experiments/slider-marks/
Joy Design slider that integrates Slider and Tooltip: https://deploy-preview-373--base-ui.netlify.app/experiments/slider-tooltip

mui-bot · 2024-05-11T01:01:23Z

Netlify deploy preview

https://deploy-preview-373--base-ui.netlify.app/

Generated by 🚫 dangerJS against 37059b8

packages/mui-base/src/Slider/Root/SliderRoot.tsx

packages/mui-base/src/Slider/SliderControl/SliderControl.tsx

colmtuite · 2024-06-27T08:58:26Z

@mj12albert

When you focus a single thumb slider, VO announces “[label], group, [value], group”. I think it should announce “[label], slider, [value], group” or “[value], [label], slider, [label], group”?
In a range slider, when you move focus from the first thumb to the second thumb, VO announces “end range, [value], group”. I think it should announce “[value], end range, slider”.
What is the scale prop on Root?

mj12albert · 2024-06-28T04:19:35Z

In a range slider, when you move focus from the first thumb to the second thumb, VO announces “end range, [value], group”. I think it should announce “[value], end range, slider”.

I've updated this so it announces "value" before "start/end range", it's announcing correctly as "slider" for me and not group though, I think this is just a VO quirk:

range.mov

When you focus a single thumb slider, VO announces “[label], group, [value], group”. I think it should announce “[label], slider, [value], group” or “[value], [label], slider, [label], group”?

You mean when VO focuses on the Root and not the Thumb right? I don't think it's possible to get it to announce "slider" unless it explicitly has the slider role (which is obviously incorrect). It also doesn't announce "group" like that for me, it could also be a VO issue:

single.mov

What is the scale prop on Root?

The scale prop enables a non-linear scale (a feature from Material UI): https://mui.com/material-ui/react-slider/#non-linear-scale
We could add a demo for this, unless you want to get rid of it? (Or are you just asking)

@colmtuite

colmtuite · 2024-06-28T14:14:14Z

@mj12albert Ok on the a11y stuff 👍

On scale, I was just asking what it was. I read the MUI docs just now and still not sure what it does? 😅

packages/mui-base/src/Slider/SliderControl/SliderControl.test.tsx

michaldudak · 2024-06-28T21:43:23Z

packages/mui-base/src/Slider/Root/useSliderRoot.ts

+    rootRef,
+    scale = Identity,
+    step = 1,
+    tabIndex,


tabIndex seems unused

@michaldudak tabIndex in the return value here gets put into context and is used by the Thumb like this:

base-ui/packages/mui-base/src/Slider/Root/SliderRoot.test.tsx

Lines 1587 to 1590 in 55a30f7

it('can be removed from the tab sequence', () => {

render(<TestSlider tabIndex={-1} value={30} />);

expect(screen.getByRole('slider')).to.have.property('tabIndex', -1);

});

I suppose it could be passed straight from the SliderRoot component into the context without going through the hook, what do you think?

Yup, if it's not used by the hook, it shouldn't go there.

packages/mui-base/src/Slider/Root/SliderRoot.types.ts

michaldudak · 2024-06-28T21:53:54Z

packages/mui-base/src/Slider/Root/useSliderRoot.ts

+
+  const handleRootRef = useForkRef(rootRef, sliderRef);
+
+  const areValuesEqual = React.useCallback(


Is there any specific reason this is returned by the hook and not defined as a static function that takes two parameters to compare?

@michaldudak The 2nd parameter (the "old" value to be compared against) would be the internal valueState which is internal to useSliderRoot, I opted to pass this function through the hook instead of passing valueState as it seemed messier to pass 3 "values" through the hook - values, percentageValues, and valueState

packages/mui-base/src/Slider/SliderThumb/SliderThumb.tsx

michaldudak · 2024-07-01T06:43:43Z

packages/mui-base/src/Slider/SliderThumb/SliderThumb.types.ts

+      ) => React.ReactElement)
+    | (React.ReactElement & { ref: React.Ref<Element> })
+    | keyof typeof defaultRenderFunctions;
+}


It should be possible anyway with the standard useComponentRenderer, right?
render={(props) => <CustomThumb {...props}><SomeOtherComponent />{props.children}</SomeOtherComponent></CustomThumb>}.
I suppose it would just require some changes to the short form of the render prop (render={<CustomThumb><SomeOtherComponent /></CustomThumb}), to merge the children with internal children.

mj12albert · 2024-07-02T05:33:36Z

I read the MUI docs just now and still not sure what it does? 😅

Not sure if I can explain it better than the docs but let me try 😓 :

First set up a slider like this: (omitted subcomponents for simplicity)

<Slider.Root min={5} step={1} max={10} defaultValue={5} />

The possible values are: 5, 6, 7, 8,9, 10

Now we add the scale prop, which is a function that can transform the slider value.

<Slider.Root min={5} step={1} max={10} defaultValue={5} scale={(value) => 2 ** value } />

So now the default value of 5 will become 2^5 = 32
And the possible values of the slider become 2^5, 2^6, 2^7... 2^10 instead, for example to make a slider that lets you pick values among: 32kb, 64kb, 128kb, 256kb, 512kb like the Material UI demo

Does this make sense? Maybe it's the name scale that is unclear 🤔 @colmtuite

70339a3

colmtuite · 2024-07-02T07:58:23Z

@mj12albert I understand now, thanks for explaining. Let's remove this functionality+prop for now.

mj12albert · 2024-07-02T08:48:37Z

@mj12albert I understand now, thanks for explaining. Let's remove this functionality+prop for now.

OK I've removed it

oliviertassinari · 2024-07-27T15:51:44Z

docs/data/base/components/slider/slider.md

+```jsx
+<Slider.Thumb
+  render={(props, inputProps) => {
+    const { children, ...rest } = props;


We might want to change the naming convention in the codebase for this, but for now since it's one of the only variations we have in the codebase, it's simpler to stick to it (it's simpler to do a mass codebase update all at once if we want to change this).

Suggested change

const { children, ...rest } = props;

const { children, ...other } = props;

Done in 89821a4

(I found this variation from reading the docs in MUI X Charts initially, I then did a quick search in VS Code where my environment has all the codebase of the organization in the VS Code workspace, to see the codebase patterns, and I found this in Base UI too).

mj12albert added the component: slider Changes related to the slider component. label Apr 29, 2024

mj12albert force-pushed the feat/slider-rewrite branch 13 times, most recently from dd496d9 to 366c478 Compare May 6, 2024 10:31

mj12albert mentioned this pull request May 8, 2024

[slider] Thumb shifts to the right upon clicking #76

Closed

This comment was marked as resolved.

Sign in to view

mj12albert force-pushed the feat/slider-rewrite branch from 366c478 to b1707f7 Compare May 8, 2024 10:32

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

mj12albert force-pushed the feat/slider-rewrite branch 2 times, most recently from b4723a8 to 1c97949 Compare May 11, 2024 00:58

mj12albert added 3 commits June 20, 2024 14:39

Remove internal open state

e4da263

Update thumb render function

70339a3

Merge remote-tracking branch 'upstream/master' into feat/slider-rewrite

2122858

michaldudak reviewed Jun 24, 2024

View reviewed changes

mj12albert added 3 commits June 24, 2024 17:03

Fix component renderer params

2797c1d

Merge remote-tracking branch 'upstream/master' into feat/slider-rewrite

e4e2118

Merge remote-tracking branch 'upstream/master' into feat/slider-rewrite

4211af7

Merge remote-tracking branch 'upstream/master' into feat/slider-rewrite

20012fd

Update range slider default aria-valuetext

55a30f7

michaldudak reviewed Jul 1, 2024

View reviewed changes

mj12albert added 3 commits July 2, 2024 12:27

Merge remote-tracking branch 'upstream/master' into feat/slider-rewrite

4495a10

Update types

dc1aeb2

Return native events instead of synthetic

1f865dd

mj12albert added 3 commits July 2, 2024 13:34

Rename directories

fbe5ef1

Revert "Update thumb render function"

8556c03

70339a3

Update Thumb render shorthand

d07a34b

mj12albert added 2 commits July 2, 2024 16:44

Remove scale prop

c6b4565

Merge remote-tracking branch 'upstream/master' into feat/slider-rewrite

37059b8

colmtuite approved these changes Jul 2, 2024

View reviewed changes

michaldudak approved these changes Jul 2, 2024

View reviewed changes

oliviertassinari mentioned this pull request Jul 19, 2024

[slider] Home / End regression #484

Closed

oliviertassinari reviewed Jul 27, 2024

View reviewed changes

ertrzyiks mentioned this pull request Sep 24, 2024

[SliderUnstyled] TypeError Cannot read property 'getBoundingClientRect' of null mui/material-ui#26754

Open

	it('can be removed from the tab sequence', () => {
	render(<TestSlider tabIndex={-1} value={30} />);
	expect(screen.getByRole('slider')).to.have.property('tabIndex', -1);
	});


		const handleRootRef = useForkRef(rootRef, sliderRef);

		const areValuesEqual = React.useCallback(

	const { children, ...rest } = props;
	const { children, ...other } = props;

Uh oh!

Conversation

mj12albert commented Apr 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preview

Major changes

Summary

Demos

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

mui-bot commented May 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Netlify deploy preview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

colmtuite commented Jun 27, 2024

Uh oh!

mj12albert commented Jun 28, 2024

Uh oh!

colmtuite commented Jun 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

michaldudak Jun 28, 2024

Choose a reason for hiding this comment

Uh oh!

mj12albert Jul 2, 2024

Choose a reason for hiding this comment

Uh oh!

michaldudak Jul 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

michaldudak Jun 28, 2024

Choose a reason for hiding this comment

Uh oh!

mj12albert Jul 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michaldudak Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

mj12albert commented Jul 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

colmtuite commented Jul 2, 2024

Uh oh!

mj12albert commented Jul 2, 2024

Uh oh!

oliviertassinari Jul 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mj12albert commented Apr 29, 2024 •

edited

Loading

mui-bot commented May 11, 2024 •

edited

Loading

colmtuite commented Jun 28, 2024 •

edited

Loading

mj12albert commented Jul 2, 2024 •

edited

Loading

oliviertassinari Jul 27, 2024 •

edited

Loading