[Tooltip] displaying twice when used with SVG icon

[cmacdonnacha]

In order to have both a styled tooltip and an accessibility friendly icon I am using the Tooltip component along with setting titleAccess on the Icon component, but now two tooltips appear when hovering over. I know that by removing titleAccess from the icon component it will prevent this but then you have a poor accessibility experience.

import React from "react";
import CheckIcon from "@material-ui/icons/CheckCircle";
import { Tooltip } from "@material-ui/core";

export default class Demo extends React.Component {
  render() {
    return (
      <Tooltip title="My tooltip">
        <CheckIcon titleAccess="My tooltip" />
      </Tooltip>
    );
  }
}

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Two tooltips appear when hovering over

Expected Behavior 🤔

Only one should appear

Steps to Reproduce 🕹

https://codesandbox.io/s/material-ui-tooltip-icon-forked-ox00e?file=/demo.js

Steps:

1. Hover over icon for a few seconds

Context 🔦

Trying to have both a styled tooltip and an accessibility friendly icon so screen readers know what the icon represents.

Your Environment 🌎

See reproduction

[oliviertassinari]

Unless I'm missing something, this is enough, for the given situation, cc @eps1lon

import React from "react";
import CheckIcon from "@material-ui/icons/CheckCircle";
import { Tooltip } from "@material-ui/core";

export default class Demo extends React.Component {
  render() {
    return (
      <div tabIndex={-1}>
        <Tooltip title="My tooltip">
          <CheckIcon aria-hidden={undefined} />
        </Tooltip>
      </div>
    );
  }
}

https://codesandbox.io/s/material-ui-tooltip-icon-forked-yhsyz?file=/demo.js:0-358

[eps1lon]

In order to have both a styled tooltip and an accessibility friendly icon

This should be the default. The Tooltip should give the icon an accessible name. All you need to do is make sure the wrapped element is part of the a11y tree. In our case this would be <CheckIcon aria-hidden={null} /> as @oliviertassinari already pointed out.

Considering our default aria-hidden on icons is a bit problematic (they are visible after all) it might be a good idea not make the icon not aria-hidden if we know the icon will be labelled.

[cmacdonnacha]

Unless I'm missing something, this is enough, for the given situation, cc @eps1lon

Thanks @oliviertassinari @eps1lon, that example actually throws a typescript error. Apologies I probably should have looked for a Typescript codesanbox. Want me to update or can you see the same in your Typescript environment?

[eps1lon]

that example actually throws a typescript error

Yeah, we really should solve that at the icon level.

[oliviertassinari]

@cmacdonnacha I have updated my first comment with undefined instead of null, it works the same way.

[cmacdonnacha]

Excellent thanks @oliviertassinari . That doesn't seem to give the icon an img role, do I need to add that manually? LIke <CheckIcon aria-hidden={undefined} role="img"/>

[oliviertassinari]

Regarding the history of the a11y of this component, we seem to inherit #6829.

[cmacdonnacha]

Hey guys, should I also add a role and alt to this?

<CheckIcon aria-hidden={undefined} role="img" alt="Check is complete"/>

[eps1lon]

You neither need role nor alt. The role should be implied by <svg> and the accessible name is provided by the Tooltip so alt would cause alt to be considered the accessible name and the title of Tooltip being considered the accessible description. I don't think this is what you want.

[cmacdonnacha]

You neither need role nor alt. The role should be implied by <svg> and the accessible name is provided by the Tooltip so alt would cause alt to be considered the accessible name and the title of Tooltip being considered the accessible description. I don't think this is what you want.

Nope I'm good with that, as long as a screen reader knows what the CheckIcon means I'm all good with that and it gets that from the title so that's perfect.

[eps1lon]

I'm keeping it open since that is a legitimate a11y issue with SvgIcon. Ideally SvgIcon shouldn't need extra attention with Tooltip (or any other labeling mechanism).

[cmacdonnacha]

Yea good point 👍

[Gautam-Arora24]

Working on this 🙂

[Gautam-Arora24]

So what I can get from the above conversation is that we should set the aria-hidden prop to true whenever there is a titleAccess prop available ( to avoid two tooltips) and set it to false if no titleAccess prop is available.

[ggascoigne]

Related, but I wish that SvgIcon just used the Tooltip component in the first place rather than the title element, rather than needing to wrap it like this everywhere.

[eps1lon]

Related, but I wish that SvgIcon just used the Tooltip component in the first place rather than the title element, rather than needing to wrap it like this everywhere.

If this is a common pattern, then I would recommend you abstract it into a separate component that composes the Tooltip and SvgIcon.

Using the Tooltip by default in the SVGIcon penalizes every usage of SVGIcon that doesn't use a Tooltip.

[ggascoigne]

And that's what I'm doing, it just loses us the convenience of simply using the icon directly. It's frustrating to want to use the Tooltip control and then having to track down all of the places that use title and fiddle with them when then have such different behavior. It makes for a very inconsistent DX.