Add confirmation dialog before closing tabs

[stevenvo]

Summary

Prevents accidental tab closures by showing a confirmation modal.

Problem

Users can accidentally close tabs by clicking the X button, losing their work and terminal session.

Solution

• Shows confirmation modal: "Close Tab? This action cannot be undone"
• Modal appears when:
  • Clicking X button on tab
  • Selecting 'Close Tab' from context menu
• User can confirm (OK) or cancel

Implementation

• New ConfirmCloseTabModal component
• Registered in modal registry
• Modified handleCloseTab in tabbar to show modal
• Preserves existing close behavior after confirmation

Test Plan

• Click X on tab -> modal appears
• Right-click -> Close Tab -> modal appears
• Click OK -> tab closes
• Click Cancel -> tab stays open
• Press Escape -> modal closes, tab stays

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@stevenvo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 21 minutes and 41 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 837fc86 and e64f9bf.

📒 Files selected for processing (4)

• frontend/app/modals/confirmclosetab.tsx
• frontend/app/modals/modalregistry.tsx
• frontend/app/store/keymodel.ts
• frontend/app/tab/tabbar.tsx

Walkthrough

This pull request introduces a confirmation dialog for closing tabs. A new ConfirmCloseTabModal component is created that displays a confirmation message and handles tab closure upon user confirmation. The modal is registered in the modal registry and integrated into the tab bar's close handler, replacing the immediate close action with a confirmation step.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• The changes follow established patterns (modal confirmation flow, registry integration)
• Logic is straightforward with standard React component structure
• Three related files with clear, single-purpose modifications
• Primary concerns for review:
  • Verify the tab closure logic in ConfirmCloseTabModal matches the original implementation from tabbar.tsx
  • Confirm proper data flow of tabId through the modal chain
  • Ensure modal registry key and display name consistency

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add confirmation dialog before closing tabs' accurately reflects the main change: introducing a confirmation modal to prevent accidental tab closures.
Description check	✅ Passed	The description is well-related to the changeset, clearly outlining the problem, solution, implementation details, and test plan for the confirmation dialog feature.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

frontend/app/modals/confirmclosetab.tsx (1)

14-23: Guard against null workspace before dereferencing ws.oid

The atoms.workspace atom can return null (when windowData is unavailable), but this code accesses ws.oid without a null check. While the workspace likely exists when the modal opens, a defensive check aligns with patterns used elsewhere in the codebase (e.g., contextmenu.ts) and prevents crashes during transient states:

-        const ws = globalStore.get(atoms.workspace);
-        getApi().closeTab(ws.oid, tabId);
-        deleteLayoutModelForTab(tabId);
-        modalsModel.popModal();
+        const ws = globalStore.get(atoms.workspace);
+        if (!ws) {
+            modalsModel.popModal();
+            return;
+        }
+        getApi().closeTab(ws.oid, tabId);
+        deleteLayoutModelForTab(tabId);
+        modalsModel.popModal();

Note: The same pattern appears in keymodel.ts (simpleCloseStaticTab and isStaticTabPinned).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fbb0c4d and 837fc86.

📒 Files selected for processing (3)

• frontend/app/modals/confirmclosetab.tsx (1 hunks)
• frontend/app/modals/modalregistry.tsx (2 hunks)
• frontend/app/tab/tabbar.tsx (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

frontend/app/tab/tabbar.tsx (1)

frontend/app/store/modalmodel.ts (1)

• modalsModel (45-45)

frontend/app/modals/confirmclosetab.tsx (3)

frontend/app/store/global.ts (2)

• atoms (842-842)
• getApi (852-852)

frontend/app/store/modalmodel.ts (1)

• modalsModel (45-45)

frontend/app/modals/modal.tsx (1)

• Modal (149-149)

frontend/app/modals/modalregistry.tsx (1)

frontend/app/modals/confirmclosetab.tsx (1)

• ConfirmCloseTabModal (39-39)

🔇 Additional comments (2)

frontend/app/modals/confirmclosetab.tsx (1)

25-35: Modal wiring and UX text look solid

onOk/onCancel/onClose are consistently wired, and the copy matches the PR intent (“Close Tab? … cannot be undone”). Component is minimal and focused, which fits the existing modal patterns.

frontend/app/modals/modalregistry.tsx (1)

10-20: Registration keyed by displayName is consistent with existing modals

Importing ConfirmCloseTabModal and registering it under [ConfirmCloseTabModal.displayName || "ConfirmCloseTabModal"] lines up with the existing pattern, and matches the "ConfirmCloseTabModal" key used in pushModal. No issues here.

[ljbred08]

@coderabbitai Is the tab closing confirmation toggleable in Settings?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Read the confirmation modal implementation
echo "=== ConfirmCloseTabModal Implementation ==="
cat frontend/app/modals/confirmclosetab.tsx

echo -e "\n=== TabBar handleCloseTab Implementation ==="
# Extract the handleCloseTab function from tabbar.tsx
sed -n '/const handleCloseTab/,/^[[:space:]]*};/p' frontend/app/tab/tabbar.tsx | head -50

echo -e "\n=== Search for settings files ==="
fd -e tsx -e ts -e json settings | head -20

echo -e "\n=== Search for user preferences patterns ==="
rg -n "userPreferences|UserSettings|appSettings" --type=ts --type=tsx -g '!node_modules' -g '!dist' -C2 | head -50

Length of output: 1856

---

🏁 Script executed:

#!/bin/bash

echo "=== Settings Schema ==="
cat schema/settings.json

echo -e "\n=== Default Config Settings ==="
cat pkg/wconfig/defaultconfig/settings.json

echo -e "\n=== Search for settings usage patterns ==="
rg -n "settings\." --type=ts -C2 | head -100

echo -e "\n=== Search for feature toggles or confirmation-related settings ==="
rg -ni "confirm|toggle|enable.*modal|modal.*enable" schema/settings.json pkg/wconfig/defaultconfig/settings.json

Length of output: 10096

---

🏁 Script executed:

#!/bin/bash

echo "=== Search for window:confirmclose usage ==="
rg -n "window:confirmclose|confirmclose" --type=ts -C3

echo -e "\n=== Check if tabbar.tsx checks any settings ==="
rg -n "settings|globalStore.get|atoms\.(settings|config)" frontend/app/tab/tabbar.tsx -C3

echo -e "\n=== Check if ConfirmCloseTabModal checks any settings ==="
rg -n "settings|globalStore.get|atoms\.(settings|config)" frontend/app/modals/confirmclosetab.tsx -C3

Length of output: 2976

---

@ljbred08 No, the tab closing confirmation is not currently toggleable in Settings. The implementation always shows the confirmation modal when you attempt to close a tab.

Based on my analysis of the code:

1. Current behavior: The handleCloseTab function in tabbar.tsx unconditionally shows the ConfirmCloseTabModal every time without checking any settings.

2. Existing similar setting: There is a window:confirmclose setting (defaults to true) that controls whether to show a confirmation dialog when closing a window, but this doesn't apply to individual tab closures.

If you'd like to make the tab closing confirmation toggleable, the implementation would need to be modified to:

• Either reuse the existing window:confirmclose setting, or
• Add a new setting like tab:confirmclose or window:confirmtabclose to the settings schema
• Update handleCloseTab to check this setting before showing the modal

Would you like me to create an enhancement issue to make this feature toggleable, or would you prefer to configure this differently?

[ljbred08]

@stevenvo This "feature" really should have a setting to turn it on and off as is standard for these types of confirmation dialogs, otherwise some users might get pretty annoyed at suddenly having to press OK for every tab close.