A11y: Create a11y test provider and revamp a11y addon #29643
Conversation
…/unified-a11y-testing
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 66931ce. 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. |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 3 | 3 | 0 |
| Self size | 44 KB | 55 KB | 🚨 +10 KB 🚨 |
| Dependency size | 2.80 MB | 2.80 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-docs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 17 | 17 | 0 |
| Self size | 2.20 MB | 2.20 MB | 0 B |
| Dependency size | 7.88 MB | 7.87 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-essentials
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 36 | 36 | 0 |
| Self size | 17 KB | 17 KB | 0 B |
| Dependency size | 13.83 MB | 13.82 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-interactions
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 56 | 56 | 0 |
| Self size | 129 KB | 129 KB | 0 B |
| Dependency size | 12.44 MB | 12.43 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-links
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 4 | 0 |
| Self size | 19 KB | 19 KB | 🚨 +21 B 🚨 |
| Dependency size | 283 KB | 272 KB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-storysource
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 7 | 7 | 0 |
| Self size | 1.89 MB | 1.89 MB | 0 B |
| Dependency size | 10.12 MB | 10.11 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/experimental-addon-test
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 61 | 61 | 0 |
| Self size | 587 KB | 588 KB | 🚨 +201 B 🚨 |
| Dependency size | 13.86 MB | 13.85 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/core
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 46 | 46 | 0 |
| Self size | 19.04 MB | 19.05 MB | 🚨 +8 KB 🚨 |
| Dependency size | 14.29 MB | 14.28 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/experimental-nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 87 | 87 | 0 |
| Self size | 231 KB | 231 KB | 0 B |
| Dependency size | 31.30 MB | 31.29 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 587 | 587 | 0 |
| Self size | 464 KB | 464 KB | 0 B |
| Dependency size | 83.68 MB | 83.67 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 241 | 241 | 0 |
| Self size | 14 KB | 14 KB | 0 B |
| Dependency size | 31.08 MB | 31.07 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/blocks
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 4 | 4 | 0 |
| Self size | 604 KB | 604 KB | 🚨 +21 B 🚨 |
| Dependency size | 1.49 MB | 1.48 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/source-loader
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 5 | 5 | 0 |
| Self size | 41 KB | 41 KB | 🚨 +21 B 🚨 |
| Dependency size | 10.07 MB | 10.06 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/test
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 53 | 53 | 0 |
| Self size | 1.52 MB | 1.52 MB | 🚨 +21 B 🚨 |
| Dependency size | 8.09 MB | 8.08 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-server-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 10 KB | 10 KB | 0 B |
| Dependency size | 3.84 MB | 3.83 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/server
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 9 | 9 | 0 |
| Self size | 13 KB | 13 KB | 🚨 +21 B 🚨 |
| Dependency size | 972 KB | 961 KB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
46 file(s) reviewed, 34 comment(s)
Edit PR Review Bot Settings | Greptile
| if (!htmlElement) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
logic: Should emit an error event when htmlElement is not found rather than silently returning undefined
| status: 'initial', | ||
| handleManual: vi.fn(), | ||
| error: null, | ||
| } as Partial<A11yContextStore> as any); |
There was a problem hiding this comment.
style: avoid double type assertion - consider creating a proper mock object that satisfies A11yContextStore
| passes: [{ id: 'pass1' } as any], | ||
| incomplete: [{ id: 'incomplete1' } as any], | ||
| violations: [{ id: 'violation1', nodes: [] } as any], |
There was a problem hiding this comment.
style: test data should include all required properties of the Result type instead of using 'as any'
| expect( | ||
| component.getByText('Please wait while the accessibility scan is running ...') | ||
| ).toBeInTheDocument(); |
There was a problem hiding this comment.
style: text string assertion is brittle due to trailing spaces - consider using a more flexible matcher
| @@ -43,7 +43,7 @@ | |||
| "prep": "jiti ../../../scripts/prepare/bundle.ts" | |||
| }, | |||
| "dependencies": { | |||
| "@storybook/csf": "^0.1.11", | |||
| "@storybook/csf": "0.1.12--canary.110.7611f0c.0", | |||
There was a problem hiding this comment.
logic: Pinning to a canary version could cause issues if this package is used by projects that require a different CSF version. Consider using a stable version or maintaining the flexible range.
| await expect(mocked(console.log).mock.calls).toEqual([ | ||
| ['1 - [from loaders]'], | ||
| ['2 - [from meta beforeEach]'], | ||
| ['3 - [from story beforeEach]'], | ||
| ['4 - [before mount]'], | ||
| ['5 - [from decorator]'], | ||
| ['6 - [after mount]'], | ||
| ['7 - [from onClick]'], | ||
| ['8 - [from story afterEach]'], | ||
| ['9 - [from meta afterEach]'], | ||
| ]); |
There was a problem hiding this comment.
logic: The expected order of hooks seems incorrect. The decorator (5) is being called after the play function's 'before mount' (4), but decorators should wrap the entire story execution including the play function. This could lead to inconsistent behavior in real-world scenarios.
| spyOn(console, 'log').mockName('console.log'); | ||
| console.log('1 - [from loaders]'); |
There was a problem hiding this comment.
style: The spyOn call is using mockName() but the name isn't being used in any assertions. Consider removing mockName() if not needed.
| @@ -123,7 +123,7 @@ | |||
| "@storybook/codemod": "workspace:*", | |||
| "@storybook/core": "workspace:*", | |||
| "@storybook/core-webpack": "workspace:*", | |||
| "@storybook/csf": "0.1.11", | |||
| "@storybook/csf": "0.1.12--canary.110.7611f0c.0", | |||
There was a problem hiding this comment.
logic: Using a canary version (0.1.12--canary.110.7611f0c.0) in production could lead to instability. Consider using a stable release.
| @@ -46,7 +46,7 @@ | |||
| }, | |||
| "dependencies": { | |||
| "@storybook/components": "workspace:*", | |||
| "@storybook/csf": "^0.1.11", | |||
| "@storybook/csf": "0.1.12--canary.110.7611f0c.0", | |||
There was a problem hiding this comment.
style: Using a canary version in production dependencies could cause instability. Consider waiting for a stable release.
…ncy-handling Add discrepancy handling to A11yPanel
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, here's a concise summary of the pull request:
Comprehensive revamp of Storybook's accessibility (a11y) addon with a new test provider and reporting system, focusing on improved test lifecycle management and result reporting.
Key changes:
ReporterAPIand test lifecycle hooks for standardized accessibility test reportinga11yRunnerby removing state management and focusing on core axe-core functionalityA11yContextSTORY_FINISHEDevent and phases for proper test completion handlingafterEachhooks throughout the testing pipeline for proper cleanupThe changes appear well-structured but there are potential concerns around error handling and hook execution order that should be reviewed carefully.