From 90bffd2b4c2f1554c65218379ade8e9e32f7e500 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 27 Aug 2020 19:18:43 +0200 Subject: [PATCH] [Tooltip] Meet dismissable WCAG criterion (#22376) --- packages/material-ui/src/Tooltip/Tooltip.d.ts | 4 +- packages/material-ui/src/Tooltip/Tooltip.js | 58 ++++++++++++++----- .../material-ui/src/Tooltip/Tooltip.test.js | 26 +++++++++ 3 files changed, 71 insertions(+), 17 deletions(-) diff --git a/packages/material-ui/src/Tooltip/Tooltip.d.ts b/packages/material-ui/src/Tooltip/Tooltip.d.ts index 724e8914595f17..906dc9521c3241 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.d.ts +++ b/packages/material-ui/src/Tooltip/Tooltip.d.ts @@ -88,13 +88,13 @@ export interface TooltipProps extends StandardProps) => void; + onClose?: (event: React.SyntheticEvent | Event) => void; /** * Callback fired when the component requests to be open. * * @param {object} event The event source of the callback. */ - onOpen?: (event: React.ChangeEvent<{}>) => void; + onOpen?: (event: React.SyntheticEvent) => void; /** * If `true`, the tooltip is shown. */ diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js index 6ffdb3303702e7..8b4ecea9b5ab35 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.js +++ b/packages/material-ui/src/Tooltip/Tooltip.js @@ -7,6 +7,7 @@ import withStyles from '../styles/withStyles'; import capitalize from '../utils/capitalize'; import Grow from '../Grow'; import Popper from '../Popper'; +import useEventCallback from '../utils/useEventCallback'; import useForkRef from '../utils/useForkRef'; import useId from '../utils/useId'; import useIsFocusVisible from '../utils/useIsFocusVisible'; @@ -326,22 +327,27 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { } }; - const handleClose = (event) => { - clearTimeout(hystersisTimer); - hystersisTimer = setTimeout(() => { - hystersisOpen = false; - }, 800 + leaveDelay); - setOpenState(false); - - if (onClose) { - onClose(event); - } + const handleClose = useEventCallback( + /** + * @param {React.SyntheticEvent | Event} event + */ + (event) => { + clearTimeout(hystersisTimer); + hystersisTimer = setTimeout(() => { + hystersisOpen = false; + }, 800 + leaveDelay); + setOpenState(false); + + if (onClose) { + onClose(event); + } - clearTimeout(closeTimer.current); - closeTimer.current = setTimeout(() => { - ignoreNonTouchEvents.current = false; - }, theme.transitions.duration.shortest); - }; + clearTimeout(closeTimer.current); + closeTimer.current = setTimeout(() => { + ignoreNonTouchEvents.current = false; + }, theme.transitions.duration.shortest); + }, + ); const handleLeave = (forward = true) => (event) => { const childrenProps = children.props; @@ -402,6 +408,28 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { }, leaveTouchDelay); }; + React.useEffect(() => { + if (!open) { + return undefined; + } + + /** + * @param {KeyboardEvent} nativeEvent + */ + function handleKeyDown(nativeEvent) { + // IE 11, Edge (prior to using Bink?) use 'Esc' + if (nativeEvent.key === 'Escape' || nativeEvent.key === 'Esc') { + handleClose(nativeEvent); + } + } + + document.addEventListener('keydown', handleKeyDown); + + return () => { + document.removeEventListener('keydown', handleKeyDown); + }; + }, [handleClose, open]); + const handleUseRef = useForkRef(setChildNode, ref); const handleFocusRef = useForkRef(focusVisibleRef, handleUseRef); const handleRef = useForkRef(children.ref, handleFocusRef); diff --git a/packages/material-ui/src/Tooltip/Tooltip.test.js b/packages/material-ui/src/Tooltip/Tooltip.test.js index e2abd64bdb83f0..17e7857a57333f 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.test.js +++ b/packages/material-ui/src/Tooltip/Tooltip.test.js @@ -8,6 +8,7 @@ import { act, createClientRender, fireEvent, + screen, } from 'test/utils'; import { camelCase } from 'lodash/string'; import Tooltip, { testReset } from './Tooltip'; @@ -235,6 +236,31 @@ describe('', () => { clock.runAll(); }); + it('is dismissable by pressing Escape', () => { + const transitionTimeout = 0; + render( + + + , + ); + + expect(screen.getByRole('tooltip')).not.toBeInaccessible(); + + act(() => { + fireEvent.keyDown( + // We don't care about the target. Any Escape should dismiss the tooltip + // eslint-disable-next-line material-ui/disallow-active-element-as-key-event-target + document.activeElement, + { key: 'Escape' }, + ); + }); + act(() => { + clock.tick(transitionTimeout); + }); + + expect(screen.queryByRole('tooltip')).to.equal(null); + }); + describe('touch screen', () => { it('should not respond to quick events', () => { const { getByRole, queryByRole } = render(