diff --git a/changelogs/upcoming/7698.md b/changelogs/upcoming/7698.md new file mode 100644 index 00000000000..de8dbcb613c --- /dev/null +++ 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 index 824060b885b..4f659ef275d 100644 --- 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 index 3f594ac0703..cd702168fe5 100644 --- 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,7 +139,11 @@ export const getHideColumnAction = ({ */ type MoveColumnActions = Pick< GetColumnActions, - 'column' | 'columns' | 'switchColumnPos' | 'setFocusedCell' + | 'column' + | 'columns' + | 'switchColumnPos' + | 'setFocusedCell' + | 'columnFocusIndex' >; const getMoveColumnActions = ({ @@ -144,17 +151,27 @@ const getMoveColumnActions = ({ 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 index af1d7a9b953..9aeef041114 100644 --- 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 0;