Skip to content

Announce Button's disabled links to assistive tech - #5609

Merged
burieberry merged 2 commits into
mainfrom
cs-12305-button-disabled-link-a11y
Jul 27, 2026
Merged

Announce Button's disabled links to assistive tech#5609
burieberry merged 2 commits into
mainfrom
cs-12305-button-disabled-link-a11y

Conversation

@burieberry

Copy link
Copy Markdown
Contributor

Button has styled a hrefless anchor as a disabled link for a while — the CSS matches a.boxel-button:not([href]), [href=''] and .disabled-link, applying opacity: 0.5, the disabled colour and pointer-events: none. But the @as='anchor' branch only suppresses the href: no disabled attribute, no aria-disabled. So the state has been visual-only for every caller, and @disabled={{true}} on an anchor produced DOM identical to just omitting @href.

  <a
    class={{classes}}
    href={{unless @disabled (if @href (sanitizeHtml @href))}}
+   role={{if @disabled 'link'}}
+   aria-disabled={{if @disabled 'true'}}
    data-test-boxel-button
    ...attributes
  >

Two things worth reviewing rather than skimming

role='link' is the part that does the work. An <a> we've stripped the href from is not a link — it maps to the generic role, where aria-disabled is not supported and browsers drop it. Adding aria-disabled alone would have looked like a fix and changed nothing observable.

Keyed on @disabled only, deliberately never on a falsy @href. Inferring from @href was my first instinct and it would have introduced a bug: callers legitimately pass href as a plain attribute through ...attributes, where @href is undefined but the link is real. Two in-tree examples, both href='#top':

  • packages/base/skill-plus.gts
  • packages/base/default-templates/theme-dashboard.gts

Those would have been labelled unavailable. There's a test guarding exactly that case.

Blast radius

Anchor-Button callers surveyed. Only one passes @disabled, and it gains the intended behaviour for free:

  • packages/host/app/components/operator-mode/publish-realm-modal.gts@as='anchor' with @disabled={{this.isUnpublishingAnyRealms}}, in two places

The rest pass a real @href or a plain href attribute and are unaffected: host-submode.gts, host-submode/open-site-popover.gts, skill-plus.gts, theme-dashboard.gts.

Tests

Adds packages/boxel-ui/test-app/tests/integration/components/button-test.gtsthe first test coverage Button has. Six tests: default/@as='button' tag, anchor href sanitisation, the disabled-anchor role/aria-disabled pair, the enabled-anchor negative case, the ...attributes regression guard, and the native disabled attribute on <button>.

I could not run them locally, and I'd rather say so than imply otherwise. The boxel-ui test-app harness dies at boot with Uncaught TypeError: window.require is not a function before any test executes. It fails identically on a stashed clean tree with an untouched accordion-test, and again after mise run build:ui (the step CI runs first), so it is pre-existing and unrelated to this change. What did pass locally, in both addon and test-app: eslint, ember-template-lint, ember-tsc --noEmit. CI's Boxel UI Tests job is the first real execution of these assertions — please don't merge on my word that they pass.

That local breakage may deserve its own issue; it's plausibly why Button had no tests to begin with.

Follow-ups this unblocks

Tracked on CS-12305, which lists the three places that assert the current behaviour and go stale together when this lands:

  1. boxel-home sections/hero-section.gts — drop the hand-passed aria-disabled and its TODO (boxel-ui, CS-12305) comment (boxel-home#70)
  2. boxel-skills skills/boxel-ui-guidelines/references/use-boxel-ui-components.md — remove the dated version note (boxel-skills#103)
  3. boxel-skills Skill/boxel-ui-guidelines.md — the same note in the mirrored tree

Also noted there: this makes the surrounding convention more load-bearing, not less. Today a plain label written as a hrefless anchor is merely silent to assistive tech; afterwards it will actively announce itself as an unavailable link. Whether an optional href means "unavailable link" or "not a link" becomes a decision authors have to make deliberately.

Draft until CI has actually run the tests.

🤖 Drafted by Claude (Opus 5).

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files      1 suites   2h 55m 38s ⏱️
3 668 tests 3 653 ✅ 15 💤 0 ❌
3 687 runs  3 672 ✅ 15 💤 0 ❌

Results for commit 7044743.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   10m 47s ⏱️ - 2m 43s
1 943 tests +8  1 943 ✅ +8  0 💤 ±0  0 ❌ ±0 
2 022 runs  +8  2 022 ✅ +8  0 💤 ±0  0 ❌ ±0 

Results for commit 7044743. ± Comparison against earlier commit 21b5ff5.

burieberry and others added 2 commits July 27, 2026 12:29
Button already styles a hrefless anchor as a disabled link — the CSS has
matched a.boxel-button:not([href]), [href=''] and .disabled-link for a
while (opacity, disabled colour, pointer-events: none). But the @as='anchor'
branch only suppresses the href: no disabled attribute, no aria-disabled.
So the state has been visual-only for every caller, and @disabled={{true}}
on an anchor produced DOM identical to just omitting @href.

role='link' is the part that makes this work. An <a> we've stripped the
href from is not a link — it maps to the generic role, where aria-disabled
is not supported and browsers drop it. Setting aria-disabled alone would
have looked like a fix and changed nothing.

Keyed on @disabled only, deliberately never on a falsy @href: callers pass
href as a plain attribute through ...attributes (base/skill-plus.gts,
base/default-templates/theme-dashboard.gts), where @href is undefined and
the link is real. Inferring from @href would have labelled those
unavailable. There is a test guarding exactly that.

Also adds the first test coverage for Button.

Not verified at runtime: the boxel-ui test-app harness fails at boot
locally with "window.require is not a function", before any test runs. It
does so identically on a clean checkout with an untouched accordion test,
and after mise run build:ui, so it is pre-existing and not caused by this
change. eslint, ember-template-lint and ember-tsc pass in both the addon
and test-app; CI is the real gate for the assertions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new button-test caught a second, older bug in the branch this PR
did not otherwise touch. `aria-disabled={{@disabled}}` passes the raw
boolean, and Glimmer renders a bare true as aria-disabled="" — a value
ARIA treats as neither "true" nor "false", so it falls back to the
default, false. Every disabled Button has been claiming it is enabled.

Benign in practice: the native disabled attribute alongside it carries
the real state, and that is what assistive tech reads. But the pair
contradicted itself, and the empty value matches none of the
[aria-disabled='true'] selectors used in boxel-ui global.css, Select,
and several realm field styles.

Line 88 was also the only place in the repo passing the raw boolean.
Select, Dropdown, detail-panel-selector and the anchor branch added in
this PR all use the {{if x 'true'}} idiom; this aligns with them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@burieberry
burieberry force-pushed the cs-12305-button-disabled-link-a11y branch from 21b5ff5 to 7044743 Compare July 27, 2026 17:04
@burieberry
burieberry marked this pull request as ready for review July 27, 2026 17:32
@burieberry
burieberry requested a review from a team July 27, 2026 19:56
@burieberry
burieberry merged commit 0f1f05f into main Jul 27, 2026
81 of 102 checks passed
@burieberry
burieberry deleted the cs-12305-button-disabled-link-a11y branch July 27, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants