fix(sidebar): make file tree rows reliably clickable (#2737)
fix(sidebar): make file tree rows reliably clickable
This commit is contained in:
@@ -47,6 +47,7 @@ import { isFilesystemError } from '@/lib/api/files-errors';
|
|||||||
import { notifyFileContentInvalidated } from '@/lib/fileContentInvalidation';
|
import { notifyFileContentInvalidated } from '@/lib/fileContentInvalidation';
|
||||||
import { isBrowserClientRuntime } from '@/lib/desktop';
|
import { isBrowserClientRuntime } from '@/lib/desktop';
|
||||||
import { useI18n } from '@/lib/i18n';
|
import { useI18n } from '@/lib/i18n';
|
||||||
|
import { recordFileTreeDragStart, shouldTreatFileTreeDragEndAsClick } from './fileTreeDragClick';
|
||||||
|
|
||||||
type FileNode = {
|
type FileNode = {
|
||||||
name: string;
|
name: string;
|
||||||
@@ -388,12 +389,20 @@ const FileRow: React.FC<FileRowProps> = ({
|
|||||||
);
|
);
|
||||||
|
|
||||||
const handleDragStart = React.useCallback((e: React.DragEvent) => {
|
const handleDragStart = React.useCallback((e: React.DragEvent) => {
|
||||||
|
recordFileTreeDragStart(e);
|
||||||
const path = getRelativePath(root, node.path);
|
const path = getRelativePath(root, node.path);
|
||||||
if (!path || path === '.') return;
|
if (!path || path === '.') return;
|
||||||
e.dataTransfer.setData('application/x-openchamber-file-path', path);
|
e.dataTransfer.setData('application/x-openchamber-file-path', path);
|
||||||
e.dataTransfer.effectAllowed = 'copy';
|
e.dataTransfer.effectAllowed = 'copy';
|
||||||
}, [node.path, root]);
|
}, [node.path, root]);
|
||||||
|
|
||||||
|
const handleDragEnd = React.useCallback((e: React.DragEvent) => {
|
||||||
|
// A micro-drag suppressed the click this gesture was meant to be (#2368).
|
||||||
|
if (shouldTreatFileTreeDragEndAsClick(e)) {
|
||||||
|
handleInteraction();
|
||||||
|
}
|
||||||
|
}, [handleInteraction]);
|
||||||
|
|
||||||
const handleExternalDragOver = React.useCallback((event: React.DragEvent) => {
|
const handleExternalDragOver = React.useCallback((event: React.DragEvent) => {
|
||||||
if (!canUpload || !uploadDirectory || !hasExternalFiles(event.dataTransfer)) return;
|
if (!canUpload || !uploadDirectory || !hasExternalFiles(event.dataTransfer)) return;
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
@@ -434,12 +443,12 @@ const FileRow: React.FC<FileRowProps> = ({
|
|||||||
onContextMenu={handleContextMenu}
|
onContextMenu={handleContextMenu}
|
||||||
draggable
|
draggable
|
||||||
onDragStart={handleDragStart}
|
onDragStart={handleDragStart}
|
||||||
|
onDragEnd={handleDragEnd}
|
||||||
className={cn(
|
className={cn(
|
||||||
'flex w-full items-center gap-1.5 rounded-md px-2 py-1 text-left text-foreground transition-colors pr-8 select-none',
|
'flex w-full items-center gap-1.5 rounded-md px-2 py-1 text-left text-foreground transition-colors pr-8 select-none',
|
||||||
isDropTarget
|
isDropTarget
|
||||||
? 'bg-interactive-selection ring-2 ring-inset ring-primary'
|
? 'bg-interactive-selection ring-2 ring-inset ring-primary'
|
||||||
: (isActive ? 'bg-interactive-selection/70' : 'hover:bg-interactive-hover/40'),
|
: (isActive ? 'bg-interactive-selection/70' : 'hover:bg-interactive-hover/40')
|
||||||
'cursor-grab active:cursor-grabbing'
|
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
{isDir ? (
|
{isDir ? (
|
||||||
@@ -1423,13 +1432,20 @@ export const SidebarFilesTree: React.FC = () => {
|
|||||||
onClick={() => handleOpenFile(node)}
|
onClick={() => handleOpenFile(node)}
|
||||||
draggable
|
draggable
|
||||||
onDragStart={(e) => {
|
onDragStart={(e) => {
|
||||||
|
recordFileTreeDragStart(e);
|
||||||
const path = node.relativePath || getRelativePath(root ?? '', node.path);
|
const path = node.relativePath || getRelativePath(root ?? '', node.path);
|
||||||
if (!path || path === '.') return;
|
if (!path || path === '.') return;
|
||||||
e.dataTransfer.setData('application/x-openchamber-file-path', path);
|
e.dataTransfer.setData('application/x-openchamber-file-path', path);
|
||||||
e.dataTransfer.effectAllowed = 'copy';
|
e.dataTransfer.effectAllowed = 'copy';
|
||||||
}}
|
}}
|
||||||
|
onDragEnd={(e) => {
|
||||||
|
// A micro-drag suppressed the click this gesture was meant to be (#2368).
|
||||||
|
if (shouldTreatFileTreeDragEndAsClick(e)) {
|
||||||
|
void handleOpenFile(node);
|
||||||
|
}
|
||||||
|
}}
|
||||||
className={cn(
|
className={cn(
|
||||||
'flex w-full items-center gap-1.5 rounded-md px-2 py-1 text-left text-foreground transition-colors cursor-grab active:cursor-grabbing',
|
'flex w-full items-center gap-1.5 rounded-md px-2 py-1 text-left text-foreground transition-colors',
|
||||||
isActive ? 'bg-interactive-selection/70' : 'hover:bg-interactive-hover/40'
|
isActive ? 'bg-interactive-selection/70' : 'hover:bg-interactive-hover/40'
|
||||||
)}
|
)}
|
||||||
title={node.path}
|
title={node.path}
|
||||||
|
|||||||
@@ -0,0 +1,76 @@
|
|||||||
|
import { beforeEach, describe, expect, test } from 'bun:test';
|
||||||
|
|
||||||
|
import {
|
||||||
|
recordFileTreeDragStart,
|
||||||
|
resetFileTreeDragClickState,
|
||||||
|
shouldTreatFileTreeDragEndAsClick,
|
||||||
|
} from './fileTreeDragClick';
|
||||||
|
|
||||||
|
const dragEnd = (clientX: number, clientY: number, dropEffect = 'none') => ({
|
||||||
|
clientX,
|
||||||
|
clientY,
|
||||||
|
dataTransfer: { dropEffect },
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
resetFileTreeDragClickState();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('file tree drag-click fallback (#2368)', () => {
|
||||||
|
test('a micro-drag that ends where it began is recovered as a click', () => {
|
||||||
|
// Chromium starts a native drag after ~4px of pointer travel and then
|
||||||
|
// suppresses the click event for the rest of the gesture. On macOS
|
||||||
|
// trackpads a plain click routinely slips past that threshold, which is
|
||||||
|
// the "clicking a folder does nothing" symptom of issue #2368.
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(102, 201))).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a zero-travel drag end is recovered as a click', () => {
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(100, 200))).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a drag released far from its origin is not a click', () => {
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(180, 230))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('slop boundary: within the radius is a click, beyond it is not', () => {
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(108, 208))).toBe(true);
|
||||||
|
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(109, 200))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a drag dropped onto a target is never a click', () => {
|
||||||
|
// Dragging a file into the chat input inserts an @mention; a completed
|
||||||
|
// drop must not additionally toggle or open the row.
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(101, 200, 'copy'))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a drag end without a recorded start is ignored', () => {
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(100, 200))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('the recorded origin is consumed by the first drag end', () => {
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(100, 200))).toBe(true);
|
||||||
|
expect(shouldTreatFileTreeDragEndAsClick(dragEnd(100, 200))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a missing dataTransfer still recovers a near-origin drag as a click', () => {
|
||||||
|
recordFileTreeDragStart({ clientX: 100, clientY: 200 });
|
||||||
|
|
||||||
|
expect(
|
||||||
|
shouldTreatFileTreeDragEndAsClick({ clientX: 101, clientY: 201, dataTransfer: null }),
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,70 @@
|
|||||||
|
/**
|
||||||
|
* Click-reliability fallback for file tree rows that are both clickable and
|
||||||
|
* draggable (issue #2368).
|
||||||
|
*
|
||||||
|
* A native HTML5 drag starts after only a few pixels of pointer travel
|
||||||
|
* (4px in Chromium), and once `dragstart` fires the browser suppresses the
|
||||||
|
* `click` event for that gesture entirely. On macOS trackpads and Magic
|
||||||
|
* Mouse a plain click very often slips past that threshold, so rows that
|
||||||
|
* carry `draggable` (to drag file references into the chat input) randomly
|
||||||
|
* ignored clicks: folders neither expanded nor collapsed and files did not
|
||||||
|
* open.
|
||||||
|
*
|
||||||
|
* Arming `draggable` only after a pointer-move threshold is not a fix:
|
||||||
|
* Chromium decides drag eligibility on the first mouse move after mousedown
|
||||||
|
* and never re-evaluates, so a drag whose first movement stays below the
|
||||||
|
* threshold would never start (verified against headless Chromium).
|
||||||
|
*
|
||||||
|
* Instead the row stays draggable, and a drag that ends where it began —
|
||||||
|
* within a small slop radius and without dropping onto any target — is
|
||||||
|
* treated as the click it was meant to be. The two paths are mutually
|
||||||
|
* exclusive: when the browser suppresses `click` it fired `dragstart`, and
|
||||||
|
* when `click` fires no drag ever started, so the row action runs exactly
|
||||||
|
* once per gesture.
|
||||||
|
*
|
||||||
|
* Module-level state is safe here because the platform allows only one
|
||||||
|
* native drag at a time.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Chromium starts a native drag at 4px of travel, so a suppressed click's
|
||||||
|
* dragstart→dragend distance is near zero. The slop only needs to absorb
|
||||||
|
* the remaining wobble between drag start and release; a deliberate drag
|
||||||
|
* released mid-flight travels far beyond it.
|
||||||
|
*/
|
||||||
|
const DRAG_CLICK_SLOP_PX = 8;
|
||||||
|
|
||||||
|
type DragPointerEvent = {
|
||||||
|
clientX: number;
|
||||||
|
clientY: number;
|
||||||
|
};
|
||||||
|
|
||||||
|
let pendingDragOrigin: { x: number; y: number } | null = null;
|
||||||
|
|
||||||
|
/** Record where a file row drag started. Call from the row's `dragstart`. */
|
||||||
|
export const recordFileTreeDragStart = (event: DragPointerEvent): void => {
|
||||||
|
pendingDragOrigin = { x: event.clientX, y: event.clientY };
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True when the drag that just ended was an accidental micro-drag that
|
||||||
|
* swallowed a click: it was never dropped onto a target and it ended within
|
||||||
|
* `DRAG_CLICK_SLOP_PX` of where it started. Consumes the recorded origin.
|
||||||
|
*/
|
||||||
|
export const shouldTreatFileTreeDragEndAsClick = (
|
||||||
|
event: DragPointerEvent & { dataTransfer: { dropEffect: string } | null },
|
||||||
|
): boolean => {
|
||||||
|
const origin = pendingDragOrigin;
|
||||||
|
pendingDragOrigin = null;
|
||||||
|
if (!origin) return false;
|
||||||
|
if (event.dataTransfer && event.dataTransfer.dropEffect !== 'none') return false;
|
||||||
|
return (
|
||||||
|
Math.abs(event.clientX - origin.x) <= DRAG_CLICK_SLOP_PX
|
||||||
|
&& Math.abs(event.clientY - origin.y) <= DRAG_CLICK_SLOP_PX
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
/** Reset module state. Intended for tests. */
|
||||||
|
export const resetFileTreeDragClickState = (): void => {
|
||||||
|
pendingDragOrigin = null;
|
||||||
|
};
|
||||||
Reference in New Issue
Block a user