From 7aa958fb89f557cdc1f9051e0d5914dbd3491ff9 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sat, 22 Jun 2019 10:32:35 +0200 Subject: [PATCH 1/7] [test] Add visual regression test for select states --- test/regressions/tests/Select/SelectStates.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 test/regressions/tests/Select/SelectStates.js diff --git a/test/regressions/tests/Select/SelectStates.js b/test/regressions/tests/Select/SelectStates.js new file mode 100644 index 00000000000000..80f3148e85464c --- /dev/null +++ b/test/regressions/tests/Select/SelectStates.js @@ -0,0 +1,16 @@ +import React from 'react'; +import MenuItem from '@material-ui/core/MenuItem'; +import Select from '@material-ui/core/Select'; + +function SelectOverflow() { + return ( + + ); +} + +export default SelectOverflow; From 7f4d1a0242361bdda70afc9f300082a8fac31a9b Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sat, 22 Jun 2019 11:02:30 +0200 Subject: [PATCH 2/7] Hardcode dimensions --- test/regressions/tests/Select/SelectStates.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/regressions/tests/Select/SelectStates.js b/test/regressions/tests/Select/SelectStates.js index 80f3148e85464c..ae5c77da173158 100644 --- a/test/regressions/tests/Select/SelectStates.js +++ b/test/regressions/tests/Select/SelectStates.js @@ -4,12 +4,14 @@ import Select from '@material-ui/core/Select'; function SelectOverflow() { return ( - +
+ +
); } From 634f5c1ddfd16fe69a1c0223697eebac64febd24 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sat, 22 Jun 2019 12:01:34 +0200 Subject: [PATCH 3/7] Reduce test flakyness --- test/regressions/tests/Select/SelectStates.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/regressions/tests/Select/SelectStates.js b/test/regressions/tests/Select/SelectStates.js index ae5c77da173158..be05b8701de668 100644 --- a/test/regressions/tests/Select/SelectStates.js +++ b/test/regressions/tests/Select/SelectStates.js @@ -5,7 +5,7 @@ import Select from '@material-ui/core/Select'; function SelectOverflow() { return (
- First Second From 08d08486a69500abef409235e3748c184fc6824b Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sat, 22 Jun 2019 12:20:01 +0200 Subject: [PATCH 4/7] [core] Add focusVisible to theme.palette.action --- packages/material-ui/src/ButtonBase/ButtonBase.js | 6 +++++- packages/material-ui/src/ButtonBase/TouchRipple.js | 4 ++-- packages/material-ui/src/ListItem/ListItem.js | 4 ++-- packages/material-ui/src/styles/createPalette.d.ts | 2 ++ packages/material-ui/src/styles/createPalette.js | 4 ++++ 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.js b/packages/material-ui/src/ButtonBase/ButtonBase.js index b662fb99f98fb5..fe6f82c6c20132 100644 --- a/packages/material-ui/src/ButtonBase/ButtonBase.js +++ b/packages/material-ui/src/ButtonBase/ButtonBase.js @@ -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); } diff --git a/packages/material-ui/src/ButtonBase/TouchRipple.js b/packages/material-ui/src/ButtonBase/TouchRipple.js index 13fe3988d27c30..881a58d932052b 100644 --- a/packages/material-ui/src/ButtonBase/TouchRipple.js +++ b/packages/material-ui/src/ButtonBase/TouchRipple.js @@ -29,7 +29,7 @@ export const styles = theme => ({ }, /* Styles applied to the internal `Ripple` components `rippleVisible` class. */ rippleVisible: { - opacity: 0.3, + opacity: theme.palette.action.focusVisibleOpacity, transform: 'scale(1)', animation: `mui-ripple-enter ${DURATION}ms ${theme.transitions.easing.easeInOut}`, // Backward compatible logic between JSS v9 and v10. @@ -74,7 +74,7 @@ export const styles = theme => ({ }, '100%': { transform: 'scale(1)', - opacity: 0.3, + opacity: theme.palette.action.focusVisibleOpacity, }, }, '@keyframes mui-ripple-exit': { diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js index 054ecfc054ece6..81e0347d387587 100644 --- a/packages/material-ui/src/ListItem/ListItem.js +++ b/packages/material-ui/src/ListItem/ListItem.js @@ -22,8 +22,8 @@ export const styles = theme => ({ textAlign: 'left', paddingTop: 8, paddingBottom: 8, - '&$focusVisible': { - backgroundColor: theme.palette.action.selected, + '&$focusVisible, &$focusVisible$selected': { + backgroundColor: theme.palette.action.focusVisible, }, '&$selected, &$selected:hover': { backgroundColor: theme.palette.action.selected, diff --git a/packages/material-ui/src/styles/createPalette.d.ts b/packages/material-ui/src/styles/createPalette.d.ts index d7e31533cfcb8e..3e8a9e439975cc 100644 --- a/packages/material-ui/src/styles/createPalette.d.ts +++ b/packages/material-ui/src/styles/createPalette.d.ts @@ -12,6 +12,8 @@ export interface TypeText { export interface TypeAction { active: string; + focusVisible: string; + focusVisibleOpacity: number; hover: string; hoverOpacity: number; selected: string; diff --git a/packages/material-ui/src/styles/createPalette.js b/packages/material-ui/src/styles/createPalette.js index 41c5d9fc74c9b4..94f19a06b07885 100644 --- a/packages/material-ui/src/styles/createPalette.js +++ b/packages/material-ui/src/styles/createPalette.js @@ -31,6 +31,8 @@ export const light = { action: { // The color of an active action like an icon button. active: 'rgba(0, 0, 0, 0.54)', + focusVisible: 'rgba(0, 0, 0, 0.3)', + focusVisibleOpacity: 0.3, // The color of an hovered action. hover: 'rgba(0, 0, 0, 0.08)', hoverOpacity: 0.08, @@ -58,6 +60,8 @@ export const dark = { }, action: { active: common.white, + focusVisible: 'rgba(255, 255, 255, 0.4)', + focusVisibleOpacity: 0.4, hover: 'rgba(255, 255, 255, 0.1)', hoverOpacity: 0.1, selected: 'rgba(255, 255, 255, 0.2)', From 7e3471bf2e1cfb408d59e35bd3986108cbcd5a66 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Sat, 22 Jun 2019 12:35:31 +0200 Subject: [PATCH 5/7] Label states --- test/regressions/tests/Select/SelectStates.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/regressions/tests/Select/SelectStates.js b/test/regressions/tests/Select/SelectStates.js index be05b8701de668..cc4565915a659d 100644 --- a/test/regressions/tests/Select/SelectStates.js +++ b/test/regressions/tests/Select/SelectStates.js @@ -5,10 +5,10 @@ import Select from '@material-ui/core/Select'; function SelectOverflow() { return (
- + Selected + + Focused
From dd8294b168ff59f73ecde8cb102dc87c7f5c8a91 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sun, 23 Jun 2019 20:08:51 +0200 Subject: [PATCH 6/7] different proposal --- docs/src/pages/components/selects/MultipleSelect.js | 2 +- docs/src/pages/components/selects/MultipleSelect.tsx | 2 +- packages/material-ui/src/ButtonBase/TouchRipple.js | 4 ++-- packages/material-ui/src/ListItem/ListItem.js | 10 ++++++---- packages/material-ui/src/styles/createPalette.d.ts | 2 -- packages/material-ui/src/styles/createPalette.js | 12 +++++------- 6 files changed, 15 insertions(+), 17 deletions(-) diff --git a/docs/src/pages/components/selects/MultipleSelect.js b/docs/src/pages/components/selects/MultipleSelect.js index 4333b82f8c03e9..c65e01ab3b8fe6 100644 --- a/docs/src/pages/components/selects/MultipleSelect.js +++ b/docs/src/pages/components/selects/MultipleSelect.js @@ -115,7 +115,7 @@ export default function MultipleSelect() { > {names.map(name => ( - -1} /> + -1} /> ))} diff --git a/docs/src/pages/components/selects/MultipleSelect.tsx b/docs/src/pages/components/selects/MultipleSelect.tsx index a26c31574e8400..2efc9143d2cd7c 100644 --- a/docs/src/pages/components/selects/MultipleSelect.tsx +++ b/docs/src/pages/components/selects/MultipleSelect.tsx @@ -117,7 +117,7 @@ export default function MultipleSelect() { > {names.map(name => ( - -1} /> + -1} /> ))} diff --git a/packages/material-ui/src/ButtonBase/TouchRipple.js b/packages/material-ui/src/ButtonBase/TouchRipple.js index 881a58d932052b..13fe3988d27c30 100644 --- a/packages/material-ui/src/ButtonBase/TouchRipple.js +++ b/packages/material-ui/src/ButtonBase/TouchRipple.js @@ -29,7 +29,7 @@ export const styles = theme => ({ }, /* Styles applied to the internal `Ripple` components `rippleVisible` class. */ rippleVisible: { - opacity: theme.palette.action.focusVisibleOpacity, + opacity: 0.3, transform: 'scale(1)', animation: `mui-ripple-enter ${DURATION}ms ${theme.transitions.easing.easeInOut}`, // Backward compatible logic between JSS v9 and v10. @@ -74,7 +74,7 @@ export const styles = theme => ({ }, '100%': { transform: 'scale(1)', - opacity: theme.palette.action.focusVisibleOpacity, + opacity: 0.3, }, }, '@keyframes mui-ripple-exit': { diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js index 81e0347d387587..e971c3147d536f 100644 --- a/packages/material-ui/src/ListItem/ListItem.js +++ b/packages/material-ui/src/ListItem/ListItem.js @@ -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'; @@ -22,11 +23,12 @@ export const styles = theme => ({ textAlign: 'left', paddingTop: 8, paddingBottom: 8, - '&$focusVisible, &$focusVisible$selected': { - backgroundColor: theme.palette.action.focusVisible, + '&$focusVisible': { + backgroundColor: theme.palette.action.hover, }, '&$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, @@ -91,7 +93,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, diff --git a/packages/material-ui/src/styles/createPalette.d.ts b/packages/material-ui/src/styles/createPalette.d.ts index 3e8a9e439975cc..d7e31533cfcb8e 100644 --- a/packages/material-ui/src/styles/createPalette.d.ts +++ b/packages/material-ui/src/styles/createPalette.d.ts @@ -12,8 +12,6 @@ export interface TypeText { export interface TypeAction { active: string; - focusVisible: string; - focusVisibleOpacity: number; hover: string; hoverOpacity: number; selected: string; diff --git a/packages/material-ui/src/styles/createPalette.js b/packages/material-ui/src/styles/createPalette.js index 94f19a06b07885..9043d03b9dc50f 100644 --- a/packages/material-ui/src/styles/createPalette.js +++ b/packages/material-ui/src/styles/createPalette.js @@ -31,13 +31,12 @@ export const light = { action: { // The color of an active action like an icon button. active: 'rgba(0, 0, 0, 0.54)', - focusVisible: 'rgba(0, 0, 0, 0.3)', - focusVisibleOpacity: 0.3, // 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. @@ -60,11 +59,10 @@ export const dark = { }, action: { active: common.white, - focusVisible: 'rgba(255, 255, 255, 0.4)', - focusVisibleOpacity: 0.4, 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)', }, From 9bd33d3511738f7e3179de666bbbd0ee76bbaa30 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sun, 23 Jun 2019 20:25:25 +0200 Subject: [PATCH 7/7] =?UTF-8?q?proposal=20n=C2=B02?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/material-ui/src/ListItem/ListItem.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js index e971c3147d536f..d76581fd325bc9 100644 --- a/packages/material-ui/src/ListItem/ListItem.js +++ b/packages/material-ui/src/ListItem/ListItem.js @@ -25,6 +25,8 @@ export const styles = theme => ({ paddingBottom: 8, '&$focusVisible': { backgroundColor: theme.palette.action.hover, + outline: `${fade(theme.palette.primary.main, 0.5)} solid 2px`, + outlineOffset: -2, }, '&$selected, &$selected:hover': { backgroundColor: fade(theme.palette.primary.main, theme.palette.action.selectedOpacity),