Addon A11y: Remove warnings API #30049
Conversation
kasperpeulen
requested review from
kylegach and
jonniebigodes
as code owners
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
code/addons/a11y/src/preview.tsx
Outdated
| reporting.addReport({ | ||
| type: 'a11y', | ||
| version: 1, | ||
| result: result, | ||
| status: hasErrors ? 'failed' : hasViolations ? 'warning' : 'passed', | ||
| status: hasViolations ? 'warning' : 'passed', |
There was a problem hiding this comment.
logic: this change means all violations will now show as warnings in Storybook UI, rather than being able to configure some as errors - consider if this reduced severity is appropriate
| @@ -58,7 +53,7 @@ export const experimental_afterEach: AfterEach<any> = async ({ | |||
| * implement proper try catch handling. | |||
| */ | |||
| if (getIsVitestStandaloneRun()) { | |||
| if (hasErrors) { | |||
| if (hasViolations) { | |||
There was a problem hiding this comment.
logic: throwing on any violations is more strict than before when some violations could be configured as warnings - this may break existing tests
There was a problem hiding this comment.
logic: This file is being completely removed, which removes documentation for a feature that users may be actively using. The PR should include updates to MIGRATION.md to inform users about this breaking change and provide alternative approaches if available.
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 9c86875. 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. |
| @@ -130,18 +130,6 @@ If you enabled the addon and you're manually upgrading to Storybook 8.5 or later | |||
|
|
|||
| <CodeSnippets path="storybook-addon-a11y-test-setup.md" /> | |||
|
|
|||
| ### Override accessibility violation levels | |||
There was a problem hiding this comment.
@kasperpeulen — Heads up that you'll need to merge next because I merged #30035, which updates this file.
Also, if we're removing this, should we document reviewOnFail instead?
valentinpalkovic
changed the title
…s-api # Conflicts: # docs/writing-tests/accessibility-testing.mdx
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
Closes #
What I did
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!
Documentation
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-storybook/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>Greptile Summary
Based on the provided files and changes, I'll create a concise summary of this pull request:
Removes the warnings API from Storybook's accessibility addon, simplifying violation reporting to a binary pass/fail model instead of configurable warning levels.
minor,moderate,serious,critical) from a11y addon parameterswarning/passedstates instead offailed/warning/passedThis is a breaking change that removes configurability in favor of stricter accessibility enforcement, requiring users to update their configurations that previously used warning levels.
💡 (5/5) You can turn off certain types of comments like style here!