From f064c2e6966eedb2ba91d64e0e5e671613f3499f Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 7 Oct 2024 15:44:51 -0700 Subject: [PATCH] Add a fix for focus traps being enterable while dragging --- .../datagrid/body/cell/focus_utils.tsx | 1 + .../header/data_grid_header_cell_wrapper.tsx | 8 ++--- .../body/header/draggable_columns.spec.tsx | 29 +++++++++++++++++++ .../body/header/draggable_columns.tsx | 11 +++---- .../components/datagrid/data_grid_types.ts | 1 + 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/packages/eui/src/components/datagrid/body/cell/focus_utils.tsx b/packages/eui/src/components/datagrid/body/cell/focus_utils.tsx index 1468435059d..b77c0cb0f48 100644 --- a/packages/eui/src/components/datagrid/body/cell/focus_utils.tsx +++ b/packages/eui/src/components/datagrid/body/cell/focus_utils.tsx @@ -146,6 +146,7 @@ export const FocusTrappedChildren: FunctionComponent< isCellEntered === false && isDOMNode(event.target) && isDOMNode(event.currentTarget) && + event.currentTarget !== event.target && event.currentTarget.contains(event.target) ) { return true; diff --git a/packages/eui/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx b/packages/eui/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx index 9f35270464b..4e982027bb2 100644 --- a/packages/eui/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx +++ b/packages/eui/src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx @@ -42,13 +42,13 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent< className, children, hasColumnActions, + isDragging, onKeyDown: _onKeyDown, 'aria-label': ariaLabel, ...rest }) => { const classes = classnames('euiDataGridHeaderCell', className); const styles = useEuiMemoizedStyles(euiDataGridHeaderCellWrapperStyles); - const cssStyles = [styles.euiDataGridHeaderCell]; // Must be a state and not a ref to trigger a HandleInteractiveChildren rerender const [headerEl, setHeaderEl] = useState(null); @@ -99,7 +99,7 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent< ref={setHeaderEl} tabIndex={isFocused ? 0 : -1} onKeyDown={onKeyDown} - css={cssStyles} + css={styles.euiDataGridHeaderCell} className={classes} data-test-subj={`dataGridHeaderCell-${id}`} data-gridcell-column-id={id} @@ -111,9 +111,9 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent< {...rest} > {typeof children === 'function' ? children(renderFocusTrap) : children} diff --git a/packages/eui/src/components/datagrid/body/header/draggable_columns.spec.tsx b/packages/eui/src/components/datagrid/body/header/draggable_columns.spec.tsx index 070aad9ba44..7b78578823a 100644 --- a/packages/eui/src/components/datagrid/body/header/draggable_columns.spec.tsx +++ b/packages/eui/src/components/datagrid/body/header/draggable_columns.spec.tsx @@ -191,6 +191,35 @@ describe('draggable columns', () => { .should('have.attr', 'data-test-subj', 'dataGridHeaderCell-b') .should('have.attr', 'tabindex', '0'); }); + + it('should not open focus traps when dragging', () => { + cy.realMount(); + + // Start drag + cy.get('[data-test-subj=dataGridHeaderCell-b]') + .focus() + .realPress('Space'); + + // Should not enter focus trap + cy.realPress('Enter'); + cy.focused().should( + 'have.attr', + 'data-test-subj', + 'dataGridHeaderCell-b' + ); + + // Cancel drag + cy.realPress('Escape'); + cy.focused().should( + 'have.attr', + 'data-test-subj', + 'dataGridHeaderCell-b' + ); + + // Should still be able to enter focus trap + cy.realPress('Enter'); + cy.focused().should('have.text', 'Second'); + }); }); describe('clicking a draggable cell', () => { diff --git a/packages/eui/src/components/datagrid/body/header/draggable_columns.tsx b/packages/eui/src/components/datagrid/body/header/draggable_columns.tsx index 2a557cc9e80..af380d7bd52 100644 --- a/packages/eui/src/components/datagrid/body/header/draggable_columns.tsx +++ b/packages/eui/src/components/datagrid/body/header/draggable_columns.tsx @@ -62,12 +62,12 @@ export const DroppableColumns: FunctionComponent< if (!source) return; if (destination && destination.index !== source.index) { - const sourceColumn = columns[source.index - indexOffset]; - const destinationColumn = columns[destination.index - indexOffset]; + const sourceColumn = columns[source.index - indexOffset]; + const destinationColumn = columns[destination.index - indexOffset]; - if (sourceColumn && destinationColumn) { - switchColumnPos(sourceColumn.id, destinationColumn.id); - } + if (sourceColumn && destinationColumn) { + switchColumnPos(sourceColumn.id, destinationColumn.id); + } } // Force focus the cell to prevent the datagrid body from become unfocusable, including on drag cancel setTimeout(() => { @@ -208,6 +208,7 @@ export const DraggableColumn: FunctionComponent<{ ...restDragHandleProps, css: reapplyCellStyles, 'data-column-moving': isDraggingRef.current || undefined, + isDragging, }; // since the cloned content is in a portal outside the datagrid diff --git a/packages/eui/src/components/datagrid/data_grid_types.ts b/packages/eui/src/components/datagrid/data_grid_types.ts index 9cff09ae61a..e32f1c67379 100644 --- a/packages/eui/src/components/datagrid/data_grid_types.ts +++ b/packages/eui/src/components/datagrid/data_grid_types.ts @@ -176,6 +176,7 @@ export interface EuiDataGridHeaderCellWrapperProps { className?: string; 'aria-label'?: AriaAttributes['aria-label']; hasColumnActions?: boolean; + isDragging?: boolean; onKeyDown?: KeyboardEventHandler; }