feat(tabs): undo for closing a query tab (#747) - #818
Conversation
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.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Thanks, and the tests are real: reverting the hook to main turns five of your six cases red, and mutating the restore fails exactly the cases that name that behaviour.
Two things need changing first, both about where the undo keeps its memory.
The ref is one slot, but the UI offers one Undo per toast. sonner keeps three toasts for four seconds each, so closing two tabs in quick succession puts two Undo buttons on screen naming different tabs, and both call reopenLastClosedTab(). I closed Query 1, then Query 2, then clicked Undo in the toast reading Closed "Query 1": Query 2 came back and Query 1 was gone. Letting each toast close over the tab it names fixes that and makes the label honest.
The memory also survives a connection switch. Close a tab in connection A, switch to B while the toast is up, click Undo: A's tab lands in B's workspace, both tabs carry id: "default", React logs a duplicate-key error, and the save effect persists the duplicate into libredb_workspace_tabs_v1:connB, so editing one tab then edits both. Scope the memory to the workspace key and drop it when the workspace reloads.
Smaller: prev.length === 1 used to be checked inside the setTabs updater. Read from the closure, two batched closes both pass the guard and leave zero tabs, so keep the removal and its guard in one updater.
…redb#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.
|
@Asgabani, I needed this merged for other work of mine, so I went ahead and made the changes from my review myself instead of waiting for another round. I hope you don't mind; the commit is on top of yours, and I am merging once CI is green. It follows the review point by point: each toast now closes over the tab it names, a connection switch dismisses outstanding Undo toasts so a late click cannot cross workspaces, and the last-tab guard is back inside the The feature itself is a good one to have. Please keep going, and I look forward to your next PR. |
2 participants
Closes #747.
What changed
closeTab(src/hooks/use-tab-manager.ts) dropped the closed tab's state immediately, so a misclick on the close icon lost an unsaved query with no recovery. It now:QueryTab— query text, name, type — and its original index) in a ref before removing it.sonner, matching the existingshowStarPromptToastpattern) with an "Undo" action.reopenLastClosedTab()splices the tab back into its original position and makes it active again.Deliberately only the single most-recently-closed tab is recoverable — closing a second tab before undoing the first drops the first. That matches the issue's framing (recovering from an immediate misclick, not a multi-level history) and keeps the implementation to one ref rather than a stack.
No confirmation step was added to the close action itself, per the issue's explicit constraint ("closing a tab is a frequent, usually intentional action").
Testing
Ran locally (
bun run format && bun run lint && bun run typecheck && bun run knip && bun run chart:check && bun run channels:showcase:check && bun run readme:check && bun run security:check && bun run test && bun run build, plusbun run build:lib && bun run attwsinceuseTabManageris reachable from the embeddable workspace export surface):bun run test:components: 39/39 groups pass.bun run test: same 13 pre-existing failures as a clean checkout (Helm binary not installed, missing built standalone zip). No live Postgres/MySQL in this sandbox.bun run test:coverage && bun run coverage:check: 100.00% line coverage on the merged lcov.bun run build,bun run build:lib,bun run attw: all succeed.If CI surfaces something this sandbox couldn't (Helm chart tests, live DB integration tests), happy to fix it up.