search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.95k stars 32.27k forks source link

[Tooltip] Change `disableInteractive` default value #30821

Open leosdad opened 2 years ago

leosdad commented 2 years ago

Duplicates

Latest version

Current behavior 😯

After a tooltip is shown, moving the mouse cursor over the tooltip keeps it open indefinitely. It is hidden only when the mouse cursor is moved outside it.

msedge_2022-01-28_12-39-44

Expected behavior 🤔

The tooltip should close as soon as the mouse moves outside the main element. This is illustrated in the Material Design guidelines: Also, the statement "Continuously display the tooltip as long as the user long-presses or hovers over the element." implies that when this is no longer the case the tooltip should disappear.

See other Material Design implementations for comparison, e.g. Vue/Quasar), does it correctly:

msedge_2022-01-28_12-41-54

Steps to reproduce 🕹

To reproduce:

  1. Go to the built-in example).
  2. Move the mouse cursor over the IconButton: The tooltip is shown.
  3. Move the mouse cursor outside the IconButton and over the tooltip. The tooltip is still shown. It is hidden only when the mouse cursor is moved outside it.

Context 🔦

Any context / all uses.

Your environment 🌎

`npx @mui/envinfo` ``` Browser: MS Edge v. 97.0.1072.69, 64 bits System: OS: Windows 10 10.0.19042 Binaries: Node: 14.15.0 - C:\Program Files\nodejs\node.EXE Yarn: Not Found npm: 6.14.8 - C:\Program Files\nodejs\npm.CMD Browsers: Chrome: 97.0.4692.99 Edge: Spartan (44.19041.1266.0), Chromium (97.0.1072.69) npmPackages: @emotion/react: ^11.7.1 => 11.7.1 @emotion/styled: ^11.6.0 => 11.6.0 @mui/base: 5.0.0-alpha.66 @mui/material: ^5.3.1 => 5.3.1 @mui/private-theming: 5.3.0 @mui/styled-engine: 5.3.0 @mui/system: 5.3.0 @mui/types: 7.1.0 @mui/utils: 5.3.0 @types/react: ^17.0.38 => 17.0.38 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 ```
hbjORbj commented 2 years ago

@leosdad Thanks for the report.

If you pass true to disableInteractive prop of Tooltip, you can achieve the behaviour (Tooltip disappearing as soon as hovering stops).

Check out this codesandbox.

Maybe we should consider setting this prop to true by default. What do you think?

cc @mnajdova @oliviertassinari @danilo-leal

oliviertassinari commented 2 years ago

See #22382

oliviertassinari commented 2 years ago

I would propose:

This proposal is a breaking change, we could consider it for v6 if we have enough evidence that it's the right thing to do. I was personally not convinced at 100% when @eps1lon did the change, but I don't think that we have enough evidence to support that it wasn't our best option. I recall also talking about this case specifically to @davidluhr a few days ago.

leosdad commented 2 years ago

Hi, OP here. Thanks all for the responses and the workaround, although I can't test anything because I'm on vacation now. I'll comment further when I'm back. Cheers

hbjORbj commented 2 years ago

@oliviertassinari

Did the renaming and the label. You want me to close the issue? Isn't it better to leave it open?

leosdad commented 2 years ago

@oliviertassinari

Did the renaming and the label. You want me to close the issue? Isn't it better to leave it open?

Please don't. Need to test the workaround first and maybe suggest a couple of changes to the docs. I'll be back in mid-February, I'll surely do it then. Thanks!

ggascoigne commented 2 years ago

Related to this I think, but another use-case where this change in behavior is awkward. We use tooltips on tables to show truncated text, for better or worse, that can make for a lot of cells with a lot of potential tooltips. The new tooltip behavior breaks pretty badly since the tooltip overlays other cells and stops the user from interacting with them and getting at those tooltips. As for the AAA concerns, we address them in the underlying cell with aria-labels.

Also, the cell no longer has the aria-describedby or title attributes - though I'll admit we're still porting to v5 so this one might well be our bug.

eps1lon commented 2 years ago

See https://github.com/mui-org/material-ui/pull/22382 for why the current behavior is intended.

TL;DR: Changing the default value breaks WCAG level AA.

ggascoigne commented 2 years ago

re. my previous comment about aria-describedby being missing, that was an issue with my code, the behavior change around describeChild requires changes I'd not discovered (and aren't in the migration guide). Sorry, didn't mean to hijack issue.

eps1lon commented 2 years ago

@ggascoigne As far as I can tell, this issue is about interactive/disableInteractive which isn't related to the accessible name/accessible description. So if there's an issue with describeChild, I would recommend opening a separate issue.

leosdad commented 2 years ago

If you pass true to disableInteractive prop of Tooltip, you can achieve the behaviour (Tooltip disappearing as soon as hovering stops).

Yes, thanks! So in the end this wasn't a bug or an omission at all. Enabling it effectively solves my issue, but for some reason I missed it. My fault.

IMHO the word "interactive" would imply a hyperlink or a button. Perhaps "persistent" would be more descriptive. Also, a flag starting with 'disable' may be confusing.

Maybe we should consider setting this prop to true by default. What do you think?

I agree. IMHO that's what the Material Design guidelines say. Most implementations follow this too.

Anyway, thanks again for the solution to my problem.

eps1lon commented 2 years ago

I agree. IMHO that's what the Material Design guidelines say.

Could you quote and link this?

Most implementations follow this too.

That's not a reason to follow. With that reasoning we may just copy other component libraries and call it a day.

  1. implementors may not be aware of the issue
  2. implementors may be aware but a fix is underway
  3. implementors may be aware but can't fix it yet

Also: What about the implementations that don't follow? Why is the a contest between the number of implementations for one or the other scenario? This issue isn't about popularity but accessibility.

leosdad commented 2 years ago

Hi @eps1lon,

Could you quote and link this?

This link is above, here it is again: 'This is illustrated in the Material Design guidelines. Also, the statement "Continuously display the tooltip as long as the user long-presses or hovers over the element [my emphasis]." implies that when this is no longer the case the tooltip should disappear.'

That's not a reason to follow.

Agreed. It's just that MUI is where I found this behavior for the first time. But that's just my opinion, please do whatever you find is best.

Also please consider my suggestion to call this feature 'persistent' instead of 'interactive'. Inverting the logic to avoid 'disable' wouild also be nice. Again, just my 2 cents.

Thanks for your attention to this issue.

davebradlee commented 1 year ago

See #22382 for why the current behavior is intended.

TL;DR: Changing the default value breaks WCAG level AA.

I know I'm late to the discussion -- finally migrating our app from MUI 4 to 5 and ran into this. Your claim in the migration doc is incorrect (and I see @oliviertassinari and you were discussing back then), because WCAG 21 says "Examples of additional content controlled by the user agent include browser tooltips created through use of the HTML [title attribute]", so tooltips should not be interactive by default. Anyway, I did blanket search/replace, so easy. Thanks for all you hard work on MUI!!!

Grawl commented 1 year ago

In all out projects with MUI, we disable it using theme provider https://mui.com/material-ui/customization/theme-components/#default-props

Akxe commented 9 months ago

I think that the tooltip should be interactive if it is shown because of:

But the tooltip should have the default behaviour if opened via mouse hover.