Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tooltip] Make interactive default #22382

Merged
merged 5 commits into from
Oct 6, 2020
Merged

[Tooltip] Make interactive default #22382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2afca7a
[Tooltip] Make `interactive` default
eps1lon Aug 27, 2020
18891da
Introduce disableInteractive
eps1lon Sep 28, 2020
57f0f10
Better formatting for migration
eps1lon Oct 5, 2020
62246d1
Update tests
eps1lon Oct 5, 2020
717c973
Update TSDOC
eps1lon Oct 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/pages/api-docs/tooltip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ The `MuiTooltip` name can be used for providing [default props](/customization/g
| <span class="prop-name">describeChild</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Set to `true` if the `title` acts as an accessible description. By default the `title` acts as an accessible label for the child. |
| <span class="prop-name">disableFocusListener</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Do not respond to focus events. |
| <span class="prop-name">disableHoverListener</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Do not respond to hover events. |
| <span class="prop-name">disableInteractive</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Makes a tooltip not interactive, i.e. it will close when the user hovers over the tooltip before the `leaveDelay` is expired. |
| <span class="prop-name">disableTouchListener</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Do not respond to long press touch events. |
| <span class="prop-name">enterDelay</span> | <span class="prop-type">number</span> | <span class="prop-default">100</span> | The number of milliseconds to wait before showing the tooltip. This prop won't impact the enter touch delay (`enterTouchDelay`). |
| <span class="prop-name">enterNextDelay</span> | <span class="prop-type">number</span> | <span class="prop-default">0</span> | The number of milliseconds to wait before showing the tooltip when one was already recently opened. |
| <span class="prop-name">enterTouchDelay</span> | <span class="prop-type">number</span> | <span class="prop-default">700</span> | The number of milliseconds a user must touch the element before showing the tooltip. |
| <span class="prop-name">id</span> | <span class="prop-type">string</span> | | 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. |
| <span class="prop-name">interactive</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Makes a tooltip interactive, i.e. will not close when the user hovers over the tooltip before the `leaveDelay` is expired. |
| <span class="prop-name">leaveDelay</span> | <span class="prop-type">number</span> | <span class="prop-default">0</span> | The number of milliseconds to wait before hiding the tooltip. This prop won't impact the leave touch delay (`leaveTouchDelay`). |
| <span class="prop-name">leaveTouchDelay</span> | <span class="prop-type">number</span> | <span class="prop-default">1500</span> | The number of milliseconds after the user stops touching an element before hiding the tooltip. |
| <span class="prop-name">onClose</span> | <span class="prop-type">func</span> | | Callback fired when the component requests to be closed.<br><br>**Signature:**<br>`function(event: object) => void`<br>*event:* The event source of the callback. |
Expand All @@ -61,7 +61,7 @@ Any other props supplied will be provided to the root element (native element).
| Rule name | Global class | Description |
|:-----|:-------------|:------------|
| <span class="prop-name">popper</span> | <span class="prop-name">.MuiTooltip-popper</span> | Styles applied to the Popper component.
| <span class="prop-name">popperInteractive</span> | <span class="prop-name">.MuiTooltip-popperInteractive</span> | Styles applied to the Popper component if `interactive={true}`.
| <span class="prop-name">popperInteractive</span> | <span class="prop-name">.MuiTooltip-popperInteractive</span> | Styles applied to the Popper component unless `disableInteractive={true}`.
| <span class="prop-name">popperArrow</span> | <span class="prop-name">.MuiTooltip-popperArrow</span> | Styles applied to the Popper component if `arrow={true}`.
| <span class="prop-name">tooltip</span> | <span class="prop-name">.MuiTooltip-tooltip</span> | Styles applied to the tooltip (label wrapper) element.
| <span class="prop-name">tooltipArrow</span> | <span class="prop-name">.MuiTooltip-tooltipArrow</span> | Styles applied to the tooltip (label wrapper) element if `arrow={true}`.
Expand Down
6 changes: 3 additions & 3 deletions ...omponents/tooltips/InteractiveTooltips.js → ...onents/tooltips/NonInteractiveTooltips.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import * as React from 'react';
import Button from '@material-ui/core/Button';
import Tooltip from '@material-ui/core/Tooltip';

export default function InteractiveTooltips() {
export default function NotInteractiveTooltips() {
return (
<Tooltip title="Add" interactive>
<Button>Interactive</Button>
<Tooltip title="Add" disableInteractive>
<Button>Not interactive</Button>
</Tooltip>
);
}
6 changes: 3 additions & 3 deletions ...mponents/tooltips/InteractiveTooltips.tsx → ...nents/tooltips/NonInteractiveTooltips.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import * as React from 'react';
import Button from '@material-ui/core/Button';
import Tooltip from '@material-ui/core/Tooltip';

export default function InteractiveTooltips() {
export default function NotInteractiveTooltips() {
return (
<Tooltip title="Add" interactive>
<Button>Interactive</Button>
<Tooltip title="Add" disableInteractive>
<Button>Not interactive</Button>
</Tooltip>
);
}
6 changes: 4 additions & 2 deletions docs/src/pages/components/tooltips/tooltips.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,11 @@ The `Tooltip` wraps long text by default to make it readable.

## Interactive

A tooltip can be interactive. It won't close when the user hovers over the tooltip before the `leaveDelay` is expired.
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/InteractiveTooltips.js"}}
{{"demo": "pages/components/tooltips/NonInteractiveTooltips.js"}}

## Disabled Elements

Expand Down
17 changes: 17 additions & 0 deletions docs/src/pages/guides/migration-v4/migration-v4.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,23 @@ const theme = createMuiTheme({
+import ToggleButtonGroup from '@material-ui/core/ToggleButtonGroup';
```

### Tooltip

- Tooltips are now interactive by default.

The previous default behavior failed [success criterion 1.4.3 ("hoverable") in WCAG 2.1](https://www.w3.org/TR/WCAG21/#content-on-hover-or-focus).
To reflect the new default value, the prop was renamed to `disableInteractive`.
If you want to restore the old behavior (thus not reaching level AA), you can apply the following diff:

```diff
-<Tooltip>
+<Tooltip disableInteractive>

# Interactive tooltips no longer need the `interactive` prop.
-<Tooltip interactive>
+<Tooltip>
```

### Typography

- Replace the `srOnly` prop so as to not duplicate the capabilities of [System](https://material-ui.com/system/basics/):
Expand Down
14 changes: 7 additions & 7 deletions packages/material-ui/src/Tooltip/Tooltip.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface TooltipProps extends StandardProps<React.HTMLAttributes<HTMLDiv
classes?: {
/** Styles applied to the Popper component. */
popper?: string;
/** Styles applied to the Popper component if `interactive={true}`. */
/** Styles applied to the Popper component unless `disableInteractive={true}`. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have used the following pattern in the other components:

Suggested change
/** Styles applied to the Popper component unless `disableInteractive={true}`. */
/** Styles applied to the Popper component if `disableInteractive={false}`. */

Should we stick with it? cc @mbrookes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that you'll almost never see someBooleanProp={false} with our API design philosophy.

It's also incomplete since the styles are also applied without any code. My approach is covering false and undefined while your approach only covers false.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer "unless true" over "if false".

popperInteractive?: string;
/** Styles applied to the Popper component if `arrow={true}`. */
popperArrow?: string;
Expand Down Expand Up @@ -56,6 +56,12 @@ export interface TooltipProps extends StandardProps<React.HTMLAttributes<HTMLDiv
* @default false
*/
disableHoverListener?: boolean;
/**
* Makes a tooltip not interactive, i.e. it will close when the user
* hovers over the tooltip before the `leaveDelay` is expired.
* @default false
*/
disableInteractive?: boolean;
/**
* Do not respond to long press touch events.
* @default false
Expand All @@ -82,12 +88,6 @@ export interface TooltipProps extends StandardProps<React.HTMLAttributes<HTMLDiv
* If you don't provide this prop. It falls back to a randomly generated id.
*/
id?: string;
/**
* Makes a tooltip interactive, i.e. will not close when the user
* hovers over the tooltip before the `leaveDelay` is expired.
* @default false
*/
interactive?: boolean;
/**
* The number of milliseconds to wait before hiding the tooltip.
* This prop won't impact the leave touch delay (`leaveTouchDelay`).
Expand Down
22 changes: 11 additions & 11 deletions packages/material-ui/src/Tooltip/Tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const styles = (theme) => ({
zIndex: theme.zIndex.tooltip,
pointerEvents: 'none', // disable jss-rtl plugin
},
/* Styles applied to the Popper component if `interactive={true}`. */
/* Styles applied to the Popper component unless `disableInteractive={true}`. */
popperInteractive: {
pointerEvents: 'auto',
},
Expand Down Expand Up @@ -174,7 +174,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
enterNextDelay = 0,
enterTouchDelay = 700,
id: idProp,
interactive = false,
disableInteractive = false,
leaveDelay = 0,
leaveTouchDelay = 1500,
onClose,
Expand Down Expand Up @@ -486,7 +486,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
childrenProps.onMouseOver = handleEnter();
childrenProps.onMouseLeave = handleLeave();

if (interactive) {
if (!disableInteractive) {
interactiveWrapperListeners.onMouseOver = handleEnter(false);
interactiveWrapperListeners.onMouseLeave = handleLeave(false);
}
Expand All @@ -496,7 +496,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
childrenProps.onFocus = handleFocus();
childrenProps.onBlur = handleLeave();

if (interactive) {
if (!disableInteractive) {
interactiveWrapperListeners.onFocus = handleFocus(false);
interactiveWrapperListeners.onBlur = handleLeave(false);
}
Expand Down Expand Up @@ -534,7 +534,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
{React.cloneElement(children, childrenProps)}
<PopperComponent
className={clsx(classes.popper, {
[classes.popperInteractive]: interactive,
[classes.popperInteractive]: !disableInteractive,
[classes.popperArrow]: arrow,
})}
placement={placement}
Expand Down Expand Up @@ -609,6 +609,12 @@ Tooltip.propTypes = {
* @default false
*/
disableHoverListener: PropTypes.bool,
/**
* Makes a tooltip not interactive, i.e. it will close when the user
* hovers over the tooltip before the `leaveDelay` is expired.
* @default false
*/
disableInteractive: PropTypes.bool,
/**
* Do not respond to long press touch events.
* @default false
Expand All @@ -635,12 +641,6 @@ Tooltip.propTypes = {
* If you don't provide this prop. It falls back to a randomly generated id.
*/
id: PropTypes.string,
/**
* Makes a tooltip interactive, i.e. will not close when the user
* hovers over the tooltip before the `leaveDelay` is expired.
* @default false
*/
interactive: PropTypes.bool,
/**
* The number of milliseconds to wait before hiding the tooltip.
* This prop won't impact the leave touch delay (`leaveTouchDelay`).
Expand Down
28 changes: 14 additions & 14 deletions packages/material-ui/src/Tooltip/Tooltip.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ describe('<Tooltip />', () => {
it('should ignore event from the tooltip', () => {
const handleMouseOver = spy();
const { getByRole } = render(
<Tooltip title="Hello World" open interactive>
<Tooltip title="Hello World" open>
<button type="submit" onMouseOver={handleMouseOver}>
Hello World
</button>
Expand Down Expand Up @@ -698,13 +698,12 @@ describe('<Tooltip />', () => {
});
});

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(
<Tooltip
title="Hello World"
enterDelay={100}
interactive
leaveDelay={111}
TransitionProps={{ timeout: 10 }}
>
Expand All @@ -731,9 +730,14 @@ describe('<Tooltip />', () => {
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(
<Tooltip title="Hello World" interactive enterDelay={500} TransitionProps={{ timeout: 10 }}>
<Tooltip
title="Hello World"
enterDelay={100}
leaveDelay={111}
TransitionProps={{ timeout: 10 }}
>
<button id="testChild" type="submit">
Hello World
</button>
Expand All @@ -742,7 +746,7 @@ describe('<Tooltip />', () => {

fireEvent.mouseOver(getByRole('button'));
act(() => {
clock.tick(500);
clock.tick(100);
});

expect(getByRole('tooltip')).toBeVisible();
Expand All @@ -752,13 +756,9 @@ describe('<Tooltip />', () => {
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();
});
});

Expand Down Expand Up @@ -886,7 +886,7 @@ describe('<Tooltip />', () => {
);
});
const { getByRole } = render(
<Tooltip interactive open title="test">
<Tooltip open title="test">
<TextField onFocus={handleFocus} />
</Tooltip>,
);
Expand Down