Conversation
🏎️ Benchmark Comparison
Full mitata output |
There was a problem hiding this comment.
Pull request overview
Adds a new accessibility-focused template rule to the plugin to ensure <a href> elements expose a non-empty accessible name, aligning behavior with similar ecosystem rules while avoiding false positives for dynamic content.
Changes:
- Introduces
template-anchor-has-contentrule with recursive accessible-name source detection (text,aria-*,title,<img alt>,aria-hiddenhandling). - Adds
isNativeElementutility (HTML/SVG/MathML allowlist + scope-shadowing check) and unit tests. - Adds rule documentation and lists the new rule in the README.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
lib/rules/template-anchor-has-content.js |
Implements the new rule’s logic for detecting unlabeled <a href> anchors. |
lib/utils/is-native-element.js |
Adds shared helper for native-element vs component discrimination (including scope shadowing). |
tests/lib/rules/template-anchor-has-content.js |
Adds rule tests for both .gjs and .hbs parsing modes. |
tests/lib/utils/is-native-element-test.js |
Adds unit tests for list-based native-element detection and tag set sanity checks. |
docs/rules/template-anchor-has-content.md |
Documents rule behavior and examples. |
README.md |
Adds the 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 6 out of 6 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
force-pushed
the
feat/template-anchor-has-content
branch
from
a071bfc to
6030630
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 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 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… via shared helper (Copilot review)
Extract a new `getStaticAttrValue` util that resolves literal-valued
mustaches (`{{"foo"}}`, `{{true}}`, `{{-1}}`) and single-part concat
statements (`"{{true}}"`) to their static string value. `isAriaHiddenTruthy`
now delegates to the helper and compares the resolved string to `'true'`
(case-insensitive, whitespace-trimmed).
Behavior change: valueless `<h1 aria-hidden>`, `aria-hidden=""`, and the
mustache-empty-string equivalents (`aria-hidden={{""}}`, `aria-hidden="{{""}}"`,
`aria-hidden={{" "}}`) are no longer treated as hidden. Per WAI-ARIA 1.2
§6.6 value table, those shapes resolve to the default `undefined` — NOT
`true` — so the empty-content check still applies. Drops the previous
"fewer false positives" deviation rationale in favour of spec-literal
consistency with sibling rules (#19, #35, #41) that share the same
aria-hidden resolution.
Byte-identical carrier of lib/utils/static-attr-value.js across all PRs
that land it.
johanrd
force-pushed
the
feat/template-anchor-has-content
branch
from
211ed97 to
88237d9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 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
force-pushed
the
feat/template-anchor-has-content
branch
from
4497502 to
34cb30b
Compare
johanrd
force-pushed
the
feat/template-anchor-has-content
branch
from
33d4351 to
0172580
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // HTML boolean `hidden` (§5.4) removes the element from rendering AND | ||
| // from the accessibility tree — equivalent to aria-hidden="true" for | ||
| // accessible-name purposes. A <span hidden>Backup</span> inside an | ||
| // anchor contributes no name at runtime. | ||
| if (attrs.some((a) => a.name === 'hidden')) { | ||
| return { dynamic: false, accessible: false }; | ||
| } |
There was a problem hiding this comment.
The child hidden handling treats any presence of a hidden attribute as meaning “definitely hidden”, but in Ember templates hidden={{false}} commonly results in the attribute being omitted at runtime (meaning the element is visible and its text should contribute to the anchor’s accessible name). This can cause false positives (flagging an anchor as empty even though it has visible child text). Consider resolving hidden similarly to aria-hidden: only treat it as hidden when it’s statically true (e.g. valueless hidden, or mustache-literal {{true}} / {{"true"}} / other statically-known truthy forms), and treat truly dynamic values as “unknown” (likely best as { dynamic: true } so the anchor isn’t flagged).
| // Skip anchors the author has explicitly hidden — either via the HTML | ||
| // `hidden` boolean attribute (element is not rendered at all) or | ||
| // `aria-hidden="true"` (element removed from the accessibility tree). | ||
| // In both cases, "accessible name of an anchor" is moot. | ||
| const hasHidden = attrs.some((a) => a.name === 'hidden'); | ||
| if (hasHidden) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This skips reporting whenever a hidden attribute is present, but in templates hidden={{false}} should not skip the rule (the anchor is not hidden at runtime). This creates false negatives where empty anchors can slip through linting. Recommend changing the logic to only skip when hidden is statically true; if hidden is dynamic/unknown, consider the project’s “fewer false positives” stance and either (a) treat dynamic as hidden (skip) consistently, or (b) treat dynamic as unknown and skip reporting (similar to how dynamic content is handled), but avoid treating hidden={{false}} as hidden.
| const resolved = getStaticAttrValue(attr.value); | ||
| if (resolved === undefined) { | ||
| // Dynamic — can't prove truthy. | ||
| return false; | ||
| } | ||
| return resolved.trim().toLowerCase() === 'true'; |
There was a problem hiding this comment.
resolved.trim() assumes getStaticAttrValue always returns a string when it can resolve a value. If it can return booleans for mustache literals (e.g. {{true}} / {{false}}) or null for valueless attributes, this will throw at runtime. Making this robust (e.g., explicitly handling resolved === true, resolved === false, and only calling .trim() when typeof resolved === 'string') will prevent rule crashes on edge-case AST/value shapes.
| // Anchor itself hidden via HTML `hidden` boolean attribute — element is | ||
| // not rendered, so "accessible name of an anchor" is moot. | ||
| { | ||
| filename: 'test.gjs', | ||
| code: '<template><a href="/x" hidden /></template>', | ||
| }, |
There was a problem hiding this comment.
The suite covers statically-present hidden, but it doesn’t cover the important template case hidden={{false}} (visible at runtime) for both (a) the anchor itself and (b) child elements inside the anchor. Adding tests for hidden={{false}} would lock in correct behavior and prevent regressions given the rule currently keys off attribute presence.
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.Summary
<a href>rendered to the DOM is exposed as a link to assistive tech. Per ACCNAME 1.2 §4.3.2 Computation steps, the accessible name is computed in order fromaria-labelledby(step 2B), thenaria-label(step 2D), then host-language label sources / descendant text content (steps 2E–2F), then a tooltip attribute such astitle(step 2I). For an<a>element with none of these, the result is the empty string. An anchor with none of these has no accessible name; a screen reader announces "link" with nothing else. Authoring guidance: WCAG 2.1 SC 2.4.4 Link Purpose.template-anchor-has-content. Flags<a href>with no text, no accessible-name attribute (aria-label/aria-labelledby/title), and no child that contributes an accessible name. Dynamic cases (mustache-only content) stay accepted — we can't know at lint time whether they resolve to a non-empty name.Four ecosystem positions on valueless aria-hidden
The question "what does
<span aria-hidden>(bare),aria-hidden=""(empty), oraria-hidden={{false}}mean?" has no single authoritative answer:jsx-ast-utilscoercing valueless JSX → booleantrue. Quirk: stringaria-hidden="true"is NOT recognized. Not a deliberate ARIA interpretation."false"→ hiddenisHiddenFromScreenReader.ts: non-spec shortcut.undefined→ not hiddenDesign choice for this rule
We lean toward fewer false positives. For this rule, that means treating a child with valueless / empty aria-hidden as NOT hidden — if someone writes
<a href="/x"><span aria-hidden>X</span></a>, the child's content likely still contributes a name, so we don't flag the anchor as having no accessible content. Only explicitaria-hidden="true"/{{true}}hides the child subtree from the name computation.Prior art
Verified each peer in source:
anchor-has-content<a>without text content or a recognized accessible-name attribute.anchor-has-content<a>lacking content andaria-label; configurableaccessibleChildren/accessibleDirectiveslet callers widen what counts as a name source.anchor-has-contentequivalent. Itsanchor-is-validcovers href validity (noHref/invalidHref/preferButton aspects), not accessible-name content.Flags
Allows