fix(desktop): stop context-menu item clicks from spurious preview open
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).
This commit is contained in:
116
apps/desktop/src/app/right-sidebar/files/tree.test.tsx
Normal file
116
apps/desktop/src/app/right-sidebar/files/tree.test.tsx
Normal file
@@ -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<Parameters<typeof ProjectTree>[0]> = {}) {
|
||||
const onPreviewFile = vi.fn()
|
||||
const onActivateFile = vi.fn()
|
||||
|
||||
const view = render(
|
||||
<I18nProvider configClient={null} initialLocale="en">
|
||||
<ProjectTree
|
||||
collapseNonce={0}
|
||||
cwd="/w"
|
||||
data={data}
|
||||
onActivateFile={onActivateFile}
|
||||
onActivateFolder={vi.fn()}
|
||||
onLoadChildren={vi.fn()}
|
||||
onNodeOpenChange={vi.fn()}
|
||||
onPreviewFile={onPreviewFile}
|
||||
openState={{}}
|
||||
{...overrides}
|
||||
/>
|
||||
</I18nProvider>
|
||||
)
|
||||
|
||||
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')
|
||||
})
|
||||
})
|
||||
@@ -244,7 +244,20 @@ function ProjectTreeRowContainer({ attrs, children, innerRef, node }: RowRendere
|
||||
return (
|
||||
<div
|
||||
{...attrs}
|
||||
onClick={node.handleClick}
|
||||
onClick={e => {
|
||||
// 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%' }}
|
||||
|
||||
Reference in New Issue
Block a user