🐛 Fix storybook story URL capture and add preview upload

[Robdel12]

Summary

• Fix story URL in screenshot properties: captureAndSendScreenshot was sending the iframe.html?id=... URL (what Playwright navigates to) as the url property. This URL doesn't render in iframes on the dashboard. Now converts it to the proper Storybook story path format (?path=/story/...) via a new toStoryUrl() helper.
• Add preview upload to Storybook CI: Added an "Upload preview" step to the Storybook SDK E2E workflow, matching what the static-site SDK already does.
• Fix test mocks: Updated screenshot.test.js mocks to use the real iframe.html URL (matching what page.url() actually returns) and added story.id to mock objects. Added tests for the new toStoryUrl function.

Test plan

• screenshot.test.js — all 11 tests pass
• Verify story URLs render correctly in Vizzly dashboard iframes
• Verify Storybook preview upload works in CI

[claude]

Code Review

Overall this is a clean, well-scoped fix. The problem is real (iframe.html URLs don't render in dashboard iframes), the solution is simple, and the test coverage is good. A few notes:

toStoryUrl — potential robustness issues

1. storyId is not URL-encoded in the output

The function appends storyId directly to the path string:

return `${url.origin}/?path=/story/${storyId}`;

If storyId contains characters that need encoding in a URL path (e.g. spaces, unicode, ?, #), the resulting URL will be malformed. The second test even highlights this — it passes components/button--primary as the raw story ID, which works fine for slashes, but the same logic would silently break for other special characters.

Consider using encodeURIComponent (or encodeURI) on storyId, or at minimum documenting the assumption that Storybook story IDs only contain path-safe characters (which is generally true in practice, but worth noting).

2. The pageUrl parameter is not actually used beyond extracting the origin

toStoryUrl ignores everything in pageUrl except url.origin. The function could be simplified — or its signature made more explicit — to reflect that only the origin matters:

// Equivalent and arguably clearer:
export function toStoryUrl(pageOrigin, storyId) {
  return `${pageOrigin}/?path=/story/${storyId}`;
}

Alternatively, the function could extract the story ID from pageUrl's id query param directly, removing the need for the storyId argument entirely. Right now the caller is responsible for passing the same ID that's already in the URL — a potential source of drift.

3. No error handling for invalid URLs

new URL(pageUrl) throws a TypeError if pageUrl is not a valid URL. In tests this is fine, but if page.url() ever returns an unexpected value in the wild (e.g. about:blank during page navigation), this will crash unhandled. A minimal guard or try/catch with a fallback would make the function more resilient.

Test coverage — a gap worth noting

The three toStoryUrl unit tests are good. However, none of the captureAndSendScreenshot tests assert that the URL sent to vizzlyScreenshot is the converted story URL (i.e. that toStoryUrl is actually wired up correctly in the integration). The tests only verify that page.screenshot was called with the right options. A test asserting the properties.url value passed to vizzlyScreenshot would catch regressions if the wiring changes.

CI workflow change

The added "Upload preview" step looks correct and consistent with what the static-site job already does. One minor observation: it runs unconditionally on every push/PR, even for draft PRs or branches without a Storybook build. If the example-storybook/dist directory doesn't exist (e.g. if a prior step failed), vizzly preview will likely error and fail the job. Worth checking whether that step should have a continue-on-error: true or a conditional, or whether the existing setup already ensures the dist is always present when this job runs.

Minor nit

The JSDoc comment on toStoryUrl says @param {string} pageUrl - Current page URL (likely iframe.html?id=...&viewMode=story) — the word "likely" undersells it. Based on how Storybook works, this will always be an iframe.html URL. Saying "always" (or removing the hedge) would be more accurate.

---

The core fix is correct and the approach is sound. The main actionable items are:

1. Consider URL-encoding storyId in toStoryUrl (low risk in practice, good hygiene)
2. Add a test asserting the URL property sent to vizzlyScreenshot
3. Consider whether the "Upload preview" CI step needs a guard against a missing dist directory

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - 1 change needs review

Status	Count
Passed	17
Changed	1
Auto-approved	17

Changes needing review (1)

fullscreen-viewer · Firefox · 375×667 · 78.5% diff

Review changes

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{fix/storybook-story-url-and-preview · cf30f37d}