react icon indicating copy to clipboard operation
react copied to clipboard

[Tooltip] Markup results in redundant screen reader announcements

Open camchenry opened this issue 1 year ago • 3 comments

I'm working on improving the accessibility of tooltips in our application and I came across an automated accessibility linting issue that I'm not sure how to resolve.

Problem

Our tooltipped button is currently marked up like this (misc. attributes are omitted):

<Tooltip aria-label="View more options">
  <Button>
    <Octicon icon={KebabHorizontalIcon} />
  </Button>
</Tooltip>

When focusing this button with a screen reader such as VoiceOver, it announces for example: "button, View more options, tooltip" (not sure what NVDA announces here since I don't have access to it). That seems OK, because I at least hear the label being announced.

However, if I use the VoiceOver Rotor to access the buttons menu, it simply titles this button as: "button", ignoring the tooltip label. When I run axe against this webpage, it correctly reports that this button has no discernible text. So, I added an aria-label to the button:

<Tooltip aria-label="View more options">
  <Button aria-label="View more options">
    <Octicon icon={KebabHorizontalIcon} />
  </Button>
</Tooltip>

This allows the button to be correctly labeled in the buttons menu, but now when I focus the button, VoiceOver will announce: "View more options, button, View more options, tooltip". This results in redundant announcements, even if I have "Skip redundant labels" turned on in VoiceOver settings.

Furthermore, if I want to try and follow the WAI ARIA tooltip recommendation, it states:

Authors SHOULD ensure that elements with the role tooltip are referenced through the use of aria-describedby before or at the time the tooltip is displayed.

Which means to follow this recommendation, I need to add an aria-describedby attribute to my button:

<Tooltip aria-label="View more options" id="tooltip">
  <Button aria-label="View more options" aria-describedby="tooltip">
    <Octicon icon={KebabHorizontalIcon} />
  </Button>
</Tooltip>

Now when I focus the button, I hear announced in VoiceOver: "View more options description, View more options, button, View more options, tooltip" which means the label is now announced three times redundantly.

Thoughts

I recognize that tooltips are not generally recommended, but in this case we have a number of icon buttons that we want to ensure are accessible both to screen reader users but also to sighted keyboard users. Using just an aria-label on the button would make it accessible to screen readers, but using a tooltip also makes it more usable for sighted keyboard and mouse users too.

I think in this case the nesting ends up confusing the announcements, because the button "owns" the tooltip, but the button is nested inside of the tooltip. Based on my understanding of how tooltips should be implemented (which might be wrong, please feel free to correct/close this issue if this is expected!), I would think that markup like this:

<Tooltip aria-label="View more options">
  <Button aria-label="View more options">
    <Octicon icon={KebabHorizontalIcon} />
  </Button>
</Tooltip>

should be rendering something like this:

<button aria-label="View more options" aria-describedby="tooltip">...</button>
<span id="tooltip" role="tooltip" ...>View more options</div>

Rather than how it currently renders as:

<span role="tooltip" aria-label="View more options">
  <button aria-label="View more options">...</button>
</span>

I'm not sure how to ensure that mark up the button so that it has an accessible name and also combine that with the tooltip in a way that doesn't result in many redundant labels.

camchenry avatar Jan 11 '24 16:01 camchenry

coming at this issue "cold" without prior context, I'd agree that the more logical structure would be

should be rendering something like this:

<button aria-label="View more options" aria-describedby="tooltip">...</button>
<span id="tooltip" role="tooltip" ...>View more options</div>

as it makes no sense for the button to be wrapped inside its own tooltip

patrickhlauke avatar Jan 12 '24 15:01 patrickhlauke

Hello @camchenry 👋 Thank you so much for writing this up! We appreciate you taking the time and gave thorough thoughts.

I believe the new Tooltip should fix this issue as it is signed off from accessibility. And it has the DOM structure that @patrickhlauke mentioned.

You can subscribe to the https://github.com/github/primer/issues/1244 to get updates on when we start recommending adoption of the new Tooltip. Let us know if you have further questions or concerns! Thanks again!

broccolinisoup avatar Jan 16 '24 00:01 broccolinisoup

@broccolinisoup Thanks for the update! Glad to hear this will be fixed in the next version of the Tooltip. I'll stay subscribed to that issue so we can update our app when it's available.

camchenry avatar Jan 16 '24 15:01 camchenry

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

github-actions[bot] avatar Jul 27 '24 23:07 github-actions[bot]

I'm going to close this out, because I've tested the new Tooltip component and this appears to no longer be an issue 🎉

camchenry avatar Jul 29 '24 15:07 camchenry