feat: persist and restore main window size and position#3673
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:L
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| if (window.isDestroyed() || window.isMinimized() || window.isFullScreen()) { | ||
| return; |
There was a problem hiding this comment.
🟡 Medium window/DesktopWindow.ts:301
persistWindowState bails out when window.isMinimized() is true, and the close handler calls it after canceling the pending debounce. When the user quits the app while the window is minimized, no state is written even though getNormalBounds() still returns valid restorable bounds in that state, so the next launch restores stale geometry. Consider removing the isMinimized() guard from persistWindowState so the final close always writes the current bounds.
| if (window.isDestroyed() || window.isMinimized() || window.isFullScreen()) { | |
| return; | |
| if (window.isDestroyed() || window.isFullScreen()) { |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindow.ts around lines 301-302:
`persistWindowState` bails out when `window.isMinimized()` is true, and the `close` handler calls it after canceling the pending debounce. When the user quits the app while the window is minimized, no state is written even though `getNormalBounds()` still returns valid restorable bounds in that state, so the next launch restores stale geometry. Consider removing the `isMinimized()` guard from `persistWindowState` so the final close always writes the current bounds.
| } | ||
|
|
||
| const rect: WindowRect = { x: state.x, y: state.y, width: state.width, height: state.height }; | ||
| const onScreen = displays.some((display) => rectsIntersect(display, rect)); |
There was a problem hiding this comment.
🟡 Medium window/DesktopWindowState.ts:86
resolveInitialWindowBounds only requires any pixel of overlap with a display via rectsIntersect. A saved state like { x: 100, y: -590, width: 800, height: 600 } still intersects the screen by a few pixels, so this code restores the off-screen x/y instead of dropping them. Because the app uses a hidden title bar, this can reopen the window with its draggable area completely off-screen and effectively inaccessible, even though the PR is meant to prevent off-screen restores. Consider requiring the title-bar/caption region (or a larger minimum visible margin) to intersect a display before restoring x/y.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindowState.ts around line 86:
`resolveInitialWindowBounds` only requires any pixel of overlap with a display via `rectsIntersect`. A saved state like `{ x: 100, y: -590, width: 800, height: 600 }` still intersects the screen by a few pixels, so this code restores the off-screen `x`/`y` instead of dropping them. Because the app uses a hidden title bar, this can reopen the window with its draggable area completely off-screen and effectively inaccessible, even though the PR is meant to prevent off-screen restores. Consider requiring the title-bar/caption region (or a larger minimum visible margin) to intersect a display before restoring `x`/`y`.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cd5a7ba. Configure here.
| let windowStateSaveFiber: Fiber.Fiber<void, never> | undefined; | ||
| const persistWindowState = () => { | ||
| if (window.isDestroyed() || window.isMinimized() || window.isFullScreen()) { | ||
| return; |
There was a problem hiding this comment.
Minimized window skips close persist
Medium Severity
When the main window closes while minimized, persistWindowState returns early, preventing the final window geometry from being saved. This can lead to stale window state persistence.
Reviewed by Cursor Bugbot for commit cd5a7ba. Configure here.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces new window state persistence with unresolved review comments identifying bugs: window state isn't saved when closing while minimized, and off-screen detection may leave windows inaccessible. These substantive issues warrant human review. You can customize Macroscope's approvability policy. Learn more. |
1 participant
What
The main window now remembers its size, position, and maximized state across launches. Before this, it always reopened at the hardcoded
1100×780inDesktopWindow.ts, throwing away whatever size/position you left it at.How
DesktopWindowStateservice (apps/desktop/src/window/DesktopWindowState.ts) persists geometry towindow-state.jsonin the existing state dir, following theDesktopAppSettingsidiom (effectFileSystem+Schema, falls back to defaults on a missing/corrupt file).DesktopWindowloads the saved geometry before creating the window, then writes it back onresize/move/maximize/unmaximize/close. Writes are debounced with a restartable fiber (same pattern as the existing dev-load retry in that file) so dragging/resizing doesn't thrash the disk.Scope / notes
Testing
pnpm --filter @t3tools/desktop typecheck— cleanvp check— cleanDesktopWindowState.test.ts(off-screen / maximized / size-only cases) and updatedDesktopWindow.test.tsdoubles.Note
Low Risk
Local UX and JSON persistence in userdata only; corrupt/missing files fall back to defaults with no security or backend impact.
Overview
The desktop main window remembers size, position, and maximized state across launches instead of always opening at fixed
1100×780.A new
DesktopWindowStateservice reads/writeswindow-state.jsonin the state directory (same Effect + Schema pattern as other desktop settings).DesktopWindowloads saved geometry beforeBrowserWindowcreation, applies maximize when saved, and persists on resize/move/maximize/unmaximize/close with 400ms debounced writes. Off-screen saved positions are dropped using display work areas (e.g. unplugged monitor) while keeping size and maximized flag. Fullscreen is not persisted; writes failures are logged only.Wiring adds
windowStatePathtoDesktopEnvironment, registers the layer inmain.ts, and extends tests (DesktopWindowState.test.ts, updatedDesktopWindow.testmocks).Reviewed by Cursor Bugbot for commit cd5a7ba. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Persist and restore main window size, position, and maximized state
DesktopWindowStateservice inDesktopWindowState.tsthat reads/writes window geometry towindow-state.jsonin the app state directory.resolveInitialWindowBoundsto constrain the window to visible displays, falling back to defaults if off-screen or no saved state exists.📊 Macroscope summarized cd5a7ba. 4 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.