diff --git a/packages/trees/src/render/FileTreeView.tsx b/packages/trees/src/render/FileTreeView.tsx index de219c62b..6d7992990 100644 --- a/packages/trees/src/render/FileTreeView.tsx +++ b/packages/trees/src/render/FileTreeView.tsx @@ -1300,9 +1300,15 @@ export function FileTreeView({ item: FileTreeContextMenuItem; path: string; source: 'button' | 'keyboard' | 'right-click'; + // Monotonic open-generation token identifying this menu instance. A + // superseded menu's async close() carries the token it was opened with, so + // a stale close cannot tear down the menu that replaced it (#664). + token: number; } | null>(null); const contextMenuStateRef = useRef(contextMenuState); contextMenuStateRef.current = contextMenuState; + // Bumped on every open so each opened menu gets a unique identity token. + const contextMenuOpenTokenRef = useRef(0); const pendingStickyFocusPathRef = useRef(null); const pendingStickyKeyboardFocusPathRef = useRef(null); @@ -1576,16 +1582,27 @@ export function FileTreeView({ const restoreFocusToTreeRef = useRef(restoreFocusToTree); restoreFocusToTreeRef.current = restoreFocusToTree; const shouldRestoreContextMenuFocusRef = useRef(true); - const closeContextMenuRef = useRef<(restoreFocus?: boolean) => void>( - () => {} - ); + const closeContextMenuRef = useRef< + (restoreFocus?: boolean, expectedToken?: number) => void + >(() => {}); const closeContextMenu = useCallback( - (restoreFocus: boolean = true): void => { + (restoreFocus: boolean = true, expectedToken?: number): void => { const currentContextMenuState = contextMenuStateRef.current; if (currentContextMenuState == null) { return; } + // Ignore a close() issued by a superseded menu instance. Sequential + // right-clicks open a new menu (new token) before the previous menu's + // consumer layer fires its async onOpenChange(false); only the menu that + // is currently open may close itself (#664). + if ( + expectedToken != null && + expectedToken !== currentContextMenuState.token + ) { + return; + } + shouldRestoreContextMenuFocusRef.current = shouldRestoreContextMenuFocusRef.current && restoreFocus; setContextMenuState(null); @@ -1641,11 +1658,13 @@ export function FileTreeView({ item.focus(); updateTriggerPosition(anchorButton); shouldRestoreContextMenuFocusRef.current = true; + contextMenuOpenTokenRef.current += 1; setContextMenuState({ anchorRect: options?.anchorRect ?? null, item: createContextMenuItem(row, targetPath), path: targetPath, source: options?.source ?? 'keyboard', + token: contextMenuOpenTokenRef.current, }); }, [controller, getTriggerAnchorButton, updateTriggerPosition] @@ -2922,7 +2941,12 @@ export function FileTreeView({ currentState.anchorRect ?? serializeAnchorRect(anchorElement.getBoundingClientRect()), close: (options) => { - closeContextMenuRef.current(options?.restoreFocus ?? true); + // Capture this menu instance's token so a close() from a menu that has + // since been superseded by a sequential right-click is ignored (#664). + closeContextMenuRef.current( + options?.restoreFocus ?? true, + currentState.token + ); }, restoreFocus: () => { if (!shouldRestoreContextMenuFocusRef.current) { @@ -3616,6 +3640,7 @@ export function FileTreeView({ updateTriggerPosition(triggerButton); shouldRestoreContextMenuFocusRef.current = true; + contextMenuOpenTokenRef.current += 1; setContextMenuState({ anchorRect: null, item: { @@ -3625,6 +3650,7 @@ export function FileTreeView({ }, path: triggerItem.getPath(), source: 'button', + token: contextMenuOpenTokenRef.current, }); }; diff --git a/packages/trees/test/file-tree-context-menu-sequential-right-click.test.ts b/packages/trees/test/file-tree-context-menu-sequential-right-click.test.ts new file mode 100644 index 000000000..99790f5e5 --- /dev/null +++ b/packages/trees/test/file-tree-context-menu-sequential-right-click.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, test } from 'bun:test'; +import { JSDOM } from 'jsdom'; + +import type { FileTreeContextMenuOpenContext } from '../src/model/publicTypes'; +import { flushDom, installDom } from './helpers/dom'; + +function getItemButton( + shadowRoot: ShadowRoot | null | undefined, + dom: JSDOM, + path: string +): HTMLButtonElement { + const button = shadowRoot?.querySelector(`[data-item-path="${path}"]`); + if (!(button instanceof dom.window.HTMLButtonElement)) { + throw new Error(`missing button for ${path}`); + } + + return button; +} + +// Regression for #664: right-clicking file B while file A's menu is open must +// re-anchor the menu onto B rather than dismiss it. The failure mode is a +// stale close() from menu A's superseded consumer layer (Radix +// `onOpenChange(false)`) firing asynchronously after menu B has already opened +// and tearing menu B down because the close was not tied to a menu instance. +describe('file-tree sequential right-click context menu', () => { + test('right-clicking a second row re-anchors the menu instead of closing it', async () => { + const { cleanup, dom } = installDom(); + try { + const { FileTree } = await import('../src/render/FileTree'); + const mount = dom.window.document.createElement('div'); + dom.window.document.body.appendChild(mount); + + // Capture each opened menu's context keyed by the row it was opened for, + // so we can later fire the stale close() belonging to menu A. + const contextByPath = new Map(); + + const fileTree = new FileTree({ + composition: { + contextMenu: { + enabled: true, + render: (item, context): HTMLElement => { + contextByPath.set(item.path, context); + const menu = dom.window.document.createElement('div'); + menu.dataset.testMenu = 'true'; + menu.dataset.itemPath = item.path; + return menu as unknown as HTMLElement; + }, + }, + }, + flattenEmptyDirectories: true, + initialExpansion: 'open', + paths: ['a.ts', 'b.ts'], + initialVisibleRowCount: 120 / 30, + }); + + fileTree.render({ containerWrapper: mount }); + await flushDom(); + + const host = fileTree.getFileTreeContainer(); + const shadowRoot = host?.shadowRoot; + + // Open menu A via right-click. + getItemButton(shadowRoot, dom, 'a.ts').dispatchEvent( + new dom.window.MouseEvent('contextmenu', { + bubbles: true, + clientX: 10, + clientY: 10, + }) + ); + await flushDom(); + + let slotted = host?.querySelector( + '[slot="context-menu"][data-test-menu]' + ); + expect(slotted).not.toBeNull(); + expect((slotted as HTMLElement | null)?.dataset.itemPath).toBe('a.ts'); + + // Right-click a different row while menu A is open. Menu B should open + // anchored to b.ts. + getItemButton(shadowRoot, dom, 'b.ts').dispatchEvent( + new dom.window.MouseEvent('contextmenu', { + bubbles: true, + clientX: 20, + clientY: 40, + }) + ); + await flushDom(); + + slotted = host?.querySelector('[slot="context-menu"][data-test-menu]'); + expect(slotted).not.toBeNull(); + expect((slotted as HTMLElement | null)?.dataset.itemPath).toBe('b.ts'); + + // Simulate menu A's superseded consumer layer firing its captured close() + // asynchronously after menu B has opened. + const staleCloseA = contextByPath.get('a.ts'); + if (staleCloseA == null) { + throw new Error('expected captured context for a.ts'); + } + staleCloseA.close(); + await flushDom(2); + + // The menu must still be open and anchored to b.ts. + slotted = host?.querySelector('[slot="context-menu"][data-test-menu]'); + expect(slotted).not.toBeNull(); + expect((slotted as HTMLElement | null)?.dataset.itemPath).toBe('b.ts'); + + fileTree.cleanUp(); + } finally { + cleanup(); + } + }); +});