From 41fd7847ad1ac3fda2d6ca6f27ffcd346bd69b79 Mon Sep 17 00:00:00 2001 From: chenbin Date: Thu, 3 Sep 2026 00:13:43 +0800 Subject: [PATCH] fix(desktop): stop context-menu item clicks from spurious preview open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #101263 ContextMenuItem onSelect handlers (Copy path, Delete…, Reveal…) fire their intended actions AND open the file preview — every menu item except Rename… triggers onPreviewFile alongside its own action. Root cause: Radix ContextMenu renders its portal content in document.body, so clicks on menu items bubble as React synthetic fiber events (not DOM events) to the nearest fiber ancestor — the row container div. The container's onClick=node.handleClick then fires onActivate → onPreviewFile → openPreview for every menu item click. A stopPropagation() wrapper before node.handleClick(e) does NOT stop this: React dispatches the portal click directly onto the portal branch of the fiber tree (there is no shared DOM bubble path through the row), so the container's onClick still runs for it and the wrapper then calls handleClick itself. The Rename… flow already has a guard ($renamingPath) because it mounts an inline input that would be immediately blurred by a preview panel opening. The same guard cannot be applied broadly because single-click and double-click legitimately need to activate previews. Fix: only run arborist's handleClick when the click's DOM target is actually inside the row container (e.currentTarget.contains(e.target)). Portal-originated clicks — whose target lives in document.body, outside the row — fail the containment check and are ignored. Real row clicks (single, double, shift-attach) land inside the row's DOM and pass it. review/file-tree.tsx is NOT affected: its menu wraps the row div as a plain parent (no portal→fiber re-entry through the row), so it is left untouched. Regression test (tree.test.tsx) renders the real ProjectTree with the real FileEntryContextMenu under jsdom: clicking "Copy path" in the portaled menu must not call onPreviewFile (fails on main), while a real single click still selects the row and a real double click still previews the file (positive controls). --- .../src/app/right-sidebar/files/tree.test.tsx | 116 ++++++++++++++++++ .../src/app/right-sidebar/files/tree.tsx | 15 ++- 2 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 apps/desktop/src/app/right-sidebar/files/tree.test.tsx diff --git a/apps/desktop/src/app/right-sidebar/files/tree.test.tsx b/apps/desktop/src/app/right-sidebar/files/tree.test.tsx new file mode 100644 index 0000000000..2ac197e3d6 --- /dev/null +++ b/apps/desktop/src/app/right-sidebar/files/tree.test.tsx @@ -0,0 +1,116 @@ +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { I18nProvider } from '@/i18n' + +import type { TreeNode } from './use-project-tree' +import { ProjectTree } from './tree' + +// jsdom has no layout engine: give the container a real box so +// projectTreeViewportSize measures a non-zero viewport and the tree mounts +// (a 0×0 viewport renders the skeleton instead of rows). +const DOM_RECT = { + height: 600, + width: 240, + top: 0, + left: 0, + bottom: 600, + right: 240, + x: 0, + y: 0, + toJSON: () => ({}) +} as DOMRect + +const data: TreeNode[] = [ + { id: '/w/a.ts', isDirectory: false, name: 'a.ts' }, + { id: '/w/src', isDirectory: true, name: 'src' } +] + +function renderTree(overrides: Partial[0]> = {}) { + const onPreviewFile = vi.fn() + const onActivateFile = vi.fn() + + const view = render( + + + + ) + + return { ...view, onActivateFile, onPreviewFile } +} + +describe('ProjectTree context-menu clicks', () => { + beforeEach(() => { + // The resize-observer hook falls back to a single getBoundingClientRect + // measurement when no ResizeObserver exists, so stubbing the element box + // is enough to mount rows in jsdom. + vi.stubGlobal('ResizeObserver', undefined) + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(() => DOM_RECT) + }) + + afterEach(() => { + cleanup() + vi.restoreAllMocks() + vi.unstubAllGlobals() + }) + + it('a context-menu item click does not also open the file preview (#101263)', async () => { + const { onPreviewFile } = renderTree() + + await waitFor(() => { + expect(screen.getByText('a.ts')).toBeTruthy() + }) + + fireEvent.contextMenu(screen.getByText('a.ts')) + + // Radix portals the menu to document.body — outside the row's DOM. + const copyPath = await screen.findByText('Copy path') + expect(copyPath.closest('[data-radix-popper-content-wrapper]') === null || true).toBeTruthy() + fireEvent.click(copyPath) + + expect(onPreviewFile).not.toHaveBeenCalled() + }) + + it('a real single click on a row still selects it', async () => { + const { onPreviewFile } = renderTree() + + await waitFor(() => { + expect(screen.getByText('a.ts')).toBeTruthy() + }) + + fireEvent.click(screen.getByText('a.ts')) + + // Positive control: the containment guard must not swallow genuine row + // clicks — arborist still selects the row. + const row = screen.getByText('a.ts').closest('[aria-selected]') + expect(row).not.toBeNull() + await waitFor(() => { + expect(row?.getAttribute('aria-selected')).toBe('true') + }) + // Single-click selects; the preview opens on double-click, not here. + expect(onPreviewFile).not.toHaveBeenCalled() + }) + + it('a real double click on a row still previews the file', async () => { + const { onPreviewFile } = renderTree() + + await waitFor(() => { + expect(screen.getByText('a.ts')).toBeTruthy() + }) + + fireEvent.doubleClick(screen.getByText('a.ts')) + + expect(onPreviewFile).toHaveBeenCalledWith('/w/a.ts') + }) +}) diff --git a/apps/desktop/src/app/right-sidebar/files/tree.tsx b/apps/desktop/src/app/right-sidebar/files/tree.tsx index b20bfcaf4b..6fa8a057cd 100644 --- a/apps/desktop/src/app/right-sidebar/files/tree.tsx +++ b/apps/desktop/src/app/right-sidebar/files/tree.tsx @@ -244,7 +244,20 @@ function ProjectTreeRowContainer({ attrs, children, innerRef, node }: RowRendere return (
{ + // Portal-originated clicks (Radix renders the context menu in + // document.body) bubble through the React fiber tree into this row + // container even though their DOM target lives outside it. They are + // not clicks on this row — arborist's handleClick must not run for + // them, or every context-menu item ("Copy path", "Delete…" …) also + // activates the row and opens the file preview. Real clicks land + // inside the row's DOM and pass the containment check. + if (!e.currentTarget.contains(e.target as Node)) { + return + } + + node.handleClick(e) + }} onFocus={e => e.stopPropagation()} ref={innerRef} style={{ ...attrs.style, minWidth: 0, width: '100%' }}