-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: do not add the aria-describedby
attribute to the anchor
#66021
base: trunk
Are you sure you want to change the base?
Conversation
I've started this effort early in the 6.8 release cycle, so that we have as much time as possible to iterate over the whole repo and merge the change. I'd like for this to be a collaborative effort. Here is what we could do:
@WordPress/gutenberg-components @afercia @t-hamano @joedolson what do y'all think? |
Thank you for starting to address this issue! I agree with the approach you proposed.
Do we need to search our entire code for some kind of regex pattern? If so, the pattern I can come up with is something like this:
|
Just so we don't forget — noting that this PR also needs to add good accessibility guidance in the relevant component docs. |
Thank you for proposing this solution, I agree it makes sense, and I always appreciate us leaning on the clean / simple side of things. Could be a good opportunity to involve a broader group of contributors, just like we did for #65018. |
1da779b
to
4f7530a
Compare
I started adding some initial temporary ESLint rules for the most obvious usages — here is a list of the matches: Here is a list of the matches
Could y'all help to review and expand the ESLint rules? The tricky part is to catch and derivative usage of
@tyxla The collaboration on #65018 was great, although on many occasions reviewing the PR took me a long time (equivalent to authoring the PR myself) given the lack of testing instructions and / or some changes being applied superficially (ie. without considering style overrides, etc). Do you have ideas on how to improve on this aspect? |
Size Change: -65 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
@mirka to the rescue, she's been writing a bunch of custom ESLint rules recently. Happy to help with reviewing myself.
Yeah, that's a good question to ask ourselves. I think we can be better at setting the right expectations for everyone, detailing step-by-step the process that we expect every change to include, including where to start, what to check, and what the end result looks like. In the |
What?
With the changes from this PR, the
Tooltip
component does not add thearia-describedby
attribute to its anchor.Why?
As discussed in #65888 (and many other places), the
Tooltip
component currently can add thearia-describedby
attribute to its anchor, thus providing an accessible description for the anchor element.This approach is problematic for a few reasons:
It was therefore decided that tooltips should be assumed to be decorative only, and that anchors using a tooltip should make sure they have an accessible label and description regardless of potential tooltips.
How?
ariakit
component already doesn't add thearia-describedby
attribute.TODO
Testing Instructions
For any tooltip usage, make sure that when the tooltip is shown, there are no changes to the
aria-describedby
attribute on the anchor.