A11y: Improved toolbar a11y by fixing semantics#28672
Conversation
There was a problem hiding this comment.
PR Summary
Improved toolbar accessibility by enhancing semantic structure and ARIA labeling.
code/addons/backgrounds/src/manager.tsx: ReplacedFragmentwith styled unordered list (ToolList) and wrapped components in list items (<li>).code/core/src/manager/components/preview/Toolbar.tsx: Marked up toolbar as<section>with ARIA label and grouped tool buttons into an unordered list (<ul>).code/core/src/manager/components/preview/tools/zoom.tsx: Usedstyledfunction to createToolListcomponent, ensuring better screen reader interpretation and visual layout.
These changes significantly improve the toolbar's accessibility but require thorough testing to avoid layout or functionality issues.
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
kasperpeulen
left a comment
There was a problem hiding this comment.
Looks great, couple of suggestions
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 65a6d76. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
|
Sorry, for getting back so late, could you fix the merge conflicts? @mehm8128 |
|
@kasperpeulen I resolved the merge conflict. Could you review this again? |
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
b982c44 to
9f371ba
Compare
| shown={isShown} | ||
| data-test-id="sb-preview-toolbar" | ||
| > | ||
| <ScreenReaderLabel id={id}>Toolbar</ScreenReaderLabel> |
There was a problem hiding this comment.
@valentinpalkovic FYI I've recently added a CSS util for visually hidden content which could simplify this ScreenReaderLabel code.
There was a problem hiding this comment.
See code/core/src/theming/global.ts
| const ScreenReaderLabel = styled.span({ | ||
| position: 'absolute', | ||
| left: -10000, | ||
| top: 'auto', | ||
| width: 1, | ||
| height: 1, | ||
| overflow: 'hidden', | ||
| }); |
There was a problem hiding this comment.
See sb-sr-only in code/core/src/theming/global.ts
|
Hi @mehm8128 Thank you so much for your contribution so far. We have left some comments and the PR got a bit stale. Would you be interested in continuing, resolve the merge conflicts and address the comments? Let us know whether your time allows it. Thanks! |
0a62818 to
05846a4
Compare
|
@valentinpalkovic |
Closes #25918
What I did
Grouped toolbar with labelled section.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
yarn task --task sandbox --start-from auto --template react-vite/default-tsDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.tsMake 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/coreteam 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>