Tooltip: do not add the aria-describedby attribute to the anchor
#66021
+56
−134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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? |
2 tasks
|
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. |
ciampo
force-pushed
the
fix/remove-tooltip-aria-describedby
branch
from
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 matchesCould 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 |
This was referenced Oct 21, 2024
|
I had an idea while working on #67094. What if we do something similar to what we eventually did with the size deprecations, and use a
This will provide a way to buy us time while we check the existing usages, give us a way to mark the instances which are "safe", and spread awareness throughout the repo. I'm also wondering whether we need to provide this "migration grace period" to extenders as well, like we do with the other |
|
@mirka I think this is a neat idea! Just to be clear, how many instances are we dealing with? |
|
@mirka your approach makes sense. My questions is: how would handle other components internally using a |
Right. Whether it's worth a full audit is a separate question. Maybe we can find some heuristics and focus on commonly occurring patterns. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
With the changes from this PR, the
Tooltipcomponent does not add thearia-describedbyattribute to its anchor.Why?
As discussed in #65888 (and many other places), the
Tooltipcomponent currently can add thearia-describedbyattribute 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?
ariakitcomponent already doesn't add thearia-describedbyattribute.TODO
Testing Instructions
For any tooltip usage, make sure that when the tooltip is shown, there are no changes to the
aria-describedbyattribute on the anchor.