UI: Add isolation mode shortcut#33074
Conversation
|
View your CI Pipeline Execution ↗ for commit 631a01d
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughAdds api.openInIsolation (implementation in stories SubAPI), a keyboard shortcut for it, updates ShareMenu to call the API instead of composing a URL, extends the public API type composition, and adds documentation/snippets for the new method and examples. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ShareMenu
participant ManagerAPI as API
participant Browser as Window
User->>ShareMenu: Click "Open in isolation"
activate ShareMenu
ShareMenu->>API: api.openInIsolation(storyId, refId)
activate API
Note right of API: determine iframe URL (ref vs preview)\nbuild story href (getStoryHref)\nmerge viewMode/query params
API->>Browser: window.open(isolationUrl, '_blank', 'noopener,noreferrer')
deactivate API
Browser->>Browser: Load story in isolation
deactivate ShareMenu
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.{js,jsx,ts,tsx,json,md,html,css,scss}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,jsx,json,html,ts,tsx,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (7)📚 Learning: 2025-11-05T09:38:47.712ZApplied to files:
📚 Learning: 2025-09-24T09:39:39.233ZApplied to files:
📚 Learning: 2025-09-18T20:51:06.618ZApplied to files:
📚 Learning: 2025-09-18T20:51:06.618ZApplied to files:
📚 Learning: 2025-11-05T09:37:25.920ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.109ZApplied to files:
📚 Learning: 2025-12-22T22:03:40.109ZApplied to files:
⏰ 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). (3)
🔇 Additional comments (2)
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: 2
🧹 Nitpick comments (3)
docs/addons/addons-api.mdx (1)
168-181: openInIsolation docs align with implementation; minor wording tweak possibleThe new
api.openInIsolation(storyId, refId?, viewMode?)section accurately describes the parameters and behavior (including local vs external refs and view mode), and the snippet inclusion is consistent with other API sections. As a small style improvement, you could merge theviewModebullet into a single sentence, e.g. “The view mode to open the story in; can be'story'or'docs'(optional, defaults to the current view mode).”code/core/src/manager-api/modules/shortcuts.ts (1)
95-120: Shortcut wiring for openInIsolation is consistent; consider guarding on storyIdAdding
openInIsolationtoAPI_Shortcuts, assigning['alt', 'shift', 'I']indefaultShortcuts, and handling it inhandleShortcutFeatureby callingfullAPI.openInIsolation(storyId, refId)is consistent with how other shortcuts are integrated and will respect persisted shortcut state. You might want to add a lightweight guard so the shortcut is a no‑op when there is no active story (e.g., on settings pages), to avoid opening an isolation URL with an undefined story ID.- case 'openInIsolation': { - const { refId, storyId } = store.getState(); - fullAPI.openInIsolation(storyId, refId); - break; - } + case 'openInIsolation': { + const { refId, storyId } = store.getState(); + if (storyId) { + fullAPI.openInIsolation(storyId, refId); + } + break; + }Also applies to: 135-160, 402-406
code/core/src/manager/components/preview/tools/share.tsx (1)
163-165: Simplifymappernow that URL building lives in the manager API
Consumernow only needsstoryIdandrefIdforShareMenu, butmapperstill computesbaseUrlandqueryParamsthat are no longer used. You can trim this logic and avoid duplicating iframe URL concerns that are already encapsulated inopenInIsolation:-const mapper = ({ state }: Combo) => { - const { storyId, refId, refs } = state; - const { location } = document; - // @ts-expect-error (non strict) - const ref = refs[refId]; - let baseUrl = `${location.origin}${location.pathname}`; - - if (!baseUrl.endsWith('/')) { - baseUrl += '/'; - } - - return { - refId, - baseUrl: ref ? `${ref.url}/iframe.html` : (PREVIEW_URL as string) || `${baseUrl}iframe.html`, - storyId, - queryParams: state.customQueryParams, - }; -}; +const mapper = ({ state }: Combo) => { + const { storyId, refId } = state; + return { storyId, refId }; +};This keeps the Share tool purely concerned with selecting IDs and lets the manager API own all isolation URL construction.
Also applies to: 176-181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
code/core/src/manager-api/modules/open-in-isolation.tsx(1 hunks)code/core/src/manager-api/modules/shortcuts.ts(3 hunks)code/core/src/manager-api/root.tsx(3 hunks)code/core/src/manager/components/preview/tools/share.tsx(6 hunks)docs/_snippets/storybook-addons-api-addnotification.md(1 hunks)docs/_snippets/storybook-addons-api-getcurrentstorydata.md(1 hunks)docs/_snippets/storybook-addons-api-openineditor.md(1 hunks)docs/_snippets/storybook-addons-api-openinisolation.md(1 hunks)docs/_snippets/storybook-addons-api-togglefullscreen.md(1 hunks)docs/_snippets/storybook-addons-api-togglepanel.md(1 hunks)docs/addons/addons-api.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager-api/modules/open-in-isolation.tsxcode/core/src/manager-api/modules/shortcuts.tscode/core/src/manager/components/preview/tools/share.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager-api/modules/open-in-isolation.tsxdocs/_snippets/storybook-addons-api-openinisolation.mddocs/_snippets/storybook-addons-api-getcurrentstorydata.mddocs/addons/addons-api.mdxcode/core/src/manager/components/preview/tools/share.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager/components/preview/tools/share.tsx
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Applied to files:
code/core/src/manager/components/preview/tools/share.tsx
🧬 Code graph analysis (4)
code/core/src/manager-api/modules/open-in-isolation.tsx (2)
code/core/src/manager-api/tests/url.test.js (1)
store(143-151)code/core/src/components/index.ts (1)
getStoryHref(121-121)
code/core/src/manager-api/modules/shortcuts.ts (1)
code/core/src/manager-api/tests/url.test.js (1)
store(143-151)
code/core/src/manager-api/root.tsx (2)
code/core/src/manager-api/modules/open-in-editor.tsx (2)
openInEditor(33-49)SubAPI(19-29)code/core/src/manager-api/modules/open-in-isolation.tsx (2)
SubAPI(10-20)openInIsolation(24-48)
code/core/src/manager/components/preview/tools/share.tsx (2)
code/core/src/manager-api/root.tsx (1)
useStorybookApi(299-302)code/core/src/manager-api/modules/open-in-isolation.tsx (1)
openInIsolation(24-48)
🪛 LanguageTool
docs/addons/addons-api.mdx
[style] ~174-~174: To form a complete sentence, be sure to include a subject.
Context: ...e: The view mode to open the story in. Can be 'story'or'docs'` (optional, def...
(MISSING_IT_THERE)
[grammar] ~184-~184: Ensure spelling is correct
Context: ...eful for "Edit in IDE" functionality in addons. - payload.file: The file path to op...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (11)
docs/_snippets/storybook-addons-api-getcurrentstorydata.md (1)
1-7: LGTM! The snippet clearly demonstrates the basic usage ofapi.getCurrentStoryData()with correct addon registration and appropriate logging of story metadata. Well-suited as complementary documentation for the expanded API surface introduced in this PR.docs/_snippets/storybook-addons-api-togglepanel.md (1)
1-11: Toggle panel snippet is accurateThe example correctly shows default toggle, explicit show, and explicit hide usage for
api.togglePanel, matching the API contract.docs/_snippets/storybook-addons-api-togglefullscreen.md (1)
1-11: Fullscreen snippet matches API behaviorThe snippet clearly demonstrates toggling, enabling, and disabling fullscreen via
api.toggleFullscreenand is consistent with the described API.docs/_snippets/storybook-addons-api-addnotification.md (1)
1-27: Notification snippet is clear and representativeThe examples for
api.addNotification(with and without icon) are realistic and align with the documented shape of the notification object.docs/_snippets/storybook-addons-api-openineditor.md (1)
1-23: Open‑in‑editor snippet looks goodThe snippet correctly demonstrates both a simple call and handling the Promise response from
api.openInEditor, matching the SubAPI behavior.docs/addons/addons-api.mdx (1)
182-196: openInEditor text remains consistentThe surrounding
api.openInEditorsection and snippet reference still match the existing SubAPI (file, optional line/column, Promise result), so the new openInIsolation docs slot in cleanly without regressions.code/core/src/manager-api/root.tsx (1)
58-59: openInIsolation is correctly wired into the manager APIImporting
./modules/open-in-isolation, extending theAPIunion withopenInIsolation.SubAPI, and registering the module alongsideopenInEditorinthis.modulesfollows the existing pattern for new SubAPIs and ensuresuseStorybookApi()consumers can accessapi.openInIsolation.Also applies to: 96-111, 171-187
code/core/src/manager-api/modules/open-in-isolation.tsx (1)
10-20: Well-scoped SubAPI and documentationThe
SubAPIsurface and JSDoc are clear and narrowly focused, which should make reusingopenInIsolationfrom other manager modules straightforward. No issues from a typing/API-design standpoint.code/core/src/manager/components/preview/tools/share.tsx (3)
3-3: Import consolidation away fromgetStoryHreflooks goodSwitching the Share menu to rely only on
Button,PopoverProvider, andTooltipLinkListfromstorybook/internal/components(and delegating URL building to the manager API) keeps this component focused on UI concerns. No issues here.
75-85: ShareMenu props now correctly carrystoryId/refIdinto the APIPassing
storyIdandrefIdas explicit props toShareMenuis a nice improvement over reconstructing URLs locally, and the typings (refId: string | null | undefined,storyId: string) match the manager state usage.
91-92: Shortcut wiring and memo dependencies are consistentUsing
shortcutKeys?.openInIsolationfor the right-hand shortcut hint and delegating the click handler toapi.openInIsolation(storyId, refId)keeps the behavior aligned with the new manager API and shortcut system. TheuseMemodependency array correctly includesapi,openInIsolation,refId, andstoryId, so the handler won’t capture stale values.Also applies to: 110-115, 141-151
…okjs/storybook into yann/add-isolation-mode-shortcut
ndelangen
left a comment
ndelangen
left a comment
There was a problem hiding this comment.
LGTM, maybe apply the 1 review suggestion I made, but not a blocker.
…okjs/storybook into yann/add-isolation-mode-shortcut
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/core/src/manager-api/modules/stories.ts (1)
712-736: Consider normalizing ref.url to avoid potential double slashes.The implementation is solid and follows good security practices with
noopener,noreferrer. However, the iframe URL construction for refs (line 726) could produce double slashes ifref.urlalready ends with/.Consider applying the same trailing slash normalization used for
baseUrl:const ref = refId ? refs[refId] : null; let baseUrl = `${location.origin}${location.pathname}`; if (!baseUrl.endsWith('/')) { baseUrl += '/'; } - const iframeUrl = ref - ? `${ref.url}/iframe.html` - : (global.PREVIEW_URL as string) || `${baseUrl}iframe.html`; + let iframeUrl; + if (ref) { + let refUrl = ref.url; + if (!refUrl.endsWith('/')) { + refUrl += '/'; + } + iframeUrl = `${refUrl}iframe.html`; + } else { + iframeUrl = (global.PREVIEW_URL as string) || `${baseUrl}iframe.html`; + } const storyViewMode = viewMode ?? currentViewMode;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/manager-api/modules/stories.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/core/src/manager-api/modules/stories.ts
🧬 Code graph analysis (1)
code/core/src/manager-api/modules/stories.ts (2)
code/core/src/manager-api/tests/url.test.js (1)
store(143-151)code/core/src/components/index.ts (1)
getStoryHref(121-121)
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/manager-api/modules/stories.ts (2)
2-2: LGTM! Import correctly added for new functionality.The
getStoryHrefimport is properly sourced and required for theopenInIsolationimplementation.
288-298: LGTM! API documentation is clear and comprehensive.The JSDoc properly documents the new
openInIsolationmethod with clear parameter descriptions and usage guidance. The type signature correctly reflects optional parameters and view mode constraints.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/manager-api/modules/stories.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager-api/modules/stories.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager-api/modules/stories.ts
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
code/core/src/manager-api/modules/stories.ts (2)
2-2: LGTM!The import of
getStoryHrefis appropriate for constructing the isolation mode URL.
295-305: LGTM!The JSDoc documentation is comprehensive and clearly explains the method's purpose, parameters, and behavior.
| openInIsolation: (storyId, refId, viewMode) => { | ||
| const { location } = global.document; | ||
| const { refs, customQueryParams, viewMode: currentViewMode } = store.getState(); | ||
|
|
||
| // Get the ref object from refs map using refId | ||
| const ref = refId ? refs[refId] : null; | ||
|
|
||
| let baseUrl = `${location.origin}${location.pathname}`; | ||
|
|
||
| if (!baseUrl.endsWith('/')) { | ||
| baseUrl += '/'; | ||
| } | ||
|
|
||
| const iframeUrl = ref | ||
| ? `${ref.url}/iframe.html` | ||
| : (global.PREVIEW_URL as string) || `${baseUrl}iframe.html`; | ||
|
|
||
| const storyViewMode = viewMode ?? currentViewMode; | ||
|
|
||
| const href = getStoryHref(iframeUrl, storyId, { | ||
| ...customQueryParams, | ||
| ...(storyViewMode && { viewMode: storyViewMode }), | ||
| }); | ||
| global.window.open(href, '_blank', 'noopener,noreferrer'); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add validation for invalid refId to match existing patterns.
The implementation should validate that refId exists in the refs map when provided, similar to the resolveStory method (line 350-353). Currently, if an invalid refId is provided, the code silently falls back to the local iframe URL, which could mask configuration errors.
Apply this diff to add validation:
openInIsolation: (storyId, refId, viewMode) => {
const { location } = global.document;
const { refs, customQueryParams, viewMode: currentViewMode } = store.getState();
- // Get the ref object from refs map using refId
+ // Validate and get the ref object from refs map using refId
+ if (refId && !refs[refId]) {
+ logger.warn(`Ref with id ${refId} does not exist`);
+ return;
+ }
+
const ref = refId ? refs[refId] : null;
let baseUrl = `${location.origin}${location.pathname}`;This ensures consistent behavior across the API and provides clear feedback when an invalid refId is used.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In code/core/src/manager-api/modules/stories.ts around lines 723 to 747, the
openInIsolation function does not validate that a provided refId exists in the
refs map and silently falls back to the local iframe URL; update the function to
check if refId is provided and refs[refId] is missing, and throw a clear error
(e.g. "Unknown refId: <refId>") to match resolveStory's behavior — perform the
validation immediately after reading refs and before building iframeUrl, and
keep the rest of the logic unchanged.
|
@yannbf @ghengeveld Is there overlap going on? I think @ghengeveld has worked on a similar thing recently involving creating a better way of constructing the URL and opening it via a keyboard shortcut. |
|
@ndelangen @yannbf Yes there is 🤦 #33416 |
|
That makes sense @ghengeveld I'm trying to determine if your PR supersedes this one, and thus we should close this, or there is value in keeping both open. |
|
@ndelangen It seems to me this PR had a few things mine doesn't have:
Functionally the "open in isolation" behavior is mostly the same. The keyboard shortcut is identical. My PR has better URL handling, consistently applied across the whole manager. |
|
I spoke to @ghengeveld and gave all the context, he is following this up on another PR! |
|
We decided to drop this PR in favor of #33416 and update that with some aspects from this PR:
Since |
4 participants
Closes #
What I did
This PR introduces a new method to Storybook's API:
openInIsolation, which allows developers to programmatically open stories in the same way as the "Open in isolation" button in the share menu. This is also added as a shortcut viaoption + shift + I.API
Signature:
api.openInIsolation(storyId, refId?, viewMode?)Opens a story in isolation mode in a new tab/window.
storyId: The ID of the story to open (required)refId: The ID of the ref for external stories. Passnull/undefinedfor local stories (optional)viewMode: The view mode to open the story in. Can be'story'or'docs'(optional, defaults to current view mode)Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
option + shift + Iand expect the story to be opened in isolation (docs or story viewmodes)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.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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
User Experience
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.