[Select] - When the Select is disabled the dropdown arrow is not greyed out

[fbarbare]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The dropdown arrow is not greyed out

Expected Behavior 🤔

As per the Material Design documentation, when a TextField or Select is disabled, everything should be greyed out, even Input Adornments:

Steps to Reproduce 🕹

https://codesandbox.io/s/gifted-brattain-og6xh

Context 🔦

Small design inconsistency.

Your Environment 🌎

Tech	Version
Material-UI	v4.9.4
React	16.12
Browser
TypeScript	3.7
etc.

[oliviertassinari]

There is a reference to it in https://material.io/design/interaction/states.html#disabled. From what I understand, we could use inheritance (opacity)

[HenryLie]

I'd like to try tackling this issue, but I wonder what is the best way to do it?
Is adding MuiSvgIcon-disabled class to the icon and applying .38 opacity as per the documentation enough?

[oliviertassinari]

@HenryLie What do you think of this patch?

diff --git a/packages/material-ui/src/NativeSelect/NativeSelect.js b/packages/material-ui/src/NativeSelect/NativeSelect.js
index e3049ebab..32f04c9c5 100644
--- a/packages/material-ui/src/NativeSelect/NativeSelect.js
+++ b/packages/material-ui/src/NativeSelect/NativeSelect.js
@@ -72,7 +72,8 @@ export const styles = (theme) => ({
     position: 'absolute',
     right: 0,
     top: 'calc(50% - 12px)', // Center vertically
-    color: theme.palette.action.active,
+    color: 'currentColor',
+    opacity: 0.8,
     pointerEvents: 'none', // Don't block pointer events on the select under the icon.
   },
   /* Styles applied to the icon component if the popup is open. */

I think that it would better communicate the styles we want. The same color as the text but less visually strong to account for the bigger surface of the arrow (compared to the text).

It's to be noted that the Autocomplete component uses a different strategy:

diff --git a/packages/material-ui/src/NativeSelect/NativeSelect.js b/packages/material-ui/src/NativeSelect/NativeSelect.js
index e3049ebab..bce383f85 100644
--- a/packages/material-ui/src/NativeSelect/NativeSelect.js
+++ b/packages/material-ui/src/NativeSelect/NativeSelect.js
@@ -74,6 +74,9 @@ export const styles = (theme) => ({
     top: 'calc(50% - 12px)', // Center vertically
     color: theme.palette.action.active,
     pointerEvents: 'none', // Don't block pointer events on the select under the icon.
+    '&$disabled': {
+      color: theme.palette.action.disabled,
+    }
   },
   /* Styles applied to the icon component if the popup is open. */
   iconOpen: {
diff --git a/packages/material-ui/src/Select/SelectInput.js b/packages/material-ui/src/Select/SelectInput.js
index 595b3001b..0a5000b2e 100644
--- a/packages/material-ui/src/Select/SelectInput.js
+++ b/packages/material-ui/src/Select/SelectInput.js
@@ -348,6 +348,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
       />
       <IconComponent
         className={clsx(classes.icon, classes[`icon${capitalize(variant)}`], {
+          [classes.disabled]: disabled,
           [classes.iconOpen]: open,
         })}
       />

Maybe we should unify both, I wonder.

[HenryLie]

@oliviertassinari
The approach with currentColor looks very clean and concise.
Personally, I am in favor of unifying this behavior into Autocomplete, since it results in one less class and makes the code shorter overall.

I'll submit a PR that uses the currentColor approach on both Select and Autocomplete component.

[oliviertassinari]

@HenryLie Ok, this sounds fair 👍

[HenryLie]

@oliviertassinari

Sorry, I just noticed a drawback of applying this method on the Autocomplete.
In this component, the icon uses IconButton, and it already applies theme.palette.action.disabled color to the icon with a stronger specificity when disabled prop is passed into it.

Since we can't remove the disabled prop from the icon (since it'll make it clickable), applying the reduced opacity will make the icon have double reduced opacity. It's especially apparent in dark theme (not sure if it's visible in this screenshots)

Original style

Updated with currentColor

What do you think of doing it the other way around and use the second approach in your patch?

[oliviertassinari]

@HenryLie You would need to increase specificity too with the disabled pseudo-class.

[HenryLie]

I just submitted a PR to both fix the issue and unify the behavior with Autocomplete.
I'm not sure if the implementation is optimal, but I think discussing it in the PR will be better.