Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions packages/trees/src/render/FileTreeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string | null>(null);
const pendingStickyKeyboardFocusPathRef = useRef<string | null>(null);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -3616,6 +3640,7 @@ export function FileTreeView({

updateTriggerPosition(triggerButton);
shouldRestoreContextMenuFocusRef.current = true;
contextMenuOpenTokenRef.current += 1;
setContextMenuState({
anchorRect: null,
item: {
Expand All @@ -3625,6 +3650,7 @@ export function FileTreeView({
},
path: triggerItem.getPath(),
source: 'button',
token: contextMenuOpenTokenRef.current,
});
};

Expand Down
Original file line number Diff line number Diff line change
@@ -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<string, FileTreeContextMenuOpenContext>();

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();
}
});
});