[Switch] Create SwitchUnstyled and useSwitch #26688

michaldudak · 2021-06-10T13:43:04Z

This is an implementation of the SwitchUnstyled component and the useSwitch hook. One chunk of #6218

Summary

useSwitch is a low-level utility for both internal and external use when maximum customizability is required. It provides an object to be used as input props
SwitchUnstyled creates a structure of components and uses useSwitch internally. Root, Thumb and Input components are overridable via components prop (and component, being equivalent to components.Root).
The original Switch component used quite a deep hierarchy: Switch -> SwitchBase -> ButtonBase -> input. The new unstyled one (and, in consequence the Switch) does not use neither ButtonBase nor SwitchBase anymore. Part of the logic was moved to useSwitch (handling of focusVisible) and part was moved to a new useTouchRipple hook.
Initially I indented to implment better type checking of componentsProps, but after encountering problems with PropTypes, I created a separate issue for this: [typescript] Better type annotations for unstyled components and componentsProps props #26810
A new testing utility has been created: describeConformanceUnstyled. It executes the existing tests that apply to unstyled components + a few new ones (testing components and componentsProps props)

Preview: https://deploy-preview-26688--material-ui.netlify.app/components/switches/#unstyled-switches

To do

tests
documentation
demos
minimal styles for SwitchUnstyled -> will be done in a separate PR if needed. For now, the unstyled component is truly unstyled.

mui-pr-bot · 2021-06-10T13:45:06Z

Details of bundle changes (experimental)

@material-ui/core: parsed: +0.55% , gzip: +0.59%
@material-ui/unstyled: parsed: +9.02% , gzip: +7.55%

Generated by 🚫 dangerJS against 530e91e

mnajdova

Initial review. If we go ahead with this notion of introducing hooks, I think it's interesting to read trough this - https://kentcdodds.com/blog/the-state-reducer-pattern-with-react-hooks

docs/src/pages/components/switches/UseSwitches.js

packages/material-ui-unstyled/src/ButtonUnstyled/ButtonUnstyled.tsx

packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx

oliviertassinari · 2021-06-10T15:03:12Z

Great initiative to open a draft so we can discuss on the changes early. The first thing that got my attention is regarding the constraints. The existing components aren't using the new unstyled ones (yet?). What's the plan regarding this aspect?

I can think of two different approaches:

Unstyled button and unstyled switches are isolated from the existing components. Pros: no risk of integration that fail, more guidance by extracting what works, faster?
We start from scratch, we work on the API regardless of what already exists, and try to make it fit as a second step. Pros: reduce upfront constraints that allows to rethink it, higher quality?

It seems that we are going with option 2.

In the two cases, one big challenge is which level we extract/abstract. It has to be enough abstracted to make it valuable for developers (solve problems for them, otherwise they will write their own hook). It also has to be low level enough to allow us to implement a second design specification

michaldudak · 2021-06-10T18:42:58Z

Great initiative to open a draft so we can discuss on the changes early. The first thing that got my attention is regarding the constraints. The existing components aren't using the new unstyled ones (yet?). What's the plan regarding this aspect?

My next goal will be changing the Core Switch to use this implementation.

I can think of two different approaches:

Unstyled button and unstyled switches are isolated from the existing components. Pros: no risk of integration that fail, more guidance by extracting what works, faster?

We start from scratch, we work on the API regardless of what already exists, and try to make it fit as a second step. Pros: reduce upfront constraints that allows to rethink it, higher quality?

It seems that we are going with option 2.

I chose to build the components from scratch using the existing functionality as reference. Of course I copied a lot of code from the existing components. I find it easier to start with a blank slate and add things when I see they are necessary, than start from a full-blown component and remove things that are not needed. Additionally, I wanted to build these components in TypeScript from the start.

In the two cases, one big challenge is which level we extract/abstract. It has to be enough abstracted to make it valuable for developers (solve problems for them, otherwise they will write their own hook). It also has to be low level enough to allow us to implement a second design specification

I agree - it's hard to draw a line properly. For sure, all the accessibility features and nothing Material-specific should be in unstyled.
The sooner we start implementing the second theme, the sooner we will validate the decisions made to extracts the unstyled components. Then, we may revisit the unstyled API if necessary.

packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx

packages/material-ui-unstyled/src/ButtonUnstyled/ButtonUnstyled.tsx

docs/src/pages/components/switches/UnstyledSwitches.tsx

docs/src/pages/components/switches/switches.md

packages/material-ui-unstyled/src/ButtonUnstyled/ButtonUnstyled.tsx

packages/material-ui-unstyled/src/SwitchUnstyled/useSwitch.ts

packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx

docs/src/pages/components/switches/UseSwitches.js

oliviertassinari · 2021-06-14T00:04:19Z

My next goal will be changing the Core Switch to use this implementation.

@michaldudak This sounds like a great next step.
I'm not even sure we will need to write dedicated test cases, as we could rely on the end-to-end tests with the integration of Material Design (we have done mostly that so far for the unstyled components).

mnajdova · 2021-06-14T11:58:19Z

I'm not even sure we will need to write dedicated test cases, as we could rely on the end-to-end tests with the integration of Material Design (we have done mostly that so far for the unstyled components).

Agree, ideally we should not need to change any tests :)

michaldudak · 2021-06-15T20:05:51Z

I implemented the Switch component using the useSwitch hook. Looks just like the original and the unit tests pass. Now I'll focus more on making the rest of the tests pass and to improve the docs.

Inspired by #26303 I decided to remove the ButtonBase from the render tree. FocusVisible support is provided by useSwitch and I moved handling the ripple effect to the new useTouchRipple hook. This way, I don't need to include ButtonUnstyled in this PR (I'll move it to another one).

oliviertassinari · 2021-06-29T11:09:10Z

@michaldudak It looks great 👌✨

mnajdova

Looks pretty close to be ready for merge :)

docs/src/pages/components/switches/UnstyledSwitchesMaterial.js

docs/src/pages/components/switches/UseSwitchesCustom.js

packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx

packages/material-ui-unstyled/src/SwitchUnstyled/switchUnstyledClasses.ts

packages/material-ui-unstyled/src/SwitchUnstyled/useSwitch.ts

Co-authored-by: Marija Najdova <[email protected]>

michaldudak · 2021-07-02T08:36:14Z

@oliviertassinari, @mnajdova I addressed all the comments you made. Could you please take a final look at the PR and see if there's anything outstanding from your point of view?

oliviertassinari · 2021-07-05T13:26:56Z

packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx

+  } = props;
+
+  const Root: React.ElementType = component ?? components.Root ?? 'span';
+  const rootProps: any = { ...getForwardableProps(Root, otherProps), ...componentsProps.root };


Why is this not enough?

Suggested change

const rootProps: any = { ...getForwardableProps(Root, otherProps), ...componentsProps.root };

const rootProps: any = { ...otherProps, ...componentsProps.root };

If Root is a plain HTML element, I don't want to forward anything that doesn't apply to a HTML element.

I mean, I don't get why "foo" shouldn't warn when it does with the buttons.

import * as React from "react"; import Button from "@material-ui/core/Button"; import SwitchUnstyled from "@material-ui/unstyled/SwitchUnstyled"; export default function UnstyledSwitches() { return ( <div> <SwitchUnstyled foo /> <Button foo /> <button foo /> </div> ); }

https://codesandbox.io/s/unstyledswitches-material-demo-forked-rsrd1?file=/demo.tsx

The developers can swallow the extra prop. The new logic only check it with the "other" props on the root element, but not with the componentsProps slots.

We didn't have the logic up until, it didn't seem to be a pain point. What use case do you have in mind?

My thinking was that forwarding props from the host to the root is less explicit than just setting them in componentsProps.root and therefore it may need some additional checking. If someone explicitly passes something in componentsProps.*, I assume they know what they are doing.

This has an inconvenience of being inconsistent, though. I'm not particularly attached to this piece of code and if you feel we should always forward everything, so be it.

cc @mui-org/core for more perspectives on the matter.

Outside of this point, the PR is good to go, from my side.

I would forward all props, as it was done before. Let's not complicate the code for problems we haven't encounter so far.

mnajdova

One tiny last comment for improving the demo. Let's also resolve #26688 (comment) before merging. Well done 🚀

mnajdova · 2021-07-06T14:51:17Z

docs/src/pages/components/switches/UnstyledSwitchesMaterial.js

+const SwitchThumb = ({ isChecked, icon, checkedIcon, className }) => {
+  if (!isChecked && icon) {
+    return icon;
+  }
+
+  if (isChecked && checkedIcon) {
+    return checkedIcon;
+  }
+
+  return <DefaultSwitchThumb className={className} />;
+};


Suggested change

const SwitchThumb = ({ isChecked, icon, checkedIcon, className }) => {

if (!isChecked && icon) {

return icon;

}

if (isChecked && checkedIcon) {

return checkedIcon;

}

return <DefaultSwitchThumb className={className} />;

};

