Conversation
4a0d0c5 to
75d14cf
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a viewport configuration option for browser mode testing, allowing developers to specify iframe dimensions either through device presets (e.g., iPhone, iPad models) or custom width/height values. The feature includes a UI selector in the preview header for runtime viewport adjustments, with changes persisted to localStorage per-project. The implementation aligns device presets with Chrome DevTools standards.
Changes:
- Added
viewportconfiguration option toBrowserModeConfigwith support for device presets and custom dimensions - Implemented
ViewportSelectorandViewportFrameUI components for interactive viewport management - Refactored browser UI build from
container.htmltoindex.htmlfor standard naming conventions
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/types/config.ts | Defines DevicePreset and BrowserViewport types for configuration |
| packages/core/src/config.ts | Adds viewport validation and merges viewport into normalized config |
| packages/browser/src/protocol.ts | Adds viewport field to BrowserProjectRuntime protocol type |
| packages/browser/src/hostController.ts | Updates to pass viewport config to UI and renames container.html to index.html |
| packages/browser-ui/src/utils/viewportPresets.ts | Defines device presets aligned with Chrome DevTools specifications |
| packages/browser-ui/src/utils/viewport.ts | Implements viewport type conversions and size calculations |
| packages/browser-ui/src/components/ViewportSelector.tsx | Interactive UI component for selecting and configuring viewports |
| packages/browser-ui/src/components/ViewportFrame.tsx | Wrapper component that applies viewport dimensions to iframes with resize support |
| packages/browser-ui/src/components/PreviewHeader.tsx | Integrates ViewportSelector into the preview header |
| packages/browser-ui/src/main.tsx | Manages viewport state per-project with localStorage persistence |
| packages/browser-ui/rsbuild.config.ts | Renames entry point from 'container' to 'index' |
| e2e/browser-mode/viewport.test.ts | E2E test verifying viewport configuration |
| e2e/browser-mode/fixtures/viewport/* | Test fixture with custom viewport dimensions |
| scripts/dictionary.txt | Adds 'Asus' and 'Zenbook' for spell checking |
| packages/core/tests//snapshots/.snap | Updates snapshots to include viewport field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (typeof viewport === 'string') { | ||
| if (!viewport.trim()) { | ||
| throw new Error('browser.viewport must be a non-empty preset id.'); | ||
| } |
There was a problem hiding this comment.
The validation function accepts any non-empty string as a valid preset, but doesn't verify if the string corresponds to an actual DevicePreset value. This means users could configure an invalid preset like "iPhone99" and it would pass validation at config time, only to cause a runtime error when the UI tries to use it via getPresetInfo().
Consider importing the DevicePreset list into the core package and validating that the string is one of the known presets. Alternatively, add a comment documenting that invalid presets will gracefully fall back to 'full' mode in the UI.
| } | |
| } | |
| // Note: preset ids are only checked for being non-empty here. | |
| // The UI layer (e.g. getPresetInfo) is responsible for resolving | |
| // the preset name and will gracefully fall back to 'full' mode | |
| // if the provided preset id is not recognized. |
| viewport: { | ||
| width: 390, | ||
| height: 844, | ||
| }, |
There was a problem hiding this comment.
The test only covers viewport configuration with explicit width and height values. Consider adding a test case that uses a device preset string (e.g., viewport: 'iPhone12Pro') to verify that preset-based configuration works correctly.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75d14cf559
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Related Links
Checklist