Skip to content

Docs: Move button in ArgsTable heading to fix screenreader announcements#32238

Merged
valentinpalkovic merged 1 commit into
nextfrom
sidnioulz/issue-31436-table
Aug 12, 2025
Merged

Docs: Move button in ArgsTable heading to fix screenreader announcements#32238
valentinpalkovic merged 1 commit into
nextfrom
sidnioulz/issue-31436-table

Conversation

@Sidnioulz
Copy link
Copy Markdown
Member

@Sidnioulz Sidnioulz commented Aug 11, 2025

Addresses part 4 of #31436. Does not close.

What I did

Moved the reset controls button outside the table. Table headings should not directly contain buttons or other secondary elements, because that causes the content of these buttons to be announced on every row visit in the entire table, ruining screenreader experience.

See https://adrianroselli.com/2025/07/check-uncheck-all-in-a-table.html for examples.

Checklist for Contributors

Testing

As far as I know, this wasn't covered by tests. Adding screenreader announcement tests would be rather difficult, this is typically manually tested.

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  1. Check this branch out alongside next in another folder
  2. Using AssistivLabs, proxy to your localhost so you can run NVDA on a dev server
  3. Navigate to an ArgsTable in next, and navigate inside the table cells; notice how "reset controls" keeps bieng repeated
  4. Now switch to this branch, navigate to the same ArgsTable, and notice that "reset controls" stops repeating on every cell in the control column

Documentation

N/A

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

Greptile Summary

This PR addresses a critical accessibility issue in the ArgsTable component by relocating the "Reset controls" button from inside the table header to outside the table structure. The change prevents screen readers from announcing "reset controls" on every cell visit throughout the table, which was creating a poor user experience for assistive technology users.

The implementation uses absolute positioning to maintain the visual appearance while solving the accessibility problem. The button is now wrapped in TablePositionWrapper and ButtonPositionWrapper styled components that position it absolutely outside the table structure. The table header has been simplified to contain only semantic text ("Control") rather than the previous complex structure with an embedded button.

This change aligns with web accessibility best practices that recommend keeping table headers semantic and free of interactive elements. The ArgsTable component is a core part of Storybook's documentation system, used to display component properties and controls, making this accessibility improvement particularly valuable for developers using assistive technologies to navigate Storybook documentation.

Confidence score: 4/5

  • This PR addresses a legitimate accessibility issue with a well-reasoned solution that follows established web accessibility guidelines
  • The implementation uses absolute positioning to maintain visual consistency while solving the screen reader announcement problem
  • Pay close attention to the positioning logic and ensure it works across different screen sizes and layouts

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Bot Settings | Greptile

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Aug 11, 2025

View your CI Pipeline Execution ↗ for commit 1f4422e

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 8s View ↗

☁️ Nx Cloud last updated this comment at 2025-08-11 13:08:11 UTC

@valentinpalkovic valentinpalkovic self-assigned this Aug 11, 2025
@valentinpalkovic valentinpalkovic added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Aug 12, 2025
@valentinpalkovic valentinpalkovic merged commit e738f7b into next Aug 12, 2025
57 of 62 checks passed
@valentinpalkovic valentinpalkovic deleted the sidnioulz/issue-31436-table branch August 12, 2025 08:30
ndelangen pushed a commit that referenced this pull request Aug 20, 2025
Docs: Move button in ArgsTable heading to fix screenreader announcements
(cherry picked from commit e738f7b)
ndelangen pushed a commit that referenced this pull request Aug 21, 2025
Docs: Move button in ArgsTable heading to fix screenreader announcements
(cherry picked from commit e738f7b)
@github-actions github-actions Bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Aug 21, 2025
@ndelangen ndelangen removed the patch:yes Bugfix & documentation PR that need to be picked to main branch label Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal patch:done Patch/release PRs already cherry-picked to main/release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants