Repository navigation
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works. I ran the #58247 reproduction against the real window-state.ts on a single 1388×781 display. On the base, the window never reopens maximized and the oversized bounds are saved again on every launch. With this PR, it reopens maximized from the second launch on, and the saved state settles to bounds that fit the display, so the loop ends. The new tests fail on the base and pass with the change. I didn't run the Electron app itself.
Public API: unchanged. This is internal desktop main-process logic, and the window-state file format is the same.
One small point, inline: the fallback to the primary display can't trigger in practice, so it, sameBounds and its test can be dropped.
Related: #56970
|
|
||
| const matched = displays.matching({ x: state.x, y: state.y, width: state.width, height: state.height }) | ||
|
|
||
| const display = displays.all().find((bounds) => sameBounds(bounds, matched)) ?? displays.primary() |
There was a problem hiding this comment.
In the real app, displays.matching is screen.getDisplayMatching(bounds).bounds (windows/index.ts, windows/early.ts). It always returns one of the currently connected displays, so the displays.all().find(sameBounds) ?? displays.primary() fallback never triggers. Using const display = displays.matching({ ... }) directly would keep the fix smaller. You could then drop sameBounds and the "matched display is no longer current" test, which only checks a matching stub that returns a display that isn't connected.
Issue for this PR
Closes #58247
Type of change
What does this PR do?
When the window is maximized,
manageWindowStatekeeps saving the normal (unmaximized) bounds. On a display shorter than the 1280x800 default (e.g. 1388x781), those bounds can be a few pixels taller than the display (y: 34, height: 783). On the next launchresolveWindowStaterejects them and returns the default-size fallback, but that fallback droppedisMaximized/isFullScreenand always put the window on the primary display at0,0. So the window never reopened maximized, and on a multi-monitor setup it also jumped to the primary display.The fallback now:
isMaximizedandisFullScreenfrom the saved statedisplays.matching()(the samescreen.getDisplayMatching()used when the state is recorded), falling back to the primary display only if that display is no longer connecteddisplay.x/display.y) instead of0,0, so monitors to the right, to the left (negative x) or a primary display not at the origin all workRelated: #57088 changes how edge-snapped bounds are accepted. It doesn't cover this case, but both PRs touch
resolveWindowState.How did you verify your code works?
x: 1920and atx: -1440, which stay on that monitor with the maximized flag kept0,0devand pass with the fix. The test displays use an Electron-likematchinghelper (largest overlap, else primary).cd packages/desktop && bun run test: 136 pass, 1 skip, 0 fail.bun run check(lint + typecheck across the monorepo) passes.Screenshots / recordings
N/A, not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #56970