-
-
Notifications
You must be signed in to change notification settings - Fork 796
feat: add tab:confirmclose setting to prompt before closing tabs #2893
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 all commits
48913bf
72d4b65
1a54f77
f874888
0cdddc2
76ee878
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 |
|---|---|---|
|
|
@@ -728,15 +728,27 @@ ipcMain.on("set-waveai-open", (event, isOpen: boolean) => { | |
| } | ||
| }); | ||
|
|
||
| ipcMain.on("close-tab", async (event, workspaceId, tabId) => { | ||
| ipcMain.handle("close-tab", async (event, workspaceId: string, tabId: string, confirmClose: boolean) => { | ||
| const ww = getWaveWindowByWorkspaceId(workspaceId); | ||
| if (ww == null) { | ||
| console.log(`close-tab: no window found for workspace ws=${workspaceId} tab=${tabId}`); | ||
| return; | ||
| return false; | ||
| } | ||
| if (confirmClose) { | ||
| const choice = dialog.showMessageBoxSync(ww, { | ||
| type: "question", | ||
| defaultId: 1, // Enter activates "Close Tab" | ||
| cancelId: 0, // Esc activates "Cancel" | ||
| buttons: ["Cancel", "Close Tab"], | ||
| title: "Confirm", | ||
| message: "Are you sure you want to close this tab?", | ||
| }); | ||
| if (choice === 0) { | ||
| return false; | ||
| } | ||
| } | ||
| await ww.queueCloseTab(tabId); | ||
| event.returnValue = true; | ||
| return null; | ||
| return true; | ||
|
Comment on lines
750
to
+751
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In both cases the IPC handler returns A minimal fix is to have 🤖 Prompt for AI Agents |
||
| }); | ||
|
|
||
| ipcMain.on("switch-workspace", (event, workspaceId) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -130,8 +130,17 @@ function getStaticTabBlockCount(): number { | |
| function simpleCloseStaticTab() { | ||
| const ws = globalStore.get(atoms.workspace); | ||
| const tabId = globalStore.get(atoms.staticTabId); | ||
| getApi().closeTab(ws.oid, tabId); | ||
| deleteLayoutModelForTab(tabId); | ||
| const confirmClose = globalStore.get(getSettingsKeyAtom("tab:confirmclose")) ?? false; | ||
| getApi() | ||
| .closeTab(ws.oid, tabId, confirmClose) | ||
| .then((didClose) => { | ||
| if (didClose) { | ||
| deleteLayoutModelForTab(tabId); | ||
| } | ||
| }) | ||
| .catch((e) => { | ||
| console.log("error closing tab", e); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Missing error handling for promise rejection The Consider adding: .catch((error) => {
console.error("Failed to close tab:", error);
}); |
||
| } | ||
|
|
||
| function uxCloseBlock(blockId: string) { | ||
|
|
@@ -151,6 +160,13 @@ function uxCloseBlock(blockId: string) { | |
| const blockData = globalStore.get(blockAtom); | ||
| const isAIFileDiff = blockData?.meta?.view === "aifilediff"; | ||
|
|
||
| // If this is the last block, closing it will close the tab — route through simpleCloseStaticTab | ||
| // so the tab:confirmclose setting is respected. | ||
| if (getStaticTabBlockCount() === 1) { | ||
| simpleCloseStaticTab(); | ||
| return; | ||
| } | ||
|
|
||
| const layoutModel = getLayoutModelForStaticTab(); | ||
| const node = layoutModel.getNodeByBlockId(blockId); | ||
| if (node) { | ||
|
|
@@ -190,6 +206,13 @@ function genericClose() { | |
| return; | ||
| } | ||
|
|
||
| // If this is the last block, closing it will close the tab — route through simpleCloseStaticTab | ||
| // so the tab:confirmclose setting is respected. | ||
| if (blockCount === 1) { | ||
| simpleCloseStaticTab(); | ||
| return; | ||
| } | ||
|
|
||
| const layoutModel = getLayoutModelForStaticTab(); | ||
| const focusedNode = globalStore.get(layoutModel.focusedNode); | ||
| const blockId = focusedNode?.data?.blockId; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -592,9 +592,18 @@ const TabBar = memo(({ workspace }: TabBarProps) => { | |
| const handleCloseTab = (event: React.MouseEvent<HTMLButtonElement, MouseEvent> | null, tabId: string) => { | ||
| event?.stopPropagation(); | ||
| const ws = globalStore.get(atoms.workspace); | ||
| getApi().closeTab(ws.oid, tabId); | ||
| tabsWrapperRef.current.style.setProperty("--tabs-wrapper-transition", "width 0.3s ease"); | ||
| deleteLayoutModelForTab(tabId); | ||
| const confirmClose = globalStore.get(getSettingsKeyAtom("tab:confirmclose")) ?? false; | ||
| getApi() | ||
| .closeTab(ws.oid, tabId, confirmClose) | ||
| .then((didClose) => { | ||
| if (didClose) { | ||
| tabsWrapperRef.current?.style.setProperty("--tabs-wrapper-transition", "width 0.3s ease"); | ||
| deleteLayoutModelForTab(tabId); | ||
| } | ||
| }) | ||
| .catch((e) => { | ||
| console.log("error closing tab", e); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Missing error handling for promise rejection The Consider adding: .catch((error) => {
console.error("Failed to close tab:", error);
}); |
||
| }; | ||
|
|
||
| const handleTabLoaded = useCallback((tabId: string) => { | ||
|
|
||
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.
defaultId: 1makes the destructive action the Enter-key default — inconsistent with other dialogs.Every other confirmation dialog in this file omits
defaultId(Line 303 window-close, Line 790 delete-workspace), letting it default to index0("Cancel") — the safe option. HeredefaultId: 1explicitly maps Enter to "Close Tab", the destructive action.🛡️ Proposed fix — make Cancel the Enter-key default, keep Escape as Cancel
const choice = dialog.showMessageBoxSync(ww, { type: "question", - defaultId: 1, // Enter activates "Close Tab" cancelId: 0, // Esc activates "Cancel" buttons: ["Cancel", "Close Tab"],📝 Committable suggestion
🤖 Prompt for AI Agents