[Material You] Update Button test & add active class name #35497
Conversation
| @@ -919,6 +922,29 @@ function describeConformance( | |||
| minimalElement: React.ReactElement, | |||
| getOptions: () => InputConformanceOptions, | |||
| ) { | |||
| let originalMatchmedia: typeof window.matchMedia; | |||
There was a problem hiding this comment.
Need to mock the matchMedia for the CssVarsProvider to work.
| expect(button).not.to.have.class(classes.colorPrimary); | ||
| }); | ||
|
|
||
| ['filled', 'filledTonal', 'outlined', 'elevated'].forEach((variant) => { |
There was a problem hiding this comment.
Just simplified the tests, and changed the classes we want to test out. For example in Material You, we don't have a combination of classes with multiple props, like textPrimary, outlinedPrimary etc. This is why we need less tests.
|
| @@ -348,7 +350,7 @@ export const ButtonRoot = styled('button', { | |||
| backgroundColor: hoveredContainerColor[ownerState.variant ?? 'text'], | |||
| boxShadow: hoveredContainerElevation[ownerState.variant ?? 'text'], | |||
| }, | |||
| '&:active': { | |||
| [`&.${buttonClasses.active}`]: { | |||
There was a problem hiding this comment.
cc @siriwatknp we may need this change in Joy UI too. For e.g. see #35410 for bugs that could arise without using it.
| @@ -416,6 +418,7 @@ const Button = React.forwardRef(function Button< | |||
| centerRipple = false, | |||
| children, | |||
| className, | |||
| classes: classesProp, | |||
There was a problem hiding this comment.
Out of curiosity, does the Material You component still accept classes prop?
In Joy UI, the classes prop is removed because it can be done with slotProps.className.
There was a problem hiding this comment.
I left the API to be close with the current Material UI components. There are other adventages for the classes prop, like allowing state classes to be passed, like disabled, active etc. Also, we come to the understanding that it's a nice shortcut, as we've seen when working with the MUI Base components (that do not support this API).
I started the PR with the need to fix the
activestate for the Button, and then realized that in the test we were testing the@mui/material/Button, instead of@mui/material-next/Button, so I re-write the test and added some necessary features to thedescribeConformancetest suit, mainly around supporting differentcreateThemefeature.