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.

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] Change disableInteractive default value #30821

Open
2 tasks done
leosdad opened this issue Jan 28, 2022 · 16 comments
Open
2 tasks done

[Tooltip] Change disableInteractive default value #30821

leosdad opened this issue Jan 28, 2022 · 16 comments
Labels
accessibility a11y breaking change component: tooltip This is the name of the generic UI component, not the React module! discussion waiting for 👍 Waiting for upvotes

Comments

@leosdad
Copy link

leosdad commented Jan 28, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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 ```
@leosdad leosdad added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 28, 2022
@hbjORbj hbjORbj added bug 🐛 Something doesn't work component: tooltip This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material discussion and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer bug 🐛 Something doesn't work labels Jan 31, 2022
@hbjORbj
Copy link
Member

hbjORbj commented Jan 31, 2022

@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
Copy link
Member

See #22382

@oliviertassinari oliviertassinari added accessibility a11y component: accordion This is the name of the generic UI component, not the React module! breaking change and removed package: material-ui Specific to @mui/material component: accordion This is the name of the generic UI component, not the React module! labels Jan 31, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 31, 2022

I would propose:

  • we rename the title to be more accurate "[Tooltip] Change disableInteractive default value"
  • close this issue
  • add the "waiting for upvotes" label

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.

@hbjORbj hbjORbj changed the title [Tooltip] should close when element is not hovered anymore [Tooltip] Change disableInteractive default value Jan 31, 2022
@hbjORbj hbjORbj added the waiting for 👍 Waiting for upvotes label Jan 31, 2022
@leosdad
Copy link
Author

leosdad commented Jan 31, 2022

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
Copy link
Member

hbjORbj commented Jan 31, 2022

@oliviertassinari

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

@leosdad
Copy link
Author

leosdad commented Jan 31, 2022

@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
Copy link

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
Copy link
Member

eps1lon commented Feb 1, 2022

See #22382 for why the current behavior is intended.

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

@ggascoigne
Copy link

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
Copy link
Member

eps1lon commented Feb 1, 2022

@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
Copy link
Author

leosdad commented Feb 16, 2022

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
Copy link
Member

eps1lon commented Feb 16, 2022

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
Copy link
Author

leosdad commented Feb 16, 2022

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
Copy link

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
Copy link

Grawl commented Feb 18, 2023

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

@Akxe
Copy link

Akxe commented Feb 5, 2024

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

  • Keyboard interaction (tab or arrows in some cases)
  • Mouse press activation

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

@DiegoAndai DiegoAndai added this to the Material UI with Base UI milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y breaking change component: tooltip This is the name of the generic UI component, not the React module! discussion waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

9 participants