feat: add template-no-noninteractive-tabindex#24
Open
Conversation
🏎️ Benchmark Comparison
Full mitata output |
johanrd
added a commit
that referenced
this pull request
Apr 21, 2026
Translates jsx-a11y's no-noninteractive-tabindex test list to HBS to pin our rule's behavior against PR #24's interactive-element and tabindex="-1" exemption fixes. Parity holds on all upstream always-valid and never-valid cases. Annotated divergences: - tabpanel: we flag (jsx-a11y strict parity); jsx-a11y recommended whitelists it via `roles: ['tabpanel']`. - Dynamic role expressions: we skip conservatively; matches jsx-a11y recommended (allowExpressionValues: true), diverges from strict. - Component name collides with a native tag (e.g. <Article>): we lowercase tag before the aria-query dom lookup so <Article> validates as <article>. False positive relative to jsx-a11y's no-settings default.
There was a problem hiding this comment.
Pull request overview
This PR adds a new accessibility rule (ember/template-no-noninteractive-tabindex) to flag tabindex="0" on non-interactive elements in Ember templates, aligning with JSX a11y’s no-noninteractive-tabindex while preserving Ember-specific component/element discrimination.
Changes:
- Added
template-no-noninteractive-tabindexrule with an allowlist option for non-interactive roles (defaulting to["tabpanel"]) and exemptions liketabindex="-1". - Introduced supporting utilities for native-element detection and HTML interactive-content classification.
- Added comprehensive tests (rule + utils) and documentation, plus README rule list update.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/rules/template-no-noninteractive-tabindex.js |
Implements the new rule, including role classification and exemptions. |
lib/utils/is-native-element.js |
Adds authoritative native-element detection with scope shadowing support. |
lib/utils/html-interactive-content.js |
Adds spec-based “interactive content” classification helper used by the rule. |
tests/lib/rules/template-no-noninteractive-tabindex.js |
RuleTester coverage for valid/invalid cases and options behavior. |
tests/lib/utils/is-native-element-test.js |
Unit tests for list-based native-element identification behavior. |
tests/lib/utils/html-interactive-content-test.js |
Unit tests for HTML interactive-content classification. |
tests/audit/no-noninteractive-tabindex/peer-parity.js |
Non-CI audit fixture comparing behavior to jsx-a11y (needs comment update). |
docs/rules/template-no-noninteractive-tabindex.md |
New rule documentation and examples, including options. |
README.md |
Adds the new rule to the Accessibility rules table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
force-pushed
the
feat/template-no-noninteractive-tabindex
branch
from
c710c9a to
b7bae63
Compare
johanrd
added a commit
that referenced
this pull request
Apr 23, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
added a commit
that referenced
this pull request
Apr 24, 2026
…lookup in html-interactive-content)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
added a commit
that referenced
this pull request
Apr 26, 2026
Translates jsx-a11y's no-noninteractive-tabindex test list to HBS to pin our rule's behavior against PR #24's interactive-element and tabindex="-1" exemption fixes. Parity holds on all upstream always-valid and never-valid cases. Annotated divergences: - tabpanel: we flag (jsx-a11y strict parity); jsx-a11y recommended whitelists it via `roles: ['tabpanel']`. - Dynamic role expressions: we skip conservatively; matches jsx-a11y recommended (allowExpressionValues: true), diverges from strict. - Component name collides with a native tag (e.g. <Article>): we lowercase tag before the aria-query dom lookup so <Article> validates as <article>. False positive relative to jsx-a11y's no-settings default.
johanrd
force-pushed
the
feat/template-no-noninteractive-tabindex
branch
from
66815af to
8a6332f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
johanrd
added a commit
that referenced
this pull request
Apr 27, 2026
Translates jsx-a11y's no-noninteractive-tabindex test list to HBS to pin our rule's behavior against PR #24's interactive-element and tabindex="-1" exemption fixes. Parity holds on all upstream always-valid and never-valid cases. Annotated divergences: - tabpanel: we flag (jsx-a11y strict parity); jsx-a11y recommended whitelists it via `roles: ['tabpanel']`. - Dynamic role expressions: we skip conservatively; matches jsx-a11y recommended (allowExpressionValues: true), diverges from strict. - Component name collides with a native tag (e.g. <Article>): we lowercase tag before the aria-query dom lookup so <Article> validates as <article>. False positive relative to jsx-a11y's no-settings default.
johanrd
force-pushed
the
feat/template-no-noninteractive-tabindex
branch
from
01a3cfe to
5997752
Compare
johanrd
force-pushed
the
feat/template-no-noninteractive-tabindex
branch
from
5997752 to
828d8e1
Compare
…pt-in requireExplicit
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+58
to
+62
| // tabindex="{{-1}}" → GlimmerConcatStatement with a single mustache part. | ||
| if ( | ||
| value.type === 'GlimmerConcatStatement' && | ||
| value.parts?.length === 1 && | ||
| value.parts[0].type === 'GlimmerMustacheStatement' |
There was a problem hiding this comment.
There’s a code path to handle GlimmerConcatStatement values like tabindex="{{-1}}", but the rule’s tests don’t currently cover that representation. Adding at least one valid/invalid case for the concat form would help prevent regressions in this parsing logic.
…-require-input-type feat: add template-require-input-type
Prepare Release v13.2.0
Captures empirically-verified Glimmer rendering behavior for HTML
attributes with mustache values, so rule authors classifying
GlimmerBooleanLiteral / GlimmerStringLiteral / GlimmerConcatStatement
have a ground-truth reference instead of intuition.
Notable findings the doc pins down:
- attr={{"false"}} (bare string "false") renders as attr="false" — TRUTHY,
not falsy as the literal suggests.
- attr="{{false}}" (concat) sets the IDL property to true regardless of
the literal value inside, even when HTML serialization shows nothing.
Verified against <video muted="{{false}}"> → videoEl.muted === true.
- Non-reflecting boolean attrs (muted, autoplay) and reflecting ones
(disabled, hidden) diverge in HTML serialization but agree at the IDL
property layer.
Includes a copy-pasteable reproduction template so future readers can
re-verify if Glimmer behavior changes.
Adds a pointer in README's "Creating a New Rule" section.
Replaces the prior tables (which mixed verified data with extrapolations
marked "(assumed)") with strictly-verified per-attribute tables. Every cell
populated from rendering and IDL inspection in ember-source 6.12.
Structure:
- One "Reference table" section, five per-attribute sub-tables
(muted, disabled, aria-hidden, tabindex, autocomplete)
- One "To reproduce the reference table" section with the exact template
and JS console snippet, inline (no separate fixture file)
- Cross-attribute observations summarizing the rules the data reveals
Findings the new tables make explicit:
- Bare-mustache falsy set is {{false}}/{{null}}/{{undefined}}/{{0}} for
boolean-coerced attrs (boolean HTML, ARIA, numeric); {{""}} is kept as
attr="".
- Bare-mustache string literals never coerce — attr={{"false"}} renders
as attr="false".
- Concat-mustache for boolean HTML attrs sets the IDL property to true
regardless of the literal value inside (verified for both reflecting
and non-reflecting attrs).
- Concat-mustache for ARIA/string attrs renders the stringified value
literally — no boolean coercion. aria-hidden="{{false}}" is visible.
- Plain string attrs (autocomplete) skip Glimmer's boolean coercion
entirely; autocomplete={{false}} renders as autocomplete="false".
The video.muted snapshot reads IDL muted=false for static attribute forms
(m1-m4, m7-m8, m11) because the test runs before media load — the doc
explains how defaultMuted reflects to muted at load time, so the rule's
"At play time" column is the lint-truth column rule authors should use.
…les" guide Adds a practical-implementation section between the reference table and the reproduction template. It maps each AST shape (GlimmerTextNode / GlimmerMustacheStatement with each path type / GlimmerConcatStatement) to a verdict, citing the row IDs from the reference table so rule authors can implement classification correctly without re-deriving the model. Includes: - AST-shape verdict table — direct mapping rule authors can copy from - Six common mistakes section, each tied to specific row IDs - Pointer to the (forthcoming) lib/utils/glimmer-attr-presence.js utility that will encode the verdict table once and let rules consume the resolved kind + value rather than re-walking the AST The audit of master rules and the open feature PRs found 18 REAL_BUG findings (12 in PRs, 6 in master) — all classifiable into the bullet-1 through bullet-4 footguns this guide enumerates.
…ng model
Adds lib/utils/glimmer-attr-presence.js exporting:
- classifyAttribute(attrNode, options?) → { presence, value }
Maps every AST shape (valueless / GlimmerTextNode / GlimmerMustacheStatement
with each path type / GlimmerConcatStatement) to a (presence, value) pair
per the verified model in docs/glimmer-attribute-behavior.md. Each branch
cites the relevant doc row IDs (m1–m19, h1–h15, d1–d10, t1–t7, i1–i5).
- inferAttrKind(name) → 'boolean' | 'aria' | 'numeric' | 'plain-string'
Used when classifyAttribute callers don't pass options.kind explicitly.
- BOOLEAN_HTML_ATTRS, NUMERIC_ATTRS — exported sets, useful for callers
that want to extend the kind model.
Key empirical asymmetries this util encodes correctly (and that audit
findings show several rules currently misclassify):
- Bare {{false}} / {{null}} / {{undefined}} on falsy-coerced kinds
(boolean / aria / numeric) → presence='absent' (Glimmer omits attribute).
Same forms on plain-string → presence='present', value='false' / etc.
- Bare {{"false"}} (StringLiteral) is JS-truthy, never coerced — renders
the literal value across all attribute kinds.
- aria-hidden={{true}} renders aria-hidden="" (h5, contested), not
aria-hidden="true" — the util surfaces value='' here so callers
comparing value === 'true' don't false-match.
- Concat is never falsy: any concat form is presence='present'; the
resolved value comes from the existing getStaticAttrValue helper.
Tests: 35 unit tests covering every doc row + the kind-override option.
Updates docs/glimmer-attribute-behavior.md to reference the actual file
and replaces the "(forthcoming)" sketch with a working example.
…ation / html-void-elements Correctness fixes from PR ember-cli#2769 review: - Boolean concat now returns canonical `value: 'true'` instead of leaking the inner literal. Per doc rows m13-m19, d7-d10 the IDL is set true regardless of inner value, so callers checking `value === 'false'` to detect "off" no longer get a wrong answer for `<video muted="{{false}}">`. - {{true}} on numeric / plain-string now returns `unknown` (untested in doc; was previously claiming `value: 'true'` by extrapolation). - `inferAttrKind` is now case-insensitive (`Disabled`, `ARIA-Hidden`, etc.). Drop hand-rolled spec lists in favor of upstream packages: - `property-information` for boolean / overloadedBoolean / number attribute classification, replacing the 24-entry BOOLEAN_HTML_ATTRS and 3-entry NUMERIC_ATTRS Sets. `colspan` is added as a small NUMERIC_OVERRIDES Set to compensate for an upstream gap in property-information 7.1.0 (rowspan and cols are marked `number: true`, colspan isn't). - `html-void-elements` in template-block-indentation.js and template-self-closing-void-elements.js, deduplicating two parallel 16-entry VOID_TAGS Sets. Internal API change: BOOLEAN_HTML_ATTRS and NUMERIC_ATTRS are no longer exported from glimmer-attr-presence. The util's public surface is now `classifyAttribute` and `inferAttrKind`. Callers wanting the underlying classification can use property-information directly.
…s for parsing integers)
… tabindex="{{-1}}" case
johanrd
force-pushed
the
feat/template-no-noninteractive-tabindex
branch
from
56c8425 to
ca9b971
Compare
This file contains hidden or 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
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.
Note
This is part of a series where Claude has audited
eslint-plugin-emberagainst jsx-a11y, vuejs-accessibility, angular-eslint, lit-a11y and html-validate,ember-template-lint, and the HTML and WCAG specs.tabindex="0"on a<div>puts the element in the keyboard tab order without supplying any keyboard semantics — users reach it but can't operate it, and screen readers announce the tag with no hint of interactivity. Normative basis: WCAG 2.1 SC 2.1.1 Keyboard (Level A) + SC 4.1.2 Name, Role, Value. Implementation patterns align with jsx-a11y'sno-noninteractive-tabindex.Fix: add
template-no-noninteractive-tabindex.Not flagged (interactive):
html-interactive-contentutil):button,details,embed,iframe,label,select,summary,textarea, plus conditional<a href>,<input>(excepttype=hidden),<img usemap>,<audio controls>,<video controls>. Note: for<audio>/<video>we align with §3.2.5.2.7 (onlycontrolscounts) — stricter than axobject-query, which marks bare<audio>/<video>as widget unconditionally.<canvas>— not in §3.2.5.2.7, but commonly wired as a drawing/game surface with tabindex; preserved via rule-level defensive for upstream ember-template-lint parity.button,checkbox,tab, …).tabindex="-1"— per HTML Living Standard §6.6.3, negative values make the element focusable but omit it from sequential focus navigation. Legitimate use cases include programmatic focus (modals, error messages) and roving-tabindex composite widgets. MDN documents these. Consistent withtemplate-require-aria-activedescendant-tabindexwhich accepts both0and-1, and jsx-a11y's same exemption.role="tabpanel"— exempt by default via therolesoption (see below). The WAI-ARIA APG Tabs pattern gives panelstabindex="0"when the panel's content isn't itself focusable; flagging that would break canonical tab code. Matches jsx-a11y's recommended config.Options
roles(default["tabpanel"]) — non-interactive ARIA roles exempted. Useroles: []for strict jsx-a11y-strict-equivalent behavior; widen to["tabpanel", "region", …]where your project usestabindexon scrollable regions or similar.Prior art
no-noninteractive-tabindextabindexon elements that are neither natively interactive nor carry an interactive role; acceptstabindex="-1"as the roving-tabindex exemption.src/rules/; onlytabindex-no-positiveexists, which is a different check).packages/eslint-plugin-template/src/rules/; onlyno-positive-tabindexexists, a different check).lib/rules/; onlytabindex-no-positiveexists, a different check).