Manager: Restructure review feature state, layout integration, and thumbnail pipeline by ghengeveld · Pull Request #35351 · storybookjs/storybook

ghengeveld · 2026-07-02T11:52:58Z

What I did

Behavior-preserving restructuring of the review (agentic/display review) feature, addressing the structural findings of a deep code-quality audit. Three independent tracks, merged here:

1. Review routes as a first-class layout policy (manager-api)

Added 'review' to API_ViewMode and introduced getNavAvailability(): 'shown' | 'hidden' | 'unavailable' in manager-api/modules/layout.ts — the single place that consults review routing.
Replaced five scattered isReviewManagerRoute conditionals in shared chrome code (Layout.tsx, shortcuts.ts, Toolbar.tsx, tools/menu.tsx, the summary portal) with that one policy.
Unified isPagesShown between Layout and Sidebar via a shared isPagesViewMode selector (they previously disagreed on review routes), and unified the two divergent isReviewCollectionStory derivations via isReviewCollectionStoryRoute.

2. Review summary host inside the Layout grid

Deleted ReviewSummaryPortal (body-level position: fixed portal that re-derived nav/panel geometry by hand, duplicated the 600px breakpoint, and parked itself at left: -10000px to keep thumbnail iframes alive).
Replaced with ReviewSummaryHost rendered in a new Layout slotOverlay inside the always-mounted content cell: position: absolute; inset: 0, hidden via visibility + inert. The grid supplies geometry for free; iframe persistence verified empirically (visibility: hidden preserves iframe documents and in-frame state).

3. Review store as single source of truth

reviewStore now owns core state (state, pendingReview, isStale, isInReviewMode, isExiting, overlay suppression) with explicit actions; channel handlers write to it directly. Deleted the React-state-mirrored-into-store layout effect, the internalPendingReview side channel, and the notificationKey-as-effect-trigger pattern.
The sessionStorage "exiting" mutex became an ordinary store field; review-mode entry now rolls back on filter-setter failure.
The stale/pending-update banner precedence (previously implemented three times with already-diverged guards) is derived once in the store layer as AttentionBannerProps | null.

4. Thumbnail pipeline: explicit phase machine

Per-cell lifecycle is now a pure reducer (idle → queued → booting → measured → settled, evict → idle) in previewThumbnailState.ts, with src, spinner, and frame style derived from one state value. Deleted the load-generation counter, duplicated resets, and the module-level Map<Window, handler> resize registry (replaced by a per-cell listener comparing event.source).
previewScheduler now owns the full slot lifecycle (opaque { forceStart, release } handles, deadline-based auto-release), instead of splitting accounting across three files.
The iframe.resize payload parser moved next to its payload type in shared/constants/iframe-resize.ts; the aliasing re-export layer is gone; frame-style helpers live next to the Frame component that defines their contract.

No intended behavior changes. One known nuance: grid cells first rendered while the summary is hidden now boot when it becomes visible, instead of preloading while parked off-screen.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Run the internal Storybook UI: cd code && yarn storybook:ui
Trigger a review via the MCP display-review tool (or replay one of the ReviewPage stories under manager/review).
Verify: the review summary hides the sidebar and toolbar chrome; sidebar shortcuts (s, /) are inactive on review routes; the fullscreen and "show sidebar" tools are hidden.
Open a reviewed story from the summary grid, then return to the summary (Escape) — thumbnail iframes must NOT reload.
Resize/drag the panel with the summary open — the summary must track the content area.
Push a second review while reviewing — the "new review available" banner appears once (summary and toolbar header); accepting swaps the payload.
Dismiss the review — sidebar filters restore, navigation returns to the pre-review canvas, no stale notifications.
Scroll a large review grid — previews boot at most 3 concurrently, hover starts one immediately, offscreen cells evict, spinners clear after thumbnails settle.

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip
Make 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-35351-sha-068e5c71. Try it out in a new sandbox by running npx storybook@0.0.0-pr-35351-sha-068e5c71 sandbox or in an existing project with npx storybook@0.0.0-pr-35351-sha-068e5c71 upgrade.

More information


Published version	`0.0.0-pr-35351-sha-068e5c71`
Triggered by	@ghengeveld
Repository	storybookjs/storybook
Branch	`review-quality-refactor`
Commit	`068e5c71`
Datetime	Thu Jul 2 14:02:00 UTC 2026 (`1783000920`)
Workflow run	28596005139

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=35351

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Review pages now use a persistent summary overlay, keeping thumbnail previews alive while navigating.
- Navigation and fullscreen controls now adapt more consistently across review, story, and docs views.
Bug Fixes
- Improved review-mode handling so updates, dismissals, and pending changes behave more reliably.
- Fixed preview loading and resize behavior for embedded review thumbnails.
- Review collection/story routes are now recognized more accurately.

