mui / material-ui

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

Consistent disabled state for all components #19343

Open LorenzHenk opened 4 years ago

LorenzHenk commented 4 years ago

At the moment, the disabled color of the slider is theme.palette.grey[400]. Other inputs use theme.palette.action.disabled (also used e.g. here) and theme.palette.text.disabled.

The Slider should use theme.palette.action.disabled as well.

Current Behavior 😯

The Slider uses theme.palette.grey[400].

Expected Behavior πŸ€”

The Slider should use theme.palette.action.disabled.

Steps to Reproduce πŸ•Ή

Example showing that theme.palette.grey[400] is used: https://codesandbox.io/s/material-demo-qpzdk

image

Note: I've forced the color with !important for the screenshot above.

Actually, this demo shows another problem with the current style system - the custom disabled style is not applied, because the css specifity of the default disabled class is higher than the one of the custom class - is this wished? Should I create another issue for this problem?

image

image

Steps:

  1. Create a custom theme to see the difference between grey[400] and action.disabled
  2. set disabled to true

Context πŸ”¦

We edit some values of the palette, which leads to a difference between grey[400] and actions.disabled. All disabled inputs have the same color, except for the Slider component. image

Your Environment 🌎

Tech Version
Material-UI v4.8.3
oliviertassinari commented 4 years ago

@LorenzHenk Interesting. The specification has a section about the disabled state:

A disabled state communicates when a component or element isn’t interactive, and should be deemphasized in a UI. Disabled states are displayed at 38% opacity of the enabled state. Disabled states can also indicate they are inactive through color changes and reduced elevation.

https://material.io/design/interaction/states.html#disabled.

