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.

Sign up for GitHub

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

Jump to bottom

[Select][NativeSelect] Polish CSS classes #26186

Merged
merged 9 commits into from
May 13, 2021
Merged

[Select][NativeSelect] Polish CSS classes #26186

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ea9903c
[Select] Polish CSS classes
m4theushw May 7, 2021
a51c65c
rename root slot to select
m4theushw May 8, 2021
c1a3d54
apply suggestions
m4theushw May 11, 2021
921447a
add note about root slot
m4theushw May 11, 2021
44140be
apply styles to the select element
m4theushw May 11, 2021
e1da464
update migration guide
m4theushw May 11, 2021
e4b3230
apply root class properly
m4theushw May 12, 2021
8e1e2fe
update docs
m4theushw May 12, 2021
edd965d
increase specificity
m4theushw May 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions docs/pages/api-docs/native-select.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@
"name": "NativeSelect",
"styles": {
"classes": [
"root",
"select",
"filled",
"outlined",
"standard",
"selectMenu",
"disabled",
"icon",
"iconOpen",
Expand Down
1 change: 0 additions & 1 deletion docs/pages/api-docs/select.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"filled",
"outlined",
"standard",
"selectMenu",
"disabled",
"icon",
"iconOpen",
Expand Down
16 changes: 16 additions & 0 deletions docs/src/pages/guides/migration-v4/migration-v4.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,13 @@ As the core components use emotion as a styled engine, the props used by emotion
+<Select label="Gender" />
```

- Merge the `selectMenu` slot into `select`. Slot `selectMenu` was redundant. The `root` slot is no longer applied to the select, but to the input.

```diff
-<Select classes={{ root: 'class1', select: 'class2', selectMenu: 'class3' }} />
+<Select classes={{ root: 'class1', select: 'class2 class3' }} />
```

### Skeleton

- Move the component from the lab to the core. The component is now stable.
Expand Down Expand Up @@ -1308,6 +1315,15 @@ As the core components use emotion as a styled engine, the props used by emotion
/>
```

- Move the custom class on `input` to `select`. The `input` key is being applied on another element.

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

- Rename the `default` value of the `padding` prop to `normal`.

```diff
Expand Down
8 changes: 0 additions & 8 deletions docs/translations/api-docs/native-select/native-select.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@
"variant": "The variant to use."
},
"classDescriptions": {
"root": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the select component `root` class"
},
"select": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the select component `select` class"
Expand All @@ -35,10 +31,6 @@
"nodeName": "the select component",
"conditions": "<code>variant=\"standard\"</code>"
},
"selectMenu": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the select component `selectMenu` class"
},
"disabled": {
"description": "Pseudo-class applied to {{nodeName}}.",
"nodeName": "the select component `disabled` class"
Expand Down
9 changes: 1 addition & 8 deletions docs/translations/api-docs/select/select.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@
"variant": "The variant to use."
},
"classDescriptions": {
"root": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the select component `root` class"
},
"root": { "description": "Styles applied to the root element." },
"select": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the select component `select` class"
Expand All @@ -49,10 +46,6 @@
"nodeName": "the select component",
"conditions": "<code>variant=\"standard\"</code>"
},
"selectMenu": {
"description": "Styles applied to {{nodeName}}.",
"nodeName": "the select component `selectMenu` class"
},
"disabled": {
"description": "Pseudo-class applied to {{nodeName}}.",
"nodeName": "the select component `disabled` class"
Expand Down
4 changes: 0 additions & 4 deletions packages/material-ui/src/NativeSelect/NativeSelect.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ export interface NativeSelectProps
* Override or extend the styles applied to the component.
*/
classes?: {
/** Styles applied to the select component `root` class. */
root?: string;
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
/** Styles applied to the select component `select` class. */
select?: string;
/** Styles applied to the select component if `variant="filled"`. */
Expand All @@ -25,8 +23,6 @@ export interface NativeSelectProps
outlined?: string;
/** Styles applied to the select component if `variant="standard"`. */
standard?: string;
/** Styles applied to the select component `selectMenu` class. */
selectMenu?: string;
/** Pseudo-class applied to the select component `disabled` class. */
disabled?: string;
/** Styles applied to the icon component. */
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/NativeSelect/NativeSelect.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('<NativeSelect />', () => {

it('should provide the classes to the select component', () => {
const { getByRole } = render(<NativeSelect {...defaultProps} />);
expect(getByRole('combobox')).to.have.class(classes.root);
expect(getByRole('combobox')).to.have.class(classes.select);
});

it('slots overrides should work', function test() {
Expand Down
15 changes: 7 additions & 8 deletions packages/material-ui/src/NativeSelect/NativeSelectInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ const useUtilityClasses = (styleProps) => {
const { classes, variant, disabled, open } = styleProps;

const slots = {
root: ['root', 'select', variant, disabled && 'disabled'],
select: ['select', variant, disabled && 'disabled'],
icon: ['icon', `icon${capitalize(variant)}`, open && 'iconOpen', disabled && 'disabled'],
};

return composeClasses(slots, getNativeSelectUtilityClasses, classes);
};

export const nativeSelectRootStyles = ({ styleProps, theme }) => ({
export const nativeSelectSelectStyles = ({ styleProps, theme }) => ({
MozAppearance: 'none', // Reset
WebkitAppearance: 'none', // Reset
// When interacting quickly, the text can end up selected.
Expand Down Expand Up @@ -66,23 +66,22 @@ export const nativeSelectRootStyles = ({ styleProps, theme }) => ({
}),
});

const NativeSelectRoot = experimentalStyled(
const NativeSelectSelect = experimentalStyled(
'select',
{},
{
name: 'MuiNativeSelect',
slot: 'Root',
slot: 'Select',
overridesResolver: (props, styles) => {
const { styleProps } = props;

return {
...styles.root,
...styles.select,
...styles[styleProps.variant],
};
},
},
)(nativeSelectRootStyles);
)(nativeSelectSelectStyles);

export const nativeSelectIconStyles = ({ styleProps, theme }) => ({
// We use a position absolute over a flexbox in order to forward the pointer events
Expand Down Expand Up @@ -138,9 +137,9 @@ const NativeSelectInput = React.forwardRef(function NativeSelectInput(props, ref
const classes = useUtilityClasses(styleProps);
return (
<React.Fragment>
<NativeSelectRoot
<NativeSelectSelect
styleProps={styleProps}
className={clsx(classes.root, className)}
className={clsx(classes.select, className)}
disabled={disabled}
ref={inputRef || ref}
{...other}
Expand Down
1 change: 0 additions & 1 deletion packages/material-ui/src/NativeSelect/nativeSelectClasses.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const nativeSelectClasses = generateUtilityClasses('MuiNativeSelect', [
'filled',
'outlined',
'standard',
'selectMenu',
'disabled',
'icon',
'iconOpen',
Expand Down
4 changes: 1 addition & 3 deletions packages/material-ui/src/Select/Select.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface SelectProps<T = unknown>
* @default {}
*/
classes?: {
/** Styles applied to the select component `root` class. */
/** Styles applied to the root element. */
root?: string;
/** Styles applied to the select component `select` class. */
select?: string;
Expand All @@ -36,8 +36,6 @@ export interface SelectProps<T = unknown>
outlined?: string;
/** Styles applied to the select component if `variant="standard"`. */
standard?: string;
/** Styles applied to the select component `selectMenu` class. */
selectMenu?: string;
/** Pseudo-class applied to the select component `disabled` class. */
disabled?: string;
/** Styles applied to the icon component. */
Expand Down
7 changes: 5 additions & 2 deletions packages/material-ui/src/Select/Select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const Select = React.forwardRef(function Select(inProps, ref) {
const {
autoWidth = false,
children,
classes = {},
classes: classesProp = {},
className,
displayEmpty = false,
IconComponent = ArrowDropDownIcon,
Expand Down Expand Up @@ -57,6 +57,8 @@ const Select = React.forwardRef(function Select(inProps, ref) {
filled: <FilledInput />,
}[variant];

const { root: rootClassName, ...otherClasses } = classesProp;

return React.cloneElement(InputComponent, {
// Most of the logic is implemented in `SelectInput`.
// The `Select` component is a simple API wrapper to expose something better to play with.
Expand All @@ -81,11 +83,12 @@ const Select = React.forwardRef(function Select(inProps, ref) {
SelectDisplayProps: { id, ...SelectDisplayProps },
}),
...inputProps,
classes: inputProps ? deepmerge(classes, inputProps.classes) : classes,
classes: inputProps ? deepmerge(otherClasses, inputProps.classes) : otherClasses,
...(input ? input.props.inputProps : {}),
},
...(multiple && native && variant === 'outlined' ? { notched: true } : {}),
ref,
classes: { root: rootClassName },
oliviertassinari marked this conversation as resolved.
Show resolved Hide resolved
className: clsx(className, InputComponent.props.className),
...other,
});
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/Select/Select.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ describe('<Select />', () => {
render(
<Select
inputProps={{
classes: { root: 'root' },
classes: { select: 'select' },
}}
value=""
/>,
);
expect(document.querySelector(`.${classes.root}`)).to.have.class('root');
expect(document.querySelector(`.${classes.select}`)).to.have.class('select');
});
});

Expand Down
32 changes: 15 additions & 17 deletions packages/material-ui/src/Select/SelectInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,33 @@ import { refType } from '@material-ui/utils';
import ownerDocument from '../utils/ownerDocument';
import capitalize from '../utils/capitalize';
import Menu from '../Menu/Menu';
import { nativeSelectRootStyles, nativeSelectIconStyles } from '../NativeSelect/NativeSelectInput';
import {
nativeSelectSelectStyles,
nativeSelectIconStyles,
} from '../NativeSelect/NativeSelectInput';
import { isFilled } from '../InputBase/utils';
import experimentalStyled, { slotShouldForwardProp } from '../styles/experimentalStyled';
import useForkRef from '../utils/useForkRef';
import useControlled from '../utils/useControlled';
import selectClasses, { getSelectUtilityClasses } from './selectClasses';
import { getSelectUtilityClasses } from './selectClasses';

const SelectRoot = experimentalStyled(
const SelectSelect = experimentalStyled(
'div',
{},
{
name: 'MuiSelect',
slot: 'Root',
slot: 'Select',
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 •
edited
Loading

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.

// TODO v5: remove `root` and `selectMenu`
...styles.root,
...styles.select,
...styles.selectMenu,
...styles[styleProps.variant],
},
...styles.select,
...styles[styleProps.variant],
};
},
},
)(nativeSelectRootStyles, {
)(nativeSelectSelectStyles, {
// Win specificity over the input base
[`&.${selectClasses.selectMenu}`]: {
'&': {
height: 'auto', // Resets for multiple select with chips
minHeight: '1.4375em', // Required for select\text-field height consistency
textOverflow: 'ellipsis',
Expand Down Expand Up @@ -98,7 +96,7 @@ const useUtilityClasses = (styleProps) => {
const { classes, variant, disabled, open } = styleProps;

const slots = {
root: ['root', 'select', variant, disabled && 'disabled', 'selectMenu'],
select: ['select', variant, disabled && 'disabled'],
icon: ['icon', `icon${capitalize(variant)}`, open && 'iconOpen', disabled && 'disabled'],
nativeInput: ['nativeInput'],
};
Expand Down Expand Up @@ -459,7 +457,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {

return (
<React.Fragment>
<SelectRoot
<SelectSelect
ref={handleDisplayRef}
tabIndex={tabIndex}
role="button"
Expand All @@ -475,7 +473,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
onFocus={onFocus}
{...SelectDisplayProps}
styleProps={styleProps}
className={clsx(classes.root, className, SelectDisplayProps.className)}
className={clsx(classes.select, className, SelectDisplayProps.className)}
// The id is required for proper a11y
id={buttonId}
>
Expand All @@ -487,7 +485,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
) : (
display
)}
</SelectRoot>
</SelectSelect>
<SelectNativeInput
value={Array.isArray(value) ? value.join(',') : value}
name={name}
Expand Down
1 change: 0 additions & 1 deletion packages/material-ui/src/Select/selectClasses.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const selectClasses = generateUtilityClasses('MuiSelect', [
'filled',
'outlined',
'standard',
'selectMenu',
'disabled',
'focused',
'icon',
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/TablePagination/TablePagination.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const TablePaginationSelect = experimentalStyled(
flexShrink: 0,
marginRight: 32,
marginLeft: 8,
[`& .${tablePaginationClasses.input}`]: {
[`& .${tablePaginationClasses.select}`]: {
paddingLeft: 8,
paddingRight: 24,
textAlign: 'right',
Expand Down