feat(html_analyze): implement noInteractiveElementToNoninteractiveRole#10028
feat(html_analyze): implement noInteractiveElementToNoninteractiveRole#10028Netail wants to merge 1 commit intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 45dda8b The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR introduces a new HTML accessibility lint rule Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/vue/valid.vue (1)
2-8: Please add PascalCase native-lookalike cases to lock case-sensitive matching.Nice fixture overall; adding a couple of explicit component-like tags (e.g.
Input,A) would harden regressions around template-language case sensitivity.🧪 Suggested fixture additions
<TestComponent onClick="doFoo" /> <Button onClick="doFoo" /> +<Input role="img" /> +<A href="http://x.y.z" role="img" /> <a href="http://x.y.z" role="button" /> <a href="http://x.y.z" tabIndex="0" role="button" /> <button class="foo" role="button" />Based on learnings: “When checking HTML element names in biome_html_analyze lint rules, the comparison must be case-insensitive for
.htmlfiles, but case-sensitive for.astro,.vue, and.sveltefiles.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/vue/valid.vue` around lines 2 - 8, Add PascalCase native-lookalike tag examples to the fixture to lock case-sensitive matching for .vue files: add component-like tags such as Input and A (e.g., <Input onClick="doFoo" /> and <A href="http://x.y.z" role="button" />) and include the same attribute variants already present (onClick, href, tabIndex, role, class) so the rule sees PascalCase native names (Input, A) alongside TestComponent and Button to ensure template-language case sensitivity is covered.crates/biome_html_analyze/src/lint/a11y/no_interactive_element_to_noninteractive_role.rs (1)
94-102: Consider combining the svg and canvas exemptions.These two checks have identical structure. You could consolidate them for brevity.
♻️ Optional refactor
- // A <svg> element can be given an "img" to make it non-interactive for a11y reasons. - if is_html_tag(node, source_type, "svg") && role_attribute_value == "img" { - return None; - } - - // A <canvas> element can be given an "img" to make it non-interactive for a11y reasons. - if is_html_tag(node, source_type, "canvas") && role_attribute_value == "img" { + // <svg> and <canvas> elements can be given "img" role to make them non-interactive for a11y reasons. + if (is_html_tag(node, source_type, "svg") || is_html_tag(node, source_type, "canvas")) + && role_attribute_value == "img" + { return None; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/src/lint/a11y/no_interactive_element_to_noninteractive_role.rs` around lines 94 - 102, These two identical checks for is_html_tag(node, source_type, "svg") and is_html_tag(node, source_type, "canvas") with role_attribute_value == "img" should be consolidated into a single conditional: check if role_attribute_value == "img" and the tag name is either "svg" or "canvas" (e.g., by matching the tag string or using a small set/array), then return None; replace the two separate if blocks with this single combined check in the function containing these conditions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@crates/biome_html_analyze/src/lint/a11y/no_interactive_element_to_noninteractive_role.rs`:
- Around line 94-102: These two identical checks for is_html_tag(node,
source_type, "svg") and is_html_tag(node, source_type, "canvas") with
role_attribute_value == "img" should be consolidated into a single conditional:
check if role_attribute_value == "img" and the tag name is either "svg" or
"canvas" (e.g., by matching the tag string or using a small set/array), then
return None; replace the two separate if blocks with this single combined check
in the function containing these conditions.
In
`@crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/vue/valid.vue`:
- Around line 2-8: Add PascalCase native-lookalike tag examples to the fixture
to lock case-sensitive matching for .vue files: add component-like tags such as
Input and A (e.g., <Input onClick="doFoo" /> and <A href="http://x.y.z"
role="button" />) and include the same attribute variants already present
(onClick, href, tabIndex, role, class) so the rule sees PascalCase native names
(Input, A) alongside TestComponent and Button to ensure template-language case
sensitivity is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4bb17e9a-c59a-4240-87d3-17343e38e3d7
⛔ Files ignored due to path filters (8)
crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/astro/invalid.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/astro/valid.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/svelte/invalid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/svelte/valid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/valid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/vue/invalid.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/vue/valid.vue.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (11)
.changeset/neat-signs-crash.mdcrates/biome_html_analyze/src/lint/a11y/no_interactive_element_to_noninteractive_role.rscrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/astro/invalid.astrocrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/astro/valid.astrocrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/invalid.htmlcrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/svelte/invalid.sveltecrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/svelte/valid.sveltecrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/valid.htmlcrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/vue/invalid.vuecrates/biome_html_analyze/tests/specs/a11y/noInteractiveElementToNoninteractiveRole/vue/valid.vuecrates/biome_js_analyze/src/lint/a11y/no_interactive_element_to_noninteractive_role.rs
Merging this PR will not alter performance
Comparing Footnotes
|
Summary
Port
noInteractiveElementToNoninteractiveRoleto HTMLRelated #8155
Test Plan
Added unit tests
Docs