It seems that we can simplify things. What do you think of these changes?

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 25c676bdb..5cd94371b 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -208,7 +208,7 @@ export const styles = theme => ({
       backgroundColor: theme.palette.action.hover,
     },
     '&[aria-disabled="true"]': {
-      opacity: 0.5,
+      opacity: theme.palette.action.disabledOpacity,
       pointerEvents: 'none',
     },
     '&:active': {
diff --git a/packages/material-ui-lab/src/Rating/Rating.js b/packages/material-ui-lab/src/Rating/Rating.js
index 2089fd521..130f099b3 100644
--- a/packages/material-ui-lab/src/Rating/Rating.js
+++ b/packages/material-ui-lab/src/Rating/Rating.js
@@ -40,7 +40,7 @@ export const styles = theme => ({
     cursor: 'pointer',
     WebkitTapHighlightColor: 'transparent',
     '&$disabled': {
-      opacity: 0.5,
+      opacity: theme.palette.action.disabledOpacity,
       pointerEvents: 'none',
     },
     '&$focusVisible $iconActive': {
diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js
index 70118bf44..bc014858e 100644
--- a/packages/material-ui/src/Chip/Chip.js
+++ b/packages/material-ui/src/Chip/Chip.js
@@ -38,7 +38,7 @@ export const styles = theme => {
       verticalAlign: 'middle',
       boxSizing: 'border-box',
       '&$disabled': {
-        opacity: 0.5,
+        opacity: theme.palette.action.disabledOpacity,
         pointerEvents: 'none',
       },
       '& $avatar': {
diff --git a/packages/material-ui/src/Slider/Slider.js b/packages/material-ui/src/Slider/Slider.js
index f90c5fe9b..2725d0e64 100644
--- a/packages/material-ui/src/Slider/Slider.js
+++ b/packages/material-ui/src/Slider/Slider.js
@@ -142,7 +142,10 @@ export const styles = theme => ({
     '&$disabled': {
       pointerEvents: 'none',
       cursor: 'default',
-      color: theme.palette.grey[400],
+      color:
+        theme.palette.type === 'light'
+          ? lighten('#000000', 1 - theme.palette.action.disabledOpacity)
+          : darken('#ffffff', 1 - theme.palette.action.disabledOpacity),
     },
     '&$vertical': {
       width: 2,
diff --git a/packages/material-ui/src/Tab/Tab.js b/packages/material-ui/src/Tab/Tab.js
index 56424569b..7fd51ed93 100644
--- a/packages/material-ui/src/Tab/Tab.js
+++ b/packages/material-ui/src/Tab/Tab.js
@@ -44,7 +44,7 @@ export const styles = theme => ({
       opacity: 1,
     },
     '&$disabled': {
-      opacity: 0.5,
+      opacity: theme.palette.action.disabledOpacity,
     },
   },
   /* Styles applied to the root element if the parent [`Tabs`](/api/tabs/) has `textColor="primary"`. */
diff --git a/packages/material-ui/src/styles/createPalette.js b/packages/material-ui/src/styles/createPalette.js
index 32db973fe..3e13072b1 100644
--- a/packages/material-ui/src/styles/createPalette.js
+++ b/packages/material-ui/src/styles/createPalette.js
@@ -39,7 +39,8 @@ export const light = {
     // The color of a selected action.
     selected: 'rgba(0, 0, 0, 0.14)',
     // The color of a disabled action.
-    disabled: 'rgba(0, 0, 0, 0.26)',
+    disabled: 'rgba(0, 0, 0, 0.38)',
+    disabledOpacity: 0.38,
     // The background color of a disabled action.
     disabledBackground: 'rgba(0, 0, 0, 0.12)',
   },
@@ -63,7 +64,8 @@ export const dark = {
     hover: 'rgba(255, 255, 255, 0.1)',
     hoverOpacity: 0.1,
     selected: 'rgba(255, 255, 255, 0.2)',
-    disabled: 'rgba(255, 255, 255, 0.3)',
+    disabled: 'rgba(255, 255, 255, 0.38)',
+    disabledOpacity: 0.38,
     disabledBackground: 'rgba(255, 255, 255, 0.12)',
   },
 };
elmeerr commented 4 years ago

@oliviertassinari how does the new changes regarding state anatomy impacts this issue? I might take a better look on those components

oliviertassinari commented 4 years ago

@elmeerr I don't understand what you mean? I have tried to take it into account :).

elmeerr commented 4 years ago

@oliviertassinari oh, sorry! I just wanna know if it's possible to use the new properties from theme (like you suggested in green) to update more components (if it's needed)

oliviertassinari commented 4 years ago

@elmeerr Right, I haven't looked into it. It's possible :)

LorenzHenk commented 4 years ago

@oliviertassinari I've also found the opacity hardcoded in ExpansionPanelSummary and ListItem:

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 25c676bdb..5cd94371b 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -208,7 +208,7 @@ export const styles = theme => ({
       backgroundColor: theme.palette.action.hover,
     },
     '&[aria-disabled="true"]': {
-      opacity: 0.5,
+      opacity: theme.palette.action.disabledOpacity,
       pointerEvents: 'none',
     },
     '&:active': {
diff --git a/packages/material-ui-lab/src/Rating/Rating.js b/packages/material-ui-lab/src/Rating/Rating.js
index 2089fd521..130f099b3 100644
--- a/packages/material-ui-lab/src/Rating/Rating.js
+++ b/packages/material-ui-lab/src/Rating/Rating.js
@@ -40,7 +40,7 @@ export const styles = theme => ({
     cursor: 'pointer',
     WebkitTapHighlightColor: 'transparent',
     '&$disabled': {
-      opacity: 0.5,
+      opacity: theme.palette.action.disabledOpacity,
       pointerEvents: 'none',
     },
     '&$focusVisible $iconActive': {
diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js
index 70118bf44..bc014858e 100644
--- a/packages/material-ui/src/Chip/Chip.js
+++ b/packages/material-ui/src/Chip/Chip.js
@@ -38,7 +38,7 @@ export const styles = theme => {
       verticalAlign: 'middle',
       boxSizing: 'border-box',
       '&$disabled': {
-        opacity: 0.5,
+        opacity: theme.palette.action.disabledOpacity,
         pointerEvents: 'none',
       },
       '& $avatar': {
diff --git a/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js b/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
index 53db1fdc5..ea75713bb 100644
--- a/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
+++ b/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
@@ -28,7 +28,7 @@ export const styles = theme => {
         backgroundColor: theme.palette.grey[300],
       },
       '&$disabled': {
-        opacity: 0.38,
+        opacity: theme.palette.action.disabledOpacity,
       },
     },
     /* Pseudo-class applied to the root element, children wrapper element and `IconButton` component if `expanded={true}`. */
diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js
index bb14d8aec..121c5ca73 100644
--- a/packages/material-ui/src/ListItem/ListItem.js
+++ b/packages/material-ui/src/ListItem/ListItem.js
@@ -29,7 +29,7 @@ export const styles = theme => ({
       backgroundColor: theme.palette.action.selected,
     },
     '&$disabled': {
-      opacity: 0.5,
+      opacity: theme.palette.action.disabledOpacity,
     },
   },
   /* Styles applied to the `container` element if `children` includes `ListItemSecondaryAction`. */
diff --git a/packages/material-ui/src/Slider/Slider.js b/packages/material-ui/src/Slider/Slider.js
index f90c5fe9b..2725d0e64 100644
--- a/packages/material-ui/src/Slider/Slider.js
+++ b/packages/material-ui/src/Slider/Slider.js
@@ -142,7 +142,10 @@ export const styles = theme => ({
     '&$disabled': {
       pointerEvents: 'none',
       cursor: 'default',
-      color: theme.palette.grey[400],
+      color:
+        theme.palette.type === 'light'
+          ? lighten('#000000', 1 - theme.palette.action.disabledOpacity)
+          : darken('#ffffff', 1 - theme.palette.action.disabledOpacity),
     },
     '&$vertical': {
       width: 2,
diff --git a/packages/material-ui/src/Tab/Tab.js b/packages/material-ui/src/Tab/Tab.js
index e5042d6a1..f4c5931b0 100644
--- a/packages/material-ui/src/Tab/Tab.js
+++ b/packages/material-ui/src/Tab/Tab.js
@@ -43,7 +43,7 @@ export const styles = theme => ({
       opacity: 1,
     },
     '&$disabled': {
-      opacity: 0.5,
+      opacity: theme.palette.action.disabledOpacity,
     },
   },
   /* Styles applied to the root element if the parent [`Tabs`](/api/tabs/) has `textColor="primary"`. */
diff --git a/packages/material-ui/src/styles/createPalette.js b/packages/material-ui/src/styles/createPalette.js
index cbb66038f..8075d6044 100644
--- a/packages/material-ui/src/styles/createPalette.js
+++ b/packages/material-ui/src/styles/createPalette.js
@@ -40,7 +40,8 @@ export const light = {
     selected: 'rgba(0, 0, 0, 0.08)',
     selectedOpacity: 0.08,
     // The color of a disabled action.
-    disabled: 'rgba(0, 0, 0, 0.26)',
+    disabled: 'rgba(0, 0, 0, 0.38)',
+    disabledOpacity: 0.38,
     // The background color of a disabled action.
     disabledBackground: 'rgba(0, 0, 0, 0.12)',
   },
@@ -65,7 +66,8 @@ export const dark = {
     hoverOpacity: 0.08,
     selected: 'rgba(255, 255, 255, 0.16)',
     selectedOpacity: 0.16,
-    disabled: 'rgba(255, 255, 255, 0.3)',
+    disabled: 'rgba(255, 255, 255, 0.38)',
+    disabledOpacity: 0.38,
     disabledBackground: 'rgba(255, 255, 255, 0.12)',
   },
 };
TommyJackson85 commented 4 years ago

Can I do anything to help? As a first issue?

oliviertassinari commented 4 years ago

@LorenzHenk Sorry for the delay. Do you want to work on the problem? I have updated #10870 to track all the progress on this epic (umbrella issue).

LorenzHenk commented 4 years ago

Sure!