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][NativeSelect] Polish CSS classes #26186

Merged
merged 9 commits into from
May 13, 2021

Conversation

m4theushw
Copy link
Member

@m4theushw m4theushw commented May 7, 2021

Breaking changes

  • [Select] The selectMenu slot was merged into select because selectMenu slot was redundant. The root slot is no longer applied to the select, but to the root.

    -<Select classes={{ root: 'class1', select: 'class2', selectMenu: 'class3' }} />
    +<Select classes={{ root: 'class1', select: 'class2 class3' }} />
  • [NativeSelect] The selectMenu slot was merged into select because selectMenu slot was redundant. The root slot is no longer applied to the select, but to the root.

    -<NativeSelect classes={{ root: 'class1', select: 'class2', selectMenu: 'class3' }} />
    +<NativeSelect classes={{ select: 'class1 class2 class3' }} />
  • [TablePagination] The classes.input key was changed to be applied on another element. Use classes.select instead.

    <TablePagination
    - classes={{ input: 'foo' }}
    + classes={{ select: 'foo' }}
    />

Part of #20012
Closes #11646
Closes #25313

I couldn't understand very well the discussion that lead to this change. I found #25766 (comment) about removing root and select. But the TODO in the code was to remove selectMenu. If it was not the specificity bug raised in #25763, we would only need to keep root.

@mui-pr-bot
Copy link

mui-pr-bot commented May 7, 2021

Details of bundle changes (experimental)

Generated by 🚫 dangerJS against edd965d

@oliviertassinari oliviertassinari added the component: select This is the name of the generic UI component, not the React module! label May 7, 2021
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost good to go from my end

packages/material-ui/src/Select/Select.d.ts Outdated Show resolved Hide resolved
packages/material-ui/src/Select/selectClasses.js Outdated Show resolved Hide resolved
overridesResolver: (props, styles) => {
const { styleProps } = props;
return {
[`&.${selectClasses.select}`]: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a comment on why we bump the specificity, from what I remember, it's important to guarantee that the customization is applied. The base style comes from the InputBase-input which we have no injection order guarantee with.

Ah yes, we say "// Win specificity over the input base" below

Copy link
Member

@oliviertassinari oliviertassinari May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to keep the bump:

Suggested change
[`&.${selectClasses.select}`]: {
// Win specificity over the input base
'&': {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m4theushw what do you think about this comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Missed this one because there's already a comment below.

packages/material-ui/src/Select/SelectInput.js Outdated Show resolved Hide resolved
docs/src/pages/guides/migration-v4/migration-v4.md Outdated Show resolved Hide resolved
@m4theushw m4theushw changed the title [Select] Polish CSS classes [Select][NativeSelect] Polish CSS classes May 12, 2021
@m4theushw m4theushw merged commit e526be5 into mui:next May 13, 2021
@m4theushw m4theushw deleted the polish-select-classes branch May 13, 2021 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: select This is the name of the generic UI component, not the React module!
Projects
None yet
4 participants