From 6ce9ad46f968ed35ec392c320455f10064354d62 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Mon, 31 Aug 2020 22:59:05 +0200 Subject: [PATCH] add missing test --- CHANGELOG.md | 2 +- .../material-ui/src/ButtonBase/ButtonBase.js | 1 + .../src/ButtonBase/ButtonBase.test.js | 22 +++++----- packages/material-ui/src/Chip/Chip.js | 41 ++++--------------- packages/material-ui/src/Chip/Chip.test.js | 20 +++++++-- .../material-ui/src/Tooltip/Tooltip.test.js | 26 ++++-------- .../src/utils/useIsFocusVisible.test.js | 20 +++------ .../{focusVisible.js => focusVisible.ts} | 5 +-- 8 files changed, 51 insertions(+), 86 deletions(-) rename test/utils/{focusVisible.js => focusVisible.ts} (81%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a1e9688e26726..a8981b39ac0f62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,7 +127,7 @@ const theme = createMuiTheme({ - [ButtonBase] Reset box-sizing to border-box (#22316) @su8ru - [Dialog] Fix unexpected close when releasing click on backdrop (#22310) @danbrud - [Divider] Add text in divider (#22285) @ShehryarShoukat96 -- [Slider] Respect disabled property when already focused (#22247) @pireads +- [Slider] Respect disabled property when already focused (#22247) @pireads - [Tabs] Don't fire onChange if current value (#22381) @jjoselv - [Tabs] Improve focus management on list with no active tabs (#22377) @alexmotoc - [theme] Add theme.mixins.gutters() in adaptV4Theme (#22396) @mnajdova diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.js b/packages/material-ui/src/ButtonBase/ButtonBase.js index af7badb1d08fe6..4d402033f5395e 100644 --- a/packages/material-ui/src/ButtonBase/ButtonBase.js +++ b/packages/material-ui/src/ButtonBase/ButtonBase.js @@ -97,6 +97,7 @@ const ButtonBase = React.forwardRef(function ButtonBase(props, ref) { ref: focusVisibleRef, } = useIsFocusVisible(); const [focusVisible, setFocusVisible] = React.useState(false); + if (disabled && focusVisible) { setFocusVisible(false); } diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.test.js b/packages/material-ui/src/ButtonBase/ButtonBase.test.js index 58eb90087cc7c0..174d38179791b9 100644 --- a/packages/material-ui/src/ButtonBase/ButtonBase.test.js +++ b/packages/material-ui/src/ButtonBase/ButtonBase.test.js @@ -9,8 +9,8 @@ import { act, createClientRender, fireEvent, - focusVisible, screen, + dispatchFocusVisible, simulatePointerDevice, } from 'test/utils'; import * as PropTypes from 'prop-types'; @@ -198,7 +198,7 @@ describe('', () => { const button = getByRole('button'); simulatePointerDevice(); - focusVisible(button); + dispatchFocusVisible(button); expect(button.querySelectorAll('.ripple-pulsate')).to.have.lengthOf(0); }); @@ -458,7 +458,7 @@ describe('', () => { const button = getByRole('button'); simulatePointerDevice(); - focusVisible(button); + dispatchFocusVisible(button); expect(button.querySelectorAll('.ripple-pulsate')).to.have.lengthOf(1); }); @@ -477,7 +477,7 @@ describe('', () => { const button = getByRole('button'); simulatePointerDevice(); - focusVisible(button); + dispatchFocusVisible(button); fireEvent.mouseLeave(button); expect(button.querySelectorAll('.ripple-pulsate')).to.have.lengthOf(1); @@ -499,7 +499,7 @@ describe('', () => { const button = getByRole('button'); simulatePointerDevice(); - focusVisible(button); + dispatchFocusVisible(button); fireEvent.keyDown(button, { key: ' ' }); expect(button.querySelectorAll('.ripple-pulsate .child-leaving')).to.have.lengthOf(1); @@ -522,7 +522,7 @@ describe('', () => { const button = getByRole('button'); simulatePointerDevice(); - focusVisible(button); + dispatchFocusVisible(button); fireEvent.keyDown(button, { key: ' ' }); fireEvent.keyUp(button, { key: ' ' }); @@ -545,7 +545,7 @@ describe('', () => { ); const button = getByRole('button'); simulatePointerDevice(); - focusVisible(button); + dispatchFocusVisible(button); act(() => { button.blur(); @@ -575,7 +575,7 @@ describe('', () => { const button = getByText('Hello'); simulatePointerDevice(); - focusVisible(button); + dispatchFocusVisible(button); expect(button).to.have.class(classes.focusVisible); @@ -635,7 +635,7 @@ describe('', () => { expect(button).not.to.have.class(classes.focusVisible); button.focus(); expect(button).not.to.have.class(classes.focusVisible); - focusVisible(button); + dispatchFocusVisible(button); expect(button).to.have.class(classes.focusVisible); }); @@ -677,7 +677,7 @@ describe('', () => { const focusRetarget = getByText('you cannot escape me'); simulatePointerDevice(); - focusVisible(buttonBase); + dispatchFocusVisible(buttonBase); expect(focusRetarget).toHaveFocus(); expect(eventLog).to.deep.equal(['focus-visible', 'focus', 'blur']); @@ -693,7 +693,7 @@ describe('', () => { ); simulatePointerDevice(); - focusVisible(getByRole('button')); + dispatchFocusVisible(getByRole('button')); expect(onFocusVisibleSpy.calledOnce).to.equal(true); expect(onFocusVisibleSpy.firstCall.args).to.have.lengthOf(1); diff --git a/packages/material-ui/src/Chip/Chip.js b/packages/material-ui/src/Chip/Chip.js index 958ab3d77f7cee..41fb282cfbfe1b 100644 --- a/packages/material-ui/src/Chip/Chip.js +++ b/packages/material-ui/src/Chip/Chip.js @@ -8,8 +8,6 @@ import { emphasize, fade } from '../styles/colorManipulator'; import useForkRef from '../utils/useForkRef'; import unsupportedProp from '../utils/unsupportedProp'; import capitalize from '../utils/capitalize'; -import useEventCallback from '../utils/useEventCallback'; -import useIsFocusVisible from '../utils/useIsFocusVisible'; import ButtonBase from '../ButtonBase'; export const styles = (theme) => { @@ -298,36 +296,11 @@ const Chip = React.forwardRef(function Chip(props, ref) { const chipRef = React.useRef(null); const handleRef = useForkRef(chipRef, ref); - const { - isFocusVisibleRef, - onFocus: handleFocusVisible, - onBlur: handleBlurVisible, - } = useIsFocusVisible(); - const [focusVisible, setFocusVisible] = React.useState(false); - if (disabled && focusVisible) { - setFocusVisible(false); - } - React.useEffect(() => { - isFocusVisibleRef.current = focusVisible; - }, [focusVisible, isFocusVisibleRef]); - - const handleBlur = useEventCallback((event) => { - handleBlurVisible(event); - if (isFocusVisibleRef.current === false) { - setFocusVisible(false); - } - }, false); - - const handleFocus = useEventCallback((event) => { + const handleFocus = (event) => { if (!chipRef.current) { chipRef.current = event.currentTarget; } - - handleFocusVisible(event); - if (isFocusVisibleRef.current === true) { - setFocusVisible(true); - } - }); + }; const handleDeleteIconClick = (event) => { // Stop the event from bubbling up to the `Chip` @@ -369,7 +342,10 @@ const Chip = React.forwardRef(function Chip(props, ref) { const small = size === 'small'; const Component = ComponentProp || (clickable ? ButtonBase : 'div'); - const moreProps = Component === ButtonBase ? { component: 'div' } : {}; + const moreProps = + Component === ButtonBase + ? { component: 'div', focusVisibleClassName: classes.focusVisible } + : {}; let deleteIcon = null; if (onDelete) { @@ -450,16 +426,15 @@ const Chip = React.forwardRef(function Chip(props, ref) { [classes[`clickableColor${capitalize(color)}`]]: clickable && color !== 'default', [classes.deletable]: onDelete, [classes[`deletableColor${capitalize(color)}`]]: onDelete && color !== 'default', - [classes.focusVisible]: focusVisible, [classes.outlinedPrimary]: variant === 'outlined' && color === 'primary', [classes.outlinedSecondary]: variant === 'outlined' && color === 'secondary', }, themeVariantsClasses, className, )} - aria-disabled={disabled ? true : undefined} + aria-disabled={!clickable && disabled ? true : undefined} + disabled={clickable && disabled ? true : undefined} tabIndex={clickable || onDelete ? 0 : undefined} - onBlur={handleBlur} onClick={onClick} onFocus={handleFocus} onKeyDown={handleKeyDown} diff --git a/packages/material-ui/src/Chip/Chip.test.js b/packages/material-ui/src/Chip/Chip.test.js index 6d42dc4c8ac77d..7cc82ad37338be 100644 --- a/packages/material-ui/src/Chip/Chip.test.js +++ b/packages/material-ui/src/Chip/Chip.test.js @@ -8,7 +8,7 @@ import { act, createClientRender, fireEvent, - focusVisible, + dispatchFocusVisible, simulatePointerDevice, } from 'test/utils'; import CheckBox from '../internal/svg-icons/CheckBox'; @@ -18,7 +18,7 @@ import Chip from './Chip'; describe('', () => { let classes; const mount = createMount(); - const render = createClientRender(); + const render = createClientRender({ strict: false }); before(() => { classes = getClasses(); @@ -550,9 +550,23 @@ describe('', () => { expect(chip).not.to.have.class(classes.focusVisible); chip.focus(); expect(chip).not.to.have.class(classes.focusVisible); - focusVisible(chip); + dispatchFocusVisible(chip); expect(chip).to.have.class(classes.focusVisible); }); + + it('should reset the focused state', () => { + const { container, setProps } = render( {}} />); + const chip = container.querySelector(`.${classes.root}`); + + simulatePointerDevice(); + dispatchFocusVisible(chip); + + expect(chip).to.have.class(classes.focusVisible); + + setProps({ disabled: true }); + + expect(chip).not.to.have.class(classes.focusVisible); + }); }); }); diff --git a/packages/material-ui/src/Tooltip/Tooltip.test.js b/packages/material-ui/src/Tooltip/Tooltip.test.js index 17e7857a57333f..d66398867134e7 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.test.js +++ b/packages/material-ui/src/Tooltip/Tooltip.test.js @@ -9,25 +9,13 @@ import { createClientRender, fireEvent, screen, + simulatePointerDevice, + dispatchFocusVisible, } from 'test/utils'; import { camelCase } from 'lodash/string'; import Tooltip, { testReset } from './Tooltip'; import Input from '../Input'; -function focusVisible(element) { - act(() => { - element.blur(); - fireEvent.keyDown(document.activeElement || document.body, { key: 'Tab' }); - element.focus(); - }); -} - -function simulatePointerDevice() { - // first focus on a page triggers focus visible until a pointer event - // has been dispatched - document.dispatchEvent(new window.Event('pointerdown')); -} - describe('', () => { /** * @type {ReturnType} @@ -379,7 +367,7 @@ describe('', () => { ); simulatePointerDevice(); - focusVisible(getByRole('button')); + dispatchFocusVisible(getByRole('button')); expect(queryByRole('tooltip')).to.equal(null); act(() => { @@ -408,7 +396,7 @@ describe('', () => { , ); const children = getByRole('button'); - focusVisible(children); + dispatchFocusVisible(children); expect(queryByRole('tooltip')).to.equal(null); @@ -430,7 +418,7 @@ describe('', () => { expect(queryByRole('tooltip')).to.equal(null); - focusVisible(children); + dispatchFocusVisible(children); // Bypass `enterDelay` wait, use `enterNextDelay`. expect(queryByRole('tooltip')).to.equal(null); @@ -463,7 +451,7 @@ describe('', () => { ); simulatePointerDevice(); - focusVisible(getByRole('button')); + dispatchFocusVisible(getByRole('button')); act(() => { clock.tick(enterDelay); }); @@ -762,7 +750,7 @@ describe('', () => { expect(queryByRole('tooltip')).to.equal(null); - focusVisible(getByRole('button')); + dispatchFocusVisible(getByRole('button')); expect(getByRole('tooltip')).toBeVisible(); diff --git a/packages/material-ui/src/utils/useIsFocusVisible.test.js b/packages/material-ui/src/utils/useIsFocusVisible.test.js index fdf6df9a0d9094..c532b27bab106b 100644 --- a/packages/material-ui/src/utils/useIsFocusVisible.test.js +++ b/packages/material-ui/src/utils/useIsFocusVisible.test.js @@ -1,21 +1,10 @@ import { expect } from 'chai'; import * as React from 'react'; import * as ReactDOM from 'react-dom'; -import { createMount } from 'test/utils'; +import { createClientRender, dispatchFocusVisible, simulatePointerDevice } from 'test/utils'; import useIsFocusVisible, { teardown as teardownFocusVisible } from './useIsFocusVisible'; import useForkRef from './useForkRef'; -function dispatchFocusVisible(element) { - element.ownerDocument.dispatchEvent(new window.Event('keydown')); - element.focus(); -} - -function simulatePointerDevice() { - // first focus on a page triggers focus visible until a pointer event - // has been dispatched - document.dispatchEvent(new window.Event('pointerdown')); -} - const SimpleButton = React.forwardRef(function SimpleButton(props, ref) { const { isFocusVisibleRef, @@ -55,7 +44,7 @@ const SimpleButton = React.forwardRef(function SimpleButton(props, ref) { }); describe('focus-visible polyfill', () => { - const mount = createMount(); + const render = createClientRender(); before(() => { // isolate test from previous component test that use the polyfill in the document scope @@ -85,12 +74,13 @@ describe('focus-visible polyfill', () => { it('should set focus state for shadowRoot children', () => { const buttonRef = React.createRef(); - mount( + render( Hello , + {}, { - attachTo: rootElement.shadowRoot, + container: rootElement.shadowRoot, }, ); simulatePointerDevice(); diff --git a/test/utils/focusVisible.js b/test/utils/focusVisible.ts similarity index 81% rename from test/utils/focusVisible.js rename to test/utils/focusVisible.ts index 078f86408a7545..5802cc386fc106 100644 --- a/test/utils/focusVisible.js +++ b/test/utils/focusVisible.ts @@ -1,9 +1,6 @@ import { act, fireEvent } from './createClientRender'; -/** - * @param {HTMLElement} element - */ -export function focusVisible(element) { +export function dispatchFocusVisible(element: HTMLElement) { act(() => { element.blur(); fireEvent.keyDown(document.body, { key: 'Tab' });