Addon Vitest: Set default viewport if applicable #28905
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
| }); | ||
|
|
||
| it('should set the viewport to the specified dimensions from INITIAL_VIEWPORTS', async () => { | ||
| const viewportsParam: any = { |
There was a problem hiding this comment.
style: Using 'any' type here might be too permissive. Consider using a more specific type
| }); | ||
|
|
||
| it('should fallback to DEFAULT_VIEWPORT_DIMENSIONS if defaultViewport does not exist', async () => { | ||
| const viewportsParam: any = { |
There was a problem hiding this comment.
style: Using 'any' type here might be too permissive. Consider using a more specific type
| height: Number.parseInt(styles.height, 10), | ||
| }; | ||
| await page.viewport(width, height); | ||
| const validPixelOrNumber = /^\d+(px)?$/; |
There was a problem hiding this comment.
style: Consider using a more descriptive variable name, e.g. isValidDimension
| // if both dimensions are not valid numbers e.g. 'calc(100vh - 10px)' or '100%', use the default dimensions instead | ||
| if (validPixelOrNumber.test(styles.width) && validPixelOrNumber.test(styles.height)) { |
There was a problem hiding this comment.
style: This comment is slightly misleading. The code uses default dimensions if either width or height is invalid, not only if both are invalid
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 0969121. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
Closes #28901
What I did
The Vitest integration supports viewports, but it only sets them if they match the viewports object. So if you have a viewport like mobile1 which has
320pxwidth and568pxheight, then the plugin will use playwright to set the viewport to 320, 568. The thing is, the default viewport (responsive) is actually invalid, in the sense that it doesn’t exist in the viewports object and therefore doesn’t relate to any width and height values. This PR makes the plugin fall back to a desktop-like dimension of 1200 width and 900 height.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
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/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 canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
This PR enhances viewport handling in the Vitest integration for Storybook, introducing a default viewport size and improving edge case management.
DEFAULT_VIEWPORT_DIMENSIONS(1200x900) incode/addons/vitest/src/plugin/viewports.tsfor fallbacksetViewportfunctioncode/addons/vitest/src/plugin/viewports.test.tsto cover various scenarios