diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index 8d2b0cc1..3facab49 100644 --- a/.ai/contexts/viewer-panel.md +++ b/.ai/contexts/viewer-panel.md @@ -167,7 +167,7 @@ It is shown with `open(…, restore)` and re-read at once, like any return to th Nothing ends the window with a dirty file tab without the user saying so. The scope is the file tabs of the file panel (the shown one and `heldFileTabs`, in every session of `filePanelState`); the Memory and Work Files panels, MCP diff tabs and Changes buffers are not covered. -- **Main** (`unsaved-guard.js`, one guard, `attach(win)` per window, `beforeQuit(event, win)` for the app). `main.js`'s `before-quit` handler calls `beforeQuit` first: it prevents the quit, asks the renderer, and calls `app.quit()` again on a yes, so the cleanup that follows (PTYs killed, MCP servers, watchers) runs only once the quit is confirmed and a Cancel leaves the app intact. This covers every `app.quit()` caller (☰ Quit, the last window closing). `updater-install` asks first (`confirmQuit`), because electron-updater's `quitAndInstall()` starts the installer before it calls `app.quit()`: a Cancel must leave the installer unstarted, and a yes pre-approves the quit that follows. A Windows `query-session-end` or `session-end` approves the quit, so logoff and shutdown never wait on the dialog. A window close, a quit and an install share one question while it is open. The window's own `close` event is held the same way for a plain window close. The question is `unsaved-check` (`id`, `'quit'` or `'reload'`); the renderer's `unsaved-check-result` (`id`, `proceed`) approves it. A `will-prevent-unload` (the renderer's `beforeunload` veto, which a reload hits) asks with `'reload'`; on a yes the next unload is allowed once (`allowNextUnload`, answered with `preventDefault()`, which in Electron means "unload anyway") and `webContents.reload()` is called. Every close asks the renderer, even with nothing dirty; a main-side dirty flag would save that round trip and is not built. +- **Main** (`unsaved-guard.js`, one guard, `attach(win)` per window, `beforeQuit(event, win)` for the app). `main.js`'s `before-quit` handler calls `beforeQuit` first: it prevents the quit, asks the renderer, and calls `app.quit()` again on a yes, so the cleanup that follows (PTYs killed, MCP servers, watchers) runs only once the quit is confirmed and a Cancel leaves the app intact. This covers every `app.quit()` caller (☰ Quit, the last window closing). `updater-install` asks first (`confirmQuit`), because electron-updater's `quitAndInstall()` starts the installer before it calls `app.quit()`: a Cancel must leave the installer unstarted, and a yes pre-approves the quit that follows. A Windows `query-session-end` or `session-end` approves the quit, so logoff and shutdown never wait on the dialog. A window close, a quit and an install share one question while it is open. The window's own `close` event is held the same way for a plain window close. The question is `unsaved-check` (`id`, `'close'` for the window's own close, `'quit'` or `'reload'`; `'close'` also gets a "Close Switchboard?" confirm, see [window-frame](window-frame.md), "Closing the window"); the renderer's `unsaved-check-result` (`id`, `proceed`) approves it. A `will-prevent-unload` (the renderer's `beforeunload` veto, which a reload hits) asks with `'reload'`; on a yes the next unload is allowed once (`allowNextUnload`, answered with `preventDefault()`, which in Electron means "unload anyway") and `webContents.reload()` is called. Every close asks the renderer, even with nothing dirty; a main-side dirty flag would save that round trip and is not built. - **Bounded.** The renderer acknowledges (`unsaved-check-ack`) as soon as it receives the check, before any dialog. The 2.5 s bound (`DEFAULT_TIMEOUT_MS`) covers only send to ack: no ack answers yes, so a hung renderer never keeps the app from quitting. After the ack the guard waits for the answer without a limit, so a slow user or a slow save loses nothing, and answers yes only if the renderer process is gone (`render-process-gone`, `destroyed`) or a send fails. A late answer is ignored. - **Renderer** (`file-panel.js`). `askAboutUnsavedEdits()` lists the dirty tabs (`collectUnsavedFileTabs`) in the `#unsaved-edits-dialog` dialog, built on the add-project dialog's classes (already in the frameless no-drag list). Save writes every dirty tab: the shown one through the viewer's own save (`ViewerPanel.saveNow()`, with its stale-disk confirm), a tab kept aside through `saveFileForPanel` with its snapshot's `agreedBase`. A save that fails (including a disk that moved) keeps the dialog open with the reason, and nothing is answered. Discard answers yes. Cancel and Escape answer no. A second request while the dialog is open gets the same dialog's answer. - **`beforeunload`.** The renderer vetoes an unload while a file tab is dirty, unless `unloadApproved`, set for 10 s after the user answered yes. That is what stops a reload from the keyboard or devtools from slipping past, and what keeps the veto from asking a second time after an approved close. diff --git a/.ai/contexts/window-frame.md b/.ai/contexts/window-frame.md index c94f4999..8439dafa 100644 --- a/.ai/contexts/window-frame.md +++ b/.ai/contexts/window-frame.md @@ -240,3 +240,24 @@ controls back, and the restored window keeps its saved bounds. If the drag region were unreachable, the window manager's own bindings still move and resize the window: `Super`+drag on GNOME, `Alt`+`F7`/`Alt`+`F8`, or `Alt`+`Space` then Move/Size on Windows. + +## Closing the window + +The close button sits a few pixels from the strip's own controls and the +terminal's top-right corner, so a stray click there ended the app and every +session in it. A **window close** — the close button, `Alt`+`F4`, the window +manager's own close — now asks "Close Switchboard?" first; Cancel or `Escape` +leaves everything running. + +It rides on the unsaved-edits guard (`unsaved-guard.js`, see +[viewer-panel](viewer-panel.md), "Unsaved edits on quit, reload and close"): +the window's `close` event asks the renderer with the reason `'close'`, while +`before-quit` and the updater ask with `'quit'`. The renderer +(`file-panel.js`, the `onUnsavedCheck` handler) adds the confirm only for +`'close'`, and only when the unsaved-edits dialog was not shown — answering +that dialog with Save or Discard is already a decision to close. A **quit** +(☰ → Quit, `Ctrl`+`Q`, an update install) is a deliberate command and is not +asked about again. A Windows logoff or shutdown approves the quit before the +question (`query-session-end`), and a renderer that does not acknowledge the +check within 2.5 s still lets the window close, so the confirm can never keep +the app from exiting on its own. diff --git a/CHANGELOG.md b/CHANGELOG.md index 84bf75a9..c48b0532 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased ### Changed +- Closing the window (the close button, `Alt`+`F4`) asks for confirmation first. Quit from the ☰ menu does not ask again. - Markdown files open formatted in Touched, with a toggle back to the source that is remembered. (#472) ### Fixed - Remote triggers refuse commands containing invisible format characters, default-ignorable characters or braille blanks, including joined emoji, emoji with variation selectors (such as hearts), soft hyphens and right-to-left marks. Fullwidth slash, exclamation and number-sign prefixes are refused too. (#440) diff --git a/public/file-panel.js b/public/file-panel.js index cc6b76f5..9b60f1bd 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -175,10 +175,15 @@ function initFilePanel() { event.returnValue = false; }); if (window.api.onUnsavedCheck) { - window.api.onUnsavedCheck(async (id) => { + window.api.onUnsavedCheck(async (id, reason) => { window.api.unsavedCheckAck(id); let proceed = true; - try { proceed = await askAboutUnsavedEdits(); } catch (err) { console.error('[unsaved-check]', err); } + try { + const asksAboutEdits = collectUnsavedFileTabs().length > 0; + proceed = await askAboutUnsavedEdits(); + // see .ai/contexts/window-frame.md ("Closing the window") + if (proceed && reason === 'close' && !asksAboutEdits) proceed = confirm('Close Switchboard?'); + } catch (err) { console.error('[unsaved-check]', err); } window.api.unsavedCheckResult(id, proceed); }); } diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index ff1bb086..9962b6aa 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -234,3 +234,43 @@ test('the check is acknowledged on receipt, before any dialog is shown', async ( await flush(); } finally { ctx.destroy(); } }); + +// see .ai/contexts/window-frame.md ("Closing the window") +test('a window close with nothing unsaved asks to close Switchboard; no keeps it open', async () => { + const ctx = setup(); + try { + ctx.calls.check(1, 'close'); + await flush(); + assert.deepEqual(ctx.calls.confirms, ['Close Switchboard?']); + assert.deepEqual(ctx.calls.answers, [{ id: 1, proceed: false }]); + + ctx.window.confirm = (msg) => { ctx.calls.confirms.push(msg); return true; }; + ctx.calls.check(2, 'close'); + await flush(); + assert.deepEqual(ctx.calls.answers[1], { id: 2, proceed: true }); + } finally { ctx.destroy(); } +}); + +test('a quit (menu, updater) is not asked about a second time', async () => { + const ctx = setup(); + try { + ctx.calls.check(1, 'quit'); + await flush(); + assert.deepEqual(ctx.calls.confirms, []); + assert.deepEqual(ctx.calls.answers, [{ id: 1, proceed: true }]); + } finally { ctx.destroy(); } +}); + +test('a window close over unsaved edits asks once: Discard closes without a second question', async () => { + const ctx = setup(); + try { + await dirtyTab(ctx, 's1', A); + ctx.calls.check(3, 'close'); + await flush(); + assert.ok(dialog(ctx)); + button(ctx, 'unsaved-discard').click(); + await flush(); + assert.deepEqual(ctx.calls.confirms, []); + assert.deepEqual(ctx.calls.answers, [{ id: 3, proceed: true }]); + } finally { ctx.destroy(); } +}); diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index bed9fe4c..da94cd0e 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -43,7 +43,7 @@ test('a close is held while the renderer is asked, and goes through on yes', asy assert.equal(e.prevented, true); assert.equal(t.sent.length, 1); assert.equal(t.sent[0].channel, 'unsaved-check'); - assert.equal(t.sent[0].args[1], 'quit'); + assert.equal(t.sent[0].args[1], 'close', 'a window close is told apart from a quit'); assert.equal(t.win.closes, 0); t.answer(t.sent[0].args[0], true); diff --git a/unsaved-guard.js b/unsaved-guard.js index 5958f90f..a5e6f8ef 100644 --- a/unsaved-guard.js +++ b/unsaved-guard.js @@ -85,7 +85,7 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou event.preventDefault(); if (closing) return; closing = true; - ask(win, 'quit').then((proceed) => { + ask(win, 'close').then((proceed) => { closing = false; if (!proceed) return; approved = true;