refactor(aria): generate role metadata #4488
Merged
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
force-pushed
the
conaclos/biome-aria-roles
branch
5 times, most recently
from
e9205de
to
b713542
Compare
Conaclos
commented
Nov 7, 2024
| <link onClick={() => {}} href="#" /> | ||
|
|
There was a problem hiding this comment.
Conaclos
force-pushed
the
conaclos/biome-aria-roles
branch
from
b713542
to
d75d834
Compare
Conaclos
commented
Nov 7, 2024
Comment on lines
-19
to
-31
| <input type="radio" aria-invalid /> | ||
| <input type="radio" aria-selected /> | ||
| <input type="radio" aria-haspopup /> | ||
| <input type="checkbox" aria-haspopup /> | ||
| <input type="reset" aria-invalid /> | ||
| <input type="submit" aria-invalid /> | ||
| <input type="image" aria-invalid /> | ||
| <input type="button" aria-invalid /> | ||
| <menu type="toolbar" aria-haspopup /> | ||
| <menu type="toolbar" aria-invalid /> | ||
| <menu type="toolbar" aria-expanded /> | ||
| <area href="#" aria-invalid /> | ||
| <a href="#" aria-invalid /> |
There was a problem hiding this comment.
I moved its cases in deprecated because these attributes are not forbidden, they are just deprecated.
Conaclos
force-pushed
the
conaclos/biome-aria-roles
branch
from
d75d834
to
e3d9726
Compare
Conaclos
commented
Nov 7, 2024
| <img alt="foobar" aria-busy /> | ||
| <menu type="toolbar" aria-activedescendant /> | ||
| <button role="tab" aria-selected="true"/> |
There was a problem hiding this comment.
Non regression test for #4486
Conaclos
commented
Nov 7, 2024
| @@ -23,7 +23,6 @@ | |||
| <a href="#" aria-live /> | |||
| <a href="#" aria-owns /> | |||
| <a href="#" aria-relevant /> | |||
| <a aria-checked /> | |||
There was a problem hiding this comment.
This is one of the case where our implementation diverges from the ESLint one.
The ESLint rule allows any aria attribute when it is not able to determine a role for an element.
We diverge in two ways:
- Our code assigns the
genericrole toaelements withouthref.
Conversely,aria-querydoesn't assign any implicit role.
I don't know which approach is the best.
This is something we should investigate - Our rule doesn't give up if a role cannot be determined. Instead, we report any non-global aria attribute as invalid.
Conaclos
commented
Nov 7, 2024
Comment on lines
-59
to
-61
| <img alt="" aria-checked /> | ||
| <img alt="foobar" aria-busy /> | ||
| <menu type="toolbar" aria-activedescendant /> |
There was a problem hiding this comment.
Same as the previous case.
Conaclos
changed the title
Conaclos
commented
Nov 7, 2024
Comment on lines
-98
to
-129
| match role_name { | ||
| "biome_aria::roles::LinkRole" => { | ||
| matches!( | ||
| aria_attribute, | ||
| "aria-expanded" | "aria-haspopup" | "aria-current" | ||
| ) | ||
| } | ||
| "biome_aria::roles::ButtonRole" => { | ||
| matches!(aria_attribute, "aria-expanded" | "aria-pressed") | ||
| } | ||
| "biome_aria::roles::CheckboxRole" | ||
| | "biome_aria::roles::RadioRole" | ||
| | "biome_aria::roles::MenuItemCheckboxRole" | ||
| | "biome_aria::roles::MenuItemRadioRole" => { | ||
| matches!(aria_attribute, "aria-checked") | ||
| } | ||
| "biome_aria::roles::ComboBoxRole" => { | ||
| matches!(aria_attribute, "aria-expanded") | ||
| } | ||
| "biome_aria::roles::SliderRole" => { | ||
| matches!( | ||
| aria_attribute, | ||
| "aria-valuemax" | "aria-valuemin" | "aria-valuenow" | ||
| ) | ||
| } | ||
| "biome_aria::roles::ListRole" => { | ||
| matches!(aria_attribute, "aria-activedescendant") | ||
| } | ||
| "biome_aria::roles::HeadingRole" => matches!(aria_attribute, "aria-level"), | ||
| // This rule is not concerned with the abstract role | ||
| "biome_aria::roles::PresentationRole" | "biome_aria::roles::GenericRole" => true, | ||
| _ => false, |
There was a problem hiding this comment.
This code seems to patch our handwritten data. We no longer need this :)
Conaclos
force-pushed
the
conaclos/biome-aria-roles
branch
from
e3d9726
to
75a3294
Compare
CodSpeed Performance ReportMerging #4488 will not alter performanceComparing Summary
|
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.
Since the refactor is already broad and painful enough, I only partially moved the responsibility from
biom_ariatobiome_aria_metadatafor aria roles.This PR extends the build script of
biome_aria_metadatato generate role-related data.Aria roles are divided between abstract roles and non-abstract (concrete) roles.
Abstract roles cannot be used by a user.
Thus, I created two enumerations to represent roles:
AriaAbstraactRoleandAriaRole.AriaRolecorresponds to all concrete roles.I introduced the types
AriaAbstractRoles,AriaRoles, andAriaAttributesthat encapsulate a list ofAriaAbstraactRole,AriaRole, orAriaAttribute. This allows us to evolve the internal representation without breaking code that use them.I currently use a static slice as internal representation. We could switch to bitflags or perfect hash map in the future.
The handwritten roles in
biome_ariaand the related data are a mix of Aria 1.1, Aria 1.2, and the current draft of Aria 1.3.To ensure we have the same roles (The
markrole was added in the Aria 1.3 draft), I had to switch from Aria 1.2 to the Aria 1.3 draft.I noticed a trend of removing required attributes in newer versions of ARIA.
Aria 1.1 has more required attributes than Aria 1.2, that is turn has more required attributes than the Aria 1.3 draft.
This explains some changes in the snapshots where previously required attributes are no longer required.
I noticed that the handwritten data omitted deprecated attributes. My current implementation include deprecated attributes because they are still valid. This explains why some code that was flagged as invalid are now valid (e.g.
aria-haspopupandaria-invalidare deprecated since Aria 1.2).Some ARIA rules have slightly different behavior to their ESLInt equivalent.
This led to painful refactoring because I got some snapshot change that looked like regression, but actually was the demonstration of a distinct implementation to the equivalent ESLint rule.
During this refactor, I noticed that I missed a source of data for aria: the WAI-ARIA Graphics Module. This module adds additional roles such as
graphics-document. I will add this in a future PR.As a nice side effect, this PR fixes #4486
Test Plan
I updated the snapshots