Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Select] Add comment on why padding-right is enforced on Select component #24453

Closed
nebbles opened this issue Jan 17, 2021 · 2 comments · Fixed by #24509
Closed

[Select] Add comment on why padding-right is enforced on Select component #24453

nebbles opened this issue Jan 17, 2021 · 2 comments · Fixed by #24509
Labels
component: select This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@nebbles
Copy link

nebbles commented Jan 17, 2021

https://github.com/mui-org/material-ui/blob/42954448866f204b0cf7e35127d5d0af9c24d6fb/packages/material-ui/src/NativeSelect/NativeSelect.js#L42-L44

In the above src code, the paddingRight style is applied with higher (double) class specificity to the component. Strangely, this is the only style rule applied in this way. This means, to override this style, I have to use the exact same pattern in my makeStyles class, i.e.

const useStyles = makeStyles((theme: Theme) => {
    return {
        select: {
                "paddingTop": 12,
                "paddingBottom": 12,
                "paddingLeft": 16,
                "&&": {
                    paddingRight: 40, // only way to override
                },
        },

//...

		<Select
                classes={{ select: classes.select }}

the above works.

However, by not following the exact same pattern to override, and instead applying the paddingRight in the same way as the other padding values, then the underlying NativeSelect style takes priority. As you can see in the Chrome style window (see image), the specificity of Mui's implementation means it cannot be overridden using the simple class prop.

(.MuiSelect-select.MuiSelect-select is ranked higher than .makeStyles-select-2357)

Screenshot of Chrome style window

image

Why does the style specified in NativeSelect use the higher specificity (double ampersand)? Is what I have done the expected way to override the right padding in this case?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 17, 2021

@nebbles This style was added in #18520, a similar issue was raised in #18520 (comment). You can better see in #18771 why this is required.

To make it short, the CSS injection order between the input components and the select component is nondeterministic. To ensure the select always wins on the same CSS property (padding-right), we use a CSS specificity of 2 instead of 1.

I think that we could add a comment to explain it. It's not a common situation. We wouldn't need it with emotion, but because we also support sc, and because the select is meant to extend custom inputs, we need to keep this rule.

Maybe something like this:

diff --git a/packages/material-ui/src/NativeSelect/NativeSelect.js b/packages/material-ui/src/NativeSelect/NativeSelect.js
index f41e1a4de2..6e62ec6eaa 100644
--- a/packages/material-ui/src/NativeSelect/NativeSelect.js
+++ b/packages/material-ui/src/NativeSelect/NativeSelect.js
@@ -39,6 +39,7 @@ export const styles = (theme) => ({
     '&:not([multiple]) option, &:not([multiple]) optgroup': {
       backgroundColor: theme.palette.background.paper,
     },
+    // Bump specificity to allow extending custom inputs
     '&&': {
       paddingRight: 24,
     },

@oliviertassinari oliviertassinari changed the title Why is padding-right enforced on Select component? [Select] Add comment on why is padding-right enforced on Select component Jan 17, 2021
@oliviertassinari oliviertassinari added component: select This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation labels Jan 17, 2021
@mbrookes mbrookes changed the title [Select] Add comment on why is padding-right enforced on Select component [Select] Add comment on why padding-right is enforced on Select component Jan 17, 2021
@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jan 18, 2021
@nebbles
Copy link
Author

nebbles commented Jan 19, 2021

Ah I see. I did find the tangle between the atomic components that Select combines a bit fiddly when working on my own style override.

🙏 Thanks for the explanation. I'd agree that adding in the docs would be helpful but can understand this is a little niche. For now, I'll be sticking with && specificity.

Looking forward to v5! Can't wait for the upgraded styling system with much better custom palettes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants