mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.86k stars 32.26k forks source link

[Slider] Can not use prop classes #24033

Closed mtr1990 closed 3 years ago

mtr1990 commented 3 years ago

I'm using Slider component and apply
classes={{ markLabel: classes.markLabel, markLabelActive: classes.markLabelActive }}

but it's not working. Here is demo: https://codesandbox.io/s/discretesliderlabel-material-demo-forked-u3g57?file=/demo.js

mnajdova commented 3 years ago

Thanks for reporting @mtr1990 I could find two problems here.

The first one is a bug in the SliderUnstyled component, where we are not sending the input classes when generating the class names for each part of the slider. This diff will solve that issue:

diff --git a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
index 715e0c7f1b..71c927df75 100644
--- a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
+++ b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
@@ -615,7 +615,7 @@ const SliderUnstyled = React.forwardRef(function SliderUnstyled(props, ref) {
     marked: marks.length > 0 && marks.some((mark) => mark.label),
   };

-  const utilityClasses = useSliderClasses(stateAndProps);
+  const utilityClasses = useSliderClasses({ ...stateAndProps, classes: classesProps });

   return (
     <Root

The second issue I could found was around specificity. Basically, for the markLabelActive to be applied we need to increase the specificity, so the styles could look something like this:

const useStyles = makeStyles((theme) => ({
  root: {},
  markLabel: { 
    color: "red",
+    '&.MuiSlider-markLabelActive': { // the blue color will be applied
+      color: 'blue',
+    },
  },
-  markLabelActive: { color: "green" }
}));

Of course this is not what we want, so I will create a PR today to restructure the styles and make this scenario working.

oliviertassinari commented 3 years ago
  1. @mnajdova has been working on the CSS injection conflict between emotion and JSS during the time of the migration in #23934. You will need wrap your root.
  2. The classes is no longer applied, at all. This diff seems to fix it, with regression tests:
diff --git a/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.js b/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.js
index e877278e9b..f22966ed64 100644
--- a/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.js
+++ b/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.js
@@ -6,7 +6,7 @@ import isHostComponent from '../utils/isHostComponent';
 import badgeUnstyledClasses, { getBadgeUtilityClass } from './badgeUnstyledClasses';

 const useBadgeClasses = (props) => {
-  const { variant, anchorOrigin, overlap, invisible, classes = {} } = props;
+  const { variant, anchorOrigin, overlap, invisible, classes } = props;

   const utilityClasses = {
     root: clsx(badgeUnstyledClasses['root'], classes['root']),
@@ -40,7 +40,6 @@ const BadgeUnstyled = React.forwardRef(function BadgeUnstyled(props, ref) {
       vertical: 'top',
       horizontal: 'right',
     },
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
     classes: classesProp = {},
     badgeContent: badgeContentProp,
     component: Component = 'span',
@@ -85,6 +84,7 @@ const BadgeUnstyled = React.forwardRef(function BadgeUnstyled(props, ref) {

   const stateAndProps = {
     ...props,
+    classes: classesProp,
     anchorOrigin,
     badgeContent,
     invisible,
diff --git a/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.test.js b/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.test.js
index a98fd528f9..5e03a99457 100644
--- a/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.test.js
+++ b/packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.test.js
@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { expect } from 'chai';
 import { createMount, createClientRender, describeConformance } from 'test/utils';
-import BadgeUnstyled from '@material-ui/unstyled/BadgeUnstyled';
+import BadgeUnstyled, { badgeUnstyledClasses as classes } from '@material-ui/unstyled/BadgeUnstyled';

 describe('<BadgeUnstyled />', () => {
   const mount = createMount();
@@ -12,7 +12,7 @@ describe('<BadgeUnstyled />', () => {
       <div />
     </BadgeUnstyled>,
     () => ({
-      classes: {},
+      classes,
       inheritComponent: 'span',
       mount,
       refInstanceof: window.HTMLSpanElement,
diff --git a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
index 715e0c7f1b..52463cdd38 100644
--- a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
+++ b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
@@ -146,7 +146,7 @@ function doesSupportTouchActionNone() {
 }

 const useSliderClasses = (props) => {
-  const { disabled, marked, orientation, track, classes = {} } = props;
+  const { disabled, marked, orientation, track, classes } = props;

   const utilityClasses = {
     root: clsx(sliderUnstyledClasses['root'], classes['root'], {
@@ -183,7 +183,6 @@ const SliderUnstyled = React.forwardRef(function SliderUnstyled(props, ref) {
     'aria-valuetext': ariaValuetext,
     className,
     component: Component = 'span',
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
     classes: classesProps = {},
     defaultValue,
     disabled = false,
@@ -601,7 +600,7 @@ const SliderUnstyled = React.forwardRef(function SliderUnstyled(props, ref) {
   // consider extracting to hook an reusing the lint rule for the varints
   const stateAndProps = {
     ...props,
-    classes: {},
+    classes: classesProps,
     disabled,
     max,
     min,
diff --git a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.test.js b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyl
ed.test.js
index d23c6caf57..55c6b30924 100644
--- a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.test.js
+++ b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.test.js
@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { expect } from 'chai';
 import { createMount, createClientRender, describeConformance } from 'test/utils';
-import SliderUnstyled from './SliderUnstyled';
+import SliderUnstyled, { sliderClasses as classes } from '@material-ui/unstyled/SliderUnstyled';

 describe('<SliderUnstyled />', () => {
   before(function beforeHook() {
@@ -14,7 +14,7 @@ describe('<SliderUnstyled />', () => {
   const render = createClientRender();

   describeConformance(<SliderUnstyled value={0} />, () => ({
-    classes: {},
+    classes,
     inheritComponent: 'span',
     mount,
     refInstanceof: window.HTMLSpanElement,
diff --git a/packages/material-ui/src/Badge/Badge.test.js b/packages/material-ui/src/Badge/Badge.test.js
index b7e7aa46a4..20fd2221d4 100644
--- a/packages/material-ui/src/Badge/Badge.test.js
+++ b/packages/material-ui/src/Badge/Badge.test.js
@@ -1,8 +1,8 @@
 import * as React from 'react';
 import { expect } from 'chai';
-import { BadgeUnstyled } from '@material-ui/unstyled';
 import { createMount, createClientRender, describeConformanceV5 } from 'test/utils';
-import Badge, { badgeClasses as classes } from './Badge';
+import BadgeUnstyled from '@material-ui/unstyled/BadgeUnstyled';
+import Badge, { badgeClasses as classes } from '@material-ui/core/Badge';

 function findBadge(container) {
   return container.firstChild.querySelector('span');
@@ -25,7 +25,7 @@ describe('<Badge />', () => {
       <div />
     </Badge>,
     () => ({
-      classes: {},
+      classes,
       inheritComponent: BadgeUnstyled,
       mount,
       refInstanceof: window.HTMLSpanElement,
diff --git a/packages/material-ui/src/Slider/Slider.test.js b/packages/material-ui/src/Slider/Slider.test.js
index 9fa52c894b..134fbc6622 100644
--- a/packages/material-ui/src/Slider/Slider.test.js
+++ b/packages/material-ui/src/Slider/Slider.test.js
@@ -1,12 +1,12 @@
 import * as React from 'react';
 import PropTypes from 'prop-types';
+import clsx from 'clsx';
 import { spy, stub } from 'sinon';
 import { expect } from 'chai';
 import { createMount, describeConformanceV5, act, createClientRender, fireEvent } from 'test/utils';
 import { ThemeProvider, createMuiTheme } from '@material-ui/core/styles';
-import { SliderUnstyled } from '@material-ui/unstyled';
-import clsx from 'clsx';
-import Slider, { sliderClasses as classes } from './Slider';
+import SliderUnstyled from '@material-ui/unstyled/SliderUnstyled';
+import Slider, { sliderClasses as classes } from '@material-ui/core/Slider';

 function createTouches(touches) {
   return {
@@ -32,7 +32,7 @@ describe('<Slider />', () => {
   const render = createClientRender();

   describeConformanceV5(<Slider value={0} />, () => ({
-    classes: {},
+    classes,
     inheritComponent: SliderUnstyled,
     mount,
     refInstanceof: window.HTMLSpanElement,
diff --git a/test/utils/describeConformance.js b/test/utils/describeConformance.js
index 2af297e853..8b496186ac 100644
--- a/test/utils/describeConformance.js
+++ b/test/utils/describeConformance.js
@@ -98,12 +98,12 @@ function testComponentProp(element, getOptions) {
 export function testPropsSpread(element, getOptions) {
   it(`spreads props to the root component`, () => {
     // type def in ConformanceOptions
-    const { classes, inheritComponent, mount } = getOptions();
+    const { inheritComponent, mount } = getOptions();
     const testProp = 'data-test-props-spread';
     const value = randomStringValue();

     const wrapper = mount(React.cloneElement(element, { [testProp]: value }));
-    const root = findRootComponent(wrapper, { classes, component: inheritComponent });
+    const root = findRootComponent(wrapper, { component: inheritComponent });

     expect(root.props()[testProp]).to.equal(value);
   });
diff --git a/test/utils/describeConformanceV5.js b/test/utils/describeConformanceV5.js
index 1df49cdcb9..2f904fa1f3 100644
--- a/test/utils/describeConformanceV5.js
+++ b/test/utils/describeConformanceV5.js
@@ -21,11 +21,11 @@ import {
 function testComponentsProp(element, getOptions) {
   describe('prop: components', () => {
     it('can render another root component with the `components` prop', () => {
-      const { classes, mount, testComponentsRootPropWith: component = 'em' } = getOptions();
+      const { mount, testComponentsRootPropWith: component = 'em' } = getOptions();

       const wrapper = mount(React.cloneElement(element, { components: { Root: component } }));

-      expect(findRootComponent(wrapper, { classes, component }).exists()).to.equal(true);
+      expect(findRootComponent(wrapper, { component }).exists()).to.equal(true);
     });
   });
 }
  1. In v4, the specificity used for .MuiSlider-markLabelActive is 1, we didn't do .MuiSlider-markLabel.Mui-active. However, in v5, we have increased the specificity.