mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-08-18 22:42:25 +00:00
feat(window): persist main window size/position across launches (#97)
* feat(window): persist main window size/position across launches The main window always opened at a fixed 1400x900, forcing a manual resize every session. Save the window geometry (normal bounds + maximized flag) to the existing desktop prefs store on close, and restore it in createWindow. Saved bounds are validated by a pure sanitizer against the current display layout before use, so stale state degrades safely instead of producing an off-screen or absurd window: - garbage/partial config -> 1400x900 centered defaults - size clamped between the 800x600 window minimums and the largest display's workArea - position kept only when the window overlaps a display by at least 100x50 px (unplugged monitor / resolution change -> re-center); negative multi-monitor coordinates remain valid - maximized sessions save getNormalBounds() and re-maximize on restore; fullscreen deliberately restores windowed No new dependency; reuses get/setDesktopConfig (atomic write, fail-soft) in soundfont-manager.ts. The store file is already in the reset-app-settings delete-set, so a config reset also resets bounds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: gionnibgud <gionnibgud@gmail.com> * fix(window): don't crash shutdown if bounds persistence write fails The close listener called setDesktopConfig synchronously with no error handling; a disk-full or permissions failure during the write would throw unhandled inside the close handler, risking a shutdown crash. Wrap the write in try/catch and log a warning instead. Flagged by CodeRabbit on PR #97. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: gionnibgud <gionnibgud@gmail.com> --------- Signed-off-by: gionnibgud <gionnibgud@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
f66c27b459
commit
6349ed4c5f
+34
-5
@@ -56,7 +56,7 @@ if (process.platform !== 'linux') {
|
|||||||
}
|
}
|
||||||
// ──────────────────────────────────────────────────────────────────────────
|
// ──────────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
import { app, BrowserWindow, ipcMain, dialog, shell, session, crashReporter, powerSaveBlocker, systemPreferences } from 'electron';
|
import { app, BrowserWindow, ipcMain, dialog, shell, session, crashReporter, powerSaveBlocker, systemPreferences, screen } from 'electron';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import { execFileSync } from 'child_process';
|
import { execFileSync } from 'child_process';
|
||||||
@@ -125,6 +125,7 @@ import { initSoundfontManager, getDesktopConfig, setDesktopConfig } from './soun
|
|||||||
import * as updateManager from './update-manager';
|
import * as updateManager from './update-manager';
|
||||||
import type { UpdateChannel } from './update-manager';
|
import type { UpdateChannel } from './update-manager';
|
||||||
import { installAppMenu } from './app-menu';
|
import { installAppMenu } from './app-menu';
|
||||||
|
import { sanitizeWindowBounds, MIN_WIDTH, MIN_HEIGHT } from './window-bounds';
|
||||||
|
|
||||||
// Linux: enable Chromium's PipeWire capturer feature so getUserMedia can see
|
// Linux: enable Chromium's PipeWire capturer feature so getUserMedia can see
|
||||||
// audio devices on PipeWire-only distros (Fedora 36+, recent Ubuntu, Arch).
|
// audio devices on PipeWire-only distros (Fedora 36+, recent Ubuntu, Arch).
|
||||||
@@ -455,15 +456,43 @@ function isLocalServiceUrl(url: string, backendPort: number): boolean {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function createWindow(port: number): void {
|
function createWindow(port: number): void {
|
||||||
|
// Restore the previous session's window geometry, validated against the
|
||||||
|
// current display layout so stale bounds (unplugged monitor, resolution
|
||||||
|
// change, hand-edited config) can't put the window off-screen. When x/y
|
||||||
|
// are absent Electron centers the window as before.
|
||||||
|
const restored = sanitizeWindowBounds(
|
||||||
|
getDesktopConfig().windowBounds,
|
||||||
|
screen.getAllDisplays().map((d) => d.workArea),
|
||||||
|
);
|
||||||
mainWindow = new BrowserWindow({
|
mainWindow = new BrowserWindow({
|
||||||
width: 1400,
|
x: restored.x,
|
||||||
height: 900,
|
y: restored.y,
|
||||||
minWidth: 800,
|
width: restored.width,
|
||||||
minHeight: 600,
|
height: restored.height,
|
||||||
|
minWidth: MIN_WIDTH,
|
||||||
|
minHeight: MIN_HEIGHT,
|
||||||
title: 'fee[dB]ack',
|
title: 'fee[dB]ack',
|
||||||
backgroundColor: '#0f172a', // slate-900 to match Slopsmith UI
|
backgroundColor: '#0f172a', // slate-900 to match Slopsmith UI
|
||||||
webPreferences: rendererWebPreferences,
|
webPreferences: rendererWebPreferences,
|
||||||
});
|
});
|
||||||
|
if (restored.maximized) mainWindow.maximize();
|
||||||
|
|
||||||
|
// Persist geometry on close. getNormalBounds() so a maximized session
|
||||||
|
// saves the underlying windowed size, restored + re-maximized next launch.
|
||||||
|
// ponytail: fullscreen is deliberately not persisted (launching straight
|
||||||
|
// into fullscreen is jarring, especially on macOS) and we save on close
|
||||||
|
// only — a crash loses the last session's geometry; add debounced
|
||||||
|
// resize/move saving if that ever matters.
|
||||||
|
mainWindow.on('close', () => {
|
||||||
|
if (!mainWindow || mainWindow.isDestroyed()) return;
|
||||||
|
try {
|
||||||
|
setDesktopConfig({
|
||||||
|
windowBounds: { ...mainWindow.getNormalBounds(), maximized: mainWindow.isMaximized() },
|
||||||
|
});
|
||||||
|
} catch (err) {
|
||||||
|
console.warn('[main] Failed to persist window bounds on close:', err);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// Forward renderer console to main process stdout
|
// Forward renderer console to main process stdout
|
||||||
mainWindow.webContents.on('console-message', (_event, level, message, line, sourceId) => {
|
mainWindow.webContents.on('console-message', (_event, level, message, line, sourceId) => {
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import * as https from 'https';
|
|||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import * as crypto from 'crypto';
|
import * as crypto from 'crypto';
|
||||||
import { restartPython } from './python';
|
import { restartPython } from './python';
|
||||||
|
import type { SavedWindowBounds } from './window-bounds';
|
||||||
|
|
||||||
// ── Source of truth for the high-quality soundfont ──────────────────────────
|
// ── Source of truth for the high-quality soundfont ──────────────────────────
|
||||||
// Public mirror of FluidR3_GM.sf2 on the feedback-soundfonts repo. When a new
|
// Public mirror of FluidR3_GM.sf2 on the feedback-soundfonts repo. When a new
|
||||||
@@ -34,6 +35,9 @@ interface DesktopConfig {
|
|||||||
// 127.0.0.1, so other devices on the network can reach the library / sync
|
// 127.0.0.1, so other devices on the network can reach the library / sync
|
||||||
// room. Opt-in (default loopback) — see python.ts and issue #441.
|
// room. Opt-in (default loopback) — see python.ts and issue #441.
|
||||||
lanAccess?: boolean;
|
lanAccess?: boolean;
|
||||||
|
// Last main-window geometry, restored (after sanitization against the
|
||||||
|
// current display layout) on next launch — see window-bounds.ts.
|
||||||
|
windowBounds?: SavedWindowBounds;
|
||||||
}
|
}
|
||||||
|
|
||||||
function configPath(): string {
|
function configPath(): string {
|
||||||
|
|||||||
@@ -0,0 +1,77 @@
|
|||||||
|
// Pure window-bounds sanitization for main-window persistence. No electron
|
||||||
|
// imports so tests/window-bounds.test.js can load it headless via _load-ts.
|
||||||
|
|
||||||
|
export interface SavedWindowBounds {
|
||||||
|
x: number;
|
||||||
|
y: number;
|
||||||
|
width: number;
|
||||||
|
height: number;
|
||||||
|
maximized?: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
// A display's workArea (same shape as Electron's Rectangle).
|
||||||
|
export interface DisplayRect {
|
||||||
|
x: number;
|
||||||
|
y: number;
|
||||||
|
width: number;
|
||||||
|
height: number;
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface RestoredWindowBounds {
|
||||||
|
width: number;
|
||||||
|
height: number;
|
||||||
|
x?: number;
|
||||||
|
y?: number;
|
||||||
|
maximized: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Keep in sync with createWindow() in main.ts.
|
||||||
|
export const DEFAULT_WIDTH = 1400;
|
||||||
|
export const DEFAULT_HEIGHT = 900;
|
||||||
|
export const MIN_WIDTH = 800;
|
||||||
|
export const MIN_HEIGHT = 600;
|
||||||
|
|
||||||
|
// Minimum overlap with some display's workArea for the saved position to be
|
||||||
|
// trusted — enough of the title bar to grab with the mouse.
|
||||||
|
const MIN_VISIBLE_W = 100;
|
||||||
|
const MIN_VISIBLE_H = 50;
|
||||||
|
|
||||||
|
function isFiniteNumber(v: unknown): v is number {
|
||||||
|
return typeof v === 'number' && Number.isFinite(v);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Validate saved bounds against the current display layout. Untrusted input
|
||||||
|
// (hand-edited/corrupt config, unplugged monitor, resolution change) degrades
|
||||||
|
// to defaults rather than producing an off-screen or absurd window. Omitted
|
||||||
|
// x/y means "let Electron center the window".
|
||||||
|
export function sanitizeWindowBounds(saved: unknown, displays: DisplayRect[]): RestoredWindowBounds {
|
||||||
|
const defaults: RestoredWindowBounds = { width: DEFAULT_WIDTH, height: DEFAULT_HEIGHT, maximized: false };
|
||||||
|
if (displays.length === 0) return defaults;
|
||||||
|
|
||||||
|
const b = saved as SavedWindowBounds | undefined;
|
||||||
|
if (!b || typeof b !== 'object') return defaults;
|
||||||
|
if (!isFiniteNumber(b.x) || !isFiniteNumber(b.y) || !isFiniteNumber(b.width) || !isFiniteNumber(b.height)) {
|
||||||
|
return defaults;
|
||||||
|
}
|
||||||
|
const maximized = b.maximized === true;
|
||||||
|
|
||||||
|
// Clamp size: never below the window minimums, never above the largest
|
||||||
|
// display's workArea (window bigger than any screen → shrink to fit).
|
||||||
|
const maxW = Math.max(...displays.map((d) => d.width));
|
||||||
|
const maxH = Math.max(...displays.map((d) => d.height));
|
||||||
|
const width = Math.min(Math.max(Math.round(b.width), MIN_WIDTH), maxW);
|
||||||
|
const height = Math.min(Math.max(Math.round(b.height), MIN_HEIGHT), maxH);
|
||||||
|
|
||||||
|
// Trust the position only if the window meaningfully overlaps some
|
||||||
|
// display. Negative coordinates are valid multi-monitor layouts — this is
|
||||||
|
// an intersection test, not a sign check.
|
||||||
|
const x = Math.round(b.x);
|
||||||
|
const y = Math.round(b.y);
|
||||||
|
const visible = displays.some((d) => {
|
||||||
|
const overlapW = Math.min(x + width, d.x + d.width) - Math.max(x, d.x);
|
||||||
|
const overlapH = Math.min(y + height, d.y + d.height) - Math.max(y, d.y);
|
||||||
|
return overlapW >= MIN_VISIBLE_W && overlapH >= MIN_VISIBLE_H;
|
||||||
|
});
|
||||||
|
|
||||||
|
return visible ? { x, y, width, height, maximized } : { width, height, maximized };
|
||||||
|
}
|
||||||
@@ -0,0 +1,59 @@
|
|||||||
|
const test = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const { loadTs } = require('./_load-ts');
|
||||||
|
|
||||||
|
const { sanitizeWindowBounds, DEFAULT_WIDTH, DEFAULT_HEIGHT, MIN_WIDTH, MIN_HEIGHT } =
|
||||||
|
loadTs('src/main/window-bounds.ts');
|
||||||
|
|
||||||
|
// A common single-display workArea (1920×1080 minus a 40px taskbar).
|
||||||
|
const PRIMARY = { x: 0, y: 0, width: 1920, height: 1040 };
|
||||||
|
const DEFAULTS = { width: DEFAULT_WIDTH, height: DEFAULT_HEIGHT, maximized: false };
|
||||||
|
|
||||||
|
test('valid bounds on a matching display round-trip', () => {
|
||||||
|
const saved = { x: 100, y: 50, width: 1200, height: 800, maximized: true };
|
||||||
|
assert.deepEqual(sanitizeWindowBounds(saved, [PRIMARY]), saved);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('garbage input falls back to defaults', () => {
|
||||||
|
for (const bad of [undefined, null, 'wat', 42, {}, { x: 1, y: 2 }, { x: 'a', y: 0, width: 1200, height: 800 }, { x: NaN, y: 0, width: 1200, height: 800 }, { x: 0, y: 0, width: Infinity, height: 800 }]) {
|
||||||
|
assert.deepEqual(sanitizeWindowBounds(bad, [PRIMARY]), DEFAULTS);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('empty display list falls back to defaults', () => {
|
||||||
|
assert.deepEqual(sanitizeWindowBounds({ x: 0, y: 0, width: 1200, height: 800 }, []), DEFAULTS);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('oversize bounds clamp to the largest display workArea', () => {
|
||||||
|
const out = sanitizeWindowBounds({ x: 0, y: 0, width: 5000, height: 4000 }, [PRIMARY]);
|
||||||
|
assert.deepEqual(out, { x: 0, y: 0, width: 1920, height: 1040, maximized: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('undersize bounds clamp up to the window minimums', () => {
|
||||||
|
const out = sanitizeWindowBounds({ x: 10, y: 10, width: 300, height: 200 }, [PRIMARY]);
|
||||||
|
assert.deepEqual(out, { x: 10, y: 10, width: MIN_WIDTH, height: MIN_HEIGHT, maximized: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('position on a now-unplugged monitor is dropped, size kept', () => {
|
||||||
|
// Saved on a second display to the right that no longer exists.
|
||||||
|
const out = sanitizeWindowBounds({ x: 2000, y: 100, width: 1200, height: 800 }, [PRIMARY]);
|
||||||
|
assert.deepEqual(out, { width: 1200, height: 800, maximized: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('negative coordinates on a left-of-primary monitor are kept', () => {
|
||||||
|
const leftMonitor = { x: -1920, y: 0, width: 1920, height: 1040 };
|
||||||
|
const saved = { x: -1800, y: 100, width: 1200, height: 800 };
|
||||||
|
const out = sanitizeWindowBounds(saved, [leftMonitor, PRIMARY]);
|
||||||
|
assert.deepEqual(out, { ...saved, maximized: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('sliver overlap below the grab threshold drops the position', () => {
|
||||||
|
// Only 50px of the window's left edge on screen — not enough to grab.
|
||||||
|
const out = sanitizeWindowBounds({ x: 1870, y: 100, width: 1200, height: 800 }, [PRIMARY]);
|
||||||
|
assert.deepEqual(out, { width: 1200, height: 800, maximized: false });
|
||||||
|
});
|
||||||
|
|
||||||
|
test('fractional coordinates are rounded to integers', () => {
|
||||||
|
const out = sanitizeWindowBounds({ x: 10.6, y: 20.4, width: 1200.5, height: 800.2 }, [PRIMARY]);
|
||||||
|
assert.deepEqual(out, { x: 11, y: 20, width: 1201, height: 800, maximized: false });
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user