diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4aed83df853..4439faa251f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
**Bug fixes**
- Fixed logo icons with static SVG IDs causing accessibility errors when multiples of the same logo were present ([#5204](https://github.com/elastic/eui/pull/5204))
+- Fixed several `EuiDataGrid` console errors that occur on column drag/drop reorder ([#5209](https://github.com/elastic/eui/pull/5209))
**Reverts**
diff --git a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx
index 4e5a54bb515..e68327abd22 100644
--- a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx
+++ b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.test.tsx
@@ -20,7 +20,7 @@ describe('EuiDataGridHeaderCellWrapper', () => {
id: 'someColumn',
index: 0,
headerIsInteractive: true,
- children: ,
+ children: ,
};
const focusContext = {
@@ -57,9 +57,7 @@ describe('EuiDataGridHeaderCellWrapper', () => {
style={Object {}}
tabIndex={-1}
>
-
+
`);
@@ -85,51 +83,56 @@ describe('EuiDataGridHeaderCellWrapper', () => {
}
tabIndex={-1}
>
-
+
`);
});
describe('focus behavior', () => {
- it('warns when a header cell contains multiple interactive children', () => {
- const consoleWarnSpy = jest
- .spyOn(console, 'warn')
- .mockImplementationOnce(() => {}); // Silence expected warning
-
- mountWithContext({
- children: (
-
-
-
-
-
- ),
+ // Reusable assertions for DRYness
+ const expectCellFocused = (headerCell: Element) => {
+ expect(document.activeElement).toEqual(headerCell);
+ expect(headerCell.getAttribute('tabIndex')).toEqual('0');
+ };
+ const expectCellChildrenFocused = (headerCell: Element) => {
+ expect(document.activeElement).toEqual(
+ headerCell.querySelector('[data-euigrid-tab-managed]')
+ );
+ expect(headerCell.getAttribute('tabIndex')).toEqual('-1');
+ };
+ const expectCellNotFocused = (headerCell: Element) => {
+ expect(document.activeElement).toBeInstanceOf(HTMLBodyElement);
+ expect(headerCell.getAttribute('tabIndex')).toEqual('-1');
+ };
+
+ describe('isFocused context', () => {
+ describe('when true', () => {
+ it('focuses the interactive cell children when present', () => {
+ const isFocused = true;
+ const headerCell = mountWithContext({}, isFocused).getDOMNode();
+ expectCellChildrenFocused(headerCell);
+ });
+
+ it('focuses the cell when no interactive children are present', () => {
+ const isFocused = true;
+ const headerCell = mountWithContext(
+ { children: },
+ isFocused
+ ).getDOMNode();
+ expectCellFocused(headerCell);
+ });
});
- expect(consoleWarnSpy).toHaveBeenCalledWith(
- 'EuiDataGridHeaderCell expects at most 1 tabbable element, 3 found instead'
- );
+ describe('when false', () => {
+ it('sets isCellEntered to false and disables interactives', () => {
+ const isFocused = false;
+ const headerCell = mountWithContext({}, isFocused).getDOMNode();
+ expectCellNotFocused(headerCell);
+ });
+ });
});
describe('events', () => {
- // Reusable assertions for DRYness
- const expectCellFocused = (headerCell: Element) => {
- expect(document.activeElement).toEqual(headerCell);
- expect(headerCell.getAttribute('tabIndex')).toEqual('0');
- };
- const expectCellChildrenFocused = (headerCell: Element) => {
- expect(document.activeElement).toEqual(
- headerCell.querySelector('[data-euigrid-tab-managed]')
- );
- expect(headerCell.getAttribute('tabIndex')).toEqual('-1');
- };
- const expectCellNotFocused = (headerCell: Element) => {
- expect(document.activeElement).toBeInstanceOf(HTMLBodyElement);
- expect(headerCell.getAttribute('tabIndex')).toEqual('-1');
- };
-
describe('keyup', () => {
test('enter key sets isCellEntered to true (which focuses the first interactive child in the header cell)', () => {
const headerCell = mountWithContext().getDOMNode();
@@ -150,22 +153,6 @@ describe('EuiDataGridHeaderCellWrapper', () => {
});
expectCellFocused(headerCell);
});
-
- test('F2 key toggles between cell child focus and cell focus', () => {
- const headerCell = mountWithContext().getDOMNode();
- act(() => {
- headerCell.dispatchEvent(
- new KeyboardEvent('keyup', { key: keys.F2 })
- );
- });
- expectCellFocused(headerCell);
- act(() => {
- headerCell.dispatchEvent(
- new KeyboardEvent('keyup', { key: keys.F2 })
- );
- });
- expectCellChildrenFocused(headerCell);
- });
});
describe('focus', () => {
@@ -179,44 +166,36 @@ describe('EuiDataGridHeaderCellWrapper', () => {
jest.restoreAllMocks();
});
- describe('when header is not interactive', () => {
- it('does not focus in on the cell', () => {
- const headerCell = mountWithContext(
- { headerIsInteractive: false },
- false
- ).getDOMNode();
- act(() => {
- headerCell.dispatchEvent(new FocusEvent('focusin'));
+ describe('focusin', () => {
+ describe('when header is not interactive', () => {
+ it('does not focus in on the cell', () => {
+ const headerCell = mountWithContext(
+ { headerIsInteractive: false },
+ false
+ ).getDOMNode();
+ act(() => {
+ headerCell.dispatchEvent(new FocusEvent('focusin'));
+ });
+ expectCellNotFocused(headerCell);
});
- expectCellNotFocused(headerCell);
- });
- });
-
- describe('when header is interactive', () => {
- it('calls setFocusedCell when not already focused', () => {
- const headerCell = mountWithContext({}, false).getDOMNode();
- act(() => {
- headerCell.dispatchEvent(new FocusEvent('focusin'));
- });
- expect(focusContext.setFocusedCell).toHaveBeenCalled();
});
- it('focuses the interactive cell children when present', () => {
- const headerCell = mountWithContext().getDOMNode();
- act(() => {
- headerCell.dispatchEvent(new FocusEvent('focusin'));
+ describe('when header is interactive', () => {
+ it('calls setFocusedCell when not already focused', () => {
+ const headerCell = mountWithContext({}, false).getDOMNode();
+ act(() => {
+ headerCell.dispatchEvent(new FocusEvent('focusin'));
+ });
+ expect(focusContext.setFocusedCell).toHaveBeenCalled();
});
- expectCellChildrenFocused(headerCell);
- });
- it('focuses the cell when no interactive children are present', () => {
- const headerCell = mountWithContext({
- children: ,
- }).getDOMNode();
- act(() => {
- headerCell.dispatchEvent(new FocusEvent('focusin'));
+ it('re-enables and focuses cell interactives when already focused', () => {
+ const headerCell = mountWithContext({}, true).getDOMNode();
+ act(() => {
+ headerCell.dispatchEvent(new FocusEvent('focusin'));
+ });
+ expectCellChildrenFocused(headerCell);
});
- expectCellFocused(headerCell);
});
});
@@ -251,5 +230,43 @@ describe('EuiDataGridHeaderCellWrapper', () => {
});
});
});
+
+ describe('multiple interactive children', () => {
+ const children = (
+
+
+
+
+
+ );
+
+ let consoleWarnSpy: jest.SpyInstance;
+
+ beforeEach(() => {
+ consoleWarnSpy = jest
+ .spyOn(console, 'warn')
+ .mockImplementation(() => {}); // Silence expected warning
+ });
+ afterEach(() => {
+ consoleWarnSpy.mockRestore();
+ });
+
+ it('emits a console warning', () => {
+ mountWithContext({ children });
+ expect(consoleWarnSpy).toHaveBeenCalledWith(
+ 'EuiDataGridHeaderCell expects at most 1 tabbable element, 3 found instead'
+ );
+ });
+
+ it('will still focus in to the first interactable element on cell enter', () => {
+ const headerCell = mountWithContext({ children }).getDOMNode();
+ act(() => {
+ headerCell.dispatchEvent(
+ new KeyboardEvent('keyup', { key: keys.ENTER })
+ );
+ });
+ expectCellChildrenFocused(headerCell);
+ });
+ });
});
});
diff --git a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx
index 92ab59cf53e..9ba11f7edb4 100644
--- a/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx
+++ b/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx
@@ -13,7 +13,6 @@ import React, {
useEffect,
useRef,
useState,
- useCallback,
} from 'react';
import tabbable from 'tabbable';
import { keys } from '../../../../services';
@@ -47,52 +46,15 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent(null);
const [isCellEntered, setIsCellEntered] = useState(false);
- const focusInteractives = useCallback((headerNode: Element) => {
- const tabbables = tabbable(headerNode);
- if (tabbables.length === 1) {
- tabbables[0].focus();
- setIsCellEntered(true);
- }
- }, []);
-
- const enableInteractives = useCallback((headerNode: Element) => {
- const interactiveElements = headerNode.querySelectorAll(
- '[data-euigrid-tab-managed]'
- );
- for (let i = 0; i < interactiveElements.length; i++) {
- interactiveElements[i].setAttribute('tabIndex', '0');
- }
- }, []);
-
- const disableInteractives = useCallback((headerNode: Element) => {
- const tababbles = tabbable(headerNode);
- if (tababbles.length > 1) {
- console.warn(
- `EuiDataGridHeaderCell expects at most 1 tabbable element, ${tababbles.length} found instead`
- );
- }
- for (let i = 0; i < tababbles.length; i++) {
- const element = tababbles[i];
- element.setAttribute('data-euigrid-tab-managed', 'true');
- element.setAttribute('tabIndex', '-1');
- }
- }, []);
-
useEffect(() => {
const headerNode = headerRef.current!;
if (isCellEntered) {
- enableInteractives(headerNode);
- focusInteractives(headerNode);
+ enableAndFocusInteractives(headerNode);
} else {
disableInteractives(headerNode);
}
- }, [
- disableInteractives,
- enableInteractives,
- focusInteractives,
- isCellEntered,
- ]);
+ }, [isCellEntered]);
useEffect(() => {
const headerNode = headerRef.current!;
@@ -111,7 +73,7 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent {
if (!headerIsInteractive) {
// header is not interactive, avoid focusing
requestAnimationFrame(() => headerNode.blur());
@@ -122,25 +84,23 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent {
// wait for the next element to receive focus, then update interactives' state
requestAnimationFrame(() => {
if (!headerNode.contains(document.activeElement)) {
setIsCellEntered(false);
}
});
- }
+ };
- function onKeyUp(event: KeyboardEvent) {
+ const onKeyUp = (event: KeyboardEvent) => {
switch (event.key) {
case keys.ENTER: {
event.preventDefault();
@@ -154,20 +114,8 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent
);
};
+
+/**
+ * Utility fns for managing child interactive tabIndex state
+ */
+
+const disableInteractives = (headerNode: Element) => {
+ const tabbables = tabbable(headerNode);
+ if (tabbables.length > 1) {
+ console.warn(
+ `EuiDataGridHeaderCell expects at most 1 tabbable element, ${tabbables.length} found instead`
+ );
+ }
+ tabbables.forEach((element) => {
+ element.setAttribute('data-euigrid-tab-managed', 'true');
+ element.setAttribute('tabIndex', '-1');
+ });
+};
+
+const enableAndFocusInteractives = (headerNode: Element) => {
+ const interactiveElements = headerNode.querySelectorAll(
+ '[data-euigrid-tab-managed]'
+ );
+ interactiveElements.forEach((element, i) => {
+ element.setAttribute('tabIndex', '0');
+ if (i === 0) {
+ (element as HTMLElement).focus();
+ }
+ });
+};
diff --git a/src/components/datagrid/column_selector.tsx b/src/components/datagrid/column_selector.tsx
index d3210fdd23e..f87f152d130 100644
--- a/src/components/datagrid/column_selector.tsx
+++ b/src/components/datagrid/column_selector.tsx
@@ -94,13 +94,15 @@ export const useDataGridColumnSelector = (
source: { index: sourceIndex },
destination,
}: DropResult) {
- const destinationIndex = destination!.index;
- const nextSortedColumns = euiDragDropReorder(
- sortedColumns,
- sourceIndex,
- destinationIndex
- );
- setColumns(nextSortedColumns);
+ if (destination) {
+ const destinationIndex = destination.index;
+ const nextSortedColumns = euiDragDropReorder(
+ sortedColumns,
+ sourceIndex,
+ destinationIndex
+ );
+ setColumns(nextSortedColumns);
+ }
}
const numberOfHiddenFields = availableColumns.length - visibleColumns.length;