[EuiDataGrid] Fix column header move left focus bug (#7698)

elastic · May 8, 2024 · 8cd69bd · 8cd69bd
1 parent cba31a5
commit 8cd69bd
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 3 deletions.
diff --git a/changelogs/upcoming/7698.md b/changelogs/upcoming/7698.md
@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Fixed a focus bug with `EuiDataGrid`s with `leadingControlColumns` when moving columns to the left/right
diff --git a/src/components/datagrid/body/header/column_actions.test.tsx b/src/components/datagrid/body/header/column_actions.test.tsx
@@ -27,6 +27,7 @@ describe('getColumnActions', () => {
   const testArgs = {
     column: { id: 'B' },
     columns: [{ id: 'A' }, { id: 'B' }, { id: 'C' }],
+    columnFocusIndex: 1,
     schema: {},
     schemaDetectors,
     setVisibleColumns,
@@ -150,6 +151,8 @@ describe('getColumnActions', () => {
   });
 
   describe('column reordering', () => {
+    jest.useFakeTimers();
+
     describe('default enabled behavior', () => {
       const items = getColumnActions(testArgs);
       const moveLeft = items[1];
@@ -189,10 +192,12 @@ describe('getColumnActions', () => {
 
       it('calls switchColumnPos and updates the focused cell column index on click', () => {
         callActionOnClick(moveLeft);
+        jest.runAllTimers();
         expect(switchColumnPos).toHaveBeenCalledWith('B', 'A');
         expect(setFocusedCell).toHaveBeenLastCalledWith([0, -1]);
 
         callActionOnClick(moveRight);
+        jest.runAllTimers();
         expect(switchColumnPos).toHaveBeenCalledWith('B', 'C');
         expect(setFocusedCell).toHaveBeenLastCalledWith([2, -1]);
 
@@ -202,6 +207,27 @@ describe('getColumnActions', () => {
       });
     });
 
+    describe('focus behavior with leading control columns', () => {
+      const items = getColumnActions({
+        ...testArgs,
+        columnFocusIndex: testArgs.columnFocusIndex + 2, // "Add" 2 leading control columns
+      });
+      const moveLeft = items[1];
+      const moveRight = items[2];
+
+      it('correctly calls the focused cell x index accounting for leading control columns', () => {
+        callActionOnClick(moveLeft);
+        jest.runAllTimers();
+        expect(switchColumnPos).toHaveBeenCalledWith('B', 'A');
+        expect(setFocusedCell).toHaveBeenLastCalledWith([2, -1]);
+
+        callActionOnClick(moveRight);
+        jest.runAllTimers();
+        expect(switchColumnPos).toHaveBeenCalledWith('B', 'C');
+        expect(setFocusedCell).toHaveBeenLastCalledWith([4, -1]);
+      });
+    });
+
     describe('disabled behavior', () => {
       it('renders "Move left" as disabled if already on the first/left-most column', () => {
         const items = getColumnActions({

diff --git a/src/components/datagrid/body/header/column_actions.tsx b/src/components/datagrid/body/header/column_actions.tsx
@@ -34,6 +34,7 @@ interface GetColumnActions {
   sorting: EuiDataGridSorting | undefined;
   switchColumnPos: (colFromId: string, colToId: string) => void;
   setFocusedCell: DataGridFocusContextShape['setFocusedCell'];
+  columnFocusIndex: number; // Index including leadingControlColumns
 }
 
 export const getColumnActions = ({
@@ -47,6 +48,7 @@ export const getColumnActions = ({
   sorting,
   switchColumnPos,
   setFocusedCell,
+  columnFocusIndex,
 }: GetColumnActions): EuiListGroupItemProps[] => {
   if (column.actions === false) {
     return [];
@@ -70,6 +72,7 @@ export const getColumnActions = ({
       columns,
       switchColumnPos,
       setFocusedCell,
+      columnFocusIndex,
     }),
     ...(column.actions?.additional || []),
   ];
@@ -136,25 +139,39 @@ export const getHideColumnAction = ({
  */
 type MoveColumnActions = Pick<
   GetColumnActions,
-  'column' | 'columns' | 'switchColumnPos' | 'setFocusedCell'
+  | 'column'
+  | 'columns'
+  | 'switchColumnPos'
+  | 'setFocusedCell'
+  | 'columnFocusIndex'
 >;
 
 const getMoveColumnActions = ({
   column,
   columns,
   switchColumnPos,
   setFocusedCell,
+  columnFocusIndex,
 }: MoveColumnActions): EuiListGroupItemProps[] => {
   const items = [];
 
   const colIdx = columns.findIndex((col) => col.id === column.id);
 
+  const moveFocus = (direction: 'left' | 'right') => {
+    const newIndex = direction === 'left' ? -1 : 1;
+    // Wait a beat to move focus, otherwise the EuiPopover's EuiFocusTrap's
+    // returnFocus logic sometimes steals it (depending on rerenders)
+    setTimeout(() => {
+      setFocusedCell([columnFocusIndex + newIndex, -1]); // -1 is the static y-index of the header
+    });
+  };
+
   if (isColumnActionEnabled('showMoveLeft', column.actions)) {
     const onClickMoveLeft = () => {
       const targetCol = columns[colIdx - 1];
       if (targetCol) {
         switchColumnPos(column.id, targetCol.id);
-        setFocusedCell([colIdx - 1, -1]);
+        moveFocus('left');
       }
     };
     const action = {
@@ -174,7 +191,7 @@ const getMoveColumnActions = ({
       const targetCol = columns[colIdx + 1];
       if (targetCol) {
         switchColumnPos(column.id, targetCol.id);
-        setFocusedCell([colIdx + 1, -1]);
+        moveFocus('right');
       }
     };
     const action = {

diff --git a/src/components/datagrid/body/header/data_grid_header_cell.tsx b/src/components/datagrid/body/header/data_grid_header_cell.tsx
@@ -87,6 +87,7 @@ export const EuiDataGridHeaderCell: FunctionComponent<EuiDataGridHeaderCellProps
           sorting,
           switchColumnPos,
           setFocusedCell,
+          columnFocusIndex: index,
         });
       }, [
         column,
@@ -99,6 +100,7 @@ export const EuiDataGridHeaderCell: FunctionComponent<EuiDataGridHeaderCellProps
         sorting,
         switchColumnPos,
         setFocusedCell,
+        index,
       ]);
 
       const showColumnActions = columnActions && columnActions.length > 0;