Merge pull request #154 from glideapps/ie/fix_cell_click

Fix onCellClick firing when it shouldn't
glideapps · Feb 3, 2022 · 2bb8d4f · 2bb8d4f
2 parents 4fca17c + e703342
commit 2bb8d4f
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 8 deletions.
diff --git a/packages/core/src/data-editor/data-editor-beautiful.stories.tsx b/packages/core/src/data-editor/data-editor-beautiful.stories.tsx
@@ -573,7 +573,7 @@ export const AddData: React.VFC = () => {
         }
         setNumRows(cv => cv + 1);
     }, [getCellContent, numRows, setCellValueRaw]);
-    
+
     return (
         <BeautifulWrapper
             title="Add data"

diff --git a/packages/core/src/data-editor/data-editor.test.tsx b/packages/core/src/data-editor/data-editor.test.tsx
@@ -293,6 +293,52 @@ describe("data-editor", () => {
         expect(spy).toHaveBeenCalledWith([1, 1], expect.anything());
     });
 
+    test("Doesn't emit cell click if mouseDown happened in a different cell", async () => {
+        const spy = jest.fn();
+
+        jest.useFakeTimers();
+        render(<DataEditor {...basicProps} onCellClicked={spy} />, {
+            wrapper: Context,
+        });
+        prep();
+
+        const canvas = screen.getByTestId("data-grid-canvas");
+        fireEvent.mouseDown(canvas, {
+            clientX: 300, // Col B, ends at x = 310
+            clientY: 36 + 32 + 16, // Row 1 (0 indexed)
+        });
+
+        fireEvent.mouseUp(canvas, {
+            clientX: 320, // Col C, started at x = 310
+            clientY: 36 + 32 + 16, // Row 1 (0 indexed)
+        });
+
+        expect(spy).not.toHaveBeenCalled();
+    });
+
+    test("Doesn't emit header click if mouseDown happened in a different cell", async () => {
+        const spy = jest.fn();
+
+        jest.useFakeTimers();
+        render(<DataEditor {...basicProps} onHeaderClicked={spy} />, {
+            wrapper: Context,
+        });
+        prep();
+
+        const canvas = screen.getByTestId("data-grid-canvas");
+        fireEvent.mouseDown(canvas, {
+            clientX: 300, // Col B, ends at x = 310
+            clientY: 16, // Header
+        });
+
+        fireEvent.mouseUp(canvas, {
+            clientX: 320, // Col C, started at x = 310
+            clientY: 16, // Header
+        });
+
+        expect(spy).not.toHaveBeenCalled();
+    });
+
     test("Uneven rows cell click", async () => {
         const spy = jest.fn();
 

diff --git a/packages/core/src/data-editor/data-editor.tsx b/packages/core/src/data-editor/data-editor.tsx
@@ -609,6 +609,8 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
 
     const lastSelectedRowRef = React.useRef<number>();
     const lastSelectedColRef = React.useRef<number>();
+
+    const lastMouseDownCellLocation = React.useRef<[number, number]>();
     const onMouseDown = React.useCallback(
         (args: GridMouseEventArgs) => {
             if (args.button !== 0) {
@@ -617,10 +619,16 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
             mouseState.current = {
                 previousSelection: gridSelection,
             };
+
+            lastMouseDownCellLocation.current = undefined;
+
             const isMultiKey = browserIsOSX.value ? args.metaKey : args.ctrlKey;
+            const [col, row] = args.location;
             if (args.kind === "cell") {
                 lastSelectedColRef.current = undefined;
-                const [col, row] = args.location;
+
+                lastMouseDownCellLocation.current = [col, row];
+
                 if (col === 0 && hasRowMarkers) {
                     if ((showTrailingBlankRow === true && row === rows) || rowMarkers === "number") return;
                     setGridSelection(undefined);
@@ -694,7 +702,7 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
                     }
                 }
             } else if (args.kind === "header") {
-                const [col] = args.location;
+                lastMouseDownCellLocation.current = [col, row];
                 setGridSelection(undefined);
                 setOverlay(undefined);
                 if (hasRowMarkers && col === 0) {
@@ -740,7 +748,7 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
                 lastSelectedRowRef.current = undefined;
                 lastSelectedColRef.current = undefined;
             } else if (args.kind === "group-header") {
-                const [col] = args.location;
+                lastMouseDownCellLocation.current = [col, row];
 
                 if (col < rowMarkerOffset) return;
 
@@ -816,16 +824,19 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
                 window.clearInterval(scrollTimer.current);
             }
 
+            const [lastMouseDownCol, lastMouseDownRow] = lastMouseDownCellLocation.current ?? [];
+            const [col, row] = args.location;
+
             if (args.kind === "header") {
-                if (args.button === 0) {
+                if (args.button === 0 && col === lastMouseDownCol && row === lastMouseDownRow) {
                     onHeaderClicked?.(args.location[0] - rowMarkerOffset, { ...args, preventDefault });
                 } else if (args.button === 2) {
                     onHeaderContextMenu?.(args.location[0] - rowMarkerOffset, { ...args, preventDefault });
                 }
             }
 
             if (args.kind === "group-header") {
-                if (args.button === 0) {
+                if (args.button === 0 && col === lastMouseDownCol && row === lastMouseDownRow) {
                     onGroupHeaderClicked?.(args.location[0] - rowMarkerOffset, { ...args, preventDefault });
                 } else if (args.button === 2) {
                     onGroupHeaderContextMenu?.(args.location[0] - rowMarkerOffset, { ...args, preventDefault });
@@ -836,9 +847,13 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
                 return;
             }
             if (args.button === 0) {
-                onCellClicked?.([args.location[0] - rowMarkerOffset, args.location[1]], { ...args, preventDefault });
+                if (lastMouseDownCol === col && lastMouseDownRow === row) {
+                    onCellClicked?.([col - rowMarkerOffset, row], {
+                        ...args,
+                        preventDefault,
+                    });
+                }
                 if (gridSelection !== undefined && mouse?.previousSelection?.cell !== undefined && !prevented) {
-                    const [col, row] = args.location;
                     const [selectedCol, selectedRow] = gridSelection.cell;
                     const [prevCol, prevRow] = mouse.previousSelection.cell;
                     const c = getMangedCellContent([col, row]);