…les into CollectionGrid The review thumbnail message layer had a renaming re-export module (iframeResizeMessage.ts) that gave the shared iframe.resize wire protocol a second vocabulary. The parser now lives in shared/constants/iframe-resize.ts next to the payload type it validates, and the frame-style helpers live in CollectionGrid next to the Frame styled component that consumes their CSS variables. Also drops the tautological THUMBNAIL_BOOTSTRAP_SCALE constant test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Slot accounting for the review thumbnail concurrency cap was split across the scheduler (start/finish), the hook (a settle timer after src assignment), and CollectionGrid (iframe load/error). The scheduler now owns the deadline: it starts a slot timer when it starts a task and auto-releases when it fires. Callers get an opaque {forceStart, release} handle instead of a task object with public mutable booleans, so the accounting-drift clamp is no longer needed. Also fixes the header comment that pointed at a CollectionGrid explanation which no longer exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Add 'review' to the API_ViewMode union and introduce a single nav-availability policy (api.getNavAvailability) in the manager-api layout module. Layout, the sidebar menu tool, the fullscreen tool, keyboard shortcuts, and the review summary portal now consume that policy instead of each re-deriving sidebar suppression from isReviewManagerRoute. Also unify the isPagesShown derivation between Layout and Sidebar via isPagesViewMode, and share the review-collection story predicate as isReviewCollectionStoryRoute. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…achine One pure reducer now drives each thumbnail cell through idle -> queued -> booting -> measured -> settled (evicted -> idle), with src, the loading spinner, and remembered dimensions derived from a single state value instead of four independent atoms reset from three places. Each event (enqueued, started, resized, settled, evicted) is one atomic dispatch, which deletes the load-generation counter: stale rAF callbacks and timers are now ignored by phase guards instead of generation bookkeeping. Supporting changes: - Per-hook message listener comparing event.source against the iframe's contentWindow, replacing the module-level handler registry, its never-removed window listener, and the attach/detach-on-load dance. - Visibility is only the mount/evict IntersectionObserver pair (hysteresis via differing rootMargins); the bespoke preload geometry and ResizeObserver are gone. While the summary portal parks the grid off-screen (previewsPaused), the whole visibility subsystem freezes: one gate instead of three. - Deleted the dead frameRef and its cast in CollectionGrid, and the speculative SSR/IntersectionObserver guards in browser-only manager code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Channel handlers and actions now write directly to the module-level reviewStore instead of React state that was mirrored into the store after the fact. ReviewProvider shrinks to channel wiring plus the derivation of index/status/route-dependent values, pushed via a single setDerived path. - Delete the setState mirror, the internalPendingReview side channel, the setState(getState(), next) self-copy, and the notificationKey trigger; ReviewNotification subscribes to the real state/pendingReview fields. - The store owns isInReviewMode including its session persistence, so the path-change resync effect is gone; enterReviewMode/exitReviewMode update the store (optimistically on entry, rolled back on failure). - Replace the REVIEW_EXITING_SESSION_KEY storage mutex with a store field. - dismissReview, acceptPendingReview and getStoryPreviewHref are no longer state fields; they are exported actions / portal-local helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The pending-update-beats-stale precedence rule was implemented three times (ReviewSummaryPortal pre-masked isStale, SummaryScreen and ReviewToolbarHeader each re-implemented the ternary with diverging guards). It is now derived once in ReviewProvider as a single ReviewBanner value (reusing AttentionBanner's discriminated-union props), and both render sites just spread it into AttentionBanner. The isStale/hasPendingUpdate/ onAcceptPendingUpdate prop triplet and the runtime callback-pairing check are gone; the union type makes the pairing impossible to violate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Replace the body-level fixed portal with a persistent overlay slot on Layout. The review layer now renders in slotOverlay, an always-mounted cell that occupies the grid's content area (the full container on mobile), so the CSS grid supplies the sidebar/panel insets that ReviewSummaryPortal previously re-derived by hand. This deletes useDesktopChromeInsets, the duplicated 600px breakpoint matchMedia listener, the -10000px off-screen parking hack, and the legacy portal-host cleanup. The hidden state is reduced to visibility: hidden plus inert, which was verified empirically to keep thumbnail iframe documents alive without reloading. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…actor

…efactor # Conflicts: # code/core/src/manager/components/review/screens/ReviewSummaryPortal.tsx

Keeping it mounted left the previously selected story's iframe alive underneath the summary, so it flashed through for a frame when navigating to a curated story. Unmounting boots a fresh preview per navigation instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The overlay's inert state was gated on a suppression flag set when navigating to a curated story and released only by an effect keyed on the summary route becoming visible. A click that never left the summary route left the flag stuck, so the overlay stayed inert on return. The main preview now unmounts on the summary route, so the suppression mechanism is no longer needed to prevent a stale-story flash. Drop it and gate the overlay purely on the summary route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

