Skip to content

fix(core): keep disabled SelectableCard focusable with aria-disabled - #6158

Open
Geervan wants to merge 1 commit into
facebook:mainfrom
Geervan:fix/selectable-card-focusable-disabled
Open

fix(core): keep disabled SelectableCard focusable with aria-disabled#6158
Geervan wants to merge 1 commit into
facebook:mainfrom
Geervan:fix/selectable-card-focusable-disabled

Conversation

@Geervan

@Geervan Geervan commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #6156

Fixes an issue where SelectableCard with isDisabled rendered native disabled on the hidden <input type="checkbox">, removing it from the tab sequence. The component now preserves focusability with aria-disabled="true" to match its documented contract.

Changes

  • SelectableCard.tsx:
    • Replaced native disabled with aria-disabled="true" on the hidden <input type="checkbox">.
    • Added form={isDisabled ? '' : undefined} to exclude the focusable control from form submissions.
    • Gated <input onChange> and suppressed Space / Enter in handleKeyDown when disabled.
  • SelectableCard.stories.tsx:
    • Updated Storybook description to reflect that disabled cards remain focusable.
  • SelectableCard.test.tsx:
    • Added tests verifying aria-disabled, focus retention, and interaction blocking.

Verification

  • vitest run packages/core/src/SelectableCard/ passed (16/16).
  • Verified in Storybook that disabled cards receive keyboard focus via Tab and cannot be toggled.

@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Sep 9, 2026 6:36am UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 8, 2026
@github-actions github-actions Bot added community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge labels Sep 8, 2026
github-actions Bot added a commit that referenced this pull request Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

Modified Components

CheckboxInput (@astryxdesign/core) · View in Storybook
Metric Before After Delta
Bundle Size (ESM) N/A N/A N/A
Lines of Code N/A 313 -
Complexity N/A Very High (38) -
SelectableCard (@astryxdesign/core) · View in Storybook
Metric Before After Delta
Bundle Size (ESM) N/A N/A N/A
Lines of Code N/A 279 -
Complexity N/A Very High (31) -

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.8KB 1.2KB

Accessibility Audit

Status: 1 accessibility violation(s) found — 1 serious.

SelectableCard - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 1/4 stories · Learn more
    • WCAG: 1.4.3 (Level AA)

Visual Regression

Status: No visual change across 4 compared shot(s).


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyboard users can now reach disabled cards, hear they are unavailable, and continue past them without changing selection. Two things still need fixing:

  • Current main moved the shared checkbox semantics into Checkbox.a11y.*, so this branch conflicts and the fixed state now reports unexpected-pass. Please rebase, remove the stale card-disabled known failure and focusability exception, and keep only component-owned callback/inert coverage in SelectableCard.test.tsx.
  • Please drop form="" and its assertion. This input has no name, so it cannot submit; detaching its form owner is an unrelated behavior change.

[Reviewed by Robohands]

@Geervan
Geervan force-pushed the fix/selectable-card-focusable-disabled branch from 8aa012c to bcb165e Compare September 9, 2026 06:32
@github-actions
github-actions Bot requested a review from cixzhang September 9, 2026 06:32
@Geervan

Geervan commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

thanks for the review, rebased onto main and applied relevant changes, let me know if i am still missing anything
-> Removed the stale card-disabled known failure from Checkbox.a11y.known-failures.ts, the declaredNotDelivered exception from Checkbox.a11y.states.ts, and the custom mismatch test from Checkbox.a11y.chromium.spec.ts.

  • Retained only component-owned callback and inert coverage in SelectableCard.test.tsx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SelectableCard disabled state is not keyboard-focusable as documented

2 participants