diff --git a/apps/desktop/src/components/sidebar/ConnectionTree.vue b/apps/desktop/src/components/sidebar/ConnectionTree.vue index 31cd662bc2..d2458e9038 100644 --- a/apps/desktop/src/components/sidebar/ConnectionTree.vue +++ b/apps/desktop/src/components/sidebar/ConnectionTree.vue @@ -35,7 +35,7 @@ import { Button } from "@/components/ui/button"; import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog"; import { codeMirrorSqlDialect } from "@/lib/database/jdbcDialect"; import { sqlFormatDialectForDbType } from "@/lib/sql/sqlFormatter"; -import { createSidebarActionTarget, findSidebarActionTarget, type SidebarActionTarget } from "@/lib/sidebar/sidebarActionTarget"; +import { createSidebarActionTarget, findSidebarActionTarget, matchesSidebarActionTarget, type SidebarActionTarget } from "@/lib/sidebar/sidebarActionTarget"; import type { SidebarDangerDialogRequest } from "@/lib/sidebar/sidebarDangerDialog"; import { resetSidebarTreeDialogState } from "./sidebarTreeDialogState"; import { SidebarDangerConfirmDialog, SidebarDdlViewDialog, SidebarObjectSourceDialog, SidebarProcedureExecutionDialog, SidebarVisibleDatabasesDialog, SidebarVisibleSchemasDialog } from "./sidebarAsyncDialogs"; @@ -611,12 +611,16 @@ const stickyHeaderStyle = computed(() => { // Reset tracking when the tree rebuilds (connect/disconnect/collapse) so a // stale scrollTop doesn't keep the overlay mounted after a structural change. -watch(flatNodes, () => { - // Menu actions originate from a rendered row instance. Close the singleton - // before a structural update can recycle that row onto another node. - sidebarContextMenuRef.value?.close(); - sidebarContextMenuItems.value = []; - sidebarContextMenuTarget.value = null; +watch(flatNodes, (nodes) => { + const contextMenuTarget = sidebarContextMenuTarget.value; + if (contextMenuTarget) { + const visibleContextMenuTarget = nodes.find(({ node }) => matchesSidebarActionTarget(node, contextMenuTarget))?.node; + if (!visibleContextMenuTarget || visibleContextMenuTarget.valid === false) { + sidebarContextMenuRef.value?.close(); + sidebarContextMenuItems.value = []; + sidebarContextMenuTarget.value = null; + } + } stickyScrollTop.value = 0; void nextTick(scheduleSidebarScrollMetricsUpdate); }); diff --git a/apps/desktop/src/lib/__tests__/sidebar/sidebarActionTarget.spec.ts b/apps/desktop/src/lib/__tests__/sidebar/sidebarActionTarget.spec.ts index cf659cf233..d701d1352f 100644 --- a/apps/desktop/src/lib/__tests__/sidebar/sidebarActionTarget.spec.ts +++ b/apps/desktop/src/lib/__tests__/sidebar/sidebarActionTarget.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { TreeNode } from "@/types/database"; -import { createSidebarActionRequest, createSidebarActionTarget, findSidebarActionTarget, releaseRemovedSidebarActionTarget } from "@/lib/sidebar/sidebarActionTarget"; +import { createSidebarActionRequest, createSidebarActionTarget, findSidebarActionTarget, matchesSidebarActionTarget, releaseRemovedSidebarActionTarget } from "@/lib/sidebar/sidebarActionTarget"; function tableNode(): TreeNode { return { @@ -38,6 +38,19 @@ describe("sidebar action targets", () => { expect(Object.isFrozen(request)).toBe(true); }); + it("matches a live node with the same action target identity", () => { + const node = tableNode(); + + expect(matchesSidebarActionTarget(node, createSidebarActionTarget(node))).toBe(true); + }); + + it("does not match the same id after identity fields change", () => { + const target = createSidebarActionTarget(tableNode()); + const changedNode = { ...tableNode(), connectionId: "connection-2", label: "accounts" }; + + expect(matchesSidebarActionTarget(changedNode, target)).toBe(false); + }); + it("does not resolve a recycled row with the same local id", () => { const target = createSidebarActionTarget(tableNode()); const recycled = tableNode(); diff --git a/apps/desktop/src/lib/sidebar/sidebarActionTarget.ts b/apps/desktop/src/lib/sidebar/sidebarActionTarget.ts index fe16a7fcf6..712468853a 100644 --- a/apps/desktop/src/lib/sidebar/sidebarActionTarget.ts +++ b/apps/desktop/src/lib/sidebar/sidebarActionTarget.ts @@ -12,13 +12,17 @@ function sameActionTarget(left: TreeNode, right: SidebarActionTarget): boolean { return left.id === right.id && left.type === right.type && left.connectionId === right.connectionId && left.database === right.database && left.schema === right.schema && left.catalog === right.catalog && left.label === right.label && left.signature === right.signature; } +export function matchesSidebarActionTarget(node: TreeNode, target: SidebarActionTarget): boolean { + return sameActionTarget(node, target); +} + export function findSidebarActionTarget(nodes: readonly TreeNode[], target: SidebarActionTarget): TreeNode | null { const visited = new WeakSet(); const find = (items: readonly TreeNode[]): TreeNode | null => { for (const node of items) { if (visited.has(node)) continue; visited.add(node); - if (sameActionTarget(node, target)) return node; + if (matchesSidebarActionTarget(node, target)) return node; const child = node.children ? find(node.children) : null; if (child) return child; const hiddenChild = node.hiddenChildren ? find(node.hiddenChildren) : null; diff --git a/packages/app-tests/sidebarTreeAffordances.test.ts b/packages/app-tests/sidebarTreeAffordances.test.ts index 2c8a5c54f6..750c1cb5bf 100644 --- a/packages/app-tests/sidebarTreeAffordances.test.ts +++ b/packages/app-tests/sidebarTreeAffordances.test.ts @@ -26,6 +26,11 @@ test("complex tree changes retain the full rebuild fallback", () => { assert.match(connectionTree, /watch\(flatNodes,/); }); +test("tree rebuilds keep a context menu only while its target row remains visible", () => { + assert.match(connectionTree, /nodes\.find\(\(\{ node \}\) => matchesSidebarActionTarget\(node, contextMenuTarget\)\)\?\.node/); + assert.match(connectionTree, /if \(!visibleContextMenuTarget \|\| visibleContextMenuTarget\.valid === false\)/); +}); + test("programmable object groups use the shared metadata loader", () => { assert.match(runtimeHost, /const databaseObjectGroup = !!objectTypesForGroupNode\(node\.type\)/); assert.match(connectionStore, /else if \(objectTypesForGroupNode\(node\.type\)\) \{\s*await loadObjectGroupChildren\(node, options\);/);