mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-10 23:04:09 +00:00
fix(panes): persist alwaysOnTop, flush geometry on quit, test the sizing override
Five more from review. 1. alwaysOnTop was RESTORED but never SAVED. A dead read: the only way to turn it on was to hand-edit the config file. There is now one snapshot() that decides what "remembered" means, used by the debounced save, the flush on close, and the flush on quit — so the three can never disagree about it again. 2. GEOMETRY WAS LOST ON QUIT. closeAllPanes() calls destroy(), and destroy() does not fire 'close' — so the flush wired to that event never ran on the one path every user takes. Combined with the 400ms debounce: move a pane, quit two seconds later, and its position was gone. Every pane is flushed before its window is destroyed. 3. The tray-icon comment claimed a 16px PNG; build:ts copies the 32px one. Also spelled out what the macOS consequence actually is (a colour icon rather than one that adapts to light/dark menu bars), rather than gesturing at it. 4. sanitizeWindowBounds' new `sizing` parameter had no tests — and it is the whole reason the function was touched. Without it a 380x560 pane restored through the MAIN window's 800x600 floor is silently inflated to three times the size the plugin asked for. Four tests now pin it: a small pane is not inflated, the min clamp uses the override, corrupt input falls back to the override's defaults, and — the one that protects everyone else — omitting `sizing` leaves the main window's behaviour byte-for-byte unchanged. window-bounds tests: 13/13. Signed-off-by: topkoa <topkoa@gmail.com>
This commit is contained in:
+18
-3
@@ -83,6 +83,17 @@ function persist(paneId: string, patch: SavedPaneWindow): void {
|
||||
}
|
||||
}
|
||||
|
||||
// Everything we remember about a pane window, read in one place so the debounced
|
||||
// save, the flush-on-close and the flush-on-quit can never disagree about what
|
||||
// "remembered" means. alwaysOnTop is in here because it was being RESTORED and never
|
||||
// written — a setting that could only ever be turned on by hand-editing the config.
|
||||
function snapshot(win: BrowserWindow): SavedPaneWindow {
|
||||
return {
|
||||
bounds: { ...win.getNormalBounds(), maximized: false },
|
||||
alwaysOnTop: win.isAlwaysOnTop(),
|
||||
};
|
||||
}
|
||||
|
||||
// setDesktopConfig is writeFileSync + renameSync. Wiring that straight to 'moved'
|
||||
// and 'resized' means a synchronous disk write for every frame of a drag — on
|
||||
// macOS, dozens per second, in the main process, where they block everything else.
|
||||
@@ -107,7 +118,7 @@ function flushGeometry(win: BrowserWindow, paneId: string): void {
|
||||
const timer = saveTimers.get(paneId);
|
||||
if (timer) { clearTimeout(timer); saveTimers.delete(paneId); }
|
||||
if (win.isDestroyed()) return;
|
||||
persist(paneId, { bounds: { ...win.getNormalBounds(), maximized: false } });
|
||||
persist(paneId, snapshot(win));
|
||||
}
|
||||
|
||||
// ── Adoption ────────────────────────────────────────────────────────────────
|
||||
@@ -146,8 +157,7 @@ export function adoptPaneWindow(win: BrowserWindow, paneId: string): void {
|
||||
// in a crash, and the whole point of remembering geometry is that you never
|
||||
// place it twice.
|
||||
const save = (): void => {
|
||||
persistSoon(paneId, () =>
|
||||
win.isDestroyed() ? null : { bounds: { ...win.getNormalBounds(), maximized: false } });
|
||||
persistSoon(paneId, () => (win.isDestroyed() ? null : snapshot(win)));
|
||||
};
|
||||
win.on('moved', save);
|
||||
win.on('resized', save);
|
||||
@@ -178,6 +188,11 @@ export function adoptPaneWindow(win: BrowserWindow, paneId: string): void {
|
||||
}
|
||||
|
||||
export function closeAllPanes(): void {
|
||||
// destroy() does NOT fire 'close', so the flush wired to that event never runs on
|
||||
// this path — and the debounced save may still be pending. Move a pane, quit two
|
||||
// seconds later, and its position would be gone. Flush every pane first.
|
||||
windows.forEach((win, paneId) => flushGeometry(win, paneId));
|
||||
|
||||
// Called when the main window goes. A pane window holds a DOM node belonging to
|
||||
// the main window's document — with the main window gone there is nothing left
|
||||
// to dock it back into. And worse: a pane HIDDEN in the tray is still an open
|
||||
|
||||
@@ -47,10 +47,13 @@ let panes: TrayPane[] = [];
|
||||
let actions: TrayPaneActions | null = null;
|
||||
|
||||
function iconPath(): string {
|
||||
// Windows wants an .ico; macOS and Linux take a PNG. macOS additionally wants
|
||||
// a monochrome template image, which the 16px PNG is not — so it will render
|
||||
// in colour there. Acceptable, and preferable to shipping no tray at all;
|
||||
// a proper …Template.png is a follow-up.
|
||||
// Windows wants an .ico; macOS and Linux take a PNG. build:ts copies
|
||||
// resources/icons/icon.ico -> tray.ico and resources/icons/32x32.png -> tray.png.
|
||||
//
|
||||
// macOS additionally wants a monochrome TEMPLATE image, which a 32px colour PNG
|
||||
// is not — so the tray icon will render in colour there rather than adapting to
|
||||
// light/dark menu bars. Acceptable, and far preferable to shipping no tray at
|
||||
// all; a proper …Template.png is a follow-up.
|
||||
return path.join(__dirname, process.platform === 'win32' ? 'tray.ico' : 'tray.png');
|
||||
}
|
||||
|
||||
|
||||
@@ -57,3 +57,41 @@ 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 });
|
||||
});
|
||||
|
||||
// ── Custom sizing (pane windows) ────────────────────────────────────────────
|
||||
//
|
||||
// sanitizeWindowBounds grew a `sizing` parameter so pane pop-outs could be small.
|
||||
// Without it, a 380×560 pane restored through the MAIN window's 800×600 floor would
|
||||
// be silently inflated to 800×600 — three times the size the plugin asked for. The
|
||||
// default argument keeps every existing call site byte-for-byte identical, so these
|
||||
// tests exist to pin the override itself, which is the part nothing else covers.
|
||||
|
||||
const PANE_SIZING = { minWidth: 240, minHeight: 180, defaultWidth: 380, defaultHeight: 560 };
|
||||
|
||||
test('custom sizing: a small pane is NOT inflated to the main window floor', () => {
|
||||
const out = sanitizeWindowBounds({ x: 100, y: 100, width: 380, height: 560 }, [PRIMARY], PANE_SIZING);
|
||||
assert.deepEqual(out, { x: 100, y: 100, width: 380, height: 560, maximized: false });
|
||||
});
|
||||
|
||||
test('custom sizing: the min clamp uses the override, not MIN_WIDTH/MIN_HEIGHT', () => {
|
||||
// Below the pane minimum (240×180) — clamped up to it, and nowhere near 800×600.
|
||||
const out = sanitizeWindowBounds({ x: 10, y: 10, width: 50, height: 20 }, [PRIMARY], PANE_SIZING);
|
||||
assert.deepEqual(out, { x: 10, y: 10, width: 240, height: 180, maximized: false });
|
||||
});
|
||||
|
||||
test('custom sizing: missing/corrupt bounds fall back to the override defaults', () => {
|
||||
assert.deepEqual(
|
||||
sanitizeWindowBounds(undefined, [PRIMARY], PANE_SIZING),
|
||||
{ width: 380, height: 560, maximized: false },
|
||||
);
|
||||
assert.deepEqual(
|
||||
sanitizeWindowBounds({ x: 'nope', y: 0, width: 380, height: 560 }, [PRIMARY], PANE_SIZING),
|
||||
{ width: 380, height: 560, maximized: false },
|
||||
);
|
||||
});
|
||||
|
||||
test('omitting sizing keeps the main window behaviour exactly as before', () => {
|
||||
// The whole point of the default argument: existing call sites must not shift.
|
||||
const out = sanitizeWindowBounds({ x: 10, y: 10, width: 50, height: 20 }, [PRIMARY]);
|
||||
assert.deepEqual(out, { x: 10, y: 10, width: MIN_WIDTH, height: MIN_HEIGHT, maximized: false });
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user