[Slider] Modernize implementation by mj12albert · Pull Request #583 · mui/base-ui

mj12albert · 2024-09-05T06:22:27Z

Summary

Replaced useCompound with Composite

Previously the id attribute of all the input elements were passed to useCompound as metadata to generate the for attribute of the output element.

However the refs provided by Composite can't be used during the render of Output for this, so the ids have to be held in a new state

Converted types to namespaces
Fix [slider] Vertical slider incorrectly exposed as horizontal in the accessibility tree in Chrome<124 #635
Hooks/context are no longer exported for now - it's no longer possible to (easily) implement a Material Design slider as it would require custom subcomponents
Matches latest export convention in [internal] Improve DX of importing Base UI components #700

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2024-09-05T06:25:20Z

Netlify deploy preview

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

Generated by 🚫 dangerJS against 129bdec

…omposite

michaldudak

It would be great if @atomiks could review the usage of the Composite component

michaldudak · 2024-10-11T12:03:24Z

docs/app/experiments/slider-marks.tsx

 import * as React from 'react';
 import * as Slider from '@base_ui/react/Slider';
-import { useSliderContext } from '@base_ui/react/Slider';
+import { useSliderContext } from '../../../packages/mui-base/src/Slider/Root/SliderContext';


If there is a legitimate use case for the context, we should export it from the package.

We discussed this and decided to revisit exporting Context(s) after the alpha phase

michaldudak · 2024-10-11T12:05:28Z

docs/data/components/slider/UnstyledSliderIntroduction/css-modules/index.tsx

+        <Slider.Control className={classes.control}>
+          <Slider.Track className={classes.track}>
+            <Slider.Indicator className={classes.indicator} />
+            <Slider.Thumb className={classes.thumb} inputId=":slider-thumb-input:" />


The inputId prop is not documented

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

packages/mui-base/src/Slider/Indicator/useSliderIndicator.ts

michaldudak · 2024-10-11T12:14:44Z

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

+/**
+ * @ignore - internal component.
+ */
+export const SliderContext = React.createContext<SliderRoot.Context | undefined>(undefined);


Context type should be defined in this file (and be called SliderContext)

Updated ~ I've also renamed it to SliderRootContext/useSliderRootContext to match other components

Yeah, that's actually what I meant 😅

…omposite

atomiks · 2024-10-15T04:11:19Z

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

+
+  // Map with index (DOM position) as the key and the id attribute of each thumb <input> element as the value
+  const [inputIdMap, setInputMap] = React.useState(() => new Map<number, string>());
+
+  const deregisterInputId = React.useCallback((index: number) => {
+    setInputMap((prevMap) => {
+      const nextMap = new Map(prevMap);
+      nextMap.delete(index);
+      return nextMap;
+    });
+  }, []);
+
+  const registerInputId = React.useCallback(
+    (index: number, inputId: string | undefined) => {
+      if (index > -1 && inputId !== undefined) {
+        setInputMap((prevMap) => new Map(prevMap).set(index, inputId));
+      }
+
+      return {
+        deregister: deregisterInputId,
+      };
+    },
+    [deregisterInputId],
+  );


Is it possible to modify the Composite source to allow for metadata instead?

Should be! How about doing this when refactoring Tabs to use Composite? Tabs uses metadata more extensively, and I wanted to get this PR merged ASAP to unblock the ESM work @michaldudak is doing 🙏

Seems ok to me as of now

This reverts commit 48ed1ff.

mj12albert added the component: slider Changes related to the slider component. label Sep 5, 2024

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 5, 2024

mj12albert force-pushed the refactor/slider-composite branch from 4293925 to b595967 Compare September 5, 2024 09:00

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 5, 2024

mj12albert force-pushed the refactor/slider-composite branch 4 times, most recently from 753b46c to 04e16b3 Compare September 10, 2024 16:34

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 11, 2024

mj12albert force-pushed the refactor/slider-composite branch from 04e16b3 to eff3fb8 Compare September 11, 2024 10:15

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

mj12albert force-pushed the refactor/slider-composite branch from eff3fb8 to cf27b23 Compare September 12, 2024 05:44

mj12albert marked this pull request as ready for review September 13, 2024 09:51

mj12albert requested review from atomiks, colmtuite and michaldudak as code owners September 13, 2024 09:51

mj12albert force-pushed the refactor/slider-composite branch from 3d02250 to f3e90f3 Compare September 19, 2024 07:09

mj12albert marked this pull request as draft September 20, 2024 04:30

mj12albert changed the title ~~[Slider] Replace useCompound with Composite~~ [Slider] Modernize implementation Sep 20, 2024

mj12albert force-pushed the refactor/slider-composite branch from f3e90f3 to 5dadbbc Compare September 26, 2024 07:53

mj12albert added 6 commits September 27, 2024 09:45

Convert to namespace types

55473b0

Replace useCompound with CompositeList

e47af47

Update demos

5f601d3

Apply CSS writing mode on input element

ee7e3fc

Clean up experiments

6ba2983

Remove useSliderContext from demos

6f8e65f

mj12albert force-pushed the refactor/slider-composite branch from 5dadbbc to 6f8e65f Compare September 27, 2024 07:45

mj12albert added 2 commits September 27, 2024 09:52

Remove useSliderContext from experiments

e1709e8

Unexport Slider hooks and context

e7e7141

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 3, 2024

mj12albert added 3 commits October 9, 2024 15:23

Merge remote-tracking branch 'upstream/master' into refactor/slider-c…

8a04fd9

…omposite

Move proptypes

f209871

Merge remote-tracking branch 'upstream/master' into refactor/slider-c…

5014a56

…omposite

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 9, 2024

Merge remote-tracking branch 'upstream/master' into refactor/slider-c…

1d2c4d2

…omposite

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

Merge remote-tracking branch 'upstream/master' into refactor/slider-c…

97dc5b8

…omposite

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 10, 2024

mj12albert marked this pull request as ready for review October 11, 2024 12:00

michaldudak reviewed Oct 11, 2024

View reviewed changes

mj12albert force-pushed the refactor/slider-composite branch 3 times, most recently from 491175e to f16af45 Compare October 13, 2024 06:39

michaldudak mentioned this pull request Oct 14, 2024

[internal] Make React context definitions consistent #735

Merged

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 14, 2024

Merge remote-tracking branch 'upstream/master' into refactor/slider-c…

364fea9

…omposite

mj12albert force-pushed the refactor/slider-composite branch from f16af45 to 364fea9 Compare October 14, 2024 09:05

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 14, 2024

Fix demo css

b78035c

atomiks reviewed Oct 15, 2024

View reviewed changes

mj12albert requested a review from michaldudak October 15, 2024 04:47

michaldudak approved these changes Oct 15, 2024

View reviewed changes

mj12albert added 2 commits October 15, 2024 17:41

Cleanup

48ed1ff

Revert "Cleanup"

129bdec

This reverts commit 48ed1ff.

mj12albert merged commit 680f5b3 into mui:master Oct 15, 2024

mj12albert deleted the refactor/slider-composite branch October 15, 2024 09:57

atomiks pushed a commit to atomiks/base-ui that referenced this pull request Oct 30, 2024

[Slider] Modernize implementation (mui#583)

68424d0

mj12albert mentioned this pull request Dec 13, 2024

[Slider] Replace internal map with Composite metadata #1082

Merged

1 task

Uh oh!

Conversation

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

Uh oh!

Summary

Uh oh!

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

Uh oh!

Netlify deploy preview

Uh oh!

michaldudak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mj12albert commented Sep 5, 2024 •

edited

Loading

mui-bot commented Sep 5, 2024 •

edited

Loading