fix(browse): add viewport auto to unpin a fixed viewport without restart#1881
Open
jbetala7 wants to merge 3 commits into
Open
fix(browse): add viewport auto to unpin a fixed viewport without restart#1881jbetala7 wants to merge 3 commits into
viewport auto to unpin a fixed viewport without restart#1881jbetala7 wants to merge 3 commits into
Conversation
…start `browse viewport WxH` pins the viewport via setViewportSize, overriding the launch-time `viewport: null` that lets the viewport follow the real window. Once pinned there was no way back to window-following short of `browse restart`, which kills the Chrome session. Skills that set a breakpoint as a side-effect (responsive testing, benchmarks) left the browser stuck at that size. Add `viewport auto` (aliases `reset`/`unpin`): it rebuilds the context with `viewport: null` the same way `viewport --scale` rebuilds it, preserving cookies, storage, and tab URLs, and resets deviceScaleFactor to 1 (Playwright forbids a custom scale with a null viewport). Refused in headed mode, where the viewport already tracks the real browser window. `viewport auto --scale` is rejected as contradictory. Closes garrytan#1059 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Asserts viewport auto clears a pinned size (page.viewportSize() -> null), that reset/unpin are accepted aliases, that auto resets deviceScaleFactor to 1, and that `viewport auto --scale` is rejected. All four fail on main, where `viewport auto` is parsed as a malformed size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a `viewport auto` example to the responsive-testing recipe and regenerates the command reference tables from commands.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1059
Problem
browse viewport WxHpins the viewport with Playwright'ssetViewportSize, overriding the launch-timeviewport: nullthat lets the viewport follow the real Chrome window. Once pinned there was no way back to window-following short ofbrowse restart, which kills the Chrome session (and drops a/connect-chromeheaded attachment back tolaunched). Skills that set a breakpoint as a side-effect (responsive testing, benchmarks) leave the browser stuck at that size, so the visible Chrome window ends up larger than the viewport Claude renders into.Repro (before)
Fix
Add
browse viewport auto(aliasesreset/unpin): it clears the pin and restores window-following without tearing down the session.viewport: nullthrough the samerecreateContext()path thatviewport --scalealready uses, so cookies, storage, and open tab URLs are preserved.deviceScaleFactorto 1, since Playwright forbids a custom scale alongside a null viewport./connect-chrome), where the viewport already tracks the real browser window — consistent with howviewport --scalerefuses headed mode.viewport auto --scale <n>is rejected as contradictory.A new private
viewportContextOptions()helper is the single source of the viewport/scale slice of the context options, used by both the initial launch and bothrecreateContextpaths, so the three sites can't drift.Testing
bun test browse/test/commands.test.ts(227 pass) andbrowse/test/handoff.test.ts(15 pass) — both exerciserecreateContext. Four new tests in theviewport auto (unpin)block assert:viewport autoclears a pinned size (page.viewportSize()->null)reset/unpinare accepted aliasesviewport autoresetsdeviceScaleFactorto 1viewport auto --scaleis rejectedAll four fail on
main, whereviewport autois parsed as a malformed size and throws the usage error — confirming the capability was genuinely absent.bun run gen:skill-docsregenerated the command-reference tables andllms.txt; skill-validation + gen-skill-docs quality suites pass (719 tests).Notes / risk
viewport WxH/viewport --scalecalls (the launch path resolves to the same pinned options whenviewportAutois false).🤖 Generated with Claude Code