✨ Capture page URL in all JS client SDKs

[Robdel12]

Summary

Sends the current page URL as a url property when capturing screenshots. This enables the backend to link individual screenshots to their corresponding deploy preview pages (preview-to-screenshot integration).

• Static Site: properties.url = page.url() — Playwright page available in captureAndSendScreenshot
• Storybook: { properties: { url: page.url() } } passed to vizzlyScreenshot — was previously sending no properties
• Vitest: url: page.url() added to the properties object built in toMatchScreenshot
• Ember: url: window.location.href added to browser-side properties payload — screenshot server forwards as-is

All placed before the user's ...customProperties spread so users can override if needed.

Test plan

• npm test — 1892 pass, 0 fail
• npm run lint — clean
• Storybook screenshot test mocks updated with url() method
• Deploy and verify url appears in screenshot records via Vizzly dashboard

[claude]

Code Review: PR #220

Summary

This PR adds url property capture across all JavaScript client SDKs (Ember, Static Site, Storybook, Vitest) to enable preview-to-screenshot integration. The implementation is clean and consistent.

---

✅ Strengths

1. Consistent Implementation
The URL property is added consistently across all 4 JS clients, properly placed before the spread of custom properties to allow user overrides:

• clients/ember/src/test-support/index.js:269 - Browser-side: url: window.location.href
• clients/static-site/src/screenshot.js:106 - Playwright: properties.url = page.url()
• clients/storybook/src/screenshot.js:85 - Playwright: { properties: { url: page.url() } }
• clients/vitest/src/setup.js:48 - Playwright: url: page.url()

2. Test Coverage
The Storybook test mocks were updated to include the url() method, preventing test failures. All tests pass (1892 pass, 0 fail).

3. Proper Placement
The URL is added before ...customProperties spread, allowing users to override if needed—good design choice.

---

🔍 Suggestions & Considerations

1. Missing Test Verification (Medium Priority)
While the Storybook mocks were updated, there are no assertions verifying that the url property is actually sent to the server. Consider adding tests like:

// clients/static-site/tests/screenshot.test.js or sdk-integration.test.js
it('should include page URL in properties', async () => {
  let mockPage = { 
    url: () => 'https://example.com/test-page',
    screenshot: mock.fn(() => Buffer.from('fake'))
  };
  // ... verify vizzlyScreenshot was called with properties.url
});

2. Ruby & Swift Clients (Question)
The PR only touches JS clients. Should the Ruby and Swift clients also send URLs for consistency? If this is a backend feature for preview-to-screenshot linking, having it in all clients would provide a better experience. If intentionally JS-only, consider documenting why.

3. Ember Client URL Format (Minor)
The Ember client uses window.location.href (browser-side) while others use Playwright's page.url(). Both work, but be aware that:

• window.location.href includes hash fragments: http://example.com/page#section
• page.url() behavior depends on Playwright version for hash handling

This is likely fine, but worth noting if the backend expects consistent URL formats.

4. Error Handling (Low Priority)
There's no try-catch around page.url() calls. While Playwright's page.url() is unlikely to throw, defensive coding might help:

properties.url = page.url?.() ?? 'unknown';

This protects against edge cases where page might be in an unexpected state.

5. Documentation (Low Priority)
Consider updating:

• Client SDK READMEs to mention the auto-captured url property
• Main docs to explain how url integrates with preview-to-screenshot feature
• Migration guide if existing users need to know about this new property

---

🔒 Security & Performance

Security: ✅ No concerns. URLs are from trusted sources (test runtime).

Performance: ✅ Negligible impact. page.url() and window.location.href are synchronous, cheap operations.

---

📋 Test Plan Feedback

The PR's test plan shows:

• ✅ Tests passing
• ✅ Lint passing
• ✅ Storybook mocks updated
• ⏳ Deploy verification pending

Recommendation: Before merging, verify on staging that the url property appears correctly in the Vizzly dashboard screenshot records. This confirms end-to-end integration.

---

🎯 Overall Assessment

Quality: High - clean, consistent implementation across all clients
Risk: Low - additive change, doesn't modify existing behavior
Recommendation: ✅ Approve with minor suggestions

The core implementation is solid. Consider addressing the test verification and potentially extending to non-JS clients for completeness, but neither is a blocker for merging.

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - 1 change needs review

Status	Count
Passed	18
Changed	1
Auto-approved	18

Changes needing review (1)

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

Review changes

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{rd/capture-page-url-in-sdk-clients · f25bf596}