ghengeveld · 2026-07-02T14:16:53Z

@copilot resolve the merge conflicts in this pull request

Co-authored-by: ghengeveld <321738+ghengeveld@users.noreply.github.com>

Copilot · 2026-07-02T14:24:46Z

@copilot resolve the merge conflicts in this pull request

Resolved in 739fecf.

coderabbitai · 2026-07-02T14:27:18Z

📝 Walkthrough

Walkthrough

This PR introduces a route/view-mode-driven NavAvailability API replacing prior review-route checks in layout, sidebar, toolbar, and menu components; refactors the review store into a core/derived-state model with new actions; replaces ReviewSummaryPortal with an always-mounted ReviewSummaryHost; and rewrites the preview thumbnail scheduler and reducer-based state machine with iframe resize message parsing.

Changes

Review mode, navigation and thumbnail pipeline

Layer / File(s)	Summary
Nav availability API contract `code/core/src/manager-api/modules/layout.ts`, `code/core/src/manager-api/modules/shortcuts.ts`, `code/core/src/manager-api/tests/layout.test.ts`	Adds `NavAvailability` type and `getNavAvailability()` method to layout `SubAPI`, replacing route-based sidebar shortcut blocking, with new tests.
Route helpers and API_ViewMode 'review' `code/core/src/shared/review/routes.ts`, `.../routes.test.ts`, `.../index.ts`, `code/core/src/types/modules/api.ts`, `code/core/src/router/utils.ts`, `.../utils.test.ts`	Adds `isReviewCollectionStoryRoute`, extends `isReviewManagerRoute` and `API_ViewMode`, and makes `parsePath` detect review summary paths.
Nav availability UI wiring `code/core/src/manager/components/layout/Layout.tsx`, `.../Layout.stories.tsx`, `.../sidebar/Sidebar.tsx`, `.../preview/Toolbar.tsx`, `.../preview/tools/menu.tsx`, `.../preview/Preview.tsx`, `.../preview/utils/types.tsx`, `.../container/Preview.tsx`	Updates layout/sidebar/toolbar/menu/preview components to consume `getNavAvailability()`, `isPagesViewMode`, and `isReviewCollectionStoryRoute` for sidebar overlay and visibility logic.
Review store core/derived state model `code/core/src/manager/components/review/review-store.ts`, `.../review-store.test.ts`	Replaces the prior store with split core/derived state, snapshot/commit updates, and new action methods.
Review mode and navigation actions `.../review-mode.ts`, `.../review-mode.test.ts`, `.../constants.ts`, `.../review-actions.ts`, `.../review-actions.test.ts`	Rewires review mode/actions to use `reviewStore` instead of `sessionStore`, removes overlay suppression, and adds `dismissReview`/`acceptPendingReview` helpers.
ReviewProvider, notification and toolbar header integration `.../components/ReviewProvider.tsx`, `.../ReviewNotification.tsx`, `.../ReviewToolbarHeader.tsx`, `.../ReviewToolbarHeader.stories.tsx`	Rewires provider/notification/header to consume `useReview()`/`reviewStore` and a computed `ReviewBanner`.
Summary screen banner and persistent host `.../screens/SummaryScreen.tsx`, `.../SummaryScreen.stories.tsx`, `.../screens/ReviewSummaryHost.tsx`, `.../ReviewSummaryPortal.tsx` (removed), `.../ReviewPersistentLayer.tsx`, `code/core/src/manager/App.tsx`, `.../ReviewPage.stories.tsx`	Replaces boolean banner props with a single `ReviewBanner`, introduces `ReviewSummaryHost` replacing the portal, and wires the persistent overlay into `App.tsx`.
Preview thumbnail scheduler and reducer state machine `.../components/previewScheduler.ts`, `.../previewThumbnailState.ts`, `.../previewThumbnailState.test.ts`, `.../usePreviewThumbnail.ts`, `.../CollectionGrid.tsx`, `code/core/src/shared/constants/iframe-resize.ts`, `.../iframe-resize.test.ts`, `.../iframeResizeMessage.ts` (removed)	Replaces the scheduler task model with a handle-based scheduler, adds a `thumbnailReducer` state machine, and adds `parseIframeResizeMessage` validation.

Estimated code review effort: 4 (Complex) | ~75 minutes

Sequence Diagram(s)

