[WIP] #31267 Sidebar 2.0 #33422
[WIP] #31267 Sidebar 2.0 #33422
Conversation
|
|
View your CI Pipeline Execution ↗ for commit 62356e7
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThis PR introduces ARIA label rendering support to Storybook's sidebar and mobile navigation. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
✨ Finishing touches
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/manager/components/sidebar/Tree.tsx (1)
690-710:Rootcomponent missing multiple required props fromNodePropsinterface.The
Rootcomponent is typed asNodeProps & { expandableDescendants: string[] }but at lines 690-710 is missing required props:isDevelopment,docsMode,statuses, andgroupStatus. WhenRootdelegates to<Node {...props} />, these missing props cascade to the child component. In particular,isDevelopmentis used inNodefor conditional spacer rendering (itemStatusButton || isDevelopment ? <SpacerDiv /> : null), so root nodes must pass it for correct layout. Compare with the non-rootNodecomponent call at line 720, which correctly provides all required props.
🧹 Nitpick comments (1)
docs/_snippets/storybook-manager-render-label-stories.md (1)
8-10: Documentation correctly reflects the new API signature.The example demonstrates the extended
renderLabelsignature with theapiand{ location }context parameters. The location-aware conditional logic is clear and practical.Consider also showing
isMobileusage in the context destructuring for completeness, as the type includes both{ isMobile, location }.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
code/.storybook/manager.tsxcode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsxcode/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Explorer.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/core/src/manager/components/sidebar/Refs.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/types/modules/api-stories.tscode/core/src/types/modules/api.tsdocs/_snippets/storybook-manager-render-label-stories.mddocs/configure/user-interface/features-and-behavior.mdx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxdocs/_snippets/storybook-manager-render-label-stories.mdcode/core/src/types/modules/api.tscode/core/src/manager/components/sidebar/Explorer.tsxcode/core/src/types/modules/api-stories.tscode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxcode/core/src/types/modules/api.tscode/core/src/manager/components/sidebar/Explorer.tsxcode/core/src/types/modules/api-stories.tscode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxcode/core/src/types/modules/api.tscode/core/src/manager/components/sidebar/Explorer.tsxcode/core/src/types/modules/api-stories.tscode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxcode/core/src/types/modules/api.tscode/core/src/manager/components/sidebar/Explorer.tsxcode/core/src/types/modules/api-stories.tscode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxcode/core/src/types/modules/api.tscode/core/src/manager/components/sidebar/Explorer.tsxcode/core/src/types/modules/api-stories.tscode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/manager-api/tests/stories.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/manager-api/tests/stories.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Usevi.mock()to mock file system, loggers, and other external dependencies in tests
Files:
code/core/src/manager-api/tests/stories.test.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/manager/components/preview/Toolbar.tsx:102-105
Timestamp: 2025-10-03T07:55:42.639Z
Learning: In code/core/src/manager/components/preview/Toolbar.tsx, we intentionally do not add aria-label/aria-labelledby to StyledToolbar (AbstractToolbar) because the enclosing section is already labeled via an sr-only heading and the toolbar is the only content. Revisit only if real user testing indicates a need.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 33140
File: code/core/src/manager/components/sidebar/TagsFilter.tsx:247-259
Timestamp: 2025-11-25T11:09:33.798Z
Learning: In the storybookjs/storybook repository, PopoverProvider creates popovers with a dialog role, so using aria-haspopup="dialog" on buttons that trigger PopoverProvider is semantically correct.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:32-37
Timestamp: 2025-09-18T20:51:52.320Z
Learning: NVDA screen reader announces the selected value in Select components even when aria-label is used, possibly due to special treatment of the role or aria-expanded attribute. Real-world screen reader testing may reveal that theoretical ARIA concerns don't always apply in practice.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/.storybook/manager.tsxcode/core/src/manager/components/sidebar/Tree.tsxdocs/_snippets/storybook-manager-render-label-stories.mdcode/core/src/manager/components/sidebar/Explorer.tsxcode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/.storybook/manager.tsxcode/core/src/manager/components/sidebar/Tree.tsx
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/components/sidebar/Explorer.stories.tsx
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Applied to files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/.storybook/manager.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager/components/sidebar/StatusButton.tsxdocs/_snippets/storybook-manager-render-label-stories.mdcode/core/src/types/modules/api.tsdocs/configure/user-interface/features-and-behavior.mdxcode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/components/sidebar/Explorer.stories.tsxcode/core/src/manager/components/sidebar/Refs.stories.tsxcode/core/src/manager/components/sidebar/Tree.tsxcode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.ts
📚 Learning: 2025-10-03T07:55:42.639Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/manager/components/preview/Toolbar.tsx:102-105
Timestamp: 2025-10-03T07:55:42.639Z
Learning: In code/core/src/manager/components/preview/Toolbar.tsx, we intentionally do not add aria-label/aria-labelledby to StyledToolbar (AbstractToolbar) because the enclosing section is already labeled via an sr-only heading and the toolbar is the only content. Revisit only if real user testing indicates a need.
Applied to files:
code/.storybook/manager.tsxdocs/configure/user-interface/features-and-behavior.mdxcode/core/src/manager-api/lib/stories.tscode/core/src/manager-api/tests/stories.test.tscode/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/.storybook/manager.tsxcode/core/src/manager-api/lib/stories.ts
📚 Learning: 2025-09-18T20:51:52.320Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:32-37
Timestamp: 2025-09-18T20:51:52.320Z
Learning: NVDA screen reader announces the selected value in Select components even when aria-label is used, possibly due to special treatment of the role or aria-expanded attribute. Real-world screen reader testing may reveal that theoretical ARIA concerns don't always apply in practice.
Applied to files:
code/core/src/manager/components/sidebar/Tree.tsx
🧬 Code graph analysis (2)
code/core/src/types/modules/api.ts (1)
code/core/src/types/modules/api-stories.ts (1)
API_HashEntry(68-74)
code/core/src/types/modules/api-stories.ts (1)
code/core/src/types/modules/api.ts (1)
API_SidebarOptions(108-128)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
- GitHub Check: nx
- GitHub Check: nx
🔇 Additional comments (19)
code/core/src/manager/components/sidebar/StatusButton.tsx (1)
34-46: Width and height are hardcoded to 28×28 pixels with no impact on existing usage.The
StyledButtoncomponent no longer acceptsheightorwidthas customizable props—they are now fixed at 28×28 pixels in the CSS styling. However, verification across the codebase shows no existingStatusButtonusages pass these props, so this change does not break any current code.docs/configure/user-interface/features-and-behavior.mdx (1)
39-49: LGTM! Clear documentation for new ARIA label rendering API.The documentation clearly explains the new
renderAriaLabelfunction, the updatedrenderLabelsignature with context, and provides helpful guidance about conditional rendering in thebottom-barlocation. The parameter table is well-structured.code/core/src/manager/components/mobile/navigation/MobileNavigation.tsx (2)
51-56: Fallback chain for ARIA label looks correct.The fallback logic where
fullStoryAriaLabeldefaults tofullStoryName(which itself defaults tocurrentStory.name) ensures there's always an accessible name even whenrenderAriaLabelis not provided.
119-123: Good accessibility pattern: separate visual and ARIA labels.Using
fullStoryNamefor the visible button text andfullStoryAriaLabelin the sr-only span ensures screen reader users get the full path context while visual users see a potentially truncated version.code/.storybook/manager.tsx (1)
6-6: LGTM! Clear ARIA label format.The
renderAriaLabelfunction provides a descriptive format (${type} ${name}) that helps screen reader users understand the nature of each sidebar item.code/core/src/manager/components/sidebar/Explorer.stories.tsx (1)
84-84: LGTM! Stories updated to exercise new prop.Adding
isDevelopment={true}to both stories ensures the new prop is exercised in the component stories.Also applies to: 98-98
code/core/src/manager/components/sidebar/Sidebar.tsx (1)
215-215: LGTM! Clean prop forwarding to child components.The
isDevelopmentprop is correctly forwarded to bothExplorerandSidebarBottom, with a sensible default derived fromglobal.CONFIG_TYPE.Also applies to: 233-233
code/core/src/manager-api/tests/stories.test.ts (1)
1506-1507: LGTM! Snapshots updated for newrenderAriaLabelfield.The inline snapshots correctly reflect the new
renderAriaLabel: undefinedfield in the filtered index entries, consistent with the type changes whererenderAriaLabelis optional and defaults to undefined when not configured.Also applies to: 1518-1519, 1532-1533
code/core/src/manager/components/sidebar/Refs.tsx (1)
18-18: LGTM! Prop threading from Refs to Tree.The
isDevelopmentprop is correctly added toRefProps, destructured, and forwarded to theTreecomponent.Also applies to: 85-85, 156-156
code/core/src/manager/components/sidebar/Explorer.tsx (1)
12-12: LGTM! Clean prop threading pattern.The
isDevelopmentprop is correctly added toExplorerProps, destructured in the component, and forwarded to eachRefchild component.Also applies to: 22-22, 50-50
code/core/src/types/modules/api.ts (1)
112-127: API surface expansion looks good.The
renderAriaLabelandrenderLabelsignatures are properly typed with consistent context objects. Theanyreturn type provides flexibility for custom renderers.code/core/src/manager/components/sidebar/Refs.stories.tsx (1)
191-202: Consistent addition ofisDevelopmentprop across all stories.The
isDevelopmentprop is properly added to allRefcomponent instances, enabling development-mode rendering in these story examples. The pattern is consistent and aligns with the prop propagation through Explorer → Refs → Tree.code/core/src/types/modules/api-stories.ts (1)
12-13: Type alignment withAPI_SidebarOptionsis correct.Using indexed access types (
API_SidebarOptions['renderAriaLabel']andAPI_SidebarOptions['renderLabel']) ensures type consistency between the base entry and sidebar options. This is a clean approach to avoid duplication.code/core/src/manager/components/sidebar/Tree.tsx (3)
202-205: Clean spacer component for development-mode alignment.The
SpacerDivstyled component provides consistent 28x28 spacing to maintain visual alignment when no status button is rendered.
232-233: Render context correctly scoped to sidebar.The
renderContextis properly constructed withisMobilefrom the layout hook andlocation: 'sidebar'hardcoded, which is appropriate since this is the Tree component used exclusively in the sidebar.
462-464: Conditional spacer logic for development mode.The logic correctly renders a
SpacerDivwhen eitheritemStatusButtonexists ORisDevelopmentis true, ensuring consistent alignment during development. However, the conditionitemStatusButton || isDevelopmentwill render the spacer even whenitemStatusButtonis truthy, which seems redundant sinceitemStatusButtonwould be rendered instead.The current logic:
showBranchStatus→ render status button with iconitemStatusButton || isDevelopment→ render spacerThis means when
itemStatusButtonis truthy butshowBranchStatusis false, you'll render a spacer instead of the button. Is this intentional?code/core/src/manager-api/lib/stories.ts (3)
208-208: Proper extraction ofrenderAriaLabelfrom sidebar config.The
renderAriaLabelis correctly destructured alongside existing sidebar options and will be propagated to all hash entries.
251-262: Consistent ARIA label propagation to root entries.The
renderAriaLabelis properly added toAPI_RootEntryalongsiderenderLabel, maintaining consistency in the entry structure.
302-311: ARIA label support for docs/story entries.The
renderAriaLabelandrenderLabelare correctly spread onto the final docs/story entries, completing the propagation chain from sidebar config through all entry types.
| @@ -1,8 +1,9 @@ | |||
| import { startCase } from 'es-toolkit/string'; | |||
| import { startCase, upperCase } from 'es-toolkit/string'; | |||
There was a problem hiding this comment.
Unused import: upperCase.
The upperCase function is imported but not used anywhere in this file.
Suggested fix
-import { startCase, upperCase } from 'es-toolkit/string';
+import { startCase } from 'es-toolkit/string';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { startCase, upperCase } from 'es-toolkit/string'; | |
| import { startCase } from 'es-toolkit/string'; |
🤖 Prompt for AI Agents
In code/.storybook/manager.tsx around line 1, the file imports upperCase from
'es-toolkit/string' but never uses it; remove the unused upperCase import (leave
startCase) to clean up the import statement and avoid lint warnings, ensuring
the import list only includes used symbols.
| // Limit length of name shown in UI due to layout constraints. | ||
| if (fullStoryName.length > 24) { | ||
| fullStoryName = `${parentName}/${fullStoryName}`; | ||
| } | ||
| fullStoryAriaLabel = `${parentAriaLabel}/${fullStoryAriaLabel}`; | ||
| } |
There was a problem hiding this comment.
Potential logic inversion in length heuristic.
The comment says "Limit length of name shown in UI due to layout constraints," but the condition appears inverted. When fullStoryName.length > 24, you're adding more content (parentName/), making the string longer rather than shorter.
Did you intend to skip prepending when the name is already long? For example:
- // Limit length of name shown in UI due to layout constraints.
- if (fullStoryName.length > 24) {
+ // Stop prepending parent names when the name is short enough for UI constraints.
+ if (fullStoryName.length <= 24) {
fullStoryName = `${parentName}/${fullStoryName}`;
}Or perhaps the intent is different—please clarify the expected behavior.
🤖 Prompt for AI Agents
In code/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
around lines 67 to 72, the length check is inverted: currently it prepends
parentName when fullStoryName.length > 24 which increases length instead of
limiting it. Change the condition to only prepend parentName when the story name
is short enough (e.g., if fullStoryName.length <= 24) and apply the same guard
to fullStoryAriaLabel so you don’t always lengthen the visible/aria string when
it’s already long.
| @@ -1,4 +1,4 @@ | |||
| import type { StatusByTypeId } from '../../../dist/types'; | |||
| import type { API_SidebarOptions, StatusByTypeId } from '../../../dist/types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, understand the file structure relative to api-stories.ts
echo "=== File location and context ==="
find . -name "api-stories.ts" -type f
# Find where API_SidebarOptions is defined
echo "=== Searching for API_SidebarOptions definition ==="
rg -n "export.*API_SidebarOptions" --type ts --type tsx
# Find where StatusByTypeId is defined
echo "=== Searching for StatusByTypeId definition ==="
rg -n "export.*StatusByTypeId" --type ts --type tsxRepository: storybookjs/storybook
Length of output: 316
🏁 Script executed:
#!/bin/bash
# Check the structure of the dist directory and if it's checked in or generated
echo "=== Checking dist directory structure ==="
find . -path "*/dist/types*" -type f | head -20
# Check if dist is in gitignore
echo "=== Checking .gitignore for dist ==="
cat .gitignore 2>/dev/null | grep -E "dist|build" | head -10
# Check for other imports from dist paths (common pattern?)
echo "=== Searching for other imports from dist paths ==="
rg "from ['\"].*dist/" --type ts --type tsx | head -20Repository: storybookjs/storybook
Length of output: 266
🏁 Script executed:
#!/bin/bash
# Read the actual file to understand context
echo "=== Content of api-stories.ts ==="
cat -n code/core/src/types/modules/api-stories.ts | head -30Repository: storybookjs/storybook
Length of output: 1148
🏁 Script executed:
#!/bin/bash
# Fix file type issue - search without type specification
echo "=== Searching for API_SidebarOptions definition (no type filter) ==="
rg -n "API_SidebarOptions" | head -30
echo ""
echo "=== Searching for StatusByTypeId definition ==="
rg -n "StatusByTypeId" | head -20
echo ""
echo "=== Understanding the path structure ==="
# From code/core/src/types/modules/api-stories.ts
# ../../../dist/types would resolve to code/core/dist/types
ls -la code/core/dist/types 2>/dev/null | head -20Repository: storybookjs/storybook
Length of output: 1646
🏁 Script executed:
#!/bin/bash
# Check where these types actually originate from in source
echo "=== Check what code/core/src contains ==="
find code/core/src -name "*.ts" -type f | grep -E "(index|types)" | head -20
echo ""
echo "=== Look for type definitions in code/core/src ==="
find code/core/src -maxdepth 3 -name "*.ts" | xargs grep -l "API_SidebarOptions" 2>/dev/null
echo ""
echo "=== Check if there's a types build output ==="
ls -la code/core/dist 2>/dev/null | head -20Repository: storybookjs/storybook
Length of output: 1070
🏁 Script executed:
#!/bin/bash
# Check what's actually in code/core/src/types/modules/api.ts
echo "=== Checking api.ts for exported types ==="
grep -n "export" code/core/src/types/modules/api.ts | head -30
echo ""
echo "=== Check status.ts to see what it exports ==="
grep -n "export" code/core/src/types/modules/status.ts | head -20
echo ""
echo "=== Check if dist/types exists and what pattern is used elsewhere ==="
find code/core/src/types/modules -name "*.ts" -exec grep -l "from.*\.\/\|from.*\.\.\/" {} \; | head -10 | xargs -I {} sh -c 'echo "=== {} ===" && head -15 {}'Repository: storybookjs/storybook
Length of output: 6084
Fix import path from build artifact to source.
Import API_SidebarOptions and StatusByTypeId from their source locations instead of the dist folder, which is generated at build time. Change the import at line 1 from '../../../dist/types' to source paths: use './api' for API_SidebarOptions and './status' for StatusByTypeId. All other type modules in the codebase follow this pattern.
🤖 Prompt for AI Agents
In code/core/src/types/modules/api-stories.ts around line 1, the file currently
imports API_SidebarOptions and StatusByTypeId from the build artifact
'../../../dist/types'; update the imports to reference the source modules
instead—import API_SidebarOptions from './api' and StatusByTypeId from
'./status'—so the types come from source files rather than the generated dist
folder, matching the pattern used by other type modules.
| import type { is } from '@babel/types'; | ||
|
|
There was a problem hiding this comment.
Unused import detected.
The is type from @babel/types is imported but not used anywhere in this file. This appears to be accidental.
🔎 Proposed fix
-import type { is } from '@babel/types';
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { is } from '@babel/types'; |
🤖 Prompt for AI Agents
In code/core/src/types/modules/api.ts around lines 3 to 4, the import "import
type { is } from '@babel/types';" is unused; remove this import line to
eliminate the dead import and keep imports minimal, then run typecheck/lint to
confirm no remaining references.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 49 | 49 | 0 |
| Self size | 20.19 MB | 19.92 MB | 🎉 -270 KB 🎉 |
| Dependency size | 16.52 MB | 16.52 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 775 KB | 775 KB | 🎉 -152 B 🎉 |
| Dependency size | 67.24 MB | 66.97 MB | 🎉 -270 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 65.82 MB | 65.55 MB | 🎉 -270 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 999 KB | 999 KB | 🚨 +42 B 🚨 |
| Dependency size | 36.71 MB | 36.44 MB | 🎉 -270 KB 🎉 |
| Bundle Size Analyzer | node | node |
Summary by CodeRabbit
Release Notes
New Features
renderAriaLabelconfiguration option to customize sidebar accessibility labels.renderLabelcallback with context parameter providing location and mobile state information.Bug Fixes
Breaking Changes
renderLabelcallback signature updated to include context parameter.✏️ Tip: You can customize this high-level summary in your review settings.