feat(html_analyze): implement noNoninteractiveElementToInteractiveRole#10022
Conversation
🦋 Changeset detectedLatest commit: 419e9a6 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 |
WalkthroughAdds 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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/svelte/valid.svelte (1)
4-5: Add a PascalCase component +rolevalid case to lock in case-sensitivity.You already cover custom components here, but not through the
rolecode path. Adding one or two PascalCase examples with interactive roles would better guard against false positives in Svelte templates.Suggested fixture addition
<TestComponent onClick={doFoo} /> <Button onClick={doFoo} /> +<TestComponent role="button" /> +<Button role="menuitem" />Based on learnings: in
.svelte/.vue/.astro, element-name checks should be case-sensitive so PascalCase components are not treated as native HTML elements.🤖 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/noNoninteractiveElementToInteractiveRole/svelte/valid.svelte` around lines 4 - 5, Add PascalCase custom-component examples that use an interactive ARIA role so the Svelte parser's role-based path is exercised and component names aren't mistaken for native elements; specifically, add one or two entries like a <TestComponent ... role="button" ...> and/or <Button ... role="switch" ...> variant (using Svelte on:click or onClick as in existing fixtures) to the valid.svelte spec so the rule treats PascalCase names case-sensitively and does not flag them as non-interactive native elements in the role code path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/major-bats-fail.md:
- Line 5: Update the changeset sentence that describes the new HTML lint rule by
fixing subject-verb agreement: in the line mentioning the rule name
noNoninteractiveElementToInteractiveRole, change “which enforce” to “which
enforces” so the user-facing copy reads correctly.
In
`@crates/biome_html_analyze/src/lint/a11y/no_noninteractive_element_to_interactive_role.rs`:
- Around line 14-18: The rule documentation in
no_noninteractive_element_to_interactive_role.rs incorrectly lists <area> as a
non-interactive element while the new allow-list permits <area role="button"/>
and <area role="menuitem"/>; update the doc comment (the top-level comment in
the no_noninteractive_element_to_interactive_role module) to remove <area> from
the non-interactive examples, or alternatively tighten the rule/tests (the
no_noninteractive_element_to_interactive_role lint logic and its fixtures) so
that <area> is not allowed—preferably remove <area> from the sentence so the
documentation matches the current allow-list and adjust any example
fixtures/comments accordingly.
- Around line 71-97: The rule currently compares element_name.text()
case-sensitively which misses HTML case-insensitivity; when
source_type.is_html() normalize the element name for comparisons (e.g. let
element_name_text = if source_type.is_html() {
element_name.text().to_ascii_lowercase() } else {
element_name.text().to_string() }) and then use element_name_text for the li
exemption and the ROLE_SENSITIVE_ELEMENTS check (replace element_name.text()
uses in the li branch and ROLE_SENSITIVE_ELEMENTS.contains(...) call), leaving
behavior unchanged for Astro/Vue/Svelte; update checks around
source_type.is_html(), element_name.text(), ROLE_SENSITIVE_ELEMENTS,
ctx.aria_roles().is_not_interactive_element and AriaRole::from_roles to use the
normalized value in HTML files and add uppercase-HTML fixtures to cover the
case.
---
Nitpick comments:
In
`@crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/svelte/valid.svelte`:
- Around line 4-5: Add PascalCase custom-component examples that use an
interactive ARIA role so the Svelte parser's role-based path is exercised and
component names aren't mistaken for native elements; specifically, add one or
two entries like a <TestComponent ... role="button" ...> and/or <Button ...
role="switch" ...> variant (using Svelte on:click or onClick as in existing
fixtures) to the valid.svelte spec so the rule treats PascalCase names
case-sensitively and does not flag them as non-interactive native elements in
the role code path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e276b173-6f37-46bc-8de8-6541a4d1de2d
⛔ Files ignored due to path filters (8)
crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/astro/invalid.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/astro/valid.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/svelte/invalid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/svelte/valid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/valid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/vue/invalid.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/vue/valid.vue.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (10)
.changeset/major-bats-fail.mdcrates/biome_html_analyze/src/lint/a11y/no_noninteractive_element_to_interactive_role.rscrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/astro/invalid.astrocrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/astro/valid.astrocrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.htmlcrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/svelte/invalid.sveltecrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/svelte/valid.sveltecrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/valid.htmlcrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/vue/invalid.vuecrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/vue/valid.vue
| /// Non-interactive HTML elements indicate _content_ and _containers_ in the user interface. | ||
| /// Non-interactive elements include `<main>`, `<area>`, `<h1>` (,`<h2>`, etc), `<img>`, `<li>`, `<ul>` and `<ol>`. | ||
| /// | ||
| /// Interactive HTML elements indicate _controls_ in the user interface. | ||
| /// Interactive elements include `<a href>`, `<button>`, `<input>`, `<select>`, `<textarea>`. |
There was a problem hiding this comment.
Keep the rule docs aligned with the new allow-list.
This text calls out <area> as non-interactive, but the new valid fixtures explicitly allow <area role="button" /> and <area role="menuitem" />. Please either drop <area> from this sentence or tighten the implementation/tests so the docs stay truthful.
🤖 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_noninteractive_element_to_interactive_role.rs`
around lines 14 - 18, The rule documentation in
no_noninteractive_element_to_interactive_role.rs incorrectly lists <area> as a
non-interactive element while the new allow-list permits <area role="button"/>
and <area role="menuitem"/>; update the doc comment (the top-level comment in
the no_noninteractive_element_to_interactive_role module) to remove <area> from
the non-interactive examples, or alternatively tighten the rule/tests (the
no_noninteractive_element_to_interactive_role lint logic and its fixtures) so
that <area> is not allowed—preferably remove <area> from the sentence so the
documentation matches the current allow-list and adjust any example
fixtures/comments accordingly.
There was a problem hiding this comment.
Because it has a role in the tests
Merging this PR will not alter performance
Comparing Footnotes
|
ematipico
left a comment
There was a problem hiding this comment.
Just need to address the bot suggestions. Good one
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_html_analyze/src/lint/a11y/no_noninteractive_element_to_interactive_role.rs (1)
14-16:⚠️ Potential issue | 🟡 MinorAlign the docs with the implemented allow-list.
Line 15 still lists
<area>as non-interactive, butcrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/valid.html(Lines 6-7) treats<area role="button" />and<area role="menuitem" />as valid. Please update the sentence so docs match behaviour.Suggested doc tweak
-/// Non-interactive elements include `<main>`, `<area>`, `<h1>` (,`<h2>`, etc), `<img>`, `<li>`, `<ul>` and `<ol>`. +/// Non-interactive elements include `<main>`, `<h1>`, `<h2>`, `<img>`, `<li>`, `<ul>` and `<ol>`.🤖 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_noninteractive_element_to_interactive_role.rs` around lines 14 - 16, Update the module doc comment in no_noninteractive_element_to_interactive_role.rs so it matches the implemented allow-list: remove `<area>` from the list of non-interactive elements (or explicitly note that `<area>` is allowed with certain interactive roles), ensuring the sentence that lists examples (`<main>`, `<area>`, `<h1>`, `<img>`, `<li>`, `<ul>`, `<ol>`) is changed to match the rule behavior; edit the docstring above the lint implementation (no_noninteractive_element_to_interactive_role) accordingly.
🧹 Nitpick comments (1)
crates/biome_html_analyze/src/lint/a11y/no_noninteractive_element_to_interactive_role.rs (1)
49-56: Add rule source metadata for this ESLint port.Given this rule is a port, adding a
sourcesentry will improve provenance andbiome migrate eslintmapping.As per coding guidelines: Use
RuleSource::Eslint("rule-name").same()when porting an ESLint rule with matching behavior.🤖 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_noninteractive_element_to_interactive_role.rs` around lines 49 - 56, The rule declaration for NoNoninteractiveElementToInteractiveRole is missing ESLint provenance metadata; add a sources field to the struct initialization using RuleSource::Eslint("no-noninteractive-element-to-interactive-role").same() so the port maps back to the original ESLint rule. Update the NoNoninteractiveElementToInteractiveRole initializer to include sources: vec![RuleSource::Eslint("no-noninteractive-element-to-interactive-role").same()], making sure RuleSource is in scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@crates/biome_html_analyze/src/lint/a11y/no_noninteractive_element_to_interactive_role.rs`:
- Around line 14-16: Update the module doc comment in
no_noninteractive_element_to_interactive_role.rs so it matches the implemented
allow-list: remove `<area>` from the list of non-interactive elements (or
explicitly note that `<area>` is allowed with certain interactive roles),
ensuring the sentence that lists examples (`<main>`, `<area>`, `<h1>`, `<img>`,
`<li>`, `<ul>`, `<ol>`) is changed to match the rule behavior; edit the
docstring above the lint implementation
(no_noninteractive_element_to_interactive_role) accordingly.
---
Nitpick comments:
In
`@crates/biome_html_analyze/src/lint/a11y/no_noninteractive_element_to_interactive_role.rs`:
- Around line 49-56: The rule declaration for
NoNoninteractiveElementToInteractiveRole is missing ESLint provenance metadata;
add a sources field to the struct initialization using
RuleSource::Eslint("no-noninteractive-element-to-interactive-role").same() so
the port maps back to the original ESLint rule. Update the
NoNoninteractiveElementToInteractiveRole initializer to include sources:
vec![RuleSource::Eslint("no-noninteractive-element-to-interactive-role").same()],
making sure RuleSource is in scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23d09f50-e536-4a07-888f-6343ec03cf12
⛔ Files ignored due to path filters (2)
crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/valid.html.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
.changeset/major-bats-fail.mdcrates/biome_html_analyze/src/lint/a11y/no_noninteractive_element_to_interactive_role.rscrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/invalid.htmlcrates/biome_html_analyze/tests/specs/a11y/noNoninteractiveElementToInteractiveRole/valid.htmlcrates/biome_html_syntax/src/element_ext.rs
✅ Files skipped from review due to trivial changes (1)
- .changeset/major-bats-fail.md
Summary
Port noNoninteractiveElementToInteractiveRole over to HTML
Related #8155
Test Plan
Unit tests
Docs