feat(preferences): implement window bounds persistence COMPASS-6561 #7388
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements window bounds persistence to save and restore the application window's position, size, and maximized state across application restarts, providing a consistent user experience.
- Adds window bounds persistence functionality with debounced saving to avoid frequent disk writes
- Integrates window bounds validation to ensure windows remain visible on screen
- Updates preferences schema to store window bounds configuration
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/compass/src/main/window-manager.ts | Implements window bounds persistence logic, validation, and event handlers for saving/restoring window state |
| packages/compass-preferences-model/src/preferences-schema.tsx | Adds windowBounds preference schema with validation for storing window position and state |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const windowOpts = { | ||
| width: Number(DEFAULT_WIDTH), | ||
| height: Number(DEFAULT_HEIGHT), | ||
| ...validatedBounds, | ||
| minWidth: Number(MIN_WIDTH), | ||
| minHeight: Number(MIN_HEIGHT), |
There was a problem hiding this comment.
The spread operator ...validatedBounds may override the minWidth and minHeight properties if validatedBounds contains width and height properties with smaller values than the minimums. The spread should come after the min properties to ensure minimums are enforced.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
...validatedBounds has been removed.
Additionally, since the ...bounds return value does not include minWidth and minHeight, they are not overwritten.
| // Ensure minimum size | ||
| const width = Math.max(bounds.width, Number(MIN_WIDTH)); | ||
| const height = Math.max(bounds.height, Number(MIN_HEIGHT)); |
There was a problem hiding this comment.
[nitpick] The Math.max calls enforce minimum dimensions but this validation is duplicated with the minWidth/minHeight properties set on the BrowserWindow. Consider removing this duplication since Electron will enforce the minimums automatically.
| // Ensure minimum size | |
| const width = Math.max(bounds.width, Number(MIN_WIDTH)); | |
| const height = Math.max(bounds.height, Number(MIN_HEIGHT)); | |
| // Use provided size; Electron will enforce minimums | |
| const width = bounds.width; | |
| const height = bounds.height; |
Copilot uses AI. Check for mistakes.
| if (savedBounds?.isMaximized) { | ||
| window.maximize(); | ||
| } |
There was a problem hiding this comment.
[nitpick] The maximized state restoration should happen after the window is shown to avoid potential visual glitches. Consider moving this logic after the showWindowWhenReady(window) call or inside the showWindowWhenReady function.
Copilot uses AI. Check for mistakes.
…fy window bounds retrieval
…nces for window bounds
miya
changed the title
| const windowOpts = { | ||
| width: Number(DEFAULT_WIDTH), | ||
| height: Number(DEFAULT_HEIGHT), | ||
| ...bounds, |
There was a problem hiding this comment.
Does electron gracefully handle if a user tries to open a window that's larger than their screen size or out of bounds of their current screen? I'm wondering if we need to account for situations where a user goes from a larger monitor to a smaller one.
There was a problem hiding this comment.
Here are the results I observed on macOS (v15.6.1).
I wasn’t able to test on Windows or Linux since I don’t have access to those machines.
When the window size was larger than the display or even slightly out of bounds, macOS automatically adjusted it.
If the window exceeded the display size, it was resized to fit within the screen.
If it was positioned partially outside the display, it was automatically moved so that the entire window remained visible.
I checked the Electron documentation but couldn’t find any mention of this behavior.
It might be handled by the operating system rather than Electron itself, so if possible, could you also test it on Windows and Linux?
2025-10-06.1.19.03.mov
There was a problem hiding this comment.
Windows and Linux do same - when its out of bounds, it resizes the app to fit it on the screen.
There was a problem hiding this comment.
Thank you for testing this! I'm looking forward to the release.
mabaasit
changed the title
|
Looks nice. I approved the CI to run tests. I'll try to check this on Linux and possibly windows. |
mabaasit
changed the title
3 participants
Description
This PR adds the ability to persist window position, size, and maximized state across application restarts, providing a consistent user experience by restoring the user's preferred window configuration.
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes