Announce Button's disabled links to assistive tech#5609
Draft
burieberry wants to merge 1 commit into
Draft
Conversation
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>
Contributor
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.
Buttonhas styled a hrefless anchor as a disabled link for a while — the CSS matchesa.boxel-button:not([href]),[href='']and.disabled-link, applyingopacity: 0.5, the disabled colour andpointer-events: none. But the@as='anchor'branch only suppresses thehref: nodisabledattribute, noaria-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 thehreffrom is not a link — it maps to thegenericrole, wherearia-disabledis not supported and browsers drop it. Addingaria-disabledalone would have looked like a fix and changed nothing observable.Keyed on
@disabledonly, deliberately never on a falsy@href. Inferring from@hrefwas my first instinct and it would have introduced a bug: callers legitimately passhrefas a plain attribute through...attributes, where@hrefisundefinedbut the link is real. Two in-tree examples, bothhref='#top':packages/base/skill-plus.gtspackages/base/default-templates/theme-dashboard.gtsThose would have been labelled unavailable. There's a test guarding exactly that case.
Blast radius
Anchor-
Buttoncallers 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 placesThe rest pass a real
@hrefor a plainhrefattribute 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.gts— the first test coverageButtonhas. Six tests: default/@as='button'tag, anchor href sanitisation, the disabled-anchorrole/aria-disabledpair, the enabled-anchor negative case, the...attributesregression guard, and the nativedisabledattribute 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 functionbefore any test executes. It fails identically on a stashed clean tree with an untouchedaccordion-test, and again aftermise run build:ui(the step CI runs first), so it is pre-existing and unrelated to this change. What did pass locally, in bothaddonandtest-app:eslint,ember-template-lint,ember-tsc --noEmit. CI'sBoxel UI Testsjob 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
Buttonhad 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:
boxel-homesections/hero-section.gts— drop the hand-passedaria-disabledand itsTODO (boxel-ui, CS-12305)comment (boxel-home#70)boxel-skillsskills/boxel-ui-guidelines/references/use-boxel-ui-components.md— remove the dated version note (boxel-skills#103)boxel-skillsSkill/boxel-ui-guidelines.md— the same note in the mirrored treeAlso 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
hrefmeans "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