-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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] Migrate to emotion #25653
[Select] Migrate to emotion #25653
Conversation
…grate-native-select
@@ -7,106 +7,6 @@ import ArrowDropDownIcon from '../internal/svg-icons/ArrowDropDown'; | |||
import Input from '../Input'; | |||
import useThemeProps from '../styles/useThemeProps'; | |||
|
|||
export const styles = (theme) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not used anymore.
{}, | ||
{ name: 'MuiNativeSelect', slot: 'Root', overridesResolver }, | ||
)(({ styleProps, theme }) => ({ | ||
export const nativeSelectRootStyles = ({ styleProps, theme }) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exporting the styles as they should be used in the SelectInput
too.
paddingRight: 32, | ||
}, | ||
}), | ||
...(styleProps.selectMenu && { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to SelectInput
, not used here anyway.
|
||
const IconRoot = experimentalStyled( | ||
'svg', | ||
const NativeSelectRoot = experimentalStyled( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved naming consistency.
@@ -77,24 +77,19 @@ const SelectRoot = experimentalStyled( | |||
'&:focus': { | |||
borderRadius: theme.shape.borderRadius, // Reset the reset for Chrome style | |||
}, | |||
'&&': { | |||
'&&&': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For bumping the specificity we need to use now specificity of 3, as naturally the style overrides for the slot would override these definitions
); | ||
container = innerContainer; | ||
}).toErrorDev( | ||
'Material-UI: The key `root` provided to the classes prop is not implemented in Select.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It throws error as classes
inside the Select
is not empty object, as it is not created with withStyles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have done b244bbb as a proposal to solve the issue. The alternative would be to have our own mergeClasses helper. The one from @material-ui/styles
is legacy. The accepted classes are so dynamic now that we can't make a warning work for classes that don't exist (as far as I understand the problem)
...(styleProps.variant && styles[`icon${capitalize(styleProps.variant)}`]), | ||
...(styleProps.open && styles.iconOpen), | ||
|
||
return deepmerge( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed merging.
This comment has been minimized.
This comment has been minimized.
@@ -0,0 +1,21 @@ | |||
import { generateUtilityClass, generateUtilityClasses } from '@material-ui/unstyled'; | |||
|
|||
export function getSelectUtilitiyClasses(slot) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: typo. 🙂 getSelectUtilityClasses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in #25804
One of #24405