refactor(aria): use aria metadata to find equivalent HTML element #4495
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
github-actions
bot
added
A-Linter
Conaclos
commented
Nov 10, 2024
| @@ -1,22 +0,0 @@ | |||
| use biome_aria_metadata::{IsoCountries, IsoLanguages, ISO_COUNTRIES, ISO_LANGUAGES}; | |||
| use std::str::FromStr; | |||
|
|
|||
There was a problem hiding this comment.
I moved this code in the biome_aria_metadata crate.
Conaclos
changed the title
Open
1 task
Conaclos
force-pushed
the
conaclos/aria-elements
branch
from
9756b1b
to
5445e56
Compare
Conaclos
commented
Nov 10, 2024
| <div role="blockquote" ></div> | ||
| <div role="associationlist" ></div> |
There was a problem hiding this comment.
The role was part of the Aria 1.2 draft and was eventually removed before the stabilization of Aria 1.2.
Conaclos
commented
Nov 10, 2024
Comment on lines
-38
to
-39
| <div role="p" ></div> | ||
| <div role="aside" ></div> |
There was a problem hiding this comment.
These roles don't exist.
Conaclos
commented
Nov 10, 2024
Comment on lines
-16
to
-17
| <div role="graphics-document" ></div> | ||
| <div role="graphics-object" ></div> |
There was a problem hiding this comment.
Related concepts for these two roles are missing. I plan to add them back in a future PR.
CodSpeed Performance ReportMerging #4495 will not alter performanceComparing Summary
|
Conaclos
commented
Nov 10, 2024
Comment on lines
-10
to
-13
| <div role="dialog" ></div> | ||
|
|
||
| <div role="cell" ></div> | ||
| <div role="columnheader" ></div> | ||
| <div role="definition" ></div> |
There was a problem hiding this comment.
Related concepts for these two roles are missing. I plan to add them back in a future PR.
Conaclos
force-pushed
the
conaclos/aria-elements
branch
from
5445e56
to
fa97efa
Compare
Conaclos
force-pushed
the
conaclos/aria-elements
branch
from
fa97efa
to
a823d3c
Compare
ematipico
approved these changes
Nov 11, 2024
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.
Summary
Part of #4241.
This PR removes the remaining role definitions from
biome_aria.I noticed some errors: we had invalid roles
pandaside.Also we had a never stabilized role
associationlist. It was part of ARIA 1.2 draft and was finally removed before is stabilization. Some MDN docs still have some documentation for this role.I refactored
is_not_interactive_elementto useget_implicit_roleinstead of traversing all roles looking for a role with the current element as concept.This allowed me to identify some missing elements in
get_implicit_role.This also makes the code simpler to read and to understand.
I added an exception for
aandareabecause our previous code considered them as interactive, and our current code that relies onget_implicit_roleflagged them as non-interactive ifhrefis not set.Taking a look at the implementation of JSX A11y for interactive/non-interactive roles,
I noticed that some roles are neither interactive nor non-interactive.
This is the case of the
toolbarrole.Thus, I created the
AriaRole::is_non_interactivemethod.In contrast to the implementation of JSX A11y, I didn't add
genericbecause it created many regressions in our test suite.Some of our rules have not the same test suite as JSX A11y and some of our rules have slight or subtle behavioral differences.
This makes it hard to detect if a change is expected or not.
Thus, I tried to avoid regression in our current test suite.
I think we will have to review every rule and also the implementation of JSX A11y because I noticed that there are numerous hacks and limitations in their code base.
As a side note, JSX A11y uses also the axobject-query NPM package that provides data from Chrome AXObject. This complements the data from ARIA query and is more browser-centered. We will have to investigate if we should/want to use this extra source of data to determine if an element is interactive or non-interactive.
I added
std::fmt::Displayimpl forHtmlElementInstance. This allowed me to drastically reduce the complexity of the implementation of theuseSematicElementsrule.Test Plan