[EuiDataGrid] Fix scrolling on focus return (#9453)

Co-authored-by: Weronika Olejniczak <weronika.olejniczak@elastic.co>

Author: tkajtoch

packages/eui/changelogs/upcoming/9453.md

@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Fixed `EuiDataGrid` scroll bouncing back to the focused element in certain cases

packages/eui/src/components/datagrid/utils/scrolling.tsx

@@ -11,12 +11,14 @@ import React, {
   useEffect,
   useCallback,
   useMemo,
+  useRef,
   MutableRefObject,
   ReactNode,
 } from 'react';
 import { VariableSizeGrid as Grid } from 'react-window';
 
-import { useEuiMemoizedStyles, useIsPointerDown } from '../../../services';
+import { useEuiMemoizedStyles } from '../../../services';
+import { useIsPointerDown } from '../../../services/hooks';
 import { logicalStyles } from '../../../global_styling';
 import { DataGridCellPopoverContext } from '../body/cell';
 import { EuiDataGridStyle } from '../data_grid_types';
@@ -50,21 +52,48 @@ export const useScroll = (args: Dependencies) => {
   const { scrollCellIntoView } = useScrollCellIntoView(args);
 
   const { focusedCell } = useContext(DataGridFocusContext);
-  const isPointerDown = useIsPointerDown(args.outerGridRef);
+  const isPointerDownRef = useIsPointerDown(args.outerGridRef);
+
+  /**
+   * Set when `focusedCell` changes while the pointer is held down (e.g. clicking a cell).
+   * Allows the `pointerup` listener below to scroll on release without
+   * causing snap-back when the user scrolls the grid without changing focus.
+   */
+  const pendingScrollRef = useRef(false);
 
   useEffect(() => {
-    if (focusedCell) {
-      // do not scroll if text is being selected
-      if (isPointerDown || window?.getSelection()?.type === 'Range') {
-        return;
-      }
+    if (!focusedCell) return;
+    if (isPointerDownRef.current) {
+      // Pointer is down - defer scroll decision to the pointerup listener
+      pendingScrollRef.current = true;
+      return;
+    }
+
+    scrollCellIntoView({ rowIndex: focusedCell[1], colIndex: focusedCell[0] });
+  }, [focusedCell, scrollCellIntoView, isPointerDownRef]);
+
+  useEffect(() => {
+    const handlePointerUp = () => {
+      if (!pendingScrollRef.current || !focusedCell) return;
+
+      pendingScrollRef.current = false;
+
+      // Skip if the interaction resulted in text being selected
+      if (window?.getSelection()?.type === 'Range') return;
 
       scrollCellIntoView({
         rowIndex: focusedCell[1],
         colIndex: focusedCell[0],
       });
-    }
-  }, [focusedCell, isPointerDown, scrollCellIntoView]);
+    };
+
+    document.addEventListener('pointerup', handlePointerUp, { capture: true });
+
+    return () =>
+      document.removeEventListener('pointerup', handlePointerUp, {
+        capture: true,
+      });
+  }, [focusedCell, scrollCellIntoView]);
 
   const { popoverIsOpen, cellLocation } = useContext(
     DataGridCellPopoverContext

packages/eui/src/services/hooks/index.ts

@@ -6,14 +6,14 @@
  * Side Public License, v 1.
  */
 
-export * from './useDependentState';
-export * from './useCombinedRefs';
-export * from './useForceRender';
-export * from './useLatest';
-export * from './useDeepEqual';
-export * from './useMouseMove';
-export * from './useIsPointerDown';
-export * from './useUpdateEffect';
+export { useDependentState } from './useDependentState';
+export { useCombinedRefs, setMultipleRefs } from './useCombinedRefs';
+export { useForceRender } from './useForceRender';
+export { useLatest } from './useLatest';
+export { useDeepEqual } from './useDeepEqual';
+export { isMouseEvent, useMouseMove } from './useMouseMove';
+export { useIsPointerDown } from './useIsPointerDown';
+export { useUpdateEffect } from './useUpdateEffect';
 export {
   type EuiDisabledProps,
   useEuiDisabledElement,

packages/eui/src/services/hooks/useIsPointerDown.test.tsx

@@ -6,10 +6,10 @@
  * Side Public License, v 1.
  */
 
-import React, { useRef } from 'react';
+import React, { type MutableRefObject, useRef } from 'react';
 import { act } from '@testing-library/react';
 
-import { render } from '../../test/rtl';
+import { render, renderHook, renderHookAct } from '../../test/rtl';
 
 import { useIsPointerDown } from './useIsPointerDown';
 
@@ -26,39 +26,36 @@ global.PointerEvent = MockPointerEvent;
 describe('useIsPointerDown', () => {
   describe('without container', () => {
     it('returns true when pointer is down and false when pointer is up', () => {
-      let isPointerDown: boolean;
+      const {
+        result: { current: ref },
+      } = renderHook(useIsPointerDown);
 
-      const TestComponent = () => {
-        isPointerDown = useIsPointerDown();
-        return null;
-      };
-
-      render(<TestComponent />);
-      expect(isPointerDown!).toBe(false);
+      expect(ref.current).toBe(false);
 
-      act(() => {
+      renderHookAct(() => {
         document.dispatchEvent(
           new PointerEvent('pointerdown', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(true);
 
-      act(() => {
+      expect(ref.current).toBe(true);
+
+      renderHookAct(() => {
         document.dispatchEvent(
           new PointerEvent('pointerup', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(false);
+      expect(ref.current).toBe(false);
     });
   });
 
   describe('with container', () => {
     it('returns true when pointer is down inside the container', () => {
-      let isPointerDown: boolean;
+      let isPointerDownRef: MutableRefObject<boolean> = { current: false };
 
       const TestComponent = () => {
         const containerRef = useRef<HTMLDivElement | null>(null);
-        isPointerDown = useIsPointerDown(containerRef);
+        isPointerDownRef = useIsPointerDown(containerRef);
         return (
           <div>
             <div data-test-subj="outside" />
@@ -68,30 +65,30 @@ describe('useIsPointerDown', () => {
       };
 
       const { getByTestSubject } = render(<TestComponent />);
-      expect(isPointerDown!).toBe(false);
+      expect(isPointerDownRef.current).toBe(false);
 
       act(() => {
         const container = getByTestSubject('container');
         container.dispatchEvent(
           new PointerEvent('pointerdown', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(true);
+      expect(isPointerDownRef.current).toBe(true);
 
       act(() => {
         document.dispatchEvent(
           new PointerEvent('pointerup', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(false);
+      expect(isPointerDownRef.current).toBe(false);
     });
 
     it('returns false when pointer is down outside the container', () => {
-      let isPointerDown: boolean;
+      let isPointerDownRef: MutableRefObject<boolean> = { current: false };
 
       const TestComponent = () => {
         const containerRef = useRef<HTMLDivElement | null>(null);
-        isPointerDown = useIsPointerDown(containerRef);
+        isPointerDownRef = useIsPointerDown(containerRef);
         return (
           <div>
             <div data-test-subj="outside" />
@@ -101,69 +98,59 @@ describe('useIsPointerDown', () => {
       };
 
       const { getByTestSubject } = render(<TestComponent />);
-      expect(isPointerDown!).toBe(false);
+      expect(isPointerDownRef.current).toBe(false);
 
       act(() => {
         const outside = getByTestSubject('outside');
         outside.dispatchEvent(
           new PointerEvent('pointerdown', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(false);
+      expect(isPointerDownRef.current).toBe(false);
     });
   });
 
   describe('pointercancel and visibilitychange events', () => {
     it('resets to false on pointercancel', () => {
-      let isPointerDown: boolean;
-
-      const TestComponent = () => {
-        isPointerDown = useIsPointerDown();
-        return null;
-      };
+      const {
+        result: { current: ref },
+      } = renderHook(useIsPointerDown);
 
-      render(<TestComponent />);
-
-      act(() => {
+      renderHookAct(() => {
         document.dispatchEvent(
           new PointerEvent('pointerdown', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(true);
+      expect(ref.current).toBe(true);
 
-      act(() => {
+      renderHookAct(() => {
         document.dispatchEvent(
           new PointerEvent('pointercancel', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(false);
+      expect(ref.current).toBe(false);
     });
 
     it('resets to false when document becomes hidden', () => {
-      let isPointerDown: boolean;
+      const {
+        result: { current: ref },
+      } = renderHook(useIsPointerDown);
 
-      const TestComponent = () => {
-        isPointerDown = useIsPointerDown();
-        return null;
-      };
-
-      render(<TestComponent />);
-
-      act(() => {
+      renderHookAct(() => {
         document.dispatchEvent(
           new PointerEvent('pointerdown', { bubbles: true })
         );
       });
-      expect(isPointerDown!).toBe(true);
+      expect(ref.current).toBe(true);
 
-      act(() => {
+      renderHookAct(() => {
         Object.defineProperty(document, 'visibilityState', {
           value: 'hidden',
           configurable: true,
         });
         document.dispatchEvent(new Event('visibilitychange'));
       });
-      expect(isPointerDown!).toBe(false);
+      expect(ref.current).toBe(false);
 
       // reset
       Object.defineProperty(document, 'visibilityState', {

packages/eui/src/services/hooks/useIsPointerDown.ts

@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import { useState, useEffect, MutableRefObject } from 'react';
+import { useRef, useEffect, type MutableRefObject } from 'react';
 
 /**
  * A hook that tracks whether the pointer is currently down/pressed.
@@ -15,7 +15,7 @@ import { useState, useEffect, MutableRefObject } from 'react';
 export function useIsPointerDown(
   container?: MutableRefObject<HTMLElement | null>
 ) {
-  const [isPointerDown, setIsPointerDown] = useState(false);
+  const isPointerDownRef = useRef(false);
 
   useEffect(() => {
     const handlePointerDown = (event: PointerEvent) => {
@@ -25,16 +25,16 @@ export function useIsPointerDown(
       ) {
         return;
       }
-      setIsPointerDown(true);
+      isPointerDownRef.current = true;
     };
 
     const handlePointerUpOrCancel = () => {
-      setIsPointerDown(false);
+      isPointerDownRef.current = false;
     };
 
     const handleVisibilityChange = () => {
       if (document.visibilityState === 'hidden') {
-        setIsPointerDown(false);
+        isPointerDownRef.current = false;
       }
     };
 
@@ -57,5 +57,5 @@ export function useIsPointerDown(
     };
   }, [container]);
 
-  return isPointerDown;
+  return isPointerDownRef;
 }

packages/eui/src/services/index.ts

@@ -85,7 +85,19 @@ export {
   formatNumber,
   formatText,
 } from './format';
-export * from './hooks';
+export {
+  useDependentState,
+  useCombinedRefs,
+  setMultipleRefs,
+  useForceRender,
+  useLatest,
+  useDeepEqual,
+  isMouseEvent,
+  useMouseMove,
+  useUpdateEffect,
+  useEuiDisabledElement,
+  type EuiDisabledProps,
+} from './hooks';
 export { isEvenlyDivisibleBy, isWithinRange } from './number';
 export { Pager } from './paging';
 export { calculatePopoverPosition, findPopoverPosition } from './popover';