From 2afca7ae24c79a83a12ae2add309e0807430de3c Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 27 Aug 2020 20:03:22 +0200 Subject: [PATCH 1/5] [Tooltip] Make `interactive` default --- docs/pages/api-docs/tooltip.md | 2 - .../tooltips/InteractiveTooltips.js | 11 --- .../tooltips/InteractiveTooltips.tsx | 11 --- .../src/pages/components/tooltips/tooltips.md | 6 -- .../pages/guides/migration-v4/migration-v4.md | 5 + packages/material-ui/src/Tooltip/Tooltip.d.ts | 8 -- packages/material-ui/src/Tooltip/Tooltip.js | 25 +---- .../material-ui/src/Tooltip/Tooltip.test.js | 97 +++++++++---------- 8 files changed, 56 insertions(+), 109 deletions(-) delete mode 100644 docs/src/pages/components/tooltips/InteractiveTooltips.js delete mode 100644 docs/src/pages/components/tooltips/InteractiveTooltips.tsx diff --git a/docs/pages/api-docs/tooltip.md b/docs/pages/api-docs/tooltip.md index ba9a00737a4323..f6070609f29192 100644 --- a/docs/pages/api-docs/tooltip.md +++ b/docs/pages/api-docs/tooltip.md @@ -39,7 +39,6 @@ The `MuiTooltip` name can be used for providing [default props](/customization/g | enterNextDelay | number | 0 | The number of milliseconds to wait before showing the tooltip when one was already recently opened. | | enterTouchDelay | number | 700 | The number of milliseconds a user must touch the element before showing the tooltip. | | id | string | | This prop is used to help implement the accessibility logic. If you don't provide this prop. It falls back to a randomly generated id. | -| interactive | bool | false | Makes a tooltip interactive, i.e. will not close when the user hovers over the tooltip before the `leaveDelay` is expired. | | leaveDelay | number | 0 | The number of milliseconds to wait before hiding the tooltip. This prop won't impact the leave touch delay (`leaveTouchDelay`). | | leaveTouchDelay | number | 1500 | The number of milliseconds after the user stops touching an element before hiding the tooltip. | | onClose | func | | Callback fired when the component requests to be closed.

**Signature:**
`function(event: object) => void`
*event:* The event source of the callback. | @@ -61,7 +60,6 @@ Any other props supplied will be provided to the root element (native element). | Rule name | Global class | Description | |:-----|:-------------|:------------| | popper | .MuiTooltip-popper | Styles applied to the Popper component. -| popperInteractive | .MuiTooltip-popperInteractive | Styles applied to the Popper component if `interactive={true}`. | popperArrow | .MuiTooltip-popperArrow | Styles applied to the Popper component if `arrow={true}`. | tooltip | .MuiTooltip-tooltip | Styles applied to the tooltip (label wrapper) element. | tooltipArrow | .MuiTooltip-tooltipArrow | Styles applied to the tooltip (label wrapper) element if `arrow={true}`. diff --git a/docs/src/pages/components/tooltips/InteractiveTooltips.js b/docs/src/pages/components/tooltips/InteractiveTooltips.js deleted file mode 100644 index a9d37f3cb3c8cc..00000000000000 --- a/docs/src/pages/components/tooltips/InteractiveTooltips.js +++ /dev/null @@ -1,11 +0,0 @@ -import * as React from 'react'; -import Button from '@material-ui/core/Button'; -import Tooltip from '@material-ui/core/Tooltip'; - -export default function InteractiveTooltips() { - return ( - - - - ); -} diff --git a/docs/src/pages/components/tooltips/InteractiveTooltips.tsx b/docs/src/pages/components/tooltips/InteractiveTooltips.tsx deleted file mode 100644 index a9d37f3cb3c8cc..00000000000000 --- a/docs/src/pages/components/tooltips/InteractiveTooltips.tsx +++ /dev/null @@ -1,11 +0,0 @@ -import * as React from 'react'; -import Button from '@material-ui/core/Button'; -import Tooltip from '@material-ui/core/Tooltip'; - -export default function InteractiveTooltips() { - return ( - - - - ); -} diff --git a/docs/src/pages/components/tooltips/tooltips.md b/docs/src/pages/components/tooltips/tooltips.md index d7e7f572833b2f..5d6fa22a21b82a 100644 --- a/docs/src/pages/components/tooltips/tooltips.md +++ b/docs/src/pages/components/tooltips/tooltips.md @@ -76,12 +76,6 @@ The `Tooltip` wraps long text by default to make it readable. {{"demo": "pages/components/tooltips/VariableWidth.js"}} -## Interactive - -A tooltip can be interactive. It won't close when the user hovers over the tooltip before the `leaveDelay` is expired. - -{{"demo": "pages/components/tooltips/InteractiveTooltips.js"}} - ## Disabled Elements By default disabled elements like ` @@ -698,68 +698,65 @@ describe('', () => { }); }); - describe('prop: interactive', () => { - it('should keep the overlay open if the popper element is hovered', () => { - const { getByRole } = render( - - - , - ); + it('should keep the overlay open if the popper element is hovered', () => { + const { getByRole } = render( + + + , + ); - fireEvent.mouseOver(getByRole('button')); - act(() => { - clock.tick(100); - }); + fireEvent.mouseOver(getByRole('button')); + act(() => { + clock.tick(100); + }); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseLeave(getByRole('button')); + fireEvent.mouseLeave(getByRole('button')); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseOver(getByRole('tooltip')); - clock.tick(111 + 10); + fireEvent.mouseOver(getByRole('tooltip')); + clock.tick(111 + 10); - expect(getByRole('tooltip')).toBeVisible(); - }); + expect(getByRole('tooltip')).toBeVisible(); + }); - it('should not animate twice', () => { - const { getByRole } = render( - - - , - ); + it('should not animate twice', () => { + const { getByRole } = render( + + + , + ); - fireEvent.mouseOver(getByRole('button')); - act(() => { - clock.tick(500); - }); + fireEvent.mouseOver(getByRole('button')); + act(() => { + clock.tick(500); + }); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseLeave(getByRole('button')); + fireEvent.mouseLeave(getByRole('button')); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseOver(getByRole('tooltip')); - clock.tick(10); + fireEvent.mouseOver(getByRole('tooltip')); + clock.tick(10); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - // TOD: Unclear why not running triggers microtasks but runAll does not trigger microtasks - // can be removed once Popper#update is sync - clock.runAll(); - }); + // TOD: Unclear why not running triggers microtasks but runAll does not trigger microtasks + // can be removed once Popper#update is sync + clock.runAll(); }); describe('prop: PopperProps', () => { @@ -886,7 +883,7 @@ describe('', () => { ); }); const { getByRole } = render( - + , ); From 18891dac3d79fa7721faf4d1d92bc98d71fd936d Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Mon, 28 Sep 2020 12:01:16 +0200 Subject: [PATCH 2/5] Introduce disableInteractive --- docs/pages/api-docs/tooltip.md | 2 + .../tooltips/NonInteractiveTooltips.js | 11 +++ .../tooltips/NonInteractiveTooltips.tsx | 11 +++ .../src/pages/components/tooltips/tooltips.md | 8 ++ .../pages/guides/migration-v4/migration-v4.md | 16 ++- packages/material-ui/src/Tooltip/Tooltip.d.ts | 7 ++ packages/material-ui/src/Tooltip/Tooltip.js | 24 ++++- .../material-ui/src/Tooltip/Tooltip.test.js | 97 ++++++++++--------- 8 files changed, 122 insertions(+), 54 deletions(-) create mode 100644 docs/src/pages/components/tooltips/NonInteractiveTooltips.js create mode 100644 docs/src/pages/components/tooltips/NonInteractiveTooltips.tsx diff --git a/docs/pages/api-docs/tooltip.md b/docs/pages/api-docs/tooltip.md index f6070609f29192..61bf6d6989b389 100644 --- a/docs/pages/api-docs/tooltip.md +++ b/docs/pages/api-docs/tooltip.md @@ -34,6 +34,7 @@ The `MuiTooltip` name can be used for providing [default props](/customization/g | describeChild | bool | false | Set to `true` if the `title` acts as an accessible description. By default the `title` acts as an accessible label for the child. | | disableFocusListener | bool | false | Do not respond to focus events. | | disableHoverListener | bool | false | Do not respond to hover events. | +| disableInteractive | bool | false | Makes a tooltip not interactive, i.e. it will close when the user hovers over the tooltip before the `leaveDelay` is expired. | | disableTouchListener | bool | false | Do not respond to long press touch events. | | enterDelay | number | 100 | The number of milliseconds to wait before showing the tooltip. This prop won't impact the enter touch delay (`enterTouchDelay`). | | enterNextDelay | number | 0 | The number of milliseconds to wait before showing the tooltip when one was already recently opened. | @@ -60,6 +61,7 @@ Any other props supplied will be provided to the root element (native element). | Rule name | Global class | Description | |:-----|:-------------|:------------| | popper | .MuiTooltip-popper | Styles applied to the Popper component. +| popperInteractive | .MuiTooltip-popperInteractive | Styles applied to the Popper component unless `disableInteractive={true}`. | popperArrow | .MuiTooltip-popperArrow | Styles applied to the Popper component if `arrow={true}`. | tooltip | .MuiTooltip-tooltip | Styles applied to the tooltip (label wrapper) element. | tooltipArrow | .MuiTooltip-tooltipArrow | Styles applied to the tooltip (label wrapper) element if `arrow={true}`. diff --git a/docs/src/pages/components/tooltips/NonInteractiveTooltips.js b/docs/src/pages/components/tooltips/NonInteractiveTooltips.js new file mode 100644 index 00000000000000..3ac4c3eafa020d --- /dev/null +++ b/docs/src/pages/components/tooltips/NonInteractiveTooltips.js @@ -0,0 +1,11 @@ +import * as React from 'react'; +import Button from '@material-ui/core/Button'; +import Tooltip from '@material-ui/core/Tooltip'; + +export default function NotInteractiveTooltips() { + return ( + + + + ); +} diff --git a/docs/src/pages/components/tooltips/NonInteractiveTooltips.tsx b/docs/src/pages/components/tooltips/NonInteractiveTooltips.tsx new file mode 100644 index 00000000000000..3ac4c3eafa020d --- /dev/null +++ b/docs/src/pages/components/tooltips/NonInteractiveTooltips.tsx @@ -0,0 +1,11 @@ +import * as React from 'react'; +import Button from '@material-ui/core/Button'; +import Tooltip from '@material-ui/core/Tooltip'; + +export default function NotInteractiveTooltips() { + return ( + + + + ); +} diff --git a/docs/src/pages/components/tooltips/tooltips.md b/docs/src/pages/components/tooltips/tooltips.md index 5d6fa22a21b82a..e338574b3428dc 100644 --- a/docs/src/pages/components/tooltips/tooltips.md +++ b/docs/src/pages/components/tooltips/tooltips.md @@ -76,6 +76,14 @@ The `Tooltip` wraps long text by default to make it readable. {{"demo": "pages/components/tooltips/VariableWidth.js"}} +## Interactive + +Tooltips are interactive by default (to pass [WCAG 2.1 success criterion 1.4.13](https://www.w3.org/TR/WCAG21/#content-on-hover-or-focus)). +It won't close when the user hovers over the tooltip before the `leaveDelay` is expired. +You can disable this behavior (thus failing the success criterion which is required to reach level AA) by passing `disableInteractive`. + +{{"demo": "pages/components/tooltips/NonInteractiveTooltips.js"}} + ## Disabled Elements By default disabled elements like ` @@ -698,65 +698,68 @@ describe('', () => { }); }); - it('should keep the overlay open if the popper element is hovered', () => { - const { getByRole } = render( - - - , - ); + describe('prop: interactive', () => { + it('should keep the overlay open if the popper element is hovered', () => { + const { getByRole } = render( + + + , + ); - fireEvent.mouseOver(getByRole('button')); - act(() => { - clock.tick(100); - }); + fireEvent.mouseOver(getByRole('button')); + act(() => { + clock.tick(100); + }); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseLeave(getByRole('button')); + fireEvent.mouseLeave(getByRole('button')); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseOver(getByRole('tooltip')); - clock.tick(111 + 10); + fireEvent.mouseOver(getByRole('tooltip')); + clock.tick(111 + 10); - expect(getByRole('tooltip')).toBeVisible(); - }); + expect(getByRole('tooltip')).toBeVisible(); + }); - it('should not animate twice', () => { - const { getByRole } = render( - - - , - ); + it('should not animate twice', () => { + const { getByRole } = render( + + + , + ); - fireEvent.mouseOver(getByRole('button')); - act(() => { - clock.tick(500); - }); + fireEvent.mouseOver(getByRole('button')); + act(() => { + clock.tick(500); + }); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseLeave(getByRole('button')); + fireEvent.mouseLeave(getByRole('button')); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - fireEvent.mouseOver(getByRole('tooltip')); - clock.tick(10); + fireEvent.mouseOver(getByRole('tooltip')); + clock.tick(10); - expect(getByRole('tooltip')).toBeVisible(); + expect(getByRole('tooltip')).toBeVisible(); - // TOD: Unclear why not running triggers microtasks but runAll does not trigger microtasks - // can be removed once Popper#update is sync - clock.runAll(); + // TOD: Unclear why not running triggers microtasks but runAll does not trigger microtasks + // can be removed once Popper#update is sync + clock.runAll(); + }); }); describe('prop: PopperProps', () => { @@ -883,7 +886,7 @@ describe('', () => { ); }); const { getByRole } = render( - + , ); From 57f0f10aeac58d628ac914501344cf842a9a6800 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Mon, 5 Oct 2020 10:12:13 +0200 Subject: [PATCH 3/5] Better formatting for migration --- docs/src/pages/guides/migration-v4/migration-v4.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/src/pages/guides/migration-v4/migration-v4.md b/docs/src/pages/guides/migration-v4/migration-v4.md index fab87cf91c5ec2..54e58389d25062 100644 --- a/docs/src/pages/guides/migration-v4/migration-v4.md +++ b/docs/src/pages/guides/migration-v4/migration-v4.md @@ -775,6 +775,7 @@ const theme = createMuiTheme({ +import ToggleButton from '@material-ui/core/ToggleButton'; +import ToggleButtonGroup from '@material-ui/core/ToggleButtonGroup'; ``` + ### Tooltip - Tooltips are now interactive by default. @@ -786,6 +787,7 @@ const theme = createMuiTheme({ ```diff - + + # Interactive tooltips no longer need the `interactive` prop. - + From 62246d12eba3d39db6a1c20449260c55fe29c3e3 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Mon, 5 Oct 2020 10:54:43 +0200 Subject: [PATCH 4/5] Update tests --- .../material-ui/src/Tooltip/Tooltip.test.js | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/material-ui/src/Tooltip/Tooltip.test.js b/packages/material-ui/src/Tooltip/Tooltip.test.js index a9ba0b66c06fd3..011bdba116c7be 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.test.js +++ b/packages/material-ui/src/Tooltip/Tooltip.test.js @@ -633,7 +633,7 @@ describe('', () => { it('should ignore event from the tooltip', () => { const handleMouseOver = spy(); const { getByRole } = render( - + @@ -698,13 +698,12 @@ describe('', () => { }); }); - describe('prop: interactive', () => { - it('should keep the overlay open if the popper element is hovered', () => { + describe('prop: disableInteractive', () => { + it('when false should keep the overlay open if the popper element is hovered', () => { const { getByRole } = render( @@ -731,9 +730,14 @@ describe('', () => { expect(getByRole('tooltip')).toBeVisible(); }); - it('should not animate twice', () => { + it('when `true` should not keep the overlay open if the popper element is hovered', () => { const { getByRole } = render( - + @@ -742,7 +746,7 @@ describe('', () => { fireEvent.mouseOver(getByRole('button')); act(() => { - clock.tick(500); + clock.tick(100); }); expect(getByRole('tooltip')).toBeVisible(); @@ -752,13 +756,9 @@ describe('', () => { expect(getByRole('tooltip')).toBeVisible(); fireEvent.mouseOver(getByRole('tooltip')); - clock.tick(10); - - expect(getByRole('tooltip')).toBeVisible(); + clock.tick(111 + 10); - // TOD: Unclear why not running triggers microtasks but runAll does not trigger microtasks - // can be removed once Popper#update is sync - clock.runAll(); + expect(getByRole('tooltip')).not.toBeVisible(); }); }); @@ -886,7 +886,7 @@ describe('', () => { ); }); const { getByRole } = render( - + , ); From 717c973686e20d75a8def8e1e915b60f6b44669a Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Mon, 5 Oct 2020 11:24:22 +0200 Subject: [PATCH 5/5] Update TSDOC --- packages/material-ui/src/Tooltip/Tooltip.d.ts | 1 + packages/material-ui/src/Tooltip/Tooltip.js | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/material-ui/src/Tooltip/Tooltip.d.ts b/packages/material-ui/src/Tooltip/Tooltip.d.ts index 59b0183ce670b7..bed17e51f14b2c 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.d.ts +++ b/packages/material-ui/src/Tooltip/Tooltip.d.ts @@ -59,6 +59,7 @@ export interface TooltipProps extends StandardProps