UI: Make preview Toolbar and core Bar implement the toolbar role #32270
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
View your CI Pipeline Execution ↗ for commit 85a2462
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 2 | 2 | 0 |
| Self size | 507 KB | 486 KB | 🎉 -21 KB 🎉 |
| Dependency size | 2.80 MB | 2.80 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-onboarding
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 368 KB | 333 KB | 🎉 -35 KB 🎉 |
| Dependency size | 667 B | 667 B | 0 B |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 43 | 44 | 🚨 +1 🚨 |
| Self size | 31.96 MB | 35.35 MB | 🚨 +3.39 MB 🚨 |
| Dependency size | 17.30 MB | 17.33 MB | 🚨 +29 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 529 | 528 | 🎉 -1 🎉 |
| Self size | 939 KB | 939 KB | 0 B |
| Dependency size | 58.25 MB | 58.22 MB | 🎉 -32 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 187 | 188 | 🚨 +1 🚨 |
| Self size | 886 KB | 886 KB | 🎉 -120 B 🎉 |
| Dependency size | 81.55 MB | 84.97 MB | 🚨 +3.42 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 169 | 170 | 🚨 +1 🚨 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 77.98 MB | 81.40 MB | 🚨 +3.42 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 44 | 45 | 🚨 +1 🚨 |
| Self size | 1.55 MB | 1.55 MB | 0 B |
| Dependency size | 49.26 MB | 52.68 MB | 🚨 +3.42 MB 🚨 |
| Bundle Size Analyzer | node | node |
Sidnioulz
force-pushed
the
sidnioulz/issue-25918-toolbar
branch
3 times, most recently
from
fdb96ac to
d85f513
Compare
Sidnioulz
force-pushed
the
sidnioulz/issue-25918-toolbar
branch
from
1ebf8e3 to
1da55db
Compare
| emptyState, | ||
| showToolsWhenEmpty, | ||
| }) => { | ||
| deprecate('The `Tabs` component is deprecated. Use `AriaTabs` instead.'); |
There was a problem hiding this comment.
Must be revisited based on target removal date/version for Tabs and on final name for AriaTabs.
| } | ||
|
|
||
| export const FlexBar = ({ children, backgroundColor, className, ...rest }: FlexBarProps) => { | ||
| deprecate('FlexBar is deprecated. Use Bar with justifyContent: "space-between" instead.'); |
There was a problem hiding this comment.
TODO: clarify date/version of removal.
Sidnioulz
force-pushed
the
sidnioulz/issue-25918-toolbar
branch
2 times, most recently
from
cb54314 to
1e92287
Compare
test-storybooks/portable-stories-kitchen-sink/react/e2e-tests/save-from-controls.spec.ts
Outdated
Show resolved
Hide resolved
Sidnioulz
changed the title
Sidnioulz
force-pushed
the
sidnioulz/issue-25918-toolbar
branch
from
1e92287 to
62c4a8a
Compare
|
I added |
Sidnioulz
force-pushed
the
sidnioulz/issue-25918-toolbar
branch
2 times, most recently
from
81ed808 to
61f9908
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces legacy Tabs/TabBar/FlexBar UI with new ARIA-first components (AriaTabs, AriaTabList, AriaTabPanel, AriaToolbar), refactors multiple manager/addon/component-testing surfaces to use them, removes old tabs modules/stories, adds react-aria/react-stately and related runtime externals, and updates e2e tests to role-based selectors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as AriaTabs (Tab Bar)
participant State as useAriaTabListState
participant Manager as Manager / Actions
participant Panel as Panel Content (TabPanel)
User->>UI: Click or keyboard-select tab
UI->>State: onSelectionChange(key)
State-->>UI: selectedKey
UI->>Manager: onSelectionChange(key) (e.g., actions.onSelect)
Manager->>Panel: render selected panel (wrapped in TabErrorBoundary)
Panel-->>User: display active panel content
Note right of UI: Keyboard navigation & ARIA semantics handled by react-aria/react-stately
sequenceDiagram
autonumber
actor User
participant PreviewToolbar as Preview Toolbar (AriaTabList)
participant TabState as useAriaTabListState
participant Router as Manager (query params)
participant Preview as Canvas / Tab Panels
User->>PreviewToolbar: Select tab
PreviewToolbar->>TabState: update selection
TabState->>Router: update selected tabId (query param)
Router->>Preview: compute visibility (canvas vs selected tab)
Preview-->>User: show canvas or selected addon panel
Note over Router,Preview: deprecation warning emitted when >1 addon tab
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/jest/src/components/Panel.tsx (1)
285-291: Update Jest addon docs linkReplace the GitHub href in code/addons/jest/src/components/Panel.tsx (lines 285–291) with Storybook’s current Jest addon docs URL: https://storybook.js.org/addons/%40junk-temporary-prototypes/addon-jest
♻️ Duplicate comments (9)
code/addons/themes/package.json (1)
75-75: gitHead metadata update looks fine.No functional changes here; see preact-vite comment for the repo-wide consistency check.
code/builders/builder-vite/package.json (1)
73-73: gitHead updated — no runtime impact.Nothing else changed; refer to the first comment for the validation script.
code/addons/docs/package.json (1)
121-121: gitHead change acknowledged.Metadata-only; align with repo-wide check from the first comment.
code/renderers/web-components/package.json (1)
74-74: gitHead bump LGTM.No additional concerns; see earlier note for consistency verification.
code/addons/links/package.json (1)
74-74: gitHead update only.Looks good; rely on the shared script to confirm SHA alignment.
code/addons/a11y/package.json (1)
82-82: gitHead updated; OK to proceed.No behavior change; cross-check with the repo-wide validation script.
code/builders/builder-webpack5/package.json (1)
92-92: gitHead metadata change confirmed.All good; refer to the initial comment for the consistency check.
code/core/src/components/components/AriaTabs/AriaTabList.stories.tsx (1)
32-44: Stories structure LGTM; parameters-based data resolves earlier TS issue.code/core/src/manager/components/preview/Toolbar.tsx (1)
112-114: Heading level in toolbar labelUsing an h2 may disrupt heading hierarchy; confirm this level is appropriate or consider
aria-labelwithout an actual heading.
🧹 Nitpick comments (54)
code/e2e-tests/component-tests.spec.ts (2)
34-36: Role-based locator switch LGTM; consider scoping to reduce ambiguity.Using getByRole aligns with the new ARIA Tabs. To avoid potential duplicate-name collisions, scope the locator to the addon panel tablist if available.
Example:
const interactionsTab = page.getByRole('tablist', { name: /Addon Panel/i }).getByRole('tab', { name: 'Interactions' });
67-69: Consistent locator; refresh adjacent skip text and typo.
- Keep this role-based locator.
- Line 22 skip reason still mentions “className locators” — update to reflect role-based locators or remove if obsolete.
- Line 55: fix “FIreFox” → “Firefox”.
scripts/build/utils/entry-utils.ts (1)
58-61: Also externalize the shim entry for use-sync-external-store.
If any packages importuse-sync-external-store/shim(common for React < 18), it won’t be externalized by listing only the base package. Add the shim path to avoid bundling it accidentally.Apply this diff:
const runtimeExternalInclude: string[] = [ 'react', - 'use-sync-external-store', + 'use-sync-external-store', + 'use-sync-external-store/shim', 'react-dom', 'react-dom/client', '@storybook/icons',code/e2e-tests/preview-api.spec.ts (1)
30-31: Prefer asserting selection state over visibility for tabsSince tabs now follow ARIA patterns, assert aria-selected to reduce flake after click.
- const interactionsTab = page.getByRole('tab', { name: 'Interactions' }); - await expect(interactionsTab).toBeVisible(); + const interactionsTab = page.getByRole('tab', { name: 'Interactions' }); + await expect(interactionsTab).toHaveAttribute('aria-selected', 'true');code/e2e-tests/addon-a11y.spec.ts (1)
69-72: Align check with ARIA semantics — good; consider updating remaining tab checksUsing aria-selected is the right assertion for tabs. For consistency, consider changing the later tab assertion (Line 78-79) to aria-selected as well to avoid relying on data-state.
- const element = page.getByRole('tab', { name: '1. <body' }); - await expect(element).toHaveAttribute('data-state', 'active'); + const element = page.getByRole('tab', { name: '1. <body' }); + await expect(element).toHaveAttribute('aria-selected', 'true');code/e2e-tests/addon-actions.spec.ts (2)
31-33: Make the text assertion slightly more robust.
Allow optional colon to reduce brittle coupling to punctuation.-const logItem = sbPage.panelContent().locator('span', { - hasText: 'onClick:', -}); +const logItem = sbPage.panelContent().locator('span', { + hasText: /onClick:?/, +});
55-57: Same here: loosen the match a bit.-const logItem = sbPage.panelContent().locator('span', { - hasText: 'console.log:', -}); +const logItem = sbPage.panelContent().locator('span', { + hasText: /console\.log:?/, +});code/e2e-tests/module-mocking.spec.ts (2)
45-49: Prefer Locator.nth() over CSS nth syntaxUse the Playwright API directly; it's clearer and less error‑prone.
- actualTexts.push(await panel.locator(`li >> nth=${i}`).innerText()); + actualTexts.push(await panel.locator('li').nth(i).innerText());
53-54: Remove noisy console logging (or guard behind a flag)Unnecessary logs slow CI and clutter output.
- console.log(actualTexts); + if (process.env.DEBUG_E2E) console.log(actualTexts);code/e2e-tests/manager.spec.ts (1)
42-51: Rename helper: it checks “hidden”, not “non‑existence”Name should reflect behavior to avoid confusion in future edits.
- const expectToolbarToNotExist = async () => { + const expectToolbarToBeHidden = async () => { const toolbar = page.getByTestId('sb-preview-toolbar').getByRole('toolbar'); await expect(toolbar).toBeHidden(); - }; + };Also update call sites:
- await expectToolbarToNotExist(); + await expectToolbarToBeHidden();code/addons/jest/src/components/Panel.tsx (2)
139-154: Remove unnecessary optional chainingtests is already an array; optional chaining hides real bugs.
- {tests.length ? ( - tests?.map((res: AssertionResult) => ( + {tests.length ? ( + tests.map((res: AssertionResult) => (
244-248: Optional: Select first non‑empty tab by defaultBetter UX: open the most relevant tab if “Failing” is empty.
- <AriaTabs - defaultSelected="failing-tests" + {/* + Prefer the first non-empty category; fall back to "todo-tests" + */} + <AriaTabs + defaultSelected={ + (testsByType.get(StatusTypes.FAILED_TYPE)?.length ?? 0) > 0 ? 'failing-tests' : + (testsByType.get(StatusTypes.PASSED_TYPE)?.length ?? 0) > 0 ? 'passing-tests' : + (testsByType.get(StatusTypes.PENDING_TYPE)?.length ?? 0) > 0 ? 'pending-tests' : + 'todo-tests' + } backgroundColor={convert(themes.light).background.hoverable} tabs={tabs} />code/core/src/components/components/bar/bar.tsx (2)
122-134: Avoid "undefined" in classNameGuard concatenation when className is falsy.
- <Bar backgroundColor={backgroundColor} className={`sb-bar ${className}`} {...rest}> + <Bar + backgroundColor={backgroundColor} + className={['sb-bar', className].filter(Boolean).join(' ')} + {...rest} + >
117-120: Drop duplicated props in FlexBarPropsborder/backgroundColor are already in BarProps. Keep a single source of truth.
-export interface FlexBarProps extends BarProps { - border?: boolean; - backgroundColor?: string; -} +export interface FlexBarProps extends BarProps {}code/core/src/controls/components/SaveStory.tsx (2)
44-48: Move responsive layout rules to Bar.innerStyleWith the new Bar, layout is driven by innerStyle on the inner wrapper; these @container rules on the outer wrapper won’t affect child ordering.
- '@container (max-width: 799px)': { - flexDirection: 'row', - justifyContent: 'flex-end', - },
149-155: Apply responsive rules via innerStyle (effective target)Ensure small‑viewport overrides hit the HeightPreserver wrapper.
- <Bar - innerStyle={{ - flexDirection: 'row-reverse', - justifyContent: 'space-between', - flexWrap: 'wrap', - }} - > + <Bar + innerStyle={{ + flexDirection: 'row-reverse', + justifyContent: 'space-between', + flexWrap: 'wrap', + '@container (max-width: 799px)': { + flexDirection: 'row', + justifyContent: 'flex-end', + }, + }} + >code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (1)
8-13: Broaden props to accept standard DOM attributes (id,data-*, etc.).Expose div attributes on the public API to avoid type friction and enable test hooks.
-export interface AbstractAriaToolbarProps { - className?: string; - children?: React.ReactNode; - 'aria-label'?: string; - 'aria-labelledby'?: string; -} +export interface AbstractAriaToolbarProps extends React.ComponentPropsWithoutRef<'div'> { + 'aria-label'?: string; + 'aria-labelledby'?: string; +}code/addons/docs/src/blocks/components/Toolbar.tsx (1)
55-64: Add buttonaria-labels and avoid unnecessarypreventDefault().Icons alone aren’t accessible; use explicit labels.
preventDefaulton a button click is redundant here.- <IconButton - key="zoomin" - onClick={(e: SyntheticEvent) => { - e.preventDefault(); - zoom(0.8); - }} - title="Zoom in" - > + <IconButton + key="zoomin" + aria-label="Zoom in" + onClick={() => zoom(0.8)} + title="Zoom in" + > <ZoomIcon /> </IconButton> @@ - <IconButton - key="zoomout" - onClick={(e: SyntheticEvent) => { - e.preventDefault(); - zoom(1.25); - }} - title="Zoom out" - > + <IconButton + key="zoomout" + aria-label="Zoom out" + onClick={() => zoom(1.25)} + title="Zoom out" + > <ZoomOutIcon /> </IconButton> @@ - <IconButton - key="zoomreset" - onClick={(e: SyntheticEvent) => { - e.preventDefault(); - resetZoom(); - }} - title="Reset zoom" - > + <IconButton + key="zoomreset" + aria-label="Reset zoom" + onClick={() => resetZoom()} + title="Reset zoom" + > <ZoomResetIcon /> </IconButton>Also applies to: 65-74, 75-84
code/core/src/components/components/AriaToolbar/AbstractAriaToolbar.stories.tsx (1)
19-25: Provide an accessible name in the story.Demonstrate best practice by labeling the toolbar.
args: { children: <Children />, + 'aria-label': 'Example toolbar', },code/core/src/component-testing/components/Toolbar.stories.tsx (1)
28-31: Optional: make “Scroll to end” interactive in stories.Provide
onScrollToEndso the button isn’t disabled in examples.args: { @@ storyFileName: 'Toolbar.stories.tsx', + onScrollToEnd: action('scrollToEnd'), hasNext: true, hasPrevious: true, },code/core/src/components/components/addon-panel/addon-panel.tsx (1)
8-17: Nit: type the refs for clarity.
useRef<any>()(or a generic) improves readability over implicitany.-const usePrevious = (value: any) => { - const ref = useRef(); +const usePrevious = <T,>(value: T) => { + const ref = useRef<T | undefined>(undefined); @@ -const useUpdate = (update: boolean, value: any) => { - const previousValue = usePrevious(value); +const useUpdate = <T,>(update: boolean, value: T) => { + const previousValue = usePrevious<T>(value);code/core/src/component-testing/components/Interaction.stories.tsx (2)
8-8: Decouple from another story’s meta; use shared fixtures instead.Importing args from
Toolbar.storiescreates cross‑story coupling and brittle dependencies. Prefer a small fixtures module.Apply this diff:
-import ToolbarStories from './Toolbar.stories'; +import { controlsFixture, controlStatesFixture } from './Toolbar.fixtures';- controls: ToolbarStories.args.controls, - controlStates: ToolbarStories.args.controlStates, + controls: controlsFixture, + controlStates: controlStatesFixture,And add a tiny fixture (new file) to support both stories:
// code/core/src/component-testing/components/Toolbar.fixtures.ts import type { Controls } from './InteractionsPanel'; export const controlsFixture: Controls = { start: () => {}, back: () => {}, goto: () => {}, next: () => {}, end: () => {}, rerun: () => {}, }; export const controlStatesFixture = { start: true, back: true, goto: true, next: true, end: true, detached: false, };Also applies to: 17-19
65-65:gotohas no visible effect in Toolbar; disable a control the Toolbar actually renders.The new Toolbar doesn’t expose a “goto” button, so
goto: falsewon’t show a disabled UI. Useend/next/backinstead.Apply this diff:
- args: { ...Done.args, controlStates: { ...ToolbarStories.args.controlStates, goto: false } }, + args: { ...Done.args, controlStates: { ...controlStatesFixture, end: false } },MIGRATION.md (3)
591-593: Duplicate paragraph and spelling error ("superceded").The "Removed x-only builtin tags" paragraph is duplicated and one instance misspells "superseded" as "superceded". Please dedupe and fix the spelling.
Apply this diff:
- During development of Storybook [Tags](https://storybook.js.org/docs/writing-stories/tags), we created `dev-only`, `docs-only`, and `test-only` built-in tags. These tags were never documented and superceded by the currently-documented `dev`, `autodocs`, and `test` tags which provide more precise control. The outdated `x-only` tags are removed in 10.0. + During development of Storybook [Tags](https://storybook.js.org/docs/writing-stories/tags), we created `dev-only`, `docs-only`, and `test-only` built-in tags. These tags were never documented and superseded by the currently-documented `dev`, `autodocs`, and `test` tags which provide more precise control. The outdated `x-only` tags are removed in 10.0.
609-611: Replace checkbox/TODO with a definitive statement and fix list style (MD004).Use a dash list and remove the question mark.
Apply this diff:
-* [ ] Tabs no longer accepts link components? +- Tabs no longer accepts link components.
11-15: Fix duplicated TOC anchors for "API and Component Changes"MIGRATION.md has two identical headings (lines 594 and 899) producing anchors
#api-and-component-changesand#api-and-component-changes-1; TOC entries at lines 11 and 34 point to them. Rename one heading or update the TOC links (e.g., add a version suffix) so each anchor uniquely targets its intended section.code/core/src/manager/components/preview/Preview.tsx (2)
66-77: Eagerly invoking tab render functions may be costly. Consider lazy panel rendering.
children: () => tab.render({ active: true })is executed during state construction (see useAriaTabListState), which can mount heavy content for all tabs. Prefer deferring panel rendering to the TabPanel layer and render only the selected tab.One approach: pass a lightweight placeholder in
childrenand move the actualtab.renderinvocation into the TabPanel renderer conditioned on selection. If that’s not feasible now, document the trade‑off and gate behind a feature flag for large addons.
79-81: Deprecation message: include removal version and actionable guidance.Message is good; consider adding a short pointer (e.g., “Use tool-type addons or AriaTabs-based panels”) to help users migrate.
code/core/src/components/components/AriaTabs/AriaTabList.stories.tsx (2)
15-18: Don’t pass unknowntabsprop to AriaTabList.It ends up as a non-standard DOM attribute on the container. Drop it from
args.Apply:
- args: { - tabs: DEFAULT_TABS, - state: undefined, - }, + args: {},
25-29: Striptabsfrom forwarded story args.Avoid leaking it to the component.
- (Story, { args, parameters }) => { - const state = useAriaTabListState({ tabs: parameters.data.tabs }); - return <Story args={{ ...args, state }} />; - }, + (Story, { args, parameters }) => { + const { tabs: _tabs, ...rest } = args as any; + const state = useAriaTabListState({ tabs: parameters.data.tabs }); + return <Story args={{ ...rest, state }} />; + },code/core/src/manager/settings/index.tsx (4)
21-29: Remove unusedHeader.Dead code; safe to delete.
-const Header = styled.div(({ theme }) => ({ - display: 'flex', - justifyContent: 'space-between', - alignItems: 'center', - height: 40, - boxShadow: `${theme.appBorderColor} 0 -1px 0 0 inset`, - background: theme.barBg, - paddingRight: 8, -}));
43-49: Drop unusedenableWhatsNewprop wiring.The tab is always present now; remove prop and its usage.
-const Pages: FC<{ - onClose: () => void; - enableShortcuts?: boolean; - changeTab: (tab: string) => void; - enableWhatsNew: boolean; -}> = ({ changeTab, onClose, enableShortcuts = true, enableWhatsNew }) => { +const Pages: FC<{ + onClose: () => void; + enableShortcuts?: boolean; + changeTab: (tab: string) => void; +}> = ({ changeTab, onClose, enableShortcuts = true }) => {return ( <Pages - enableWhatsNew={state.whatsNewData?.status === 'SUCCESS'} enableShortcuts={state.ui.enableShortcuts} changeTab={changeTab} onClose={api.closeSettings} /> );Also applies to: 128-134
96-116: Provide a deterministic default selection.Avoid controlled/uncontrolled ambiguity when the URL is
/settings/.- const selected = tabs.find((tab) => path.includes(`settings/${tab.id}`))?.id; + const selected = tabs.find((tab) => path.includes(`settings/${tab.id}`))?.id ?? 'about';
105-111:preventDefault()is unnecessary on a button.Can be removed.
- onClick={(e: SyntheticEvent) => { - e.preventDefault(); - return onClose(); - }} + onClick={() => onClose()}code/core/src/component-testing/components/Toolbar.tsx (1)
165-169: Hook up the rerun spin animation to status.Tiny UX improvement; no behavior change.
- <RerunButton aria-label="Rerun" onClick={controls.rerun}> + <RerunButton + aria-label="Rerun" + onClick={controls.rerun} + animating={status === 'playing' || status === 'rendering'} + >code/core/src/components/components/AriaTabs/AriaTabList.tsx (3)
8-11: UnifyAriaTabListPropsinto a single declaration.Duplicate interface declarations are confusing; prefer one
extends HTMLAttributes<HTMLDivElement>.-export interface AriaTabListProps { - state: TabListState<object>; -} +export interface AriaTabListProps extends HTMLAttributes<HTMLDivElement> { + state: TabListState<object>; +} -export interface AriaTabListProps extends HTMLAttributes<HTMLDivElement> { - state: TabListState<object>; -} +// (second declaration removed)Also applies to: 96-98
90-93: Type the tab ref to the actual element.Minor typing hygiene.
- const tabRef = React.useRef(null); + const tabRef = React.useRef<HTMLButtonElement>(null);
53-66: Add a disabled visual state.Improve affordance when tabs are disabled.
({ isSelected, textColor, theme }) => isSelected ? { color: textColor || theme.barSelectedColor, borderBottomColor: theme.barSelectedColor, } : { color: textColor || theme.barTextColor, borderBottomColor: 'transparent', '&:hover': { color: theme.barHoverColor, }, } + , + ({ isDisabled }) => + isDisabled + ? { + opacity: 0.5, + cursor: 'not-allowed', + pointerEvents: 'none', + } + : {}code/core/src/components/components/AriaToolbar/AriaToolbar.stories.tsx (2)
24-27: Add an accessible name to the toolbar in storiesDemonstrate best practice by labeling the toolbar so ATs announce it.
Apply this diff:
const meta = preview.meta({ title: 'AriaToolbar', component: AriaToolbar, args: { + 'aria-label': 'Example toolbar', children: <Children />, }, });
35-36: Remove redundant JSX braces in renderMinor cleanup; no behavior change.
- render: (args) => <div style={{ width: 400 }}>{<AriaToolbar {...args} />}</div>, + render: (args) => <div style={{ width: 400 }}><AriaToolbar {...args} /></div>,Also applies to: 42-43
code/addons/a11y/src/components/Tabs.tsx (1)
92-96: Ensure toggle button announces pressed stateIf IconButton’s
activedoes not map toaria-pressed, add it explicitly.Apply this diff if needed:
- <ToggleButton onClick={toggleHighlight} active={highlighted}> + <ToggleButton onClick={toggleHighlight} active={highlighted} aria-pressed={highlighted}>Please confirm whether IconButton sets
aria-pressedwhenactiveis true. If it already does, skip this change.code/core/src/components/components/AriaTabs/AriaTabPanel.stories.tsx (1)
104-113: Reduce DOM-coupling in assertionsThe
.parentNode?.parentNode?...chain is brittle. Preferclosest('[hidden]')or querying the panel container directly.code/core/src/components/components/AriaTabs/AriaTabPanel.tsx (2)
38-40: Type the ref targetTiny improvement to aid editor tooling and prevent accidental misuse.
- const ref = useRef(null); + const ref = useRef<HTMLDivElement>(null);
27-30: Verify overflow behavior whenhasScrollbaris falseWith
overflowY: 'hidden'on Panel, disabling the internal ScrollArea might clip content. Confirm the “WithoutScrollbar” story scrolls as intended via an ancestor container; if not, set Panel’soverflowYto'auto'whenhasScrollbaris false.Also applies to: 56-61
code/core/src/manager/components/preview/Toolbar.tsx (2)
105-127: Don’t render an empty toolbarArrays are truthy; current check always renders when
isShownis true. Gate on lengths.- return isShown && (tabs || tools || toolsExtra) ? ( + return isShown && (tabs.length > 0 || tools.length > 0 || toolsExtra.length > 0) ? (
195-204: Optional: Group tool clusters for semanticsConsider
role="group"witharia-labelon eachToolGroupto help AT users understand left vs. right clusters.Also applies to: 131-137
code/core/src/components/components/AriaTabs/AriaTabs.stories.tsx (1)
74-97: Controlled story doesn’t resync when args.selected changes.If the Controls panel updates selected, local state won’t follow. Sync it to avoid desync.
- import { useState } from 'react'; + import { useEffect, useState } from 'react'; @@ render: (args) => { const [selected, setSelected] = useState(args.selected); + useEffect(() => { + setSelected(args.selected); + }, [args.selected]); return (Also applies to: 1-1
code/core/src/manager/components/panel/Panel.tsx (4)
96-99: Add rel="noopener noreferrer" to external link opened in a new tab.Prevents opener hijacking and improves perf.
- <Link href={'https://storybook.js.org/addons?ref=ui'} target="_blank" withArrow> + <Link + href={'https://storybook.js.org/addons?ref=ui'} + target="_blank" + rel="noopener noreferrer" + withArrow + >
103-128: IconButtons rely on title only; add aria-labels for consistent SR names.Title isn’t reliably announced across AT. Mirror the title in aria-label.
- <IconButton + <IconButton key="position" onClick={actions.togglePosition} title={`Change addon orientation [${shortcutToHumanString(shortcuts.panelPosition)}]`} + aria-label="Change addon orientation" > @@ - <IconButton + <IconButton key="visibility" onClick={actions.toggleVisibility} title={`Hide addons [${shortcutToHumanString(shortcuts.togglePanel)}]`} + aria-label="Hide addons" >
15-19: Remove unused SafeTabProps.Dead type; not referenced after the refactor.
-export interface SafeTabProps { - title: Addon_BaseType['title']; - id: string; - children: Addon_BaseType['render']; -}
69-70: Drop obsolete absolute prop from the public type.It’s unused; leaving it can confuse integrators.
panelPosition?: 'bottom' | 'right'; - absolute?: boolean;code/core/src/components/components/AriaTabs/AriaTabs.tsx (3)
10-16: Type nit: title already covered by ReactNode; string is redundant.Simplify the union.
-export interface TabProps { +export interface TabProps { id: string; 'aria-label'?: string; - title: FC | ReactNode | string; + title: FC | ReactNode; children?: FC | ReactNode; isDisabled?: boolean; }
110-113: Empty-state early return drops wrapper props (id/className) and layout. Wrap it.Returning the node directly can break callers relying on Container’s semantics.
- if (!showToolsWhenEmpty && tabs.length === 0) { - return EmptyContent; - } + if (!showToolsWhenEmpty && tabs.length === 0) { + return <Container {...props}>{EmptyContent}</Container>; + }
93-105: Heads-up: panelProps.id cannot override the tabpanel id.AriaTabPanel spreads tabPanelProps after rest, so react-aria’s id wins by design. Document this or reject id in panelProps to avoid surprises.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
code/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (87)
MIGRATION.md(5 hunks)code/addons/a11y/package.json(1 hunks)code/addons/a11y/src/components/Tabs.tsx(3 hunks)code/addons/docs/package.json(1 hunks)code/addons/docs/src/blocks/components/ArgsTable/TabbedArgsTable.tsx(2 hunks)code/addons/docs/src/blocks/components/Toolbar.tsx(3 hunks)code/addons/jest/package.json(1 hunks)code/addons/jest/src/components/Panel.tsx(7 hunks)code/addons/jest/src/hoc/provideJestResult.tsx(1 hunks)code/addons/links/package.json(1 hunks)code/addons/themes/package.json(1 hunks)code/builders/builder-vite/package.json(1 hunks)code/builders/builder-webpack5/package.json(1 hunks)code/core/package.json(3 hunks)code/core/src/component-testing/components/Interaction.stories.tsx(3 hunks)code/core/src/component-testing/components/InteractionsPanel.stories.tsx(2 hunks)code/core/src/component-testing/components/InteractionsPanel.tsx(2 hunks)code/core/src/component-testing/components/Subnav.tsx(0 hunks)code/core/src/component-testing/components/Toolbar.stories.tsx(2 hunks)code/core/src/component-testing/components/Toolbar.tsx(1 hunks)code/core/src/components/components/AriaTabs/AriaTabList.stories.tsx(1 hunks)code/core/src/components/components/AriaTabs/AriaTabList.tsx(1 hunks)code/core/src/components/components/AriaTabs/AriaTabPanel.stories.tsx(1 hunks)code/core/src/components/components/AriaTabs/AriaTabPanel.tsx(1 hunks)code/core/src/components/components/AriaTabs/AriaTabs.stories.tsx(1 hunks)code/core/src/components/components/AriaTabs/AriaTabs.tsx(1 hunks)code/core/src/components/components/AriaToolbar/AbstractAriaToolbar.stories.tsx(1 hunks)code/core/src/components/components/AriaToolbar/AriaToolbar.stories.tsx(1 hunks)code/core/src/components/components/AriaToolbar/AriaToolbar.tsx(1 hunks)code/core/src/components/components/addon-panel/addon-panel.tsx(2 hunks)code/core/src/components/components/bar/bar.tsx(2 hunks)code/core/src/components/components/bar/button.tsx(0 hunks)code/core/src/components/components/tabs/tabs.helpers.tsx(0 hunks)code/core/src/components/components/tabs/tabs.hooks.tsx(0 hunks)code/core/src/components/components/tabs/tabs.stories.tsx(0 hunks)code/core/src/components/components/tabs/tabs.tsx(0 hunks)code/core/src/components/index.ts(1 hunks)code/core/src/controls/components/ControlsPanel.tsx(3 hunks)code/core/src/controls/components/SaveStory.tsx(3 hunks)code/core/src/controls/manager.tsx(1 hunks)code/core/src/manager/components/panel/Panel.tsx(2 hunks)code/core/src/manager/components/preview/Preview.tsx(3 hunks)code/core/src/manager/components/preview/Toolbar.tsx(3 hunks)code/core/src/manager/components/preview/utils/components.ts(1 hunks)code/core/src/manager/globals/exports.ts(1 hunks)code/core/src/manager/settings/index.tsx(3 hunks)code/e2e-tests/addon-a11y.spec.ts(1 hunks)code/e2e-tests/addon-actions.spec.ts(2 hunks)code/e2e-tests/component-tests.spec.ts(2 hunks)code/e2e-tests/manager.spec.ts(2 hunks)code/e2e-tests/module-mocking.spec.ts(4 hunks)code/e2e-tests/preview-api.spec.ts(1 hunks)code/e2e-tests/util.ts(1 hunks)code/frameworks/angular/package.json(1 hunks)code/frameworks/ember/package.json(1 hunks)code/frameworks/html-vite/package.json(1 hunks)code/frameworks/nextjs-vite/package.json(1 hunks)code/frameworks/nextjs/package.json(1 hunks)code/frameworks/preact-vite/package.json(1 hunks)code/frameworks/react-native-web-vite/package.json(1 hunks)code/frameworks/react-vite/package.json(1 hunks)code/frameworks/react-webpack5/package.json(1 hunks)code/frameworks/server-webpack5/package.json(1 hunks)code/frameworks/svelte-vite/package.json(1 hunks)code/frameworks/sveltekit/package.json(1 hunks)code/frameworks/vue3-vite/package.json(1 hunks)code/frameworks/web-components-vite/package.json(1 hunks)code/lib/cli-sb/package.json(1 hunks)code/lib/cli-storybook/package.json(1 hunks)code/lib/codemod/package.json(1 hunks)code/lib/core-webpack/package.json(1 hunks)code/lib/create-storybook/package.json(1 hunks)code/lib/csf-plugin/package.json(1 hunks)code/lib/eslint-plugin/package.json(1 hunks)code/lib/react-dom-shim/package.json(1 hunks)code/package.json(1 hunks)code/presets/create-react-app/package.json(1 hunks)code/presets/react-webpack/package.json(1 hunks)code/presets/server-webpack/package.json(1 hunks)code/renderers/html/package.json(1 hunks)code/renderers/preact/package.json(1 hunks)code/renderers/react/package.json(1 hunks)code/renderers/server/package.json(1 hunks)code/renderers/svelte/package.json(1 hunks)code/renderers/vue3/package.json(1 hunks)code/renderers/web-components/package.json(1 hunks)scripts/build/utils/entry-utils.ts(2 hunks)
💤 Files with no reviewable changes (6)
- code/core/src/components/components/tabs/tabs.helpers.tsx
- code/core/src/components/components/tabs/tabs.hooks.tsx
- code/core/src/components/components/bar/button.tsx
- code/core/src/component-testing/components/Subnav.tsx
- code/core/src/components/components/tabs/tabs.stories.tsx
- code/core/src/components/components/tabs/tabs.tsx
🧰 Additional context used
📓 Path-based instructions (1)
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/e2e-tests/preview-api.spec.tscode/e2e-tests/addon-actions.spec.tscode/e2e-tests/addon-a11y.spec.tscode/e2e-tests/manager.spec.tscode/e2e-tests/module-mocking.spec.tscode/e2e-tests/component-tests.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-09-17T08:11:47.184Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.184Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Applied to files:
code/e2e-tests/module-mocking.spec.tscode/core/src/components/components/AriaTabs/AriaTabPanel.stories.tsx
🧬 Code graph analysis (25)
code/core/src/components/components/AriaToolbar/AbstractAriaToolbar.stories.tsx (3)
code/core/src/components/index.ts (2)
Button(53-53)AbstractAriaToolbar(75-75)code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (1)
AbstractAriaToolbar(15-32)code/core/src/components/components/AriaToolbar/AriaToolbar.stories.tsx (1)
Basic(29-29)
code/core/src/component-testing/components/Toolbar.stories.tsx (1)
code/core/src/component-testing/components/Toolbar.tsx (1)
Toolbar(99-175)
code/e2e-tests/addon-a11y.spec.ts (2)
code/core/src/test/expect.ts (1)
expect(148-148)code/e2e-tests/util.ts (1)
SbPage(8-203)
code/core/src/components/components/addon-panel/addon-panel.tsx (2)
code/core/src/components/index.ts (2)
Div(10-10)ScrollArea(47-47)code/core/src/manager/components/panel/Panel.tsx (1)
AddonPanel(63-151)
code/core/src/components/components/AriaTabs/AriaTabs.tsx (3)
code/core/src/components/components/AriaTabs/AriaTabPanel.tsx (1)
AriaTabPanel(32-64)code/core/src/components/components/bar/bar.tsx (1)
Bar(50-68)code/core/src/components/components/AriaTabs/AriaTabList.tsx (1)
AriaTabList(100-111)
code/core/src/controls/components/ControlsPanel.tsx (3)
code/core/src/components/index.ts (1)
ScrollArea(47-47)code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx (1)
ArgsTable(318-474)code/core/src/controls/components/SaveStory.tsx (1)
SaveStory(96-232)
code/addons/a11y/src/components/Tabs.tsx (2)
code/core/src/components/components/AriaTabs/AriaTabs.tsx (2)
Container(44-48)AriaTabs(93-140)code/core/src/components/index.ts (4)
AriaTabs(74-74)WithTooltip(58-58)TooltipNote(60-60)IconButton(54-54)
code/core/src/components/components/AriaToolbar/AriaToolbar.stories.tsx (2)
code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (1)
AriaToolbar(36-53)code/core/src/components/components/AriaToolbar/AbstractAriaToolbar.stories.tsx (1)
Basic(27-27)
code/core/src/controls/manager.tsx (2)
code/core/src/manager/components/panel/Panel.tsx (1)
AddonPanel(63-151)code/core/src/components/components/addon-panel/addon-panel.tsx (1)
AddonPanel(35-46)
code/core/src/component-testing/components/Toolbar.tsx (6)
code/core/src/component-testing/components/InteractionsPanel.tsx (1)
Controls(16-23)code/core/src/instrumenter/types.ts (1)
ControlStates(51-58)code/core/src/component-testing/components/StatusBadge.tsx (2)
PlayStatus(5-5)StatusBadge(45-52)code/core/src/components/index.ts (7)
Button(53-53)TooltipNote(60-60)IconButton(54-54)P(22-22)AriaToolbar(75-75)Separator(69-69)WithTooltip(58-58)code/core/src/component-testing/components/Interaction.tsx (1)
StyledIconButton(102-105)code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (1)
AriaToolbar(36-53)
code/core/src/component-testing/components/InteractionsPanel.tsx (1)
code/core/src/component-testing/components/Toolbar.tsx (1)
Toolbar(99-175)
code/addons/docs/src/blocks/components/ArgsTable/TabbedArgsTable.tsx (1)
code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx (1)
ArgsTable(318-474)
code/core/src/components/components/AriaTabs/AriaTabPanel.tsx (1)
code/core/src/components/index.ts (2)
AriaTabPanel(73-73)ScrollArea(47-47)
code/core/src/components/components/AriaTabs/AriaTabList.stories.tsx (2)
code/core/src/components/components/AriaTabs/AriaTabs.tsx (2)
TabProps(10-16)useAriaTabListState(25-42)code/core/src/components/components/AriaTabs/AriaTabList.tsx (1)
AriaTabList(100-111)
code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (2)
code/core/src/components/index.ts (4)
AbstractAriaToolbar(75-75)BarProps(70-70)AriaToolbar(75-75)Bar(70-70)code/core/src/components/components/bar/bar.tsx (2)
BarProps(7-14)Bar(50-68)
code/core/src/components/components/AriaTabs/AriaTabList.tsx (1)
code/core/src/components/index.ts (1)
AriaTabList(72-72)
code/addons/docs/src/blocks/components/Toolbar.tsx (2)
code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (1)
AriaToolbar(36-53)code/core/src/component-testing/components/Toolbar.tsx (1)
Toolbar(99-175)
code/core/src/components/components/AriaTabs/AriaTabs.stories.tsx (1)
code/core/src/components/components/AriaTabs/AriaTabs.tsx (1)
AriaTabs(93-140)
code/core/src/controls/components/SaveStory.tsx (1)
code/core/src/components/components/Modal/Modal.tsx (1)
Modal(58-58)
code/core/src/manager/components/panel/Panel.tsx (3)
code/core/src/components/components/addon-panel/addon-panel.tsx (1)
AddonPanel(35-46)code/core/src/manager/components/layout/LayoutProvider.tsx (1)
useLayout(61-61)code/core/src/components/components/AriaTabs/AriaTabs.tsx (1)
AriaTabs(93-140)
code/core/src/manager/components/preview/Toolbar.tsx (2)
code/core/src/components/components/AriaTabs/AriaTabList.tsx (1)
AriaTabList(100-111)code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (1)
AbstractAriaToolbar(15-32)
code/addons/jest/src/components/Panel.tsx (1)
code/addons/jest/src/hoc/provideJestResult.tsx (2)
Test(18-26)AssertionResult(11-16)
code/core/src/components/components/bar/bar.tsx (1)
code/core/src/node-logger/index.ts (1)
deprecate(94-94)
code/core/src/components/components/AriaTabs/AriaTabPanel.stories.tsx (4)
code/core/src/components/components/AriaTabs/AriaTabs.tsx (2)
TabProps(10-16)useAriaTabListState(25-42)code/core/src/components/components/AriaTabs/AriaTabPanel.tsx (1)
AriaTabPanel(32-64)code/core/src/components/components/AriaTabs/AriaTabList.tsx (1)
AriaTabList(100-111)code/core/src/test/testing-library.ts (1)
userEvent(119-122)
code/core/src/manager/components/preview/Preview.tsx (2)
code/core/src/components/components/AriaTabs/AriaTabs.tsx (1)
useAriaTabListState(25-42)code/core/src/node-logger/index.ts (1)
deprecate(94-94)
🪛 Biome (2.1.2)
code/core/src/components/components/addon-panel/addon-panel.tsx
[error] 40-40: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 42-42: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
code/addons/jest/src/components/Panel.tsx
[error] 163-163: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 167-167: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 markdownlint-cli2 (0.17.2)
MIGRATION.md
609-609: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
⏰ 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). (1)
- GitHub Check: normal
🔇 Additional comments (55)
code/frameworks/server-webpack5/package.json (1)
65-65: LGTM: metadata-only gitHead bumpNo functional or runtime impact from this change.
If your release/publish pipeline keys off package.json changes, confirm metadata-only edits like gitHead don’t trigger a publish.
code/renderers/svelte/package.json (1)
78-78: gitHead updated; no runtime/API change here.code/addons/jest/package.json (1)
69-69: gitHead bump only; addon behavior unchanged.code/frameworks/react-vite/package.json (1)
80-80: gitHead bump only; framework package unaffected functionally.code/lib/cli-storybook/package.json (1)
75-75: gitHead bump; CLI package semantics unchanged.code/renderers/html/package.json (1)
62-62: gitHead bump only; no export or dependency changes.code/lib/core-webpack/package.json (1)
56-56: gitHead metadata updated; no functional delta.code/lib/eslint-plugin/package.json (1)
71-71: gitHead bump; ESLint plugin contents unchanged.code/lib/csf-plugin/package.json (1)
73-73: Metadata-only gitHead bump — verified consistent, LGTM.
All package.json files (40) contain the same gitHead: a8e7fd8a655c69780bc20b9749d2699e45beae17.code/frameworks/nextjs/package.json (1)
146-146: Metadata-only gitHead bump — OK.No action needed.
code/frameworks/svelte-vite/package.json (1)
79-79: Metadata-only gitHead bump — OK.No action needed.
code/presets/react-webpack/package.json (1)
73-73: Metadata-only gitHead bump — OK.No action needed.
code/frameworks/ember/package.json (1)
74-74: Metadata-only gitHead bump — OK.No action needed.
code/core/package.json (3)
239-243: use-sync-external-store addition — OK.Reasonable choice for subscription-based state; no further action.
384-384: Metadata-only gitHead bump — OK.No action needed.
239-243: Add runtime deps here if core imports ARIA libs.If core’s AriaTabs/AriaToolbar import react-aria/react-stately, declare them as dependencies (or peers) in this package to satisfy consumers.
Apply if applicable:
@@ "dependencies": { "semver": "^7.6.2", + "react-aria": "^3.42.0", + "react-stately": "^3.40.0", "use-sync-external-store": "^1.5.0", "ws": "^8.18.0" }⛔ Skipped due to learnings
Learnt from: ndelangen PR: storybookjs/storybook#32484 File: code/core/package.json:326-326 Timestamp: 2025-09-17T07:31:04.408Z Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.code/frameworks/nextjs-vite/package.json (1)
108-108: Metadata-only gitHead bump — OK.No action needed.
code/lib/create-storybook/package.json (1)
66-66: Metadata-only gitHead bump — LGTM.
No runtime or API impact.code/lib/react-dom-shim/package.json (1)
55-55: gitHead updated — looks fine.
No behavioral change.code/frameworks/react-webpack5/package.json (1)
74-74: gitHead bump only — OK to proceed.code/lib/cli-sb/package.json (1)
34-34: gitHead bump only — LGTM.code/renderers/react/package.json (1)
94-94: gitHead metadata update — approved.code/presets/server-webpack/package.json (1)
59-59: gitHead metadata update — approved.code/frameworks/angular/package.json (1)
126-126: gitHead update only — LGTM.scripts/build/utils/entry-utils.ts (2)
3-3: Import reordering — fine.
No semantic change.
58-75: Verify packages importing use-sync-external-store declare it (prevent missing external at runtime).
rg in the sandbox returned "No files were searched" so importer scan is inconclusive; only package.json found that declares it is code/core/package.json -> ^1.5.0. Ensure any package that imports "use-sync-external-store" or "use-sync-external-store/shim" lists it in dependencies or peerDependencies, or add it to runtimeExternalInclude.code/frameworks/react-native-web-vite/package.json (1)
77-77: Metadata-only gitHead bump — OK to mergeNo functional/package surface changes in this file.
code/frameworks/html-vite/package.json (1)
66-66: Metadata-only gitHead bump — OKNo runtime impact.
code/frameworks/sveltekit/package.json (1)
77-77: gitHead update onlyLooks good.
code/renderers/preact/package.json (1)
64-64: gitHead updated — no code changesAll good.
code/renderers/vue3/package.json (1)
72-72: gitHead metadata updateNo issues.
code/presets/create-react-app/package.json (1)
56-56: gitHead bump onlySafe.
code/e2e-tests/addon-a11y.spec.ts (1)
62-64: Good fix: remove await for sync assertionString containment via expect(...) is synchronous; this change is correct.
code/renderers/server/package.json (1)
61-61: Metadata-only gitHead bump — OK.
No functional changes; safe to land.code/frameworks/vue3-vite/package.json (1)
75-75: gitHead update looks good.
Repository metadata sync only.code/frameworks/web-components-vite/package.json (1)
68-68: gitHead update acknowledged.
No runtime impact.code/e2e-tests/util.ts (1)
160-161: Role-based locator is preferable — nice.
More robust than CSS + :not([hidden]).code/core/src/manager/components/preview/utils/components.ts (1)
13-19: Ensure thehas an accessible name (heading or aria-label/aria-labelledby).
Automated search produced no matches for under code/core/src/manager — confirm the component defined at code/core/src/manager/components/preview/utils/components.ts (lines 13–19) includes a visible heading or an aria-label/aria-labelledby so theis a labeled landmark. code/addons/jest/src/hoc/provideJestResult.tsx (1)
11-16: Use Jest’s official AssertionResult type; add @jest/test-result if missing.Replace the local interface with the upstream type to keep status literals and fields in sync.
-// TODO: import type from @types/jest -export interface AssertionResult { - status: string; - fullName: string; - title: string; - failureMessages: string[]; -} +// Align with Jest's upstream types to prevent divergence +import type { AssertionResult as JestAssertionResult } from '@jest/test-result'; +export type AssertionResult = Pick< + JestAssertionResult, + 'status' | 'fullName' | 'title' | 'failureMessages' +>;Verification: AssertionResult is defined at code/addons/jest/src/hoc/provideJestResult.tsx and referenced in code/addons/jest/src/components/Panel.tsx; no "@jest/test-result" entry was found in package.json — add it as a devDependency (e.g., yarn add -D @jest/test-result or npm i -D @jest/test-result) before applying the change.
code/e2e-tests/module-mocking.spec.ts (1)
22-29: Nice: deterministic panel state before assertionsClearing and remounting ensures stable expectations in Actions panel.
code/e2e-tests/manager.spec.ts (1)
240-254: Good switch to role-based selectors on mobileUsing getByRole('tab', { name: 'Controls' }) aligns with the ARIA migration and reduces ID brittleness.
code/core/src/components/components/bar/bar.tsx (1)
16-38: LGTM: Scrollable Bar with inner wrapperThe HeightPreserver pattern preserves height while enabling native horizontal scrolling. Matches the ARIA toolbar/tab needs.
code/core/src/controls/components/SaveStory.tsx (1)
200-229: LGTM: Modal split from BarExtracting the modal sibling avoids layout coupling and is cleaner for focus management.
code/core/src/components/components/AriaToolbar/AriaToolbar.tsx (1)
24-28: Orientation is hardcoded. Verify this is intentional across all toolbars.If vertical toolbars are planned, consider an
orientation?: 'horizontal' | 'vertical'prop (default horizontal).Also applies to: 45-49
code/core/src/controls/manager.tsx (1)
125-127: LGTM: avoid double scrollbars in active panel.Passing
hasScrollbar={false}aligns with the new AriaTabs panel container.code/core/src/component-testing/components/InteractionsPanel.stories.tsx (1)
13-14: LGTM: stories now reference Toolbar controls.Import swap and args linkage look correct.
Also applies to: 52-54
code/core/src/controls/components/ControlsPanel.tsx (1)
3-3: LGTM: scroll behavior and save‑UI gating.Wrapping
ArgsTableinScrollAreaand usingshowSaveFromUIsimplifies layout and avoids double scrollbars.Also applies to: 33-39, 91-114
code/addons/docs/src/blocks/components/Toolbar.tsx (1)
49-50: Prevent leaking non-DOM props (storyId, baseUrl) to the DOM. Destructure and drop storyId/baseUrl (or use shouldForwardProp); the diff already renames them to _storyId/_baseUrl.
Location: code/addons/docs/src/blocks/components/Toolbar.tsx (around lines 49–50).
Repo-wide verification failed due to a script error — search for JSX spreads that forward {...rest} and for tokens "storyId" / "baseUrl" to confirm no other components leak these props.code/core/src/component-testing/components/InteractionsPanel.tsx (2)
14-14: LGTM: Toolbar migration import is correct.
117-123: LGTM — props forwarded correctly to AriaToolbar. Re-run E2E role-based locators.AriaToolbar is used in code/core/src/component-testing/components/Toolbar.tsx (aria-label="Component test playback controls"); AbstractAriaToolbar (code/core/src/components/components/AriaToolbar/AriaToolbar.tsx) accepts 'aria-label'/'aria-labelledby' via useToolbar — controls/controlStates/status/storyFileName/onScrollToEnd are forwarded.
code/core/src/manager/globals/exports.ts (1)
477-483: Exports verified in components barrelcode/core/src/components/index.ts re-exports the symbols — lines 72–75 export AriaTabList, AriaTabPanel, AriaTabs, useAriaTabListState, AriaToolbar, AbstractAriaToolbar.
code/core/src/manager/components/preview/Preview.tsx (1)
2-2: Import hygiene after heading-id fix.Once the
useIdchange above is applied, this import becomes used; otherwise, drop it to avoid lint noise.code/core/src/components/index.ts (1)
70-75: Verify TypeScript ≥ 4.5 and downstream imports
- BarProps is re-exported as a type in code/core/src/components/index.ts (export { Bar, type BarProps }) — ensure the repository/build uses TypeScript ≥ 4.5.
- Confirm no consumers import Tabs, TabBar, TabsState, TabWrapper, TabButton, or FlexBar from the public components entry; found one local usage only: code/addons/a11y/src/components/A11YPanel.tsx imports ./Tabs (local), so that file isn't affected.
code/addons/a11y/src/components/Tabs.tsx (1)
85-91: Tooltips should also appear on keyboard focusConfirm
WithTooltip trigger="hover"also shows on focus for keyboard users; otherwise include focus.If
WithTooltipsupports multiple triggers, prefer adding focus without removing hover (e.g.,trigger="hover focus"or equivalent API).Also applies to: 97-103, 111-117
code/core/src/manager/components/panel/Panel.tsx (1)
137-143: Don't remove panelProps.id here — update callers/tests firstAriaTabPanel applies its own id (so this prop is ignored), but the literal "panel-tab-content" is referenced across the repo; removing the prop will break tests/stories unless those references are updated.
- code/core/src/component-testing/components/InteractionsPanel.stories.tsx (StyledWrapper id="panel-tab-content")
- code/core/src/component-testing/components/Panel.tsx (IntersectionObserver root: document.querySelector('#panel-tab-content'))
- code/e2e-tests/framework-svelte.spec.ts (multiple locators using '#storybook-panel-root #panel-tab-content')
- code/e2e-tests/framework-nextjs.spec.ts (locators using '#storybook-panel-root #panel-tab-content')
- code/addons/a11y/src/components/A11YPanel.stories.tsx (StyledWrapper id="panel-tab-content")
- code/addons/a11y/src/components/Report/Report.stories.tsx (StyledWrapper id="panel-tab-content")
Either remove this prop and update all these references to use AriaTabPanel's generated id or another stable selector, or keep the prop for test compatibility and add a comment explaining it's overridden.
Sidnioulz
force-pushed
the
sidnioulz/issue-25918-toolbar
branch
from
ee627a6 to
3c5bba9
Compare
This reverts commit 24a8ac7.
Sidnioulz
force-pushed
the
sidnioulz/issue-25918-toolbar
branch
from
a83d1fb to
85a2462
Compare
Closes #25918
Closes #32235
Closes #26674
New components
AriaToolbar
We needed a bar that has the proper toolbar role and implements the toolbar navigation.
AbstractAriaToolbar
AriaTabs
We needed the proper tablist-tabpanel implementation with
Keyboard navigation (left-right in the list, tab/shift+tab to switch between list and panel)
The list and panel directly adjacent in tabbing order
Integrates all below components and hooks into a preconfigured tabpanel
Forces a layout where the tablist and tabpanel are huddled together, with the tablist in a Bar and optional tools on the side
Provides access to AriaTabPanel props to limit need to detach
Allows passing a style object and bg color to the Bar, also to limit need to detach
Has empty state support like old Tabs
Can receive tools like old Tabs (but places them before the ariatabs in the tabbing order for WCAG compliance)
AriaTabList
AriaTabPanel
hiddenattr, for React context preservation in existing addon panelsuseAriaTabListState
Changes to existing UI
Bar (@storybook/components)
Changes made
innerStyleprop to help handling recurring layout customisation needs (padding, gap, flex direction) without having to fork Bar or rely on internal DOM structureChanges not made
borderandbackgroundColoras it was not relevant to what I was fixing, though I'd be happy to smooth that out too to group up component API changes in a single majorRationale
Bar's primary issues are:
scrollableprop resulting in three variants:trueuses ScrollArea,undefineduses CSS scrollbars,falseis non-scrollable; knowing that no code inside Storybook usestrueFlexBar (@storybook/components)
Changes made
deprecatenoticeRationale
FlexBar is a layout specialisation of Bar, but it turned out that we had places in the code where we needed exactly that layout, but still used Bar. That's because FlexBar made it harder to apply paddings and gaps between items. The component likely was written for a specific use case and existing use cases likely didn't get refactored in time.
Given that the bulk of FlexBar's value can be achieved with a few CSS properties, and that using FlexBar requires writing as much CSS to override its opinionated padding, I prefer removing it entirely.
Tabs (@storybook/components)
Changes made
deprecatenoticeuseResizeDetectordependencyTabButtonsubcomponent from thebarfolder to thetabs/folder because it belongs to TabsRationale
Tabs suffers a few issues:
aria-selectedFixing those issues requires a near total rewrite of the component's logic, especially because we have patterns where we need the tablist and tabpanel to be rendered separately, and their shared state to be maintained upstream. It also requires adding props for an opaque state object that can contain all the relevant shared state without forcing callees to be aware of implementation details.
It was significantly easier to create a new component than to ensure that both new and existing components are API-compatible.
AddonPanel (@storybook/components)
hasScrollbarprop to pre-integrate the scrollbar for addons that don't need a more advanced layout; can be disabled easily enoughRationale
This component is used internally for many of our 'simpler' addon layouts, but also exposed to addon authors. Therefore, it makes sense for the component to provide the best defaults out of the box for addon panel positioning (aka. take all the space and have a scrollbar).
Toolbar (@storybook/addon-docs/blocks)
SubNav -> Toolbar (component-testing module)
Addon panel
Addon-controls panel
Toolbar
canvas, it also shows (and not just when undefined, for compat with components settingtabId)Settings page
TO DO
Solve the overflow menu dilemma
With this PR, we lose the overflow menu that shows when scrollbars with a TabList have too little space. We're considering options:
sacrifice usability and unmake some of this PR's changes to preserve the overflow button temporarily
make the entire tablist become a vertical tablist on a menu button as soon as there's an overflow
permanently replace the tablist with a menu, and add a title for the current addon
accept the scrollbar and the loss of the overflow menu for now
@MichaelArestad to try out options in Figma and make a recommendation
Implementation of the recommended solution
Implement CSS changes
Finalise naming and deprecation decisions
Please provide async comments on your preferences. Now that I know SB 10 is here, my recommendation is to entirely remove Tabs and to remove Aria prefixes from the new components' names.We want to minimize breaking changes, so we want to keep existing names and deprecate rather than remove Tabs / Bar / FlexBar
Double check source code in Tabs and Bar to make sure what's deprecated is marked as such and to delete what we choose to remove in SB 10
Ensure deprecation / removal notices are well written
TabList: mention that the new AriaTabList does not support mixed link/tab content (tabs only)
Finish integration of new components
Doc changes
Fix E2E tests
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Documentation
N/A, only MIGRATION.md changes AFAIK
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>Summary by CodeRabbit
New Features
Improvements
Deprecations / Removals
Documentation
Chores