From 5fd5c5a0db2f6ad778022ee4f51fcaa861998047 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Wed, 20 Nov 2019 00:28:23 +0100 Subject: [PATCH] [Tooltip] Use hysteresis with the enterDelay --- packages/material-ui/src/Tooltip/Tooltip.js | 43 ++++++++---- .../material-ui/src/Tooltip/Tooltip.test.js | 68 +++++++++++++++---- 2 files changed, 82 insertions(+), 29 deletions(-) diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js index 7f4feb1a966b54..5cbf56f444d280 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.js +++ b/packages/material-ui/src/Tooltip/Tooltip.js @@ -150,6 +150,14 @@ export const styles = theme => ({ }, }); +let hystersisOpen = false; +let hystersisTimer = null; + +export function testReset() { + hystersisOpen = false; + clearTimeout(hystersisTimer); +} + const Tooltip = React.forwardRef(function Tooltip(props, ref) { const { arrow = false, @@ -160,7 +168,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { disableTouchListener = false, enterDelay = 0, enterTouchDelay = 700, - id, + id: idProp, interactive = false, leaveDelay = 0, leaveTouchDelay = 1500, @@ -176,11 +184,10 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { } = props; const theme = useTheme(); - const [, forceUpdate] = React.useState(0); const [childNode, setChildNode] = React.useState(); const [arrowRef, setArrowRef] = React.useState(null); const ignoreNonTouchEvents = React.useRef(false); - const defaultId = React.useRef(); + const closeTimer = React.useRef(); const enterTimer = React.useRef(); const leaveTimer = React.useRef(); @@ -232,19 +239,18 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { }, [isControlled, title, childNode]); } + const [defaultId, setDefaultId] = React.useState(); + const id = idProp || defaultId; React.useEffect(() => { + if (!open || defaultId) { + return; + } + // Fallback to this default id when possible. // Use the random value for client-side rendering only. // We can't use it server-side. - if (!defaultId.current) { - defaultId.current = `mui-tooltip-${Math.round(Math.random() * 1e5)}`; - } - - // Rerender with defaultId and childNode. - if (openProp) { - forceUpdate(n => !n); - } - }, [openProp]); + setDefaultId(`mui-tooltip-${Math.round(Math.random() * 1e5)}`); + }, [open, defaultId]); React.useEffect(() => { return () => { @@ -256,6 +262,9 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { }, []); const handleOpen = event => { + clearTimeout(hystersisTimer); + hystersisOpen = true; + // The mouseover event will trigger for every nested element in the tooltip. // We can skip rerendering when the tooltip is already open. // We are using the mouseover event instead of the mouseenter event to fix a hide/show issue. @@ -288,7 +297,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { clearTimeout(enterTimer.current); clearTimeout(leaveTimer.current); - if (enterDelay) { + if (enterDelay && !hystersisOpen) { event.persist(); enterTimer.current = setTimeout(() => { handleOpen(event); @@ -327,6 +336,12 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { }; const handleClose = event => { + clearTimeout(hystersisTimer); + hystersisTimer = setTimeout(() => { + hystersisOpen = false; + }, 500); + // Use 500 ms per https://github.com/reach/reach-ui/blob/3b5319027d763a3082880be887d7a29aee7d3afc/packages/tooltip/src/index.js#L214 + if (!isControlled) { setOpenState(false); } @@ -417,7 +432,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { // We are open to change the tradeoff. const shouldShowNativeTitle = !open && !disableHoverListener; const childrenProps = { - 'aria-describedby': open ? id || defaultId.current : null, + 'aria-describedby': open ? id : null, title: shouldShowNativeTitle && typeof title === 'string' ? title : null, ...other, ...children.props, diff --git a/packages/material-ui/src/Tooltip/Tooltip.test.js b/packages/material-ui/src/Tooltip/Tooltip.test.js index fa22cf9bb5a550..4266b07909376e 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.test.js +++ b/packages/material-ui/src/Tooltip/Tooltip.test.js @@ -4,19 +4,25 @@ import PropTypes from 'prop-types'; import { spy, useFakeTimers } from 'sinon'; import consoleErrorMock from 'test/utils/consoleErrorMock'; import { createMount, getClasses } from '@material-ui/core/test-utils'; +import { act, createClientRender, fireEvent } from 'test/utils/createClientRender'; import describeConformance from '../test-utils/describeConformance'; import Popper from '../Popper'; -import Tooltip from './Tooltip'; +import Tooltip, { testReset } from './Tooltip'; import Input from '../Input'; -import createMuiTheme from '../styles/createMuiTheme'; -const theme = createMuiTheme(); - -function focusVisible(wrapper) { +function focusVisibleLegacy(wrapper) { document.dispatchEvent(new window.Event('keydown')); wrapper.simulate('focus'); } +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 @@ -26,10 +32,14 @@ function simulatePointerDevice() { describe('', () => { let mount; let classes; + const render = createClientRender({ strict: false }); let clock; const defaultProps = { - children: Hello World, - theme, + children: ( + + ), title: 'Hello World', }; @@ -40,6 +50,10 @@ describe('', () => { clock = useFakeTimers(); }); + beforeEach(() => { + testReset(); + }); + after(() => { clock.restore(); mount.cleanUp(); @@ -47,9 +61,9 @@ describe('', () => { describeConformance(, () => ({ classes, - inheritComponent: 'span', + inheritComponent: 'button', mount, - refInstanceof: window.HTMLSpanElement, + refInstanceof: window.HTMLButtonElement, skip: [ 'componentProp', // react-transition-group issue @@ -193,16 +207,40 @@ describe('', () => { describe('prop: delay', () => { it('should take the enterDelay into account', () => { - const wrapper = mount(); + const wrapper = mount(); simulatePointerDevice(); const children = wrapper.find('#testChild'); - focusVisible(children); + focusVisibleLegacy(children); assert.strictEqual(wrapper.find('[role="tooltip"]').exists(), false); clock.tick(111); wrapper.update(); assert.strictEqual(wrapper.find('[role="tooltip"]').exists(), true); }); + it('should use hysteresis with the enterDelay', () => { + const { container } = render( + , + ); + const children = container.querySelector('#testChild'); + focusVisible(children); + assert.strictEqual(document.body.querySelectorAll('[role="tooltip"]').length, 0); + clock.tick(111); + assert.strictEqual(document.body.querySelectorAll('[role="tooltip"]').length, 1); + document.activeElement.blur(); + clock.tick(5); + clock.tick(6); + assert.strictEqual(document.body.querySelectorAll('[role="tooltip"]').length, 0); + + focusVisible(children); + // Bypass `enterDelay` wait, instant display. + assert.strictEqual(document.body.querySelectorAll('[role="tooltip"]').length, 1); + }); + it('should take the leaveDelay into account', () => { const childRef = React.createRef(); const wrapper = mount( @@ -212,7 +250,7 @@ describe('', () => { ); simulatePointerDevice(); const children = wrapper.find('#testChild'); - focusVisible(children); + focusVisibleLegacy(children); assert.strictEqual(wrapper.find('[role="tooltip"]').exists(), true); children.simulate('blur'); assert.strictEqual(wrapper.find('[role="tooltip"]').exists(), true); @@ -345,7 +383,7 @@ describe('', () => { describe('forward', () => { it('should forward props to the child element', () => { const wrapper = mount( - +

H1

, ); @@ -354,7 +392,7 @@ describe('', () => { it('should respect the props priority', () => { const wrapper = mount( -