Rework outside click detection (adazzle#3794)

nstepien · web-flow · commit 97c49160b06f · 2025-06-09T08:51:06.000-05:00
diff --git a/src/EditCell.tsx b/src/EditCell.tsx
@@ -1,4 +1,4 @@
-import { useEffect, useRef } from 'react';
+import { useLayoutEffect, useRef } from 'react';
 import { css } from '@linaria/core';
 
 import { useLatestFunc } from './hooks';
@@ -12,6 +12,21 @@ import type {
   RenderEditCellProps
 } from './types';
 
+declare global {
+  const scheduler: Scheduler | undefined;
+}
+
+interface Scheduler {
+  readonly postTask?: (
+    callback: () => void,
+    options?: {
+      priority?: 'user-blocking' | 'user-visible' | 'background';
+      signal?: AbortSignal;
+      delay?: number;
+    }
+  ) => Promise<unknown>;
+}
+
 /*
  * To check for outside `mousedown` events, we listen to all `mousedown` events at their birth,
  * i.e. on the window during the capture phase, and at their death, i.e. on the window during the bubble phase.
@@ -21,13 +36,15 @@ import type {
  *
  * The event can be `stopPropagation()`ed halfway through, so they may not always bubble back up to the window,
  * so an alternative check must be used. The check must happen after the event can reach the "inside" container,
- * and not before it run to completion. `requestAnimationFrame` is the best way we know how to achieve this.
+ * and not before it run to completion. `postTask`/`requestAnimationFrame` are the best way we know to achieve this.
  * Usually we want click event handlers from parent components to access the latest commited values,
  * so `mousedown` is used instead of `click`.
  *
  * We must also rely on React's event capturing/bubbling to handle elements rendered in a portal.
  */
 
+const canUsePostTask = typeof scheduler === 'object' && typeof scheduler.postTask === 'function';
+
 const cellEditing = css`
   @layer rdg.EditCell {
     padding: 0;
@@ -56,33 +73,68 @@ export default function EditCell<R, SR>({
   onKeyDown,
   navigate
 }: EditCellProps<R, SR>) {
+  const captureEventRef = useRef<MouseEvent | undefined>(undefined);
+  const abortControllerRef = useRef<AbortController>(undefined);
   const frameRequestRef = useRef<number>(undefined);
   const commitOnOutsideClick = column.editorOptions?.commitOnOutsideClick ?? true;
 
-  // We need to prevent the `useEffect` from cleaning up between re-renders,
+  // We need to prevent the `useLayoutEffect` from cleaning up between re-renders,
   // as `onWindowCaptureMouseDown` might otherwise miss valid mousedown events.
   // To that end we instead access the latest props via useLatestFunc.
   const commitOnOutsideMouseDown = useLatestFunc(() => {
     onClose(true, false);
   });
 
-  useEffect(() => {
+  useLayoutEffect(() => {
     if (!commitOnOutsideClick) return;
 
-    function onWindowCaptureMouseDown() {
-      frameRequestRef.current = requestAnimationFrame(commitOnOutsideMouseDown);
+    function onWindowCaptureMouseDown(event: MouseEvent) {
+      captureEventRef.current = event;
+
+      if (canUsePostTask) {
+        const abortController = new AbortController();
+        const { signal } = abortController;
+        abortControllerRef.current = abortController;
+        // Use postTask to ensure that the event is not called in the middle of a React render
+        // and that it is called before the next paint.
+        scheduler
+          .postTask(commitOnOutsideMouseDown, {
+            priority: 'user-blocking',
+            signal
+          })
+          // ignore abort errors
+          .catch(() => {});
+      } else {
+        frameRequestRef.current = requestAnimationFrame(commitOnOutsideMouseDown);
+      }
+    }
+
+    function onWindowMouseDown(event: MouseEvent) {
+      if (captureEventRef.current === event) {
+        commitOnOutsideMouseDown();
+      }
     }
 
     addEventListener('mousedown', onWindowCaptureMouseDown, { capture: true });
+    addEventListener('mousedown', onWindowMouseDown);
 
     return () => {
       removeEventListener('mousedown', onWindowCaptureMouseDown, { capture: true });
-      cancelFrameRequest();
+      removeEventListener('mousedown', onWindowMouseDown);
+      cancelTask();
     };
   }, [commitOnOutsideClick, commitOnOutsideMouseDown]);
 
-  function cancelFrameRequest() {
-    cancelAnimationFrame(frameRequestRef.current!);
+  function cancelTask() {
+    captureEventRef.current = undefined;
+    if (abortControllerRef.current !== undefined) {
+      abortControllerRef.current.abort();
+      abortControllerRef.current = undefined;
+    }
+    if (frameRequestRef.current !== undefined) {
+      cancelAnimationFrame(frameRequestRef.current);
+      frameRequestRef.current = undefined;
+    }
   }
 
   function handleKeyDown(event: React.KeyboardEvent<HTMLDivElement>) {
@@ -143,7 +195,7 @@ export default function EditCell<R, SR>({
       className={className}
       style={getCellStyle(column, colSpan)}
       onKeyDown={handleKeyDown}
-      onMouseDownCapture={cancelFrameRequest}
+      onMouseDownCapture={cancelTask}
     >
       {column.renderEditCell != null && (
         <>
diff --git a/src/hooks/useLatestFunc.ts b/src/hooks/useLatestFunc.ts
@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useRef } from 'react';
+import { useCallback, useLayoutEffect, useRef } from 'react';
 
 import type { Maybe } from '../types';
 
@@ -7,7 +7,7 @@ import type { Maybe } from '../types';
 export function useLatestFunc<T extends Maybe<(...args: any[]) => any>>(fn: T): T {
   const ref = useRef(fn);
 
-  useEffect(() => {
+  useLayoutEffect(() => {
     ref.current = fn;
   });
 
diff --git a/test/browser/rowSelection.test.tsx b/test/browser/rowSelection.test.tsx
@@ -44,7 +44,7 @@ function RowSelectionTest({
   );
 }
 
-function setupNew(initialRows = defaultRows) {
+function setup(initialRows = defaultRows) {
   page.render(<RowSelectionTest initialRows={initialRows} />);
 }
 
@@ -60,7 +60,7 @@ async function toggleSelection(rowIdx: number, shift = false) {
 }
 
 test('toggle selection when checkbox is clicked', async () => {
-  setupNew();
+  setup();
   await toggleSelection(0);
   testSelection(0, true);
   await toggleSelection(1);
@@ -73,7 +73,7 @@ test('toggle selection when checkbox is clicked', async () => {
 });
 
 test('toggle selection using keyboard', async () => {
-  setupNew();
+  setup();
   testSelection(0, false);
   await userEvent.click(getCellsAtRowIndex(0)[0]);
   testSelection(0, true);
@@ -88,7 +88,7 @@ test('toggle selection using keyboard', async () => {
 });
 
 test('should partially select header checkbox', async () => {
-  setupNew();
+  setup();
   const headerCheckbox = page.getByRole('checkbox', { name: 'Select All' }).element();
   expect(headerCheckbox).not.toBeChecked();
   expect(headerCheckbox).not.toBePartiallyChecked();
@@ -140,7 +140,7 @@ test('should not select row when isRowSelectionDisabled returns true', async ()
 });
 
 test('select/deselect all rows when header checkbox is clicked', async () => {
-  setupNew();
+  setup();
   const headerCheckbox = page.getByRole('checkbox', { name: 'Select All' }).element();
   expect(headerCheckbox).not.toBeChecked();
   await userEvent.click(headerCheckbox);
@@ -161,7 +161,7 @@ test('select/deselect all rows when header checkbox is clicked', async () => {
 });
 
 test('header checkbox is not checked when there are no rows', async () => {
-  setupNew([]);
+  setup([]);
   await expect.element(page.getByRole('checkbox', { name: 'Select All' })).not.toBeChecked();
 });
 
@@ -238,7 +238,7 @@ test('extra keys are preserved when updating the selectedRows Set', async () =>
 });
 
 test('select/deselect rows using shift click', async () => {
-  setupNew();
+  setup();
   await toggleSelection(0);
   await toggleSelection(2, true);
   testSelection(0, true);
@@ -252,7 +252,7 @@ test('select/deselect rows using shift click', async () => {
 });
 
 test('select rows using shift space', async () => {
-  setupNew();
+  setup();
   await userEvent.click(getCellsAtRowIndex(0)[1]);
   await userEvent.keyboard('{Shift>} {/Shift}');
   testSelection(0, true);
diff --git a/tsconfig.js.json b/tsconfig.js.json
@@ -4,7 +4,6 @@
     "allowJs": true,
     "module": "NodeNext",
     "moduleResolution": "NodeNext",
-    "resolveJsonModule": true,
     "skipLibCheck": true
   },
   "include": ["**/*.js", ".github/**/*.js", "package.json"],
