Skip to content

UI: Make disabled Buttons keyboard-focusable #34166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Sidnioulz merged 6 commits into from
Mar 17, 2026
Merged

UI: Make disabled Buttons keyboard-focusable #34166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
01b1051
fix(a11y): replace native disabled with isDisabled and aria-disabled …
WioletaKolodziej Aug 23, 2025
cff53d2
Finalise Button aria-disabled changes
Sidnioulz Mar 16, 2026
92c9867
Adjust remaining tests using toBeDisabled
Sidnioulz Mar 17, 2026
585d934
Fix mistake in E2E test locator
Sidnioulz Mar 17, 2026
9e58af5
Merge branch 'next' into sidnioulz/issue-31678-aria-disabled-finish
Sidnioulz Mar 17, 2026
609f377
Merge branch 'next' into sidnioulz/issue-31678-aria-disabled-finish
Sidnioulz Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,23 @@ export const Default: Story = {};
export const Submitting: Story = {
play: async ({ args }) => {
const button = await screen.findByRole('button', { name: 'Submit' });
await expect(button).toBeDisabled();
await expect(button).toHaveAttribute('aria-disabled', 'true');

await userEvent.click(await screen.findByText('Design system'));
await expect(button).toBeDisabled();
await expect(button).toHaveAttribute('aria-disabled', 'true');

await userEvent.click(await screen.findByText('Functional testing'));
await userEvent.click(await screen.findByText('Accessibility testing'));
await userEvent.click(await screen.findByText('Visual testing'));
await expect(button).toBeDisabled();
await expect(button).toHaveAttribute('aria-disabled', 'true');

await userEvent.selectOptions(screen.getByRole('combobox'), ['We use it at work']);
await expect(button).not.toBeDisabled();
await expect(button).not.toHaveAttribute('aria-disabled', 'true');

await userEvent.click(button);

await waitFor(async () => {
await expect(button).toBeDisabled();
await expect(button).toHaveAttribute('aria-disabled', 'true');
await expect(args.onComplete).toHaveBeenCalledWith({
building: {
'application-ui': false,
Expand Down
6 changes: 3 additions & 3 deletions code/core/src/component-testing/components/test-fn.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ TestFunctionTypes.test(
},
async ({ canvas }) => {
const button = canvas.getByText('Arg from story');
await expect(button).toBeEnabled();
await expect(button).not.toHaveAttribute('aria-disabled', 'true');
}
Comment on lines +99 to 100

@coderabbitai coderabbitai Bot Mar 17, 2026

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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find and read the target file
cat -n code/core/src/component-testing/components/test-fn.stories.tsx | head -130 | tail -40

Repository: storybookjs/storybook

Length of output: 1609

🏁 Script executed:

# Read the beginning of the file to understand the component being tested
head -90 code/core/src/component-testing/components/test-fn.stories.tsx | tail -50

Repository: storybookjs/storybook

Length of output: 1293

🏁 Script executed:

# Read from the very top of the file to find imports and component definitions
head -60 code/core/src/component-testing/components/test-fn.stories.tsx

Repository: storybookjs/storybook

Length of output: 1474

🏁 Script executed:

# Check the test framework setup - look for jest-dom or testing-library matchers
grep -r "toBeEnabled\|jest-dom\|@testing-library" code/core/src/component-testing --include="*.ts" --include="*.tsx" -A 2 -B 2

Repository: storybookjs/storybook

Length of output: 47

🏁 Script executed:

# Check what's exported from storybook/test
find code -name "*.ts" -o -name "*.tsx" | xargs grep -l "export.*expect\|from.*storybook/test" | head -5

Repository: storybookjs/storybook

Length of output: 360

🏁 Script executed:

# Find the storybook/test export
find code -path "*storybook/test*" -type f | head -10

Repository: storybookjs/storybook

Length of output: 153

🏁 Script executed:

# Search for storybook/test definition
rg "export.*expect|vitest|@vitest" code --type ts --type tsx -A 3 | head -50

Repository: storybookjs/storybook

Length of output: 92

🏁 Script executed:

# Search for storybook/test setup
rg "export.*expect" code -g "*.ts" -g "*.tsx" -A 3 | head -80

Repository: storybookjs/storybook

Length of output: 691

🏁 Script executed:

# Check the test/index.ts to see what rawExpect is
cat code/core/src/test/index.ts

Repository: storybookjs/storybook

Length of output: 1658

🏁 Script executed:

# Check expect.ts to see what's exported
cat code/core/src/test/expect.ts

Repository: storybookjs/storybook

Length of output: 4374

🏁 Script executed:

# Verify toBeEnabled is part of `@testing-library/jest-dom` matchers
grep -r "toBeEnabled" code --include="*.ts" --include="*.tsx" -B 2 -A 2 | head -40

Repository: storybookjs/storybook

Length of output: 47

🏁 Script executed:

# Check package.json for `@testing-library/jest-dom` version
grep -r "@testing-library/jest-dom" code --include="package.json"

Repository: storybookjs/storybook

Length of output: 191

🌐 Web query:

@testing-library/jest-dom 6.9.1 toBeEnabled matcher

💡 Result:

In @testing-library/jest-dom v6.9.1, toBeEnabled() is available and is defined as the logical inverse of toBeDisabled() (i.e. it’s equivalent to expect(element).not.toBeDisabled() but avoids double-negation). It considers an element disabled when it’s a disable-able form control with the disabled attribute or it’s inside a disabled ancestor that can disable descendants (e.g. a <fieldset disabled>), and therefore considers it enabled when those conditions are not true. It does not take aria-disabled into account. [1]

Example:

import '@testing-library/jest-dom'

expect(screen.getByRole('button')).toBeEnabled()

The 6.9.1 release itself is a small bugfix release (unrelated to toBeEnabled behavior). [2]

Sources: [1] [2]

Replace aria-disabled assertions with toBeEnabled() for native buttons.

Lines 99, 109, and 123 check not.toHaveAttribute('aria-disabled', 'true'), but for native <button> elements, the actual enabled state is determined by the native disabled attribute or disabled fieldset ancestor, not the aria-disabled accessibility attribute. Use toBeEnabled() instead to verify the button's actual enabled behavior.

Suggested patch 
-    await expect(button).not.toHaveAttribute('aria-disabled', 'true');
+    await expect(button).toBeEnabled();
...
-    await expect(button).not.toHaveAttribute('aria-disabled', 'true');
+    await expect(button).toBeEnabled();
...
-    await expect(button).not.toHaveAttribute('aria-disabled', 'true');
+    await expect(button).toBeEnabled();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await expect(button).not.toHaveAttribute('aria-disabled', 'true');
}
await expect(button).toBeEnabled();
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/component-testing/components/test-fn.stories.tsx` around lines
99 - 100, Replace the three assertions that check aria-disabled on the native
<button> element with the proper enabled-state matcher: locate the assertions
using the button variable (the lines containing await
expect(button).not.toHaveAttribute('aria-disabled', 'true')) and change them to
use await expect(button).toBeEnabled() so the test verifies the native disabled
behavior (respecting the disabled attribute and disabled fieldset ancestors)
rather than the aria-disabled attribute.

);

Expand All @@ -106,7 +106,7 @@ export const ExtendedStorySinglePlayExample = TestFunctionTypes.extend({
},
play: async ({ canvas }) => {
const button = canvas.getByText('Arg from extended story');
await expect(button).toBeEnabled();
await expect(button).not.toHaveAttribute('aria-disabled', 'true');
},
});

Expand All @@ -120,7 +120,7 @@ ExtendedStorySingleTestExample.test(
'this is a very long test name to explain that this story test should guarantee that the args have been extended correctly',
async ({ canvas }) => {
const button = canvas.getByText('Arg from extended story');
await expect(button).toBeEnabled();
await expect(button).not.toHaveAttribute('aria-disabled', 'true');
}
);

Expand Down
28 changes: 27 additions & 1 deletion code/core/src/components/components/Button/Button.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import { FaceHappyIcon } from '@storybook/icons';

import { fn } from 'storybook/test';
import { expect, fn } from 'storybook/test';
import { styled } from 'storybook/theming';

import preview from '../../../../../.storybook/preview';
Expand Down Expand Up @@ -291,6 +291,32 @@ export const Disabled = meta.story({
ariaLabel: false,
disabled: true,
children: 'Disabled Button',
onClick: fn(),
},
render: (args) => (
<Row>
<Button variant="solid" {...args}>
Disabled Button
</Button>
</Row>
),
play: async ({ args, canvas, step }) => {
const button = canvas.getByRole('button', { name: 'Disabled Button' });

await step('Disabled button should be aria-disabled', async () => {
expect(button).toHaveAttribute('aria-disabled', 'true');
});

await step('Disabled button should not be clickable', async () => {
button.click();
expect(args.onClick).not.toHaveBeenCalled();
});

await step('Disabled button should be focusable for accessibility', async () => {
const button = canvas.getByRole('button', { name: 'Disabled Button' });
button.focus();
expect(button).toHaveFocus();
});
},
});

Expand Down
13 changes: 7 additions & 6 deletions code/core/src/components/components/Button/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,13 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
variant={variant}
size={size}
padding={padding}
disabled={disabled || readOnly}
$disabled={disabled || readOnly}
aria-disabled={disabled || readOnly ? 'true' : undefined}
readOnly={readOnly}
active={active}
animating={isAnimating}
animation={animation}
onClick={handleClick}
onClick={disabled || readOnly ? undefined : handleClick}
aria-label={!readOnly && ariaLabel !== false ? ariaLabel : undefined}
aria-keyshortcuts={readOnly ? undefined : shortcutAttribute}
{...(readOnly ? {} : ariaDescriptionAttrs)}
Expand All @@ -165,7 +166,7 @@ const StyledButton = styled('button', {
padding?: 'small' | 'medium' | 'none';
variant?: 'outline' | 'solid' | 'ghost';
active?: boolean;
disabled?: boolean;
$disabled?: boolean;
readOnly?: boolean;
animating?: boolean;
animation?: 'none' | 'rotate360' | 'glow' | 'jiggle';
Expand All @@ -174,15 +175,15 @@ const StyledButton = styled('button', {
theme,
variant,
size,
disabled,
$disabled,
readOnly,
active,
animating,
animation = 'none',
padding,
}) => ({
border: 0,
cursor: readOnly ? 'inherit' : disabled ? 'not-allowed' : 'pointer',
cursor: readOnly ? 'inherit' : $disabled ? 'not-allowed' : 'pointer',
display: 'inline-flex',
gap: '6px',
alignItems: 'center',
Expand Down Expand Up @@ -216,7 +217,7 @@ const StyledButton = styled('button', {
verticalAlign: 'top',
whiteSpace: 'nowrap',
userSelect: 'none',
opacity: disabled && !readOnly ? 0.5 : 1,
opacity: $disabled && !readOnly ? 0.5 : 1,
margin: 0,
fontSize: `${theme.typography.size.s1}px`,
fontWeight: theme.typography.weight.bold,
Expand Down
4 changes: 2 additions & 2 deletions code/core/src/manager/components/sidebar/TagsFilter.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,9 @@ export const ResetToDefaults: Story = {
const resetButton = await screen.findByRole('button', { name: 'Reset filters' });

expect(resetButton).toBeInTheDocument();
expect(resetButton).not.toBeDisabled();
expect(resetButton).not.toHaveAttribute('aria-disabled', 'true');
resetButton.click();
await waitFor(() => expect(resetButton).toBeDisabled());
await waitFor(() => expect(resetButton).toHaveAttribute('aria-disabled', 'true'));
},
} satisfies Story;

Expand Down
2 changes: 1 addition & 1 deletion code/core/src/shared/checklist-store/checklistData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ export const Disabled: Story = {
await userEvent.click(button);

// 👇 Make assertions
await expect(button).toBeDisabled();
await expect(button).toHaveAttribute('aria-disabled', 'true');
await expect(args.onClick).not.toHaveBeenCalled();
}
};`}
Expand Down
2 changes: 1 addition & 1 deletion code/e2e-tests/addon-toolbars.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ test.describe('addon-toolbars', () => {
await expect(sbPage.previewRoot()).toContainText('안녕하세요');

const button = sbPage.page.getByLabel('Internationalization locale');
await expect(button).toBeDisabled();
await expect(button).toHaveAttribute('aria-disabled', 'true');
});
});
2 changes: 1 addition & 1 deletion code/e2e-tests/addon-viewport.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ test.describe('addon-viewport', () => {

const toolbar = page.getByLabel('Viewport size');

await expect(toolbar).toBeDisabled();
await expect(toolbar).toHaveAttribute('aria-disabled', 'true');
});
});
4 changes: 2 additions & 2 deletions code/renderers/react/template/stories/test-fn.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Default.test(
},
async ({ canvas }) => {
const button = canvas.getByText('Arg from story');
await expect(button).toBeEnabled();
await expect(button).not.toHaveAttribute('aria-disabled', 'true');
}
);
export const Extended = Default.extend({
Expand All @@ -88,5 +88,5 @@ export const Extended = Default.extend({
});
Extended.test('should have extended args', async ({ canvas }) => {
const button = canvas.getByText('Arg from extended story');
await expect(button).toBeEnabled();
await expect(button).not.toHaveAttribute('aria-disabled', 'true');
});
2 changes: 1 addition & 1 deletion docs/api/csf/csf-next.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ PrimaryDisabled.test('should be disabled', async ({ canvas, userEvent, args }) =
const button = await canvas.findByRole('button');
await userEvent.click(button);

await expect(button).toBeDisabled();
await expect(button).toHaveAttribute('aria-disabled', 'true');
await expect(args.onClick).not.toHaveBeenCalled();
});
```
Expand Down
2 changes: 1 addition & 1 deletion docs/writing-tests/interaction-testing.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ The `expect` utility here combines the methods available in [Vitest’s `expect`
| -------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ |
| [`toBeInTheDocument()`](https://github.com/testing-library/jest-dom#tobeinthedocument) | Checks if the element is in the DOM<br />`await expect(<element>).toBeInTheDocument()` |
| [`toBeVisible()`](https://github.com/testing-library/jest-dom#tobevisible) | Checks if the element is visible to the user<br />`await expect(<element>).toBeVisible()` |
| [`toBeDisabled()`](https://github.com/testing-library/jest-dom#tobedisabled) | Checks if an element is disabled<br />`await expect(<element>).toBeDisabled()` |
| [`toHaveAttribute()`](https://github.com/testing-library/jest-dom#tohaveattribute) | Checks if an element has an attribute<br />`await expect(<element>).toHaveAttribute('aria-disabled', 'true')` |
| [`toHaveBeenCalled()`](https://vitest.dev/api/expect.html#tohavebeencalled) | Checks that a spied function was called<br />`await expect(<function-spy>).toHaveBeenCalled()` |
| [`toHaveBeenCalledWith()`](https://vitest.dev/api/expect.html#tohavebeencalledwith) | Checks that a spied function was called with specific parameters<br />`await expect(<function-spy>).toHaveBeenCalledWith('example')` |

Expand Down
10 changes: 5 additions & 5 deletions ...orybooks/portable-stories-kitchen-sink/react-vitest-3/e2e-tests/component-testing.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,21 +237,21 @@ test.describe("component testing", () => {
const watchModeButton = await page.getByRole("switch", {
name: "Watch mode",
});
await expect(runTestsButton).toBeEnabled();
await expect(watchModeButton).toBeEnabled();
await expect(runTestsButton).not.toHaveAttribute("aria-disabled", "true");
await expect(watchModeButton).not.toHaveAttribute("aria-disabled", "true");

await runTestsButton.click();

await expect(watchModeButton).toBeDisabled();
await expect(watchModeButton).toHaveAttribute("aria-disabled", "true");

// Wait for test results to appear
await expect(page.locator("#testing-module-description")).toHaveText(
/Ran \d+ tests/,
{ timeout: 30000 }
);

await expect(runTestsButton).toBeEnabled();
await expect(watchModeButton).toBeEnabled();
await expect(runTestsButton).not.toHaveAttribute("aria-disabled", "true");
await expect(watchModeButton).not.toHaveAttribute("aria-disabled", "true");

const errorFilter = page.getByLabel(
/Filter main navigation to show \d+ tests with errors/
Expand Down
11 changes: 6 additions & 5 deletions test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/component-testing.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,21 +233,22 @@ test.describe("component testing", () => {
const watchModeButton = await page.getByRole("switch", {
name: "Watch mode",
});
await expect(runTestsButton).toBeEnabled();
await expect(watchModeButton).toBeEnabled();
await expect(runTestsButton).not.toHaveAttribute("aria-disabled", "true");
await expect(watchModeButton).not.toHaveAttribute("aria-disabled", "true");

await runTestsButton.click();

await expect(watchModeButton).toBeDisabled();
// The test button will be disabled as tests are running
expect(watchModeButton).toHaveAttribute("aria-disabled", "true"),
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Wait for test results to appear
await expect(page.locator("#testing-module-description")).toHaveText(
/Ran \d+ tests/,
{ timeout: 30000 }
);

await expect(runTestsButton).toBeEnabled();
await expect(watchModeButton).toBeEnabled();
await expect(runTestsButton).not.toHaveAttribute("aria-disabled", "true");
await expect(watchModeButton).not.toHaveAttribute("aria-disabled", "true");

const errorFilter = page.getByLabel(
/Filter main navigation to show \d+ tests with errors/
Expand Down
Loading