sequenceDiagram
  participant App
  participant Layout
  participant ReviewPersistentLayer
  participant ReviewSummaryHost
  participant reviewStore
  App->>Layout: slotOverlay=ReviewPersistentLayer, slotMain=MainPreview
  Layout->>ReviewPersistentLayer: render overlay slot
  ReviewPersistentLayer->>ReviewSummaryHost: render always-mounted host
  ReviewSummaryHost->>reviewStore: read isSummaryVisible/isInReviewMode
  reviewStore-->>ReviewSummaryHost: state
  ReviewSummaryHost-->>App: toggle visibility/pointerEvents/inert

sequenceDiagram
  participant IntersectionObserver
  participant usePreviewThumbnail
  participant previewScheduler
  participant thumbnailReducer
  participant iframe
  IntersectionObserver->>usePreviewThumbnail: isInView=true
  usePreviewThumbnail->>previewScheduler: enqueuePreview(start)
  previewScheduler-->>usePreviewThumbnail: PreviewHandle
  usePreviewThumbnail->>thumbnailReducer: dispatch(enqueued/started)
  iframe->>usePreviewThumbnail: postMessage resize payload
  usePreviewThumbnail->>thumbnailReducer: dispatch(resized/settled)
  usePreviewThumbnail->>previewScheduler: handle.release()

Possibly related PRs

storybookjs/storybook#35220: Both PRs modify parsePath/review routing behavior in core/src/router/utils.ts, directly related to review view-mode detection.
storybookjs/storybook#35232: Both PRs touch the ReviewPersistentLayer/review summary overlay flow in core, with this PR swapping in ReviewSummaryHost for what the related PR introduced as ReviewSummaryPortal.
storybookjs/storybook#35322: Both PRs modify Layout.tsx sidebar visibility/collapse logic for review mode.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

storybook-app-bot · 2026-07-03T07:09:28Z

Package Benchmarks

^{Commit: 11a890b, ran on 3 July 2026 at 08:47:59 UTC}

The following packages have significant changes to their size or dependencies:

`@storybook/angular-vite`

	Before	After	Difference
Dependency count	34	32	🎉 -2 🎉
Self size	22.41 MB	22.41 MB	0 B
Dependency size	22.54 MB	20.07 MB	🎉 -2.47 MB 🎉
Bundle Size Analyzer	Link	Link

parsePath now maps /review/ and /review to viewMode review so manager state matches the layout policy. Rename previewsPaused to summaryHidden across the thumbnail pipeline for clearer intent.

ghengeveld and others added 9 commits July 2, 2026 13:07

Merge branch 'review-thumbnail-phase-machine' into review-quality-ref…

97a19c7

…actor

Merge branch 'worktree-agent-abb41cfa8d3e35cae' into review-quality-r…

8b20f9c

…efactor # Conflicts: # code/core/src/manager/components/review/screens/ReviewSummaryPortal.tsx

ghengeveld added maintenance User-facing maintenance tasks ci:normal Run our default set of CI jobs (choose this for most PRs). qa:needed Pull Requests that will need manual QA prior to release. labels Jul 2, 2026

ghengeveld self-assigned this Jul 2, 2026

ghengeveld and others added 2 commits July 2, 2026 15:45

ghengeveld marked this pull request as ready for review July 2, 2026 14:16

Copilot started work on behalf of ghengeveld July 2, 2026 14:17

View session

Merge origin/next into review-quality-refactor

739fecf

Co-authored-by: ghengeveld <321738+ghengeveld@users.noreply.github.com>

Copilot finished work on behalf of ghengeveld July 2, 2026 14:25

ghengeveld added qa:success Pull Requests that were successfully QA'ed by the release team. and removed qa:needed Pull Requests that will need manual QA prior to release. labels Jul 3, 2026

Sidnioulz reviewed Jul 3, 2026

View reviewed changes

Comment thread code/core/src/manager/components/preview/tools/menu.tsx

Wire review summary paths to viewMode review and rename summaryHidden

5a4b09c

parsePath now maps /review/ and /review to viewMode review so manager state matches the layout policy. Rename previewsPaused to summaryHidden across the thumbnail pipeline for clearer intent.

Navigation Menu

Repository navigation

Manager: Restructure review feature state, layout integration, and thumbnail pipeline - #35351

Manager: Restructure review feature state, layout integration, and thumbnail pipeline#35351
ghengeveld merged 14 commits into
nextfrom
review-quality-refactor

ghengeveld commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

ghengeveld commented Jul 2, 2026

Uh oh!

Copilot AI commented Jul 2, 2026

Uh oh!

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

storybook-app-bot Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ghengeveld commented Jul 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Summary by CodeRabbit

Uh oh!

ghengeveld commented Jul 2, 2026

Uh oh!

Copilot AI commented Jul 2, 2026

Uh oh!

coderabbitai Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

storybook-app-bot Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Package Benchmarks

@storybook/angular-vite

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghengeveld commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

storybook-app-bot Bot commented Jul 3, 2026 •

edited

Loading

`@storybook/angular-vite`