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 component improvements #2369

Closed
afercia opened this issue Aug 11, 2017 · 4 comments
Closed

Tooltip component improvements #2369

afercia opened this issue Aug 11, 2017 · 4 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@afercia
Copy link
Contributor

afercia commented Aug 11, 2017

I'd like to propose a couple changes for the recently introduced Tooltip component, see #2293 and #2366.

It currently uses an ARIA role="tooltip" but actually it's not an ARIA tooltip. Real ARIA tooltips need to be associated with an element that uses aria-describedby pointing to the ID of the tooltip:
https://www.w3.org/TR/wai-aria/roles#tooltip
https://www.w3.org/TR/wai-aria-1.1/#tooltip

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.

Without that, the role="tooltip" alone doesn't do anything.

However, I think there's no need to do this. The UI controls the tooltip is used for already have an aria-label attribute and in most of the cases the tooltip uses the same text used for the aria-label. If the tooltip was associated using aria-describedby, then the same text would be read out twice: the first time as accessible name (aria-label) and the second time as accessible description (aria-describedby). For example:

<button type="button" aria-label="Undo" aria-describedby="tooltip-1"></button>
<div id="tooltip-1" class="components-popover components-tooltip ..." tabindex="0" role="tooltip">
	<div class="components-popover__content">Undo</div>
</div>

Would be announced as:
Button Undo {brief pause} Undo

Since we don't want this to happen, the role="tooltip" is basically pointless and I'd recommend to remove it.

I'd also recommend to completely hide the tooltip to assistive technologies, since it is meant to just repeat the UI control aria-label text. Also, it shouldn't be focusable. To recap, I'd consider to:

  • remove role="tooltip"
  • remove tabindex="0"
  • add aria-hidden="true"
  • document in a clear way that the tooltip is meant to be just a "visual" thing, while the content for assistive technologies is provided by a required aria-label attribute on the UI control

Aside: from a visual perspective, sometimes the tooltip text is just too long, for example:

screen shot 2017-08-11 at 15 19 02

I guess this is already a known issue. In these cases, while the text used for the aria-label should be informative and can also be a bit long when necessary, the text used for the visible tooltip should probably be a bit shorter.

Lastly: I'm not sure about the default positioning. In all the big app I've seen (Gmail, Twitter, GitHub), tooltips are displayed below the control.

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Aug 11, 2017
@aduth
Copy link
Member

aduth commented Aug 11, 2017

I added a few more commits in #2366 to address requested changes.

document in a clear way that the tooltip is meant to be just a "visual" thing, while the content for assistive technologies is provided by a required aria-label attribute on the UI control

Where should this occur you think?

Lastly: I'm not sure about the default positioning. In all the big app I've seen (Gmail, Twitter, GitHub), tooltips are displayed below the control.

Hmm, is this because their tooltips (or ones I can find) are toward top of the page and would otherwise risk exceeding top page bounds?

@afercia
Copy link
Contributor Author

afercia commented Aug 11, 2017

Where should this occur you think?

Not sure, in the readme maybe? I do think tooltip should be used for very short text: 2-3 words max. I don't see them as something to be used longer messages.

About the default position: definitely something for designers. It seems a bit weird to me they're at the top. Gmail puts them at the top just when the control are... at the bottom of the page:

screen shot 2017-08-11 at 19 00 13

@afercia
Copy link
Contributor Author

afercia commented Aug 12, 2017

Thanks, I see role, aria-hidden, and tabindex have been addressed! What is left here is:

  • consider alternative, shorter text when the text is too long
  • tooltip default position

@afercia
Copy link
Contributor Author

afercia commented Aug 28, 2017

For reference: about the positioning, see also discussion on the related PR #2366 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

No branches or pull requests

2 participants