Repository navigation
Review: Simplify layout handling - #35322
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors review-mode session handling into smaller helpers, updates review entry points to capture and restore return navigation state, changes review navigation to validate return targets, simplifies the review summary portal positioning, and hides the sidebar when the manager is in review view mode. ChangesReview Cycle Refactor and Layout Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/manager/components/layout/Layout.tsx`:
- Around line 240-266: The no-sidebar branch in Layout.tsx defines a 2-column
grid via gridTemplateColumns, but the gridTemplateAreas cases for showSidebar
=== false still emit 3 area tokens per row, making the template invalid. Update
the no-sidebar return strings in the layout logic to match the 2-column desktop
grid, especially the branches that depend on showPanel and panelPosition. Keep
the area names aligned with the actual columns so the review layout renders
correctly.
In `@code/core/src/manager/components/review/review-entry.ts`:
- Around line 12-24: capturePreReviewReturn currently overwrites
PRE_REVIEW_RETURN_KEY on every story/docs navigation, so update the
capturePreReviewReturn function to write only once per review cycle by checking
whether review mode is already active or whether sessionStore already has a
stored pre-review return value before calling sessionStore.write. Keep the
existing story/docs path parsing and normalizeSearch flow, but add the guard
around the write using the capturePreReviewReturn and PRE_REVIEW_RETURN_KEY
symbols. Add a regression test that calls capturePreReviewReturn multiple times
within one cycle and verifies the first snapshot is preserved.
In `@code/core/src/manager/components/review/review-mode.ts`:
- Around line 66-92: The session snapshot keys are never cleared after being
restored, so later calls can replay stale chrome/filter state. Update the
review-mode flow in exitReviewMode and the related restore logic to consume
CHROME_SNAPSHOT_SESSION_KEY and FILTERS_SNAPSHOT_SESSION_KEY after applying
them, and make initializeSessionFiltersIfNeeded / the chrome snapshot write path
only persist fresh state once per cycle so subsequent exits do not overwrite
newer user changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b45caa5-cf18-4419-b706-c43756034006
📒 Files selected for processing (17)
code/core/src/manager/components/layout/Layout.tsxcode/core/src/manager/components/review/ReviewPersistentLayer.tsxcode/core/src/manager/components/review/ReviewProvider.tsxcode/core/src/manager/components/review/ReviewSummaryPortal.tsxcode/core/src/manager/components/review/constants.tscode/core/src/manager/components/review/review-actions.tscode/core/src/manager/components/review/review-entry.test.tscode/core/src/manager/components/review/review-entry.tscode/core/src/manager/components/review/review-mode.test.tscode/core/src/manager/components/review/review-mode.tscode/core/src/manager/components/review/review-navigation.test.tscode/core/src/manager/components/review/review-navigation.tscode/core/src/manager/components/review/useReviewFiltersRef.tscode/core/src/manager/components/review/useReviewNavigationInterceptor.tscode/core/src/manager/components/review/useReviewShortcuts.tscode/core/src/manager/components/sidebar/ReviewWidget.stories.tsxcode/core/src/manager/components/sidebar/ReviewWidget.tsx
💤 Files with no reviewable changes (1)
- code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx
ghengeveld
changed the title
ghengeveld
force-pushed
the
ghengeveld/review-fullscreen-summary
branch
from
June 29, 2026 14:16
63525ee to
c4e6851
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx`:
- Around line 111-113: Keep ReviewWidget.stories in sync with review navigation
by updating the Storybook state path alongside the MemoryRouter route. The issue
is that ReviewProvider reads useStorybookState().path, but this story leaves
path at '/' so the summary-side effects for the /review/ flow never run. Adjust
the story setup in ReviewWidget.stories and the relevant review-navigation
render path so the state path reflects '/review/' when testing OpenReview and
the chrome/filter initialization behavior.
In `@code/core/src/manager/components/sidebar/ReviewWidget.tsx`:
- Around line 89-92: The return-state capture in ReviewWidget’s onOpen is using
window.location.search, which can get out of sync with the router state. Update
the call to capturePreReviewReturn to use the current router search value from
location first, falling back to window.location.search only when needed, so
dismiss navigation restores the correct place under MemoryRouter. Use the
existing onOpen handler and location reference in ReviewWidget to make the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 889603d3-cf66-47a8-9420-ccec1868b4a5
📒 Files selected for processing (17)
code/core/src/manager/components/layout/Layout.tsxcode/core/src/manager/components/review/components/ReviewNotification.tsxcode/core/src/manager/components/review/components/ReviewProvider.tsxcode/core/src/manager/components/review/constants.tscode/core/src/manager/components/review/review-actions.tscode/core/src/manager/components/review/review-entry.test.tscode/core/src/manager/components/review/review-entry.tscode/core/src/manager/components/review/review-mode.test.tscode/core/src/manager/components/review/review-mode.tscode/core/src/manager/components/review/review-navigation.test.tscode/core/src/manager/components/review/review-navigation.tscode/core/src/manager/components/review/screens/ReviewSummaryPortal.tsxcode/core/src/manager/components/review/useReviewFiltersRef.tscode/core/src/manager/components/review/useReviewNavigationInterceptor.tscode/core/src/manager/components/review/useReviewShortcuts.tscode/core/src/manager/components/sidebar/ReviewWidget.stories.tsxcode/core/src/manager/components/sidebar/ReviewWidget.tsx
✅ Files skipped from review due to trivial changes (2)
- code/core/src/manager/components/review/useReviewFiltersRef.ts
- code/core/src/manager/components/review/constants.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- code/core/src/manager/components/review/review-navigation.ts
- code/core/src/manager/components/review/review-entry.ts
- code/core/src/manager/components/review/review-entry.test.ts
- code/core/src/manager/components/review/review-mode.ts
- code/core/src/manager/components/review/review-navigation.test.ts
- code/core/src/manager/components/review/useReviewNavigationInterceptor.ts
- code/core/src/manager/components/review/review-mode.test.ts
- code/core/src/manager/components/review/useReviewShortcuts.ts
- code/core/src/manager/components/review/review-actions.ts
- code/core/src/manager/components/layout/Layout.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx`:
- Around line 111-113: Keep ReviewWidget.stories in sync with review navigation
by updating the Storybook state path alongside the MemoryRouter route. The issue
is that ReviewProvider reads useStorybookState().path, but this story leaves
path at '/' so the summary-side effects for the /review/ flow never run. Adjust
the story setup in ReviewWidget.stories and the relevant review-navigation
render path so the state path reflects '/review/' when testing OpenReview and
the chrome/filter initialization behavior.
In `@code/core/src/manager/components/sidebar/ReviewWidget.tsx`:
- Around line 89-92: The return-state capture in ReviewWidget’s onOpen is using
window.location.search, which can get out of sync with the router state. Update
the call to capturePreReviewReturn to use the current router search value from
location first, falling back to window.location.search only when needed, so
dismiss navigation restores the correct place under MemoryRouter. Use the
existing onOpen handler and location reference in ReviewWidget to make the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 889603d3-cf66-47a8-9420-ccec1868b4a5
📒 Files selected for processing (17)
code/core/src/manager/components/layout/Layout.tsxcode/core/src/manager/components/review/components/ReviewNotification.tsxcode/core/src/manager/components/review/components/ReviewProvider.tsxcode/core/src/manager/components/review/constants.tscode/core/src/manager/components/review/review-actions.tscode/core/src/manager/components/review/review-entry.test.tscode/core/src/manager/components/review/review-entry.tscode/core/src/manager/components/review/review-mode.test.tscode/core/src/manager/components/review/review-mode.tscode/core/src/manager/components/review/review-navigation.test.tscode/core/src/manager/components/review/review-navigation.tscode/core/src/manager/components/review/screens/ReviewSummaryPortal.tsxcode/core/src/manager/components/review/useReviewFiltersRef.tscode/core/src/manager/components/review/useReviewNavigationInterceptor.tscode/core/src/manager/components/review/useReviewShortcuts.tscode/core/src/manager/components/sidebar/ReviewWidget.stories.tsxcode/core/src/manager/components/sidebar/ReviewWidget.tsx
✅ Files skipped from review due to trivial changes (2)
- code/core/src/manager/components/review/useReviewFiltersRef.ts
- code/core/src/manager/components/review/constants.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- code/core/src/manager/components/review/review-navigation.ts
- code/core/src/manager/components/review/review-entry.ts
- code/core/src/manager/components/review/review-entry.test.ts
- code/core/src/manager/components/review/review-mode.ts
- code/core/src/manager/components/review/review-navigation.test.ts
- code/core/src/manager/components/review/useReviewNavigationInterceptor.ts
- code/core/src/manager/components/review/review-mode.test.ts
- code/core/src/manager/components/review/useReviewShortcuts.ts
- code/core/src/manager/components/review/review-actions.ts
- code/core/src/manager/components/layout/Layout.tsx
🛑 Comments failed to post (2)
code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx (1)
111-113: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the story and provider files, then inspect the relevant ranges. git ls-files 'code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx' \ 'code/core/src/manager/components/sidebar/*Review*' \ 'code/core/src/manager/components/sidebar/*Provider*' \ 'code/core/src/manager/**/*Review*' \ 'code/core/src/manager/**/*Provider*' echo "---- story outline ----" ast-grep outline code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx --view expanded || true echo "---- story excerpt ----" sed -n '1,220p' code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx echo "---- related provider/search ----" rg -n "initializeSessionChromeIfNeeded|initializeSessionFiltersIfNeeded|applyReviewingFiltersForReviewIfNeeded|useStorybookState\\(|path:" code/core/src/manager -g '*.ts' -g '*.tsx' || trueRepository: storybookjs/storybook
Length of output: 18134
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- ReviewProvider.tsx ----" sed -n '1,180p' code/core/src/manager/components/review/components/ReviewProvider.tsx echo "---- ReviewWidget.stories.tsx around OpenReview ----" sed -n '220,290p' code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx echo "---- ReviewPage.stories.tsx url-state harness for comparison ----" sed -n '1,120p' code/core/src/manager/components/review/ReviewPage.stories.tsxRepository: storybookjs/storybook
Length of output: 13249
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- ReviewProvider.tsx around path/summary logic ----" sed -n '180,320p' code/core/src/manager/components/review/components/ReviewProvider.tsx echo "---- review-navigation helpers ----" sed -n '1,180p' code/core/src/manager/components/review/review-navigation.ts echo "---- ReviewWidget.tsx interaction points ----" sed -n '1,220p' code/core/src/manager/components/sidebar/ReviewWidget.tsxRepository: storybookjs/storybook
Length of output: 12485
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' code/core/src/manager/components/review/review-actions.tsRepository: storybookjs/storybook
Length of output: 2412
Sync
state.pathwith the review navigation (lines 111-113, 265-268)
ReviewProviderkeys its summary-side effects offuseStorybookState().path, but this story keeps that value at'/'while only theMemoryRouterlocation changes. TheOpenReviewassertions can pass without covering the/review/path that triggers the chrome/filter initialization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx` around lines 111 - 113, Keep ReviewWidget.stories in sync with review navigation by updating the Storybook state path alongside the MemoryRouter route. The issue is that ReviewProvider reads useStorybookState().path, but this story leaves path at '/' so the summary-side effects for the /review/ flow never run. Adjust the story setup in ReviewWidget.stories and the relevant review-navigation render path so the state path reflects '/review/' when testing OpenReview and the chrome/filter initialization behavior.code/core/src/manager/components/sidebar/ReviewWidget.tsx (1)
89-92: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail files=( "code/core/src/manager/components/sidebar/ReviewWidget.tsx" "code/core/src/manager/components/review/components/ReviewNotification.tsx" "code/core/src/manager/components/review/components/ReviewProvider.tsx" "code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx" ) for f in "${files[@]}"; do echo "===== $f =====" wc -l "$f" echo "--- outline ---" ast-grep outline "$f" --view expanded || true echo doneRepository: storybookjs/storybook
Length of output: 3261
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' code/core/src/manager/components/sidebar/ReviewWidget.tsx echo '=====' sed -n '1,240p' code/core/src/manager/components/review/components/ReviewNotification.tsx echo '=====' sed -n '1,260p' code/core/src/manager/components/review/components/ReviewProvider.tsx echo '=====' sed -n '1,260p' code/core/src/manager/components/sidebar/ReviewWidget.stories.tsxRepository: storybookjs/storybook
Length of output: 23421
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== ReviewWidget.tsx ===" nl -ba code/core/src/manager/components/sidebar/ReviewWidget.tsx | sed -n '1,170p' echo echo "=== ReviewNotification.tsx ===" nl -ba code/core/src/manager/components/review/components/ReviewNotification.tsx | sed -n '1,220p' echo echo "=== ReviewProvider.tsx ===" nl -ba code/core/src/manager/components/review/components/ReviewProvider.tsx | sed -n '1,240p' echo echo "=== ReviewWidget.stories.tsx ===" nl -ba code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx | sed -n '1,240p'Repository: storybookjs/storybook
Length of output: 224
Use the current router search here —
window.location.searchcan diverge from Storybook’s router state underMemoryRouter; capturelocation?.search ?? window.location.searchso dismiss returns to the right place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/manager/components/sidebar/ReviewWidget.tsx` around lines 89 - 92, The return-state capture in ReviewWidget’s onOpen is using window.location.search, which can get out of sync with the router state. Update the call to capturePreReviewReturn to use the current router search value from location first, falling back to window.location.search only when needed, so dismiss navigation restores the correct place under MemoryRouter. Use the existing onOpen handler and location reference in ReviewWidget to make the change.
ghengeveld
added
maintenance
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx (1)
307-316: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFold the route assertion into the existing
waitFor. The/review/check is still a one-shot assertion and can run before the router-drivenrouter-pathupdate lands. Move it into the samewaitForas the other post-click assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx` around lines 307 - 316, The route assertion for the `router-path` in `ReviewWidget.stories.tsx` is still happening outside the async stabilization block, so it can fire before the router update is reflected. Move the `canvas.getByTestId('router-path')` expectation into the existing `waitFor` alongside the `toggleNavMock`, `togglePanelMock`, `setAllTagFiltersMock`, and `setAllStatusFiltersMock` assertions so the post-click checks all wait for the same state transition.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx`:
- Around line 172-189: `ReviewWidget.stories.tsx` is still hardcoding the
MemoryRouter start location to `/`, so the seeded `contextOptions.path` and
review collection query never take effect on first render. Update the story’s
router setup to initialize `MemoryRouter` from `contextOptions` (including any
seeded `REVIEW_COLLECTION_QUERY_PARAM`) and keep
`ManagerStateSync`/`makeManagerContext` aligned with that initial location so
preselected review routes work correctly.
---
Outside diff comments:
In `@code/core/src/manager/components/sidebar/ReviewWidget.stories.tsx`:
- Around line 307-316: The route assertion for the `router-path` in
`ReviewWidget.stories.tsx` is still happening outside the async stabilization
block, so it can fire before the router update is reflected. Move the
`canvas.getByTestId('router-path')` expectation into the existing `waitFor`
alongside the `toggleNavMock`, `togglePanelMock`, `setAllTagFiltersMock`, and
`setAllStatusFiltersMock` assertions so the post-click checks all wait for the
same state transition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eaa51635-1409-4a2c-9fd2-f92458f2157c
📒 Files selected for processing (2)
code/core/src/manager/components/sidebar/ReviewWidget.stories.tsxcode/core/src/manager/components/sidebar/ReviewWidget.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/manager/components/sidebar/ReviewWidget.tsx
Contributor
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 72 | 72 | 0 |
| Self size | 21.92 MB | 21.90 MB | 🎉 -20 KB 🎉 |
| Dependency size | 36.44 MB | 36.44 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 204 | 204 | 0 |
| Self size | 821 KB | 821 KB | 0 B |
| Dependency size | 91.76 MB | 91.74 MB | 🎉 -20 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 197 | 197 | 0 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 90.24 MB | 90.22 MB | 🎉 -20 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 1.09 MB | 1.09 MB | 0 B |
| Dependency size | 58.36 MB | 58.34 MB | 🎉 -20 KB 🎉 |
| Bundle Size Analyzer | node | node |
Member
Author
|
Babysit update while AFK:
All CodeRabbit threads are resolved. Local validation: Awaiting CI re-run and @yannbf review. |
2 of 9 tasks
Contributor
|
Failed to publish canary version of this pull request, triggered by @ghengeveld. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/28428414509 |
Review entry and exit now only snapshot and restore sidebar filters; entering a review no longer collapses or restores nav/panel layout state.
The review route no longer renders the sidebar (desktop or mobile menu), and sidebar keyboard shortcuts are ignored there.
Curated review story URLs suppress sidebar rendering and sidebar shortcuts, hide the toolbar show-sidebar control, and gate the review toolbar header on the collection query param.
The fixed-position summary overlay no longer reserves nav width on review routes, matching the layout grid when the sidebar is hidden.
Auto-accept now runs before the in-review arrival skip, so opening the summary or a collection story marks the matching createdAt as visited and clears the sidebar toast without a click.
Layout and other callers can render before router state is initialized; treat a missing path as a non-review route.
yannbf
approved these changes
Jun 30, 2026
yannbf
left a comment
yannbf
left a comment
Member
There was a problem hiding this comment.
LGTM as long as the flicker is resolved. The story has to be unmounted so there's no flicker in the navigation
Write REVIEW_MODE_SESSION_KEY only after setAllTagFilters and setAllStatusFilters resolve, and remove the filters snapshot if either setter rejects so a failed entry does not block re-entry.
Combine getIsNavShown with the review-route check so leaving review does not remount the sidebar when the user had previously hidden nav.
Close isMobileMenuOpen via useLayoutEffect when the menu is hidden on review routes so the drawer does not remount open after navigation.
This was referenced Jul 13, 2026
Merged
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Review mode no longer collapses or restores sidebar/panel visibility when entering or leaving a review; it still snapshots and restores sidebar filters only.
On the review summary page and on story routes with a
collectionquery param, the sidebar is not rendered (including the mobile menu drawer), sidebar keyboard shortcuts (toggleNav,focusNav,search) are ignored, and the toolbar “Show sidebar” control is hidden. The review toolbar header on story canvases only renders whencollectionis present.The fixed-position review summary portal no longer reserves left nav width on review routes, so the summary fills the full width when the sidebar is suppressed.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
cd code && yarn storybook:ui).?path=/review/).collectionquery param present): confirm no sidebar, no review toolbar header on normal stories withoutcollection, review header visible withcollection, and no “Show sidebar” toolbar button.Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This pull request has been released as version
0.0.0-pr-35329-sha-ec096ee6. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-35329-sha-ec096ee6 sandboxor in an existing project withnpx storybook@0.0.0-pr-35329-sha-ec096ee6 upgrade.More information
0.0.0-pr-35329-sha-ec096ee6review-sidebar-layoutec096ee61782818766)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=35329Summary by CodeRabbit