- 
                Notifications
    
You must be signed in to change notification settings  - Fork 242
 
feat(preferences): implement window bounds persistence COMPASS-6561 #7388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
2bad280
              54c08fb
              ad278b4
              6924373
              8d13821
              ebeede5
              d198c68
              1b40f27
              65171d9
              54f712f
              02836d7
              df9f359
              32f90ca
              3dff406
              8ed328c
              c7d854f
              ed70691
              2e4b687
              2750310
              8bdb6c6
              1b86957
              da94013
              f3688b6
              637f305
              a7b69fe
              9f9acc4
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -23,6 +23,8 @@ import { | |
| registerConnectionIdForBrowserWindow, | ||
| } from './auto-connect'; | ||
| 
     | 
||
| import { screen, type Display } from 'electron'; | ||
| 
     | 
||
| const { debug } = createLogger('COMPASS-WINDOW-MANAGER'); | ||
| 
     | 
||
| const earlyOpenUrls: string[] = []; | ||
| 
          
            
          
           | 
    @@ -68,6 +70,7 @@ const DEFAULT_HEIGHT = (() => { | |
| // change significantly at widths of 1024 and less. | ||
| const MIN_WIDTH = process.env.COMPASS_MIN_WIDTH ?? 1025; | ||
| const MIN_HEIGHT = process.env.COMPASS_MIN_HEIGHT ?? 640; | ||
| const SAVE_DEBOUNCE_DELAY = 500; // 500ms delay for save operations | ||
| 
     | 
||
| /** | ||
| * The app's HTML shell which is the output of `./src/index.html` | ||
| 
        
          
        
         | 
    @@ -82,6 +85,95 @@ async function showWindowWhenReady(bw: BrowserWindow) { | |
| bw.show(); | ||
| } | ||
| 
     | 
||
| /** | ||
| * Save window bounds to preferences | ||
| */ | ||
| async function saveWindowBounds( | ||
| window: BrowserWindow, | ||
| compassApp: typeof CompassApplication | ||
| ) { | ||
| try { | ||
| const bounds = window.getBounds(); | ||
| const isMaximized = window.isMaximized(); | ||
| 
     | 
||
| await compassApp.preferences.savePreferences({ | ||
| windowBounds: { | ||
| x: bounds.x, | ||
| y: bounds.y, | ||
| width: bounds.width, | ||
| height: bounds.height, | ||
| isMaximized, | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| debug('Failed to save window bounds:', error); | ||
| } | ||
| } | ||
| 
     | 
||
| /** | ||
| * Get saved window bounds from preferences | ||
| */ | ||
| function getSavedWindowBounds(compassApp: typeof CompassApplication) { | ||
| try { | ||
| const preferences = compassApp.preferences.getPreferences(); | ||
| return preferences.windowBounds; | ||
| } catch (error) { | ||
| debug('Failed to get saved window bounds:', error); | ||
| return undefined; | ||
| } | ||
| } | ||
| 
     | 
||
| /** | ||
| * Validate and adjust window bounds to ensure they're visible on screen | ||
| */ | ||
| function validateWindowBounds(bounds: { | ||
| x?: number; | ||
| y?: number; | ||
| width?: number; | ||
| height?: number; | ||
| }) { | ||
| if (bounds?.width == null || bounds?.height == null) { | ||
| return { | ||
| width: Number(DEFAULT_WIDTH), | ||
| height: Number(DEFAULT_HEIGHT), | ||
| }; | ||
| } | ||
| 
     | 
||
| // Ensure minimum size | ||
| const width = Math.max(bounds.width, Number(MIN_WIDTH)); | ||
| const height = Math.max(bounds.height, Number(MIN_HEIGHT)); | ||
| 
     | 
||
| // If no position specified, let Electron handle it | ||
| if (bounds?.x == null || bounds?.y == null) { | ||
| return { width, height }; | ||
| } | ||
| 
     | 
||
| // Check if window would be visible on any display | ||
| const windowRect = { | ||
| x: bounds.x, | ||
| y: bounds.y, | ||
| width, | ||
| height, | ||
| }; | ||
| 
     | 
||
| const displays = screen.getAllDisplays(); | ||
| const isVisible = displays.some((display: Display) => { | ||
| const { bounds: displayBounds } = display; | ||
| return ( | ||
| windowRect.x < displayBounds.x + displayBounds.width && | ||
| windowRect.x + windowRect.width > displayBounds.x && | ||
| windowRect.y < displayBounds.y + displayBounds.height && | ||
| windowRect.y + windowRect.height > displayBounds.y | ||
| ); | ||
| }); | ||
| 
     | 
||
| if (isVisible) { | ||
| return { ...windowRect }; | ||
| } | ||
| 
     | 
||
| return { width, height }; | ||
| } | ||
| 
     | 
||
| /** | ||
| * Call me instead of using `new BrowserWindow()` directly because i'll: | ||
| * | ||
| 
          
            
          
           | 
    @@ -109,9 +201,12 @@ function showConnectWindow( | |
| } | ||
| > = {} | ||
| ): BrowserWindow { | ||
| // Get saved window bounds | ||
| const savedBounds = getSavedWindowBounds(compassApp); | ||
| const validatedBounds = validateWindowBounds(savedBounds); | ||
| 
     | 
||
| const windowOpts = { | ||
| width: Number(DEFAULT_WIDTH), | ||
| height: Number(DEFAULT_HEIGHT), | ||
| ...validatedBounds, | ||
| minWidth: Number(MIN_WIDTH), | ||
| minHeight: Number(MIN_HEIGHT), | ||
| 
         
      Comment on lines
    
      178
     to 
      181
    
   
  
    
 | 
||
| /** | ||
| 
          
            
          
           | 
    @@ -156,8 +251,36 @@ function showConnectWindow( | |
| 
     | 
||
| compassApp.emit('new-window', window); | ||
| 
     | 
||
| // Set up window state persistence | ||
| let saveTimeoutId: NodeJS.Timeout | null = null; | ||
| const debouncedSaveWindowBounds = () => { | ||
| if (saveTimeoutId) { | ||
| clearTimeout(saveTimeoutId); | ||
| } | ||
| saveTimeoutId = setTimeout(() => { | ||
| if (window && !window.isDestroyed()) { | ||
| void saveWindowBounds(window, compassApp); | ||
| } | ||
| saveTimeoutId = null; | ||
| }, SAVE_DEBOUNCE_DELAY); // Debounce to avoid too frequent saves | ||
| }; | ||
| 
     | 
||
| // Save window bounds when moved or resized | ||
| window.on('moved', debouncedSaveWindowBounds); | ||
| window.on('resized', debouncedSaveWindowBounds); | ||
| window.on('maximize', debouncedSaveWindowBounds); | ||
| window.on('unmaximize', debouncedSaveWindowBounds); | ||
                
      
                  mabaasit marked this conversation as resolved.
               
          
            Show resolved
            Hide resolved
         | 
||
| 
     | 
||
| // Restore maximized state if it was saved | ||
| if (savedBounds?.isMaximized) { | ||
| window.maximize(); | ||
| } | ||
                
       | 
||
| 
     | 
||
| const onWindowClosed = () => { | ||
| debug('Window closed. Dereferencing.'); | ||
| if (saveTimeoutId) { | ||
| clearTimeout(saveTimeoutId); | ||
| } | ||
| window = null; | ||
| void unsubscribeProxyListenerPromise.then((unsubscribe) => unsubscribe()); | ||
| }; | ||
| 
          
            
          
           | 
    @@ -243,6 +366,8 @@ class CompassWindowManager { | |
| if (first) { | ||
| debug('sending `app:quit` msg'); | ||
| first.webContents.send('app:quit'); | ||
| // Save window bounds before quitting | ||
| void saveWindowBounds(first, compassApp); | ||
| } | ||
| }); | ||
| 
     | 
||
| 
          
            
          
           | 
    @@ -284,6 +409,8 @@ class CompassWindowManager { | |
| ipcMain?.handle('compass:maximize', () => { | ||
| const first = BrowserWindow.getAllWindows()[0]; | ||
| first.maximize(); | ||
| // Save the maximized state | ||
| void saveWindowBounds(first, compassApp); | ||
                
      
                  mabaasit marked this conversation as resolved.
               
              
                Outdated
          
            Show resolved
            Hide resolved
         | 
||
| }); | ||
| 
     | 
||
| await electronApp.whenReady(); | ||
| 
          
            
          
           | 
    ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The
Math.maxcalls enforce minimum dimensions but this validation is duplicated with theminWidth/minHeightproperties set on the BrowserWindow. Consider removing this duplication since Electron will enforce the minimums automatically.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed: 1b40f27