From 5cc92db14f2ee170e3b249a12823b6c3998958c9 Mon Sep 17 00:00:00 2001 From: Ankit Gabani Date: Sun, 13 Sep 2026 16:06:52 +0530 Subject: [PATCH 1/2] feat(tabs): undo for closing a query tab (#747) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closeTab dropped a tab's query and name with no way back — a misclick on the close icon lost unsaved work outright. closeTab now keeps the just-closed tab (query, name, position) and offers a toast with an Undo action that restores it and makes it active again. Only the most recent close is recoverable; the close action itself stays confirmation-free per the issue's explicit constraint. --- src/hooks/use-tab-manager.ts | 54 ++++++++-- tests/hooks/use-tab-manager.test.ts | 153 +++++++++++++++++++++++++++- 2 files changed, 197 insertions(+), 10 deletions(-) diff --git a/src/hooks/use-tab-manager.ts b/src/hooks/use-tab-manager.ts index e95ec14a1..04a38276f 100644 --- a/src/hooks/use-tab-manager.ts +++ b/src/hooks/use-tab-manager.ts @@ -1,6 +1,7 @@ "use client"; -import { useState, useCallback, useEffect, useMemo } from "react"; +import { useState, useCallback, useEffect, useMemo, useRef } from "react"; +import { toast } from "sonner"; import type { DatabaseConnection, QueryTab } from "@/lib/types"; import type { DetailedObject } from "@/lib/db/detailed-object"; import type { ProviderMetadata } from "@/hooks/use-provider-metadata"; @@ -10,6 +11,12 @@ import { resolveTabType } from "@/lib/editor/tab-language"; import { logger } from "@/lib/logger"; import { newLocalId } from "@/lib/ids"; +/** A tab `closeTab` removed, and where it sat, so `reopenLastClosedTab` can put it back (#747). */ +interface ClosedTab { + tab: QueryTab; + index: number; +} + const DEFAULT_TAB: QueryTab = { id: "default", name: "Query 1", @@ -168,19 +175,47 @@ export function useTabManager({ activeConnection, metadata, schema, persistWorks setActiveTabId(newId); }, [metadata]); + /** + * Holds exactly the one tab `closeTab` most recently removed (#747). A ref, not state: + * nothing renders from it, only `reopenLastClosedTab` reads it, and closing a second tab + * before undoing the first deliberately drops the first — undo answers the immediate + * misclick the issue describes, not a multi-level history, so there is nothing here that + * needs to survive past the next close. + */ + const lastClosedTabRef = useRef(null); + + const reopenLastClosedTab = useCallback(() => { + const closed = lastClosedTabRef.current; + if (!closed) return; + lastClosedTabRef.current = null; + setTabs((prev) => { + const next = [...prev]; + next.splice(Math.min(closed.index, next.length), 0, closed.tab); + return next; + }); + setActiveTabId(closed.tab.id); + }, []); + const closeTab = useCallback( (id: string, e: React.MouseEvent) => { e.stopPropagation(); - setTabs((prev) => { - if (prev.length === 1) return prev; - const newTabs = prev.filter((t) => t.id !== id); - if (activeTabId === id && newTabs.length > 0) { - setActiveTabId(newTabs[newTabs.length - 1].id); - } - return newTabs; + if (tabs.length === 1) return; + const index = tabs.findIndex((t) => t.id === id); + if (index === -1) return; + const closedTab = tabs[index]; + + setTabs((prev) => prev.filter((t) => t.id !== id)); + if (activeTabId === id) { + const remaining = tabs.filter((t) => t.id !== id); + if (remaining.length > 0) setActiveTabId(remaining[remaining.length - 1].id); + } + + lastClosedTabRef.current = { tab: closedTab, index }; + toast(`Closed "${closedTab.name}"`, { + action: { label: "Undo", onClick: () => reopenLastClosedTab() }, }); }, - [activeTabId], + [tabs, activeTabId, reopenLastClosedTab], ); /** @@ -272,6 +307,7 @@ export function useTabManager({ activeConnection, metadata, schema, persistWorks setEditingTabName, addTab, closeTab, + reopenLastClosedTab, updateCurrentTab, updateTabById, handleTableClick, diff --git a/tests/hooks/use-tab-manager.test.ts b/tests/hooks/use-tab-manager.test.ts index 10b679545..fe4a43a67 100644 --- a/tests/hooks/use-tab-manager.test.ts +++ b/tests/hooks/use-tab-manager.test.ts @@ -4,7 +4,7 @@ import { describe, test, expect, mock, beforeEach } from "bun:test"; import { renderHook, act, waitFor } from "@testing-library/react"; // Shared mocks — process-wide singletons (no contamination) -import "../helpers/mock-sonner"; +import { mockToastDefault } from "../helpers/mock-sonner"; import "../helpers/mock-navigation"; import { useTabManager } from "@/hooks/use-tab-manager"; @@ -73,9 +73,17 @@ const testSchema: DetailedObject[] = [ }, ]; +type ToastOptions = { action: { label: string; onClick: () => void } }; + +function lastToastCall() { + const call = mockToastDefault.mock.calls.at(-1) as unknown as [string, ToastOptions]; + return { message: call[0], options: call[1] }; +} + describe("useTabManager", () => { beforeEach(() => { localStorage.clear(); + mockToastDefault.mockClear(); }); test("starts with one default tab", () => { @@ -207,6 +215,149 @@ describe("useTabManager", () => { expect(result.current.tabs[0].id).toBe("default"); }); + test("closeTab does not toast when it can't close the only remaining tab", () => { + const { result } = renderHook(() => + useTabManager({ + activeConnection: null, + metadata: null, + schema: [], + }), + ); + + act(() => { + result.current.closeTab("default", { stopPropagation: () => {} } as React.MouseEvent); + }); + + expect(mockToastDefault).not.toHaveBeenCalled(); + }); + + test("closeTab offers an Undo toast naming the closed tab", () => { + const { result } = renderHook(() => + useTabManager({ + activeConnection: null, + metadata: null, + schema: [], + }), + ); + + act(() => { + result.current.addTab(); + }); + + act(() => { + result.current.closeTab("default", { stopPropagation: () => {} } as React.MouseEvent); + }); + + const { message, options } = lastToastCall(); + expect(message).toContain("Query 1"); + expect(options.action.label).toBe("Undo"); + }); + + test("Undo restores the closed tab's query, name and original position", () => { + const { result } = renderHook(() => + useTabManager({ + activeConnection: null, + metadata: null, + schema: [], + }), + ); + + act(() => { + result.current.updateTabById("default", { query: "SELECT 1;" }); + result.current.addTab(); + }); + const secondTabId = result.current.tabs[1].id; + + act(() => { + result.current.closeTab("default", { stopPropagation: () => {} } as React.MouseEvent); + }); + expect(result.current.tabs).toHaveLength(1); + + act(() => { + lastToastCall().options.action.onClick(); + }); + + expect(result.current.tabs).toHaveLength(2); + expect(result.current.tabs[0].id).toBe("default"); + expect(result.current.tabs[0].query).toBe("SELECT 1;"); + expect(result.current.tabs[1].id).toBe(secondTabId); + expect(result.current.activeTabId).toBe("default"); + }); + + test("Undo re-activates the closed tab even if another tab is active by then", () => { + const { result } = renderHook(() => + useTabManager({ + activeConnection: null, + metadata: null, + schema: [], + }), + ); + + act(() => { + result.current.addTab(); + }); + const secondTabId = result.current.tabs[1].id; + + act(() => { + result.current.closeTab(secondTabId, { stopPropagation: () => {} } as React.MouseEvent); + }); + // closeTab fell back to the only remaining tab. + expect(result.current.activeTabId).toBe("default"); + + act(() => { + lastToastCall().options.action.onClick(); + }); + + expect(result.current.activeTabId).toBe(secondTabId); + }); + + test("Undo is a no-op once a second tab has since been closed", () => { + const { result } = renderHook(() => + useTabManager({ + activeConnection: null, + metadata: null, + schema: [], + }), + ); + + act(() => { + result.current.addTab(); + result.current.addTab(); + }); + expect(result.current.tabs).toHaveLength(3); + + act(() => { + result.current.closeTab(result.current.tabs[0].id, { stopPropagation: () => {} } as React.MouseEvent); + }); + act(() => { + result.current.closeTab(result.current.tabs[0].id, { stopPropagation: () => {} } as React.MouseEvent); + }); + expect(result.current.tabs).toHaveLength(1); + + act(() => { + result.current.reopenLastClosedTab(); + }); + + // Undo only ever restores the MOST recent close (#747) - the first is gone for good. + expect(result.current.tabs).toHaveLength(2); + }); + + test("reopenLastClosedTab is a no-op when nothing has been closed", () => { + const { result } = renderHook(() => + useTabManager({ + activeConnection: null, + metadata: null, + schema: [], + }), + ); + + act(() => { + result.current.reopenLastClosedTab(); + }); + + expect(result.current.tabs).toHaveLength(1); + }); + test("updateCurrentTab updates the active tab properties", () => { const { result } = renderHook(() => useTabManager({ From 41e6c1519e73ee7129789140557d642511dc3a7e Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 14 Sep 2026 23:16:26 +0300 Subject: [PATCH 2/2] fix(tabs): give each close its own undo, scoped to its workspace (#747) One ref held the last closed tab, but sonner keeps several toasts on screen, so the Undo under an older toast restored the newer tab and the older one was lost. Each toast now closes over the tab it names. A connection switch dismisses outstanding Undo toasts, and a late click is checked against the workspace key. Undo after a switch put the old connection's tab into the new one with a duplicate "default" id, which the save effect persisted. The last-tab guard is back inside the setTabs updater, a restored tab goes back before its right-hand neighbour so out-of-order undos keep the order, and a restore never duplicates an id that is open again. --- src/hooks/use-tab-manager.ts | 59 +++++++----- tests/helpers/mock-sonner.ts | 2 + tests/hooks/use-tab-manager.test.ts | 140 ++++++++++++++++++++++++++-- 3 files changed, 170 insertions(+), 31 deletions(-) diff --git a/src/hooks/use-tab-manager.ts b/src/hooks/use-tab-manager.ts index 191a1078c..2dbad0c56 100644 --- a/src/hooks/use-tab-manager.ts +++ b/src/hooks/use-tab-manager.ts @@ -12,10 +12,13 @@ import { resolveTabType } from "@/lib/editor/tab-language"; import { logger } from "@/lib/logger"; import { newLocalId } from "@/lib/ids"; -/** A tab `closeTab` removed, and where it sat, so `reopenLastClosedTab` can put it back (#747). */ +/** A tab `closeTab` removed, where it sat, and the workspace it sat in, so its Undo can put it back (#747). */ interface ClosedTab { tab: QueryTab; index: number; + /** The tab to its right when it closed, or null when it was last: the anchor that survives other closes. */ + nextTabId: string | null; + workspaceKey: string; } const DEFAULT_TAB: QueryTab = { @@ -250,22 +253,35 @@ export function useTabManager({ activeConnection, metadata, schema, persistWorks setActiveTabId(newId); }, [metadata]); - /** - * Holds exactly the one tab `closeTab` most recently removed (#747). A ref, not state: - * nothing renders from it, only `reopenLastClosedTab` reads it, and closing a second tab - * before undoing the first deliberately drops the first — undo answers the immediate - * misclick the issue describes, not a multi-level history, so there is nothing here that - * needs to survive past the next close. + /* + * The Undo toasts still on screen, and the workspace the hook is showing (#747). + * + * Each toast closes over the tab it names rather than sharing one "last closed" slot: sonner + * keeps several toasts up at once, so a shared slot let the Undo under `Closed "Query 1"` + * restore Query 2. A closed tab belongs to the workspace it was closed in, so a connection + * switch dismisses the outstanding toasts, and a click that lands during the dismissal is + * checked against the workspace key: restoring it into another connection duplicated the + * `default` id there and the SAVE EFFECT persisted the duplicate. */ - const lastClosedTabRef = useRef(null); + const undoToastIdsRef = useRef>([]); + const workspaceKeyRef = useRef(workspaceKey); + useEffect(() => { + workspaceKeyRef.current = workspaceKey; + for (const id of undoToastIdsRef.current) toast.dismiss(id); + undoToastIdsRef.current = []; + }, [workspaceKey]); - const reopenLastClosedTab = useCallback(() => { - const closed = lastClosedTabRef.current; - if (!closed) return; - lastClosedTabRef.current = null; + const reopenClosedTab = useCallback((closed: ClosedTab) => { + if (closed.workspaceKey !== workspaceKeyRef.current) return; setTabs((prev) => { + // A tab id is unique in a workspace. It is already present when this Undo was clicked + // before, or when a Source tab, whose id is derived from its address, was opened again. + if (prev.some((t) => t.id === closed.tab.id)) return prev; + // Back before its right-hand neighbour when that tab is still open. The index alone is + // stale as soon as another tab to its left closes too, which is the ordinary way to tidy up. + const anchor = closed.nextTabId === null ? -1 : prev.findIndex((t) => t.id === closed.nextTabId); const next = [...prev]; - next.splice(Math.min(closed.index, next.length), 0, closed.tab); + next.splice(anchor === -1 ? Math.min(closed.index, next.length) : anchor, 0, closed.tab); return next; }); setActiveTabId(closed.tab.id); @@ -277,20 +293,22 @@ export function useTabManager({ activeConnection, metadata, schema, persistWorks if (tabs.length === 1) return; const index = tabs.findIndex((t) => t.id === id); if (index === -1) return; - const closedTab = tabs[index]; + const closed: ClosedTab = { tab: tabs[index], index, nextTabId: tabs[index + 1]?.id ?? null, workspaceKey }; - setTabs((prev) => prev.filter((t) => t.id !== id)); + // The last-tab guard is evaluated again inside the updater, against the state actually + // being written: two closes batched into one commit both pass the check above. + setTabs((prev) => (prev.length === 1 ? prev : prev.filter((t) => t.id !== id))); if (activeTabId === id) { const remaining = tabs.filter((t) => t.id !== id); - if (remaining.length > 0) setActiveTabId(remaining[remaining.length - 1].id); + setActiveTabId(remaining[remaining.length - 1].id); } - lastClosedTabRef.current = { tab: closedTab, index }; - toast(`Closed "${closedTab.name}"`, { - action: { label: "Undo", onClick: () => reopenLastClosedTab() }, + const toastId = toast(`Closed "${closed.tab.name}"`, { + action: { label: "Undo", onClick: () => reopenClosedTab(closed) }, }); + undoToastIdsRef.current.push(toastId); }, - [tabs, activeTabId, reopenLastClosedTab], + [tabs, activeTabId, workspaceKey, reopenClosedTab], ); /** @@ -456,7 +474,6 @@ export function useTabManager({ activeConnection, metadata, schema, persistWorks setEditingTabName, addTab, closeTab, - reopenLastClosedTab, updateCurrentTab, updateTabById, handleTableClick, diff --git a/tests/helpers/mock-sonner.ts b/tests/helpers/mock-sonner.ts index 3c81608cf..7521beb9a 100644 --- a/tests/helpers/mock-sonner.ts +++ b/tests/helpers/mock-sonner.ts @@ -16,6 +16,7 @@ import React from "react"; export const mockToastSuccess = mock(() => {}); export const mockToastError = mock(() => {}); export const mockToastDefault = mock(() => {}); +export const mockToastDismiss = mock((_id?: string | number) => {}); export const mockToaster = mock((props: Record) => React.createElement("div", { "data-testid": "toaster", className: props.className }), ); @@ -25,5 +26,6 @@ mock.module("sonner", () => ({ toast: Object.assign(mockToastDefault, { success: mockToastSuccess, error: mockToastError, + dismiss: mockToastDismiss, }), })); diff --git a/tests/hooks/use-tab-manager.test.ts b/tests/hooks/use-tab-manager.test.ts index bcadeb531..3973f78d7 100644 --- a/tests/hooks/use-tab-manager.test.ts +++ b/tests/hooks/use-tab-manager.test.ts @@ -4,7 +4,7 @@ import { describe, test, expect, mock, beforeEach } from "bun:test"; import { renderHook, act, waitFor } from "@testing-library/react"; // Shared mocks — process-wide singletons (no contamination) -import { mockToastDefault } from "../helpers/mock-sonner"; +import { mockToastDefault, mockToastDismiss } from "../helpers/mock-sonner"; import "../helpers/mock-navigation"; import { useTabManager } from "@/hooks/use-tab-manager"; @@ -85,6 +85,7 @@ describe("useTabManager", () => { beforeEach(() => { localStorage.clear(); mockToastDefault.mockClear(); + mockToastDismiss.mockClear(); }); test("starts with one default tab", () => { @@ -312,7 +313,7 @@ describe("useTabManager", () => { expect(result.current.activeTabId).toBe(secondTabId); }); - test("Undo is a no-op once a second tab has since been closed", () => { + test("each Undo restores the tab its own toast names, whichever is clicked first", () => { const { result } = renderHook(() => useTabManager({ activeConnection: null, @@ -322,28 +323,140 @@ describe("useTabManager", () => { ); act(() => { + result.current.updateTabById("default", { query: "SELECT 1;" }); result.current.addTab(); result.current.addTab(); }); - expect(result.current.tabs).toHaveLength(3); + const [first, second, third] = result.current.tabs.map((t) => t.id); act(() => { - result.current.closeTab(result.current.tabs[0].id, { stopPropagation: () => {} } as React.MouseEvent); + result.current.closeTab(first, { stopPropagation: () => {} } as React.MouseEvent); }); + const firstToast = lastToastCall(); act(() => { - result.current.closeTab(result.current.tabs[0].id, { stopPropagation: () => {} } as React.MouseEvent); + result.current.closeTab(second, { stopPropagation: () => {} } as React.MouseEvent); }); - expect(result.current.tabs).toHaveLength(1); + const secondToast = lastToastCall(); + expect(firstToast.message).toContain("Query 1"); + expect(secondToast.message).toContain("Query 2"); + // The OLDER toast, clicked while the newer one is still on screen, brings back Query 1. act(() => { - result.current.reopenLastClosedTab(); + firstToast.options.action.onClick(); }); + expect(result.current.tabs.map((t) => t.id)).toEqual([first, third]); + expect(result.current.tabs[0].query).toBe("SELECT 1;"); + expect(result.current.activeTabId).toBe(first); + + act(() => { + secondToast.options.action.onClick(); + }); + expect(result.current.tabs.map((t) => t.id)).toEqual([first, second, third]); + expect(result.current.activeTabId).toBe(second); + }); + + test("Undo clicked twice restores the tab once", () => { + const { result } = renderHook(() => + useTabManager({ + activeConnection: null, + metadata: null, + schema: [], + }), + ); - // Undo only ever restores the MOST recent close (#747) - the first is gone for good. + act(() => { + result.current.addTab(); + }); + act(() => { + result.current.closeTab("default", { stopPropagation: () => {} } as React.MouseEvent); + }); + const { options } = lastToastCall(); + + act(() => { + options.action.onClick(); + }); + act(() => { + options.action.onClick(); + }); + + expect(result.current.tabs.map((t) => t.id).filter((id) => id === "default")).toHaveLength(1); expect(result.current.tabs).toHaveLength(2); }); - test("reopenLastClosedTab is a no-op when nothing has been closed", () => { + test("Undo focuses a Source tab reopened since, rather than adding a second with its id", () => { + const routine: DatabaseObject = { path: ["app", "order_total(integer)"], name: "order_total", kind: "function" }; + const { result } = renderHook(() => + useTabManager({ activeConnection: makeConnection(), metadata: defaultMetadata, schema: [] }), + ); + + act(() => { + result.current.openSourceTab(routine); + }); + const sourceId = result.current.activeTabId; + act(() => { + result.current.closeTab(sourceId, { stopPropagation: () => {} } as React.MouseEvent); + }); + const { options } = lastToastCall(); + // A Source tab's id is derived from its address, so opening the object again mints the same id. + act(() => { + result.current.openSourceTab(routine); + }); + act(() => { + result.current.setActiveTabId("default"); + }); + + act(() => { + options.action.onClick(); + }); + + expect(result.current.tabs.filter((t) => t.id === sourceId)).toHaveLength(1); + expect(result.current.activeTabId).toBe(sourceId); + }); + + test("a connection switch dismisses outstanding Undo toasts, and a late click cannot cross", async () => { + localStorage.setItem( + "libredb_workspace_tabs_v1:conn-a", + JSON.stringify({ + activeTabId: "a-1", + tabs: [ + { id: "a-1", name: "A One", query: "SELECT alpha_only_secret;", type: "sql" }, + { id: "a-2", name: "A Two", query: "SELECT 2;", type: "sql" }, + ], + }), + ); + const connA = makeConnection({ id: "conn-a" }); + const connB = makeConnection({ id: "conn-b" }); + const { result, rerender } = renderHook( + ({ conn }) => useTabManager({ activeConnection: conn, metadata: null, schema: [], persistWorkspace: true }), + { initialProps: { conn: connA } }, + ); + await waitFor(() => { + expect(result.current.tabs).toHaveLength(2); + }); + + mockToastDefault.mockImplementationOnce((() => "closed-a-1") as unknown as () => void); + act(() => { + result.current.closeTab("a-1", { stopPropagation: () => {} } as React.MouseEvent); + }); + const { options } = lastToastCall(); + + rerender({ conn: connB }); + await waitFor(() => { + expect(result.current.tabs.map((t) => t.id)).toEqual(["default"]); + }); + expect(mockToastDismiss).toHaveBeenCalledWith("closed-a-1"); + + act(() => { + options.action.onClick(); + }); + expect(result.current.tabs.map((t) => t.id)).toEqual(["default"]); + + await new Promise((r) => setTimeout(r, 700)); + const parsedB = JSON.parse(localStorage.getItem("libredb_workspace_tabs_v1:conn-b")!) as { tabs: unknown[] }; + expect(parsedB.tabs).toEqual([{ id: "default", name: "Query 1", query: "", type: "sql" }]); + }); + + test("two closes batched into one commit still leave the last tab open", () => { const { result } = renderHook(() => useTabManager({ activeConnection: null, @@ -353,10 +466,17 @@ describe("useTabManager", () => { ); act(() => { - result.current.reopenLastClosedTab(); + result.current.addTab(); + }); + const secondTabId = result.current.tabs[1].id; + + act(() => { + result.current.closeTab("default", { stopPropagation: () => {} } as React.MouseEvent); + result.current.closeTab(secondTabId, { stopPropagation: () => {} } as React.MouseEvent); }); expect(result.current.tabs).toHaveLength(1); + expect(result.current.currentTab).toBeDefined(); }); test("updateCurrentTab updates the active tab properties", () => {