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] Improve visual distinction between selected and focus-visible #16331

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion docs/src/pages/components/selects/MultipleSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export default function MultipleSelect() {
>
{names.map(name => (
<MenuItem key={name} value={name}>
<Checkbox checked={personName.indexOf(name) > -1} />
<Checkbox color="primary" checked={personName.indexOf(name) > -1} />
<ListItemText primary={name} />
</MenuItem>
))}
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/selects/MultipleSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export default function MultipleSelect() {
>
{names.map(name => (
<MenuItem key={name} value={name}>
<Checkbox checked={personName.indexOf(name) > -1} />
<Checkbox color="primary" checked={personName.indexOf(name) > -1} />
<ListItemText primary={name} />
</MenuItem>
))}
Expand Down
6 changes: 5 additions & 1 deletion packages/material-ui/src/ButtonBase/ButtonBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,11 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) {
const handleBlur = useRippleHandler(
'stop',
event => {
if (focusVisible) {
/**
* If components set imperative focus a blur might be fired before the component
* has re-rendered e.g. Selects where an item is focused on opening that isn't selected
*/
if (isFocusVisible(event)) {
onBlurVisible(event);
setFocusVisible(false);
}
Expand Down
10 changes: 7 additions & 3 deletions packages/material-ui/src/ListItem/ListItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { chainPropTypes } from '@material-ui/utils';
import { fade } from '../styles/colorManipulator';
import withStyles from '../styles/withStyles';
import ButtonBase from '../ButtonBase';
import { isMuiElement, useForkRef } from '../utils/reactHelpers';
Expand All @@ -23,10 +24,13 @@ export const styles = theme => ({
paddingTop: 8,
paddingBottom: 8,
'&$focusVisible': {
backgroundColor: theme.palette.action.selected,
backgroundColor: theme.palette.action.hover,
outline: `${fade(theme.palette.primary.main, 0.5)} solid 2px`,
outlineOffset: -2,
},
'&$selected, &$selected:hover': {
backgroundColor: theme.palette.action.selected,
backgroundColor: fade(theme.palette.primary.main, theme.palette.action.selectedOpacity),
// backgroundColor: theme.palette.action.selected,
},
'&$disabled': {
opacity: 0.5,
Expand Down Expand Up @@ -91,7 +95,7 @@ const useEnhancedEffect = typeof window === 'undefined' ? React.useEffect : Reac
const ListItem = React.forwardRef(function ListItem(props, ref) {
const {
alignItems = 'center',
autoFocus,
autoFocus = false,
button = false,
children: childrenProp,
classes,
Expand Down
8 changes: 5 additions & 3 deletions packages/material-ui/src/styles/createPalette.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ export const light = {
// The color of an active action like an icon button.
active: 'rgba(0, 0, 0, 0.54)',
// The color of an hovered action.
hover: 'rgba(0, 0, 0, 0.08)',
hoverOpacity: 0.08,
hover: 'rgba(0, 0, 0, 0.07)',
hoverOpacity: 0.07,
// The color of a selected action.
selected: 'rgba(0, 0, 0, 0.14)',
selectedOpacity: 0.14,
// The color of a disabled action.
disabled: 'rgba(0, 0, 0, 0.26)',
// The background color of a disabled action.
Expand All @@ -60,7 +61,8 @@ export const dark = {
active: common.white,
hover: 'rgba(255, 255, 255, 0.1)',
hoverOpacity: 0.1,
selected: 'rgba(255, 255, 255, 0.2)',
selected: 'rgba(255, 255, 255, 0.25)',
selectedOpacity: 0.25,
disabled: 'rgba(255, 255, 255, 0.3)',
disabledBackground: 'rgba(255, 255, 255, 0.12)',
},
Expand Down
18 changes: 18 additions & 0 deletions test/regressions/tests/Select/SelectStates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react';
import MenuItem from '@material-ui/core/MenuItem';
import Select from '@material-ui/core/Select';

function SelectOverflow() {
return (
<div style={{ height: 130, width: 100 }}>
<Select MenuProps={{ transitionDuration: 0 }} open value="selected">
<MenuItem value="selected">Selected</MenuItem>
<MenuItem autoFocus value="focused">
Focused
</MenuItem>
</Select>
</div>
);
}

export default SelectOverflow;