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][material-ui] Add root class to SelectClasses #38424

Merged
merged 14 commits into from
Aug 21, 2023
1 change: 1 addition & 0 deletions docs/pages/material-ui/api/select.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
],
"styles": {
"classes": [
"root",
"select",
"multiple",
"filled",
Expand Down
1 change: 1 addition & 0 deletions docs/translations/api-docs/select/select.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"variant": { "description": "The variant to use." }
},
"classDescriptions": {
"root": { "description": "Styles applied to the root element." },
"select": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the select component <code>select</code> class"
Expand Down
6 changes: 4 additions & 2 deletions packages/mui-material/src/Select/Select.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import styled, { rootShouldForwardProp } from '../styles/styled';

const useUtilityClasses = (ownerState) => {
const { classes } = ownerState;

return classes;
};

Expand Down Expand Up @@ -73,6 +74,7 @@ const Select = React.forwardRef(function Select(inProps, ref) {

const ownerState = { ...props, variant, classes: classesProp };
const classes = useUtilityClasses(ownerState);
const { root, ...restOfClasses } = classes;

const InputComponent =
input ||
Expand Down Expand Up @@ -112,12 +114,12 @@ const Select = React.forwardRef(function Select(inProps, ref) {
SelectDisplayProps: { id, ...SelectDisplayProps },
}),
...inputProps,
classes: inputProps ? deepmerge(classes, inputProps.classes) : classes,
classes: inputProps ? deepmerge(restOfClasses, inputProps.classes) : restOfClasses,
...(input ? input.props.inputProps : {}),
},
...(multiple && native && variant === 'outlined' ? { notched: true } : {}),
ref: inputComponentRef,
className: clsx(InputComponent.props.className, className),
className: clsx(InputComponent.props.className, className, classes.root),
DiegoAndai marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

We can remove classes.root here as it will be handled down the line on InputBase

Copy link
Contributor Author

@sai6855 sai6855 Aug 17, 2023

Choose a reason for hiding this comment

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

This particular test is failing if classes.root is not added here.

Copy link
Member

Choose a reason for hiding this comment

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

Weird 🤔 I thought both classes.root and className should've ended here. Where might we be loosing classes.root so it wouldn't end here?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, my bad. classes is passed inside inputProps. So adding classes.root to the className here is correct.

Should we remove the root key when passing classes to inputProps here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think we should remove. found a similar issue related to duplicate root class. #34502.

// If a custom input is provided via 'input' prop, do not allow 'variant' to be propagated to it's root element. See https://github.com/mui/material-ui/issues/33894.
...(!input && { variant }),
...other,
Expand Down
14 changes: 14 additions & 0 deletions packages/mui-material/src/Select/Select.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import Select, { SelectChangeEvent } from '@mui/material/Select';
import MenuItem from '@mui/material/MenuItem';
import { createTheme } from '@mui/material/styles';

function genericValueTest() {
function handleChangeWithSameTypeAsSelect(event: SelectChangeEvent<number>) {}
Expand Down Expand Up @@ -44,4 +45,17 @@ function genericValueTest() {

// disabledUnderline prop should be available (inherited from InputProps) and NOT throw typescript error
<Select disableUnderline />;

// Tests presence of `root` class in SelectClasses
const theme = createTheme({
components: {
MuiSelect: {
styleOverrides: {
root: {
borderRadius: '8px',
},
},
},
},
});
}
3 changes: 3 additions & 0 deletions packages/mui-material/src/Select/selectClasses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { unstable_generateUtilityClasses as generateUtilityClasses } from '@mui/
import generateUtilityClass from '../generateUtilityClass';

export interface SelectClasses {
/** Styles applied to the root element. */
root: string;
/** Styles applied to the select component `select` class. */
select: string;
/** Styles applied to the select component if `multiple={true}`. */
Expand Down Expand Up @@ -37,6 +39,7 @@ export function getSelectUtilityClasses(slot: string): string {
}

const selectClasses: SelectClasses = generateUtilityClasses('MuiSelect', [
'root',
'select',
'multiple',
'filled',
Expand Down