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

Toolbar Example: Add missing descriptive tooltips #986 fix #998

Closed
wants to merge 3 commits into from

Conversation

ardnahcivar
Copy link

added the descriptive title for the toolbar buttons.

@mcking65
Copy link
Contributor

I'll get a reviewer assigned today. Question about changes starting at line 213, are they white space changes only? The replacement line sounds the same as the original to me.

@ardnahcivar
Copy link
Author

I'll get a reviewer assigned today. Question about changes starting at line 213, are they white space changes only? The replacement line sounds the same as the original to me.

yes at line 213 onwards, it is white space changes only

@jongund
Copy link
Contributor

jongund commented May 28, 2019

@ardnahcivar @mcking65
I don't think we want to do it this way, it you want the tooltips with the title attribute we need to remove aria-label, otherwise we are setting up a situation where the accessible name will be the same as the accessible description. This also needs to be documented in the example descriptions for use of properties and states.

@ardnahcivar
Copy link
Author

@ardnahcivar @mcking65
I don't think we want to do it this way, it you want the tooltips with the title attribute we need to remove aria-label, otherwise we are setting up a situation where the accessible name will be the same as the accessible description. This also needs to be documented in the example descriptions for use of properties and states.

@jongund , any pointers, sugesstions how to fix it

@carmacleod
Copy link
Contributor

Hi, @ardnahcivar! Thank-you so much for your pull request!

The group discussed this yesterday, and we decided that we prefer not to use the title attribute because sighted keyboard users cannot see browser tooltips, so title is not the best choice for accessible tooltips.

The W3C HTML spec for the title attribute mentions this:

Warning! Relying on the title attribute is currently discouraged as many user agents do not expose the attribute in an accessible manner as required by this specification (e.g., requiring a pointing device, such as a mouse, to cause a tooltip to appear excludes keyboard-only users and touch-only users, such as anyone with a modern phone or tablet).

We believe the direction should be similar to @ZoeBijl's example tooltip with aria-labelledby (in CodePen).

Do you want to have a go at writing this for the toolbar example?

@jongund
Copy link
Contributor

jongund commented Jul 3, 2019

@mcking65 Is this pull request going to be merged soon?

@mcking65
Copy link
Contributor

mcking65 commented Jul 5, 2019

@ardnahcivar, sorry we chose another path, which @jongund implemented in #1069. Closing this PR.

If you are available for other bugs, please comment in the relevant issue so we can talk about the approach to resolution. We don't want people to get discouraged from contributing because of situations like this where they offer some work and then we re-do it or go down another path.

@mcking65 mcking65 closed this Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants