Repository navigation
A11y: Handle lang attribute throughout preview - #35321
Conversation
Sidnioulz
added
accessibility
ci:normal
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesBCP-47 lang attribute support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
code/addons/docs/src/blocks/blocks/DocsContainer.tsx (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the
.tsextension to this relative import.
./docsLangis a new relative TypeScript import, so it should be written with an explicit extension to match the repo rule. As per coding guidelines,**/*.{ts,tsx}: "Use explicit file extensions on relative imports and exports in TypeScript source (for example./foo.tsor./bar.tsx)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/docs/src/blocks/blocks/DocsContainer.tsx` at line 13, The relative import in DocsContainer should use an explicit TypeScript extension to match the repo rule. Update the import of resolveDocsLang in DocsContainer.tsx to reference the file with its .ts extension, keeping the same symbol and relative path otherwise.Source: Coding guidelines
code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.lang.test.tsx (2)
9-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit extensions to these relative imports.
Repo rules require extensionful relative imports in TypeScript source, so these should be
./ArgRow.stories.tsxand./ArgsTable.tsx. As per coding guidelines, "**/*.{ts,tsx}: Use explicit file extensions on relative imports and exports in TypeScript source (for example./foo.tsor./bar.tsx)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.lang.test.tsx` around lines 9 - 10, The relative imports in ArgsTable.lang.test.tsx use extensionless TypeScript paths, which violates the repo rule for explicit extensions. Update the import of ArgRow.stories and ArgsTable to reference their full TypeScript file names, using the same symbols ArgRow and ArgsTable so the test file resolves them via extensionful relative imports.Source: Coding guidelines
12-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this component coverage into a story
playtest instead of a new*.test.tsxfile.This file is asserting React component behavior/accessibility, which the repo wants in Storybook stories rather than standalone
*.test.tsxtests. Please add/update a story and put these assertions in aplayfunction, then verify it with the Storybook Vitest config. As per coding guidelines, "**/*.test.tsx: For React components, do not add*.test.tsxunit tests; behavior, accessibility, and interaction assertions should live in Storybook stories withplayfunctions." and "**/*.stories.tsx: When updating component stories, verifyplayassertions withvitest --config code/vitest.config.storybook.ts <story-file>."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.lang.test.tsx` around lines 12 - 35, This ArgsTable coverage should not live in a standalone .test.tsx file; move the English-lang assertions into the component’s Storybook story play function. Update the relevant ArgsTable story so it renders the same wrapped setup and uses play to assert the header and arg-name cell keep lang="en", referencing ArgsTable and the story export that exercises it. Then verify the story behavior with the Storybook Vitest config instead of adding/keeping the separate unit test.Source: Coding guidelines
code/addons/docs/src/blocks/blocks/Description.tsx (1)
7-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse explicit extensions for the new relative imports.
This block mixes
./use-service-docgen.ts/./use-service-story-docs.tswith extensionless imports like./DocsContext,./docsLang,./Markdown, and./useOf, which breaks the repo’s TS import convention. As per coding guidelines,**/*.{ts,tsx}: Use explicit file extensions on relative imports and exports in TypeScript source (for example./foo.tsor./bar.tsx).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/docs/src/blocks/blocks/Description.tsx` around lines 7 - 13, Update the relative imports in Description.tsx to follow the repository’s TypeScript import convention by using explicit file extensions everywhere. The issue is that this block currently mixes extensionless imports like DocsContext, resolveDocsLang, Markdown, and useOf with explicit .ts imports; make all relative imports consistent by adding the proper extensions to each referenced module in this file.Source: Coding guidelines
code/addons/docs/src/blocks/blocks/docsLangProse.test.tsx (1)
31-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this coverage into Storybook
playtests instead of a*.test.tsxfile.This is a new React component behavior suite, but the repo’s test policy keeps those assertions in stories. As per coding guidelines,
**/*.test.tsx: For React components, do not add*.test.tsxunit tests; behavior, accessibility, and interaction assertions should live in Storybook stories withplayfunctions. As per coding guidelines,**/*.stories.tsx: When writing tests for components, add or update<Component>.stories.tsxand cover each behavior withplayfunctions usingexpect,userEvent, andwithinfromstorybook/test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/docs/src/blocks/blocks/docsLangProse.test.tsx` around lines 31 - 58, Move the `Title` and `Subtitle` language assertions out of `docsLangProse.test.tsx` and into the corresponding Storybook story file for these components. Update the relevant stories to cover the `docs.lang` behavior with `play` functions using `expect`/`within` (and `userEvent` if needed), and remove the React component unit test file so the coverage follows the repo’s Storybook test policy.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/addons/docs/src/blocks/blocks/docsLang.test.ts`:
- Line 3: The relative import in docsLang.test.ts should use an explicit
TypeScript extension to match the repo import rule. Update the import of
resolveDocsLang from ./docsLang to reference ./docsLang.ts so the test file
follows the same explicit-extension convention used across TypeScript sources.
In `@code/addons/docs/src/blocks/blocks/docsLang.ts`:
- Around line 10-11: The resolveDocsLang helper currently uses nullish
coalescing, which preserves empty docs.lang strings instead of falling back.
Update resolveDocsLang to treat blank or whitespace-only parameters?.docs?.lang
and projectParameters?.docs?.lang as unset, so it falls through to the project
value or the 'en' default. Keep the change localized to resolveDocsLang and
preserve the existing fallback order.
In `@code/addons/docs/src/blocks/blocks/Subtitle.tsx`:
- Around line 51-54: The Subtitle lang lookup is missing the project-level docs
language fallback, so update the `Subtitle` component to resolve the language
using both `preparedMeta?.parameters` and the project annotations’ docs
parameters. Mirror the `Title` fix by passing
`projectAnnotations.parameters.docs.lang` (or the equivalent merged docs
parameters source) into `resolveDocsLang` so a global docs language is respected
when no local override is present.
In `@code/addons/docs/src/blocks/blocks/Title.tsx`:
- Around line 53-56: The Title block is not passing the project-level docs
language into resolveDocsLang, so it can fall back to the default language
instead of the globally configured one. Update the Title component to thread the
docs lang from the project context into resolveDocsLang alongside
preparedMeta?.parameters, matching the pattern used by Description and ensuring
the rendered PureTitle gets the correct lang value.
In `@code/addons/docs/src/blocks/components/DocsPage.test.tsx`:
- Around line 83-94: Move this assertion out of the DocsContent unit test
because it bypasses DocsPageWrapper and does not validate the changed forwarding
behavior in DocsPage.tsx. Add the lang forwarding check to the appropriate
Storybook story using a play function that renders the wrapper and asserts the
content region receives the lang attribute, and remove the *.test.tsx component
test for this case.
In `@code/addons/docs/src/blocks/components/Preview.tsx`:
- Around line 234-238: The `ActionBar` in `Preview` is applying `lang="en"` to
both Storybook-owned controls and caller-provided `additionalActionItems`, which
can incorrectly force user content to be announced as English. Move the English
override off the `ActionBar` container and apply it only to the built-in
buttons/actions rendered by `Preview`, keeping `additionalActionItems` outside
that language scope.
In `@code/core/src/types/modules/csf.ts`:
- Around line 83-90: Update the JSDoc for htmlLang in csf.ts to match actual
behavior: document that WebView.prepareForStory() applies the value to the story
root element, not the document <html> element, and remove the claim that story
view sets <html lang> or that the default is always 'en'. Keep the inheritance
note (project → meta → story) and align the description with the tests around
document.documentElement.lang remaining unchanged.
---
Nitpick comments:
In `@code/addons/docs/src/blocks/blocks/Description.tsx`:
- Around line 7-13: Update the relative imports in Description.tsx to follow the
repository’s TypeScript import convention by using explicit file extensions
everywhere. The issue is that this block currently mixes extensionless imports
like DocsContext, resolveDocsLang, Markdown, and useOf with explicit .ts
imports; make all relative imports consistent by adding the proper extensions to
each referenced module in this file.
In `@code/addons/docs/src/blocks/blocks/DocsContainer.tsx`:
- Line 13: The relative import in DocsContainer should use an explicit
TypeScript extension to match the repo rule. Update the import of
resolveDocsLang in DocsContainer.tsx to reference the file with its .ts
extension, keeping the same symbol and relative path otherwise.
In `@code/addons/docs/src/blocks/blocks/docsLangProse.test.tsx`:
- Around line 31-58: Move the `Title` and `Subtitle` language assertions out of
`docsLangProse.test.tsx` and into the corresponding Storybook story file for
these components. Update the relevant stories to cover the `docs.lang` behavior
with `play` functions using `expect`/`within` (and `userEvent` if needed), and
remove the React component unit test file so the coverage follows the repo’s
Storybook test policy.
In `@code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.lang.test.tsx`:
- Around line 9-10: The relative imports in ArgsTable.lang.test.tsx use
extensionless TypeScript paths, which violates the repo rule for explicit
extensions. Update the import of ArgRow.stories and ArgsTable to reference their
full TypeScript file names, using the same symbols ArgRow and ArgsTable so the
test file resolves them via extensionful relative imports.
- Around line 12-35: This ArgsTable coverage should not live in a standalone
.test.tsx file; move the English-lang assertions into the component’s Storybook
story play function. Update the relevant ArgsTable story so it renders the same
wrapped setup and uses play to assert the header and arg-name cell keep
lang="en", referencing ArgsTable and the story export that exercises it. Then
verify the story behavior with the Storybook Vitest config instead of
adding/keeping the separate unit test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01c85115-612d-4875-89fc-ea8bca4f9960
📒 Files selected for processing (34)
code/addons/docs/src/blocks/blocks/Description.tsxcode/addons/docs/src/blocks/blocks/DocsContainer.tsxcode/addons/docs/src/blocks/blocks/Story.stories.tsxcode/addons/docs/src/blocks/blocks/Subtitle.tsxcode/addons/docs/src/blocks/blocks/Title.tsxcode/addons/docs/src/blocks/blocks/docsLang.test.tscode/addons/docs/src/blocks/blocks/docsLang.tscode/addons/docs/src/blocks/blocks/docsLangProse.test.tsxcode/addons/docs/src/blocks/components/ArgsTable/ArgRow.tsxcode/addons/docs/src/blocks/components/ArgsTable/ArgsTable.lang.test.tsxcode/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsxcode/addons/docs/src/blocks/components/DocsPage.test.tsxcode/addons/docs/src/blocks/components/DocsPage.tsxcode/addons/docs/src/blocks/components/Preview.tsxcode/addons/docs/src/blocks/components/Story.tsxcode/addons/docs/src/blocks/examples/StoryParameters.stories.tsxcode/addons/docs/src/types.tscode/core/src/preview-api/modules/preview-web/WebView.test.tscode/core/src/preview-api/modules/preview-web/WebView.tscode/core/src/types/modules/csf.tsdocs/api/main-config/main-config-indexers.mdxdocs/api/main-config/main-config-log-level.mdxdocs/api/main-config/main-config-manager-head.mdxdocs/api/main-config/main-config-preview-annotations.mdxdocs/api/main-config/main-config-preview-body.mdxdocs/api/main-config/main-config-preview-head.mdxdocs/api/main-config/main-config-refs.mdxdocs/api/main-config/main-config-static-dirs.mdxdocs/api/main-config/main-config-swc.mdxdocs/api/main-config/main-config-tags.mdxdocs/api/main-config/main-config-typescript.mdxdocs/api/main-config/main-config-vite-final.mdxdocs/api/main-config/main-config-webpack-final.mdxdocs/api/parameters.mdx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx (1)
472-525: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass
docsLangto everyArgRow, not just the ungrouped branch.Line 472 forwards
docsLang, but the sectioned and subsectionedArgRowcall sites below still omit it. SinceTableWrapperis nowlang="en", grouped description cells will keep inheriting English instead of the configured docs language.Suggested fix
{subsection.map((row) => ( <ArgRow key={row.key} row={row} arg={args && args[row.key]} expandable={expandable} + docsLang={docsLang} {...common} /> ))} @@ - {section.ungrouped.map((row) => ( - <ArgRow key={row.key} row={row} arg={args && args[row.key]} {...common} /> - ))} + {section.ungrouped.map((row) => ( + <ArgRow + key={row.key} + row={row} + arg={args && args[row.key]} + docsLang={docsLang} + {...common} + /> + ))} @@ {subsection.map((row) => ( <ArgRow key={row.key} row={row} arg={args && args[row.key]} expandable={expandable} + docsLang={docsLang} {...common} /> ))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx` around lines 472 - 525, Pass docsLang through every ArgRow render in ArgsTable, not only the top ungrouped branch. Update the ArgRow call sites inside groups.ungroupedSubsections, section.ungrouped, and section.subsections so they all receive docsLang alongside the existing props, using the same pattern already present in the first ArgRow branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/addons/docs/src/blocks/blocks/DocsContainer.tsx`:
- Around line 31-42: The DocsContainer logic leaves lang implicitly typed,
triggering the noImplicitAnyLet lint rule. Update the lang declaration in
DocsContainer so it is explicitly a string (or initialized to a string value)
and keep the existing meta/projectAnnotations fallback assignment logic intact.
Use the lang variable in the try/catch block as the uniquely identifying symbol
to locate and fix this.
---
Outside diff comments:
In `@code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx`:
- Around line 472-525: Pass docsLang through every ArgRow render in ArgsTable,
not only the top ungrouped branch. Update the ArgRow call sites inside
groups.ungroupedSubsections, section.ungrouped, and section.subsections so they
all receive docsLang alongside the existing props, using the same pattern
already present in the first ArgRow branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3fe8e0a-8bb0-47d6-a622-bfbb799f2dcc
📒 Files selected for processing (11)
code/.storybook/preview.tsxcode/addons/docs/src/blocks/blocks/ArgTypes.tsxcode/addons/docs/src/blocks/blocks/Controls.tsxcode/addons/docs/src/blocks/blocks/Description.tsxcode/addons/docs/src/blocks/blocks/DocsContainer.tsxcode/addons/docs/src/blocks/components/ArgsTable/ArgRow.tsxcode/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsxcode/addons/docs/src/blocks/components/ArgsTable/TabbedArgsTable.tsxcode/addons/docs/src/blocks/components/TableOfContents.tsxcode/addons/docs/src/blocks/components/Toolbar.tsxcode/core/src/components/components/Toolbar/Toolbar.tsx
✅ Files skipped from review due to trivial changes (1)
- code/addons/docs/src/blocks/components/Toolbar.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.lang.test.tsx`:
- Around line 37-40: The grouped ArgsTable rows are missing docsLang
propagation, so section and subsection descriptions still inherit the table’s
default lang. Update ArgsTable.tsx so the same docsLang value passed to ArgRow
in groups.ungrouped is also forwarded in the ungroupedSubsections and sections
rendering paths, then extend ArgsTable.lang.test.tsx to cover grouped rows and
verify the description lang attribute matches the requested docs language.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db33f2b0-9a20-4c05-ab5e-0cb21a3079f9
📒 Files selected for processing (4)
code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.lang.test.tsxcode/addons/docs/src/blocks/components/DocsPage.stories.tsxcode/addons/docs/src/blocks/components/Preview.tsxcode/core/src/types/modules/csf.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/types/modules/csf.ts
Co-Authored-By: Steve Dodier-Lazaro <sidnioulz@gmail.com>
099757b to
520c701
Compare
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 188 | 0 |
| Self size | 79 KB | 79 KB | 0 B |
| Dependency size | 36.17 MB | 35.23 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 160 KB | 160 KB | 0 B |
| Dependency size | 31.44 MB | 30.50 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 187 | 187 | 0 |
| Self size | 15 KB | 15 KB | 0 B |
| Dependency size | 31.52 MB | 30.57 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 533 | 533 | 0 |
| Self size | 663 KB | 662 KB | 🎉 -120 B 🎉 |
| Dependency size | 62.88 MB | 61.94 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 93 | 93 | 0 |
| Self size | 1.39 MB | 1.39 MB | 🎉 -48 B 🎉 |
| Dependency size | 24.93 MB | 23.98 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 122 | 122 | 0 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 26.00 MB | 25.05 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 36 KB | 36 KB | 0 B |
| Dependency size | 22.70 MB | 21.76 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 274 | 274 | 0 |
| Self size | 23 KB | 23 KB | 0 B |
| Dependency size | 48.66 MB | 47.72 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 200 | 200 | 0 |
| Self size | 16 KB | 16 KB | 🎉 -12 B 🎉 |
| Dependency size | 37.43 MB | 36.49 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 84 | 84 | 0 |
| Self size | 110 KB | 110 KB | 0 B |
| Dependency size | 22.74 MB | 21.79 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 204 | 204 | 0 |
| Self size | 821 KB | 821 KB | 🎉 -84 B 🎉 |
| Dependency size | 92.62 MB | 91.68 MB | 🎉 -941 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 197 | 197 | 0 |
| Self size | 32 KB | 32 KB | 🎉 -72 B 🎉 |
| Dependency size | 91.10 MB | 90.16 MB | 🎉 -941 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 163 | 163 | 0 |
| Self size | 19 KB | 19 KB | 🎉 -24 B 🎉 |
| Dependency size | 33.15 MB | 32.20 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.52 MB | 1.52 MB | 🚨 +6 B 🚨 |
| Dependency size | 13.38 MB | 12.43 MB | 🎉 -942 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
github-project-automation
Bot
moved this from In Progress
to Done
in Core Team Projects
Sidnioulz
added
qa:success
|
We've been studying how accessibility changes get reviewed across open source, and this PR is one of seven case studies in a public corpus: https://github.com/ecogetaway/oss-accessibility-inclusion Posting here because we'd rather you heard it from us than came across it, and because this case is the one we've thought hardest about. The method, brieflyEach case is scored against a six-criterion rubric published before any scoring, with every per-criterion justification public so any score can be disputed. The rubric deliberately does not judge whether a fix was technically correct, and it does not measure effort — it asks whether the review process could have verified the accessibility claim. Two of our own scores have already changed after re-verification. This case: 4/12. Two things Storybook does better than the rest of the corpusThis PR actually reached users. Four of the seven cases merged into unreleased branches or were closed because an unreleased rewrite superseded them — so the people who reported those defects still have them. This one shipped. That sounds trivial and isn't: it's the difference between a fix and a record of a fix. The labelling is correct. This carries Why the score is low despite the most review activity in the corpusThis is the most heavily reviewed pull request in our seven: nine actionable comments across three separate automated passes, which found four genuine defects in the mechanism — blank language values rendering as an empty attribute, settings not reaching title and subtitle, an English override covering caller-supplied content, and the language dropping on grouped rows. Those are real findings and the review did its job. Our rubric scores something different: whether anyone established that a screen reader actually announces the right language afterwards. That question isn't answerable from source code at all — someone has to render it and listen — and we found no evidence it was asked. We want to be explicit that this is not an argument that automated review is bad. It's a property of the problem, not a failing of the tool. Within the part of the job available to it, the review performed well. The concern is subtler, and it's the reason this case is in the corpus at all: nine actionable comments, four real defects and a green build read as a change that has been scrutinised. A maintainer with nine open PRs on a Thursday evening does not look hardest at the one that already looks reviewed. High-quality mechanism review may absorb the attention that outcome verification needed. The control case is instructive — another project in our set ran the same kind of automated review, three passes, and there a human separately opened the component with VoiceOver before approving. Same tooling, opposite outcome. The variable was never whether a bot reviewed. One observation on the testing checklistThe PR body invites the reader to run their favourite screen reader and observe how it handles languages. That's precisely the right instruction — it was phrased as a suggestion rather than a requirement, and we found no record of anyone doing it. That gap is exactly what our proposed template field is trying to close, and this PR is part of why we think it's worth having. Correction — 4 August. Two changes since posting. The labelling paragraph read "three that shipped"; it now reads "three that merged," which is what the paragraph above it already says — those three went to an unreleased branch. Separately, I've identified that commit 520c701 here is co-authored, which my write-up didn't reflect; the case study is corrected. The finding is unchanged: co-authorship is collaboration, not independent verification, and no screen-reader result was recorded before merge. |
4 participants
Closes #11706
What I did
langattribute to be set at the root of storieslangattribute for docs content (descriptions, etc.)lang="en"requiredattribute accessibility in ArgRow because I noticed it there and then and it was a trivial changeThis should ensure the whole tree is declaring the right language for screen readers regardless of what our users need to do.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Open the UI. Write some docgen in Spanish and some stories in French, or something like that. Run your favourite screenreader and observe how it handles languages as you go through the UI.
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.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
htmlLang) and docs prose/content (docs.lang), with end-to-end propagation to rendered docs and embedded story elements.langforwarding support across docs wrappers, args/controls tables, and interactive docs stories.langpropagation in stories, docs, and preview rendering behavior.