UI: Keep preview frame stable in overall layout

[Sidnioulz]

Closes #32970, hopefully

What I did

I believe crashes occured because the preview element is positioned in different DOM trees between the mobile and desktop layouts, causing React to re-render it entirely when we open DevTools with the right layout.

This causes React to use its internal restoreComponent function that attempts to restore a previously discarded element. Among other things, it attempts to restore focus. This is a useful heuristic per se, but in our case, the preview frame runs with Testing Library, and Testing Library's patchedFocus method fails when called through restoreComponent. It appears a reference to window has gone stale in the meantime.

I am not fixing the underlying issue in TL here, but rather, ensuring our preview stays stable when switching layouts. This circumvents the issue and improves performance and perceived speed by keeping the preview element mounted.

Checklist for Contributors

Testing

Manual testing

• Open DevTools
• Position it so that the mobile layout appears
• Spam F12 like your life depends on it to try and trigger the race condition / bug

edit: Check #32970 for more info on reproduction if needed

Documentation

ø

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

• 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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

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

Summary by CodeRabbit

• Refactor
  • Reorganized layout rendering logic to improve desktop and mobile view organization
  • Refined panel display behavior with enhanced conditional control
  • Updated notification rendering to mobile-only presentation

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[nx-cloud]

View your CI Pipeline Execution ↗ for commit a48e809

Command	Status	Duration	Result
nx run-many -t compile,check,knip,test,pretty-d...	✅ Succeeded	8m 35s	View ↗

---

☁️ Nx Cloud last updated this comment at 2025-12-30 14:29:57 UTC

[coderabbitai]

📝 Walkthrough

Walkthrough

Restructures the Layout component's rendering logic by relocating PanelContainer from the desktop conditional branch to a separate condition, deferring MainContentMatcher rendering, and making notifications mobile-only. The component now conditionally renders PanelContainer based on isDesktop && showPanel, with rendering order and scope modified.

Changes

Cohort / File(s)	Change Summary
Layout Component Rendering Restructure code/core/src/manager/components/layout/Layout.tsx	Reorganized conditional rendering: PanelContainer moved to separate isDesktop && showPanel condition; MainContentMatcher deferred to render after header; Notifications restricted to mobile rendering only; Desktop and Mobile container rendering unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b439f83 and a48e809.

📒 Files selected for processing (1)

• code/core/src/manager/components/layout/Layout.tsx

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{js,jsx,ts,tsx,json,md,html,css,scss}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format code using Prettier with yarn prettier --write <file>

Files:

• code/core/src/manager/components/layout/Layout.tsx

**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run ESLint checks using yarn lint:js:cmd <file> or the full command cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives to fix linting errors before committing

Files:

• code/core/src/manager/components/layout/Layout.tsx

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode across all packages

Files:

• code/core/src/manager/components/layout/Layout.tsx

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

• code/core/src/manager/components/layout/Layout.tsx

code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use logger from storybook/internal/node-logger for server-side logging in Node.js code

Files:

• code/core/src/manager/components/layout/Layout.tsx

🧠 Learnings (1)

📓 Common learnings

Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: normal
• GitHub Check: Core Unit Tests, windows-latest
• GitHub Check: nx
• GitHub Check: nx
• GitHub Check: get-parameters

🔇 Additional comments (2)

code/core/src/manager/components/layout/Layout.tsx (2)

172-188: Excellent fix for preview frame stability!

This restructuring solves the core issue by keeping MainContentMatcher (which contains the preview frame via slots.slotMain) mounted unconditionally at line 187. Previously, it was rendered within the desktop/mobile conditional branches, causing React to unmount and remount it during layout switches. This unmount/remount triggered React's restoreComponent flow, leading to the Testing Library crash described in the PR objectives.

By rendering MainContentMatcher outside the conditional branches, the preview element remains stable across layout transitions, which both prevents the crash and improves performance by avoiding unnecessary re-renders.

---

199-199: Verify that desktop notifications aren't being removed unintentionally.

The Notifications component is now gated behind isMobile (line 199), which means desktop users will not see any notifications at all. The NotificationList component has desktop-compatible styling defined and the notifications API tracks state on both viewports, suggesting notifications were previously available to desktop users.

Please confirm one of the following:

1. Desktop notifications were intentionally removed as part of a mobile-first redesign, or
2. An alternative desktop notification mechanism exists that this change accounts for, or
3. This is a regression that should be fixed.

If this change was unintentional, the Notifications component should be rendered on both desktop and mobile.

---

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

[ndelangen]

what an amazing improvement ❤️