const SwitchThumb = styled(({ isChecked, icon, checkedIcon, className }) => {

if (!isChecked && icon) {

return icon;

}

if (isChecked && checkedIcon) {

return checkedIcon;

}

return <span className={className} />;

}, {

name: 'MuiSwitch',

slot: 'Thumb',

overridesResolver: (props, styles) => styles.thumb,

})(({ theme }) => ({

boxShadow: theme.shadows[1],

backgroundColor: 'currentColor',

width: 20,

height: 20,

borderRadius: '50%',

}));

How about we merge this component with the DefaultSwitchThumb? We would avoid having to think about names for components :)

This isn't equivalent. icon and checkedIcon shouldn't be styled by us.

What do you mean? They are not, they would be styled if we add the className prop no?

~~In your suggestion, if icon or checkedIcon is provided, they would have the styles applied (boxShadow, backgroundColor, etc.), which is not what we want~~ Sorry, I made wrong assumption about how styled works. You're right. I'll correct it.

khushalsrashtasoft · 2024-04-17T06:54:45Z

@mui-pr-bot use i am import { Switch } from "@mui/material";

I have this issue. I don't know what issue. above image switch casse css reflet but When I refresh page it gets fixed

michaldudak · 2024-04-17T09:46:02Z

@khushalsrashtasoft if you're experiencing problems with the Switch, please open a new issue and provide reproduction steps.

michaldudak requested review from oliviertassinari, mnajdova, eps1lon and siriwatknp June 10, 2021 13:43

mnajdova reviewed Jun 10, 2021

View reviewed changes

michaldudak commented Jun 10, 2021

View reviewed changes

packages/material-ui-unstyled/src/SwitchUnstyled/SwitchUnstyled.tsx Outdated Show resolved Hide resolved

michaldudak mentioned this pull request Jun 11, 2021

Provide a version of the components without any styles #6218

Closed

mnajdova reviewed Jun 11, 2021

View reviewed changes

packages/material-ui-unstyled/src/ButtonUnstyled/ButtonUnstyled.tsx Outdated Show resolved Hide resolved

oliviertassinari reviewed Jun 14, 2021

View reviewed changes

oliviertassinari added the package: base-ui Specific to @mui/base label Jun 14, 2021

michaldudak added 12 commits June 15, 2021 19:16

Add initial implementation of useSwitch and SwitchUnstyled

30852a2

Use checkbox to handle switch state

9b9c56e

Add ButtonUnstyled and use it in SwitchUnstyled

fe11962

Working on the API

62dc051

Prettier

512757e

Add type checks for components and componentsProps

903b934

Fix forwardRef type definition

b5e8d04

Include tsx files in unstyled tsconfig

69a2519

Lint, prettier

7ed4a3e

PropTypes

6b18590

Implement the Switch using useSwitch

cbf19d8

Handle ripple effect

9ca3348

michaldudak force-pushed the feat/unstyled-switch branch from 2c18d8a to 9ca3348 Compare June 15, 2021 17:17

Fix failing test

6129be7

Polish the demos

305df4a

oliviertassinari mentioned this pull request Jun 29, 2021

[Switch] ref attribute is not the root element #19613

Open

2 tasks

Code style

b98cafb

mnajdova reviewed Jun 30, 2021

View reviewed changes

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

Merge branch 'next' into feat/unstyled-switch

abb7ca8

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

michaldudak and others added 4 commits July 2, 2021 08:42

Apply suggestions from code review

3ad767a

Co-authored-by: Marija Najdova <[email protected]>

Apply suggestions from CR

00fedfa

Merge branch 'next' into feat/unstyled-switch

6ba8485

Make the demos consistent

43451ad

michaldudak and others added 5 commits July 2, 2021 11:47

Add JSDoc to useSwitch

692013b

Merge className applied on host with the one from componentsProps

c2cb70c

remove blank space convention

98b0582

leading dot

37309ca

sort props asc

eb31636

oliviertassinari reviewed Jul 5, 2021

View reviewed changes

mnajdova approved these changes Jul 6, 2021

View reviewed changes

oliviertassinari changed the title ~~[SwitchUnstyled] Create SwitchUnstyled and useSwitch~~ [Switch] Create SwitchUnstyled and useSwitch Jul 6, 2021

michaldudak added 3 commits July 7, 2021 09:36

Forward all host props to the root component

05bd709

Merge branch 'next' into feat/unstyled-switch

2fe768d

Use less components in UnstyledSwitchesMaterial

530e91e

michaldudak merged commit 0230df9 into mui:next Jul 7, 2021

michaldudak deleted the feat/unstyled-switch branch July 7, 2021 10:05

michaldudak mentioned this pull request Feb 27, 2024

Unstyled components progress mui/base-ui#10

Open

32 tasks

oliviertassinari mentioned this pull request Jul 11, 2021

v5.0.0-beta.1 #27230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Switch] Create SwitchUnstyled and useSwitch #26688

[Switch] Create SwitchUnstyled and useSwitch #26688

michaldudak commented Jun 10, 2021 •

edited by oliviertassinari

Loading

mui-pr-bot commented Jun 10, 2021 •

edited

Loading

mnajdova left a comment

oliviertassinari commented Jun 10, 2021

michaldudak commented Jun 10, 2021

oliviertassinari commented Jun 14, 2021 •

edited

Loading

mnajdova commented Jun 14, 2021

michaldudak commented Jun 15, 2021

oliviertassinari commented Jun 29, 2021 •

edited

Loading

mnajdova left a comment

michaldudak commented Jul 2, 2021

oliviertassinari Jul 5, 2021

michaldudak Jul 5, 2021

oliviertassinari Jul 5, 2021 •

edited

Loading

michaldudak Jul 6, 2021

oliviertassinari Jul 6, 2021 •

edited

Loading

mnajdova Jul 6, 2021

mnajdova left a comment

mnajdova Jul 6, 2021

michaldudak Jul 7, 2021

mnajdova Jul 7, 2021

michaldudak Jul 7, 2021 •

edited

Loading

khushalsrashtasoft commented Apr 17, 2024 •

edited

Loading

michaldudak commented Apr 17, 2024

	const rootProps: any = { ...getForwardableProps(Root, otherProps), ...componentsProps.root };
	const rootProps: any = { ...otherProps, ...componentsProps.root };

-const SwitchThumb = ({ isChecked, icon, checkedIcon, className }) => {
-  if (!isChecked && icon) {
-    return icon;
-  }
-  if (isChecked && checkedIcon) {
-    return checkedIcon;
-  }
-  return <DefaultSwitchThumb className={className} />;
-};
+const SwitchThumb = styled(({ isChecked, icon, checkedIcon, className }) => {
+  if (!isChecked && icon) {
+    return icon;
+  }
+  if (isChecked && checkedIcon) {
+    return checkedIcon;
+  }
+  return <span className={className} />;
+}, {
+  name: 'MuiSwitch',
+  slot: 'Thumb',
+  overridesResolver: (props, styles) => styles.thumb,
+})(({ theme }) => ({
+  boxShadow: theme.shadows[1],
+  backgroundColor: 'currentColor',
+  width: 20,
+  height: 20,
+  borderRadius: '50%',
+}));

[Switch] Create SwitchUnstyled and useSwitch #26688

[Switch] Create SwitchUnstyled and useSwitch #26688

Conversation

michaldudak commented Jun 10, 2021 • edited by oliviertassinari Loading

Summary

To do

mui-pr-bot commented Jun 10, 2021 • edited Loading

mnajdova left a comment

Choose a reason for hiding this comment

oliviertassinari commented Jun 10, 2021

michaldudak commented Jun 10, 2021

oliviertassinari commented Jun 14, 2021 • edited Loading

mnajdova commented Jun 14, 2021

michaldudak commented Jun 15, 2021

oliviertassinari commented Jun 29, 2021 • edited Loading

mnajdova left a comment

Choose a reason for hiding this comment

michaldudak commented Jul 2, 2021

oliviertassinari Jul 5, 2021

Choose a reason for hiding this comment

michaldudak Jul 5, 2021

Choose a reason for hiding this comment

oliviertassinari Jul 5, 2021 • edited Loading

Choose a reason for hiding this comment

michaldudak Jul 6, 2021

Choose a reason for hiding this comment

oliviertassinari Jul 6, 2021 • edited Loading

Choose a reason for hiding this comment

mnajdova Jul 6, 2021

Choose a reason for hiding this comment

mnajdova left a comment

Choose a reason for hiding this comment

mnajdova Jul 6, 2021

Choose a reason for hiding this comment

michaldudak Jul 7, 2021

Choose a reason for hiding this comment

mnajdova Jul 7, 2021

Choose a reason for hiding this comment

michaldudak Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

khushalsrashtasoft commented Apr 17, 2024 • edited Loading

michaldudak commented Apr 17, 2024

michaldudak commented Jun 10, 2021 •

edited by oliviertassinari

Loading

mui-pr-bot commented Jun 10, 2021 •

edited

Loading

oliviertassinari commented Jun 14, 2021 •

edited

Loading

oliviertassinari commented Jun 29, 2021 •

edited

Loading

oliviertassinari Jul 5, 2021 •

edited

Loading

oliviertassinari Jul 6, 2021 •

edited

Loading

michaldudak Jul 7, 2021 •

edited

Loading

khushalsrashtasoft commented Apr 17, 2024 •

edited

Loading