Skip to content

✨ Default static-site and storybook screenshots to full-page#226

Merged
Robdel12 merged 1 commit intomainfrom
feat/full-page-default-static-storybook
Feb 16, 2026
Merged

✨ Default static-site and storybook screenshots to full-page#226
Robdel12 merged 1 commit intomainfrom
feat/full-page-default-static-storybook

Conversation

@Robdel12
Copy link
Contributor

@Robdel12 Robdel12 commented Feb 16, 2026

Why

Full-page capture is usually what teams expect for static pages and Storybook stories.

Today, both SDKs default to viewport-only screenshots. That works, but it also creates surprises when teams assume they are capturing the full scrollable content by default.

What Changed

  • Set screenshot.fullPage default to true in both SDKs.
  • Added explicit CLI opt-out in both SDKs with --no-full-page.
  • Updated screenshot helper fallback defaults to fullPage: true.
  • Updated docs/examples/tests to match new behavior.

Static Site SDK

  • Config/runtime defaults updated.
  • Zod schema defaults updated.
  • Init config schema updated.

Storybook SDK

  • Config/runtime defaults updated.
  • Init config schema updated.

⚠️ Breaking Change

This changes default screenshot behavior from viewport-only to full-page for users who do not explicitly set fullPage.

How To Keep Previous Behavior

If a team wants viewport-only capture (previous default), they can use either:

  • Config: screenshot: { fullPage: false }
  • CLI: --no-full-page

Test Plan

  • cd clients/static-site && npm test
  • cd clients/storybook && npm test

Flip screenshot.fullPage defaults to true across config, schema, CLI init defaults, and screenshot helpers in both SDKs.

Add --no-full-page so teams can keep viewport-only behavior explicitly.

This is a breaking default change; existing behavior can be preserved with screenshot.fullPage: false or --no-full-page.
@Robdel12 Robdel12 changed the title Default static-site and storybook screenshots to full-page ✨ Default static-site and storybook screenshots to full-page Feb 16, 2026
@vizzly-testing
Copy link

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

fullscreen-viewer

Review changes

CLI TUI - Approved

5 comparisons, no changes detected.

View build


feat/full-page-default-static-storybook · 2d194733

@claude
Copy link

claude bot commented Feb 16, 2026

Code Review Feedback

This PR successfully implements the default change from viewport-only to full-page screenshots for both the static-site and Storybook SDKs. The implementation is clean and well-tested. Here's my detailed review:


✅ Strengths

1. Comprehensive Coverage

  • Both SDKs updated consistently (static-site and Storybook)
  • All relevant locations updated: defaults, schemas, config files, function parameters, docs
  • Proper CLI option handling with both --full-page and --no-full-page

2. Well-Tested

  • Tests updated to reflect new defaults
  • Test coverage for both --full-page and --no-full-page CLI options
  • Config merging tests properly validate the new behavior
  • All test assertions correctly flipped to match new defaults

3. Clear Documentation

  • README files updated with new defaults
  • JSDoc comments updated to reflect fullPage: true as default
  • Example configs updated
  • Migration path documented in PR description

4. Backward Compatibility

  • Users can easily opt-out with screenshot: { fullPage: false } in config
  • CLI option --no-full-page provides runtime override
  • Config merging preserves explicit user settings

🔍 Potential Issues

1. Formatting Inconsistency in config-schema.js

Location: clients/static-site/src/config-schema.js:99-110

The PR introduces formatting changes to include and exclude fields that aren't related to the core change:

// Before:
include: z.union([z.string(), z.array(z.string())]).nullable().optional(),
exclude: z.union([z.string(), z.array(z.string())]).nullable().optional(),

// After:
include: z
  .union([z.string(), z.array(z.string())])
  .nullable()
  .optional(),
exclude: z
  .union([z.string(), z.array(z.string())])
  .nullable()
  .optional(),

Impact: Minor - This is just formatting, but it adds noise to the diff.
Recommendation: Either revert these formatting changes or mention in the PR description that formatting improvements were included. Based on CLAUDE.md conventions, changes should be focused and avoid unnecessary modifications.


💡 Suggestions

1. Migration Documentation

Consider adding a note in the docs about the breaking change and how teams can verify their behavior:

## Migrating from Viewport-Only Defaults

If your tests expect viewport-only screenshots, update your config:

```js
export default {
  screenshot: {
    fullPage: false, // Match previous default
  },
};

Or use the CLI flag: --no-full-page


### 2. **Changelog Entry**
Ensure this is prominently documented in CHANGELOG.md as a breaking change with migration instructions.

---

## 🔒 Security Considerations
✅ No security concerns identified. The change only affects default screenshot behavior.

---

## ⚡ Performance Considerations

**Potential Impact:** Full-page screenshots can be:
- **Slower** to capture (more scrolling/rendering)
- **Larger** in file size (more pixels)
- **More memory-intensive** for very long pages

**Recommendation:** Consider documenting performance implications in the README or adding a note about when users might want to use `fullPage: false` (e.g., for very long pages or performance-sensitive CI pipelines).

Example addition to README:
```markdown
### Performance Note
Full-page screenshots capture the entire scrollable content, which may:
- Take longer for very long pages
- Generate larger file sizes
- Use more memory

For performance-critical scenarios or very long pages, consider using `fullPage: false`.

🧪 Test Coverage

Excellent test coverage:

  • CLI option parsing for both --full-page and --no-full-page
  • Config merging preserves user settings correctly
  • Default behavior validated in schema tests
  • Screenshot function tests updated

📋 Summary

Overall Assessment: This is a solid, well-implemented PR. The breaking change is justified and clearly communicated.

Recommended Actions Before Merge:

  1. Minor: Decide on the formatting changes in config-schema.js (revert or document)
  2. 💡 Optional: Add performance note to README
  3. 💡 Optional: Add migration guide snippet to docs
  4. ⚠️ Important: Ensure CHANGELOG.md is updated with breaking change notice

Verdict: Approve with minor suggestions. The code quality is high and the change aligns with user expectations for full-page capture being the intuitive default.

@Robdel12 Robdel12 merged commit e3bf2de into main Feb 16, 2026
31 of 32 checks passed
@Robdel12 Robdel12 deleted the feat/full-page-default-static-storybook branch February 16, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant