Skip to content

Commit

Permalink
Add a fix for focus traps being enterable while dragging
Browse files Browse the repository at this point in the history
  • Loading branch information
cee-chen committed Oct 7, 2024
1 parent 85ec539 commit f064c2e
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLDivElement | null>(null);
Expand Down Expand Up @@ -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}
Expand All @@ -111,9 +111,9 @@ export const EuiDataGridHeaderCellWrapper: FunctionComponent<
{...rest}
>
<HandleInteractiveChildren
cellEl={headerEl}
cellEl={isDragging ? null : headerEl}
renderFocusTrap={isDragging ? false : renderFocusTrap}
updateCellFocusContext={updateCellFocusContext}
renderFocusTrap={renderFocusTrap}
onInteractiveChildrenFound={setInteractiveChildren}
>
{typeof children === 'function' ? children(renderFocusTrap) : children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(<StatefulDataGrid />);

// 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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions packages/eui/src/components/datagrid/data_grid_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export interface EuiDataGridHeaderCellWrapperProps {
className?: string;
'aria-label'?: AriaAttributes['aria-label'];
hasColumnActions?: boolean;
isDragging?: boolean;
onKeyDown?: KeyboardEventHandler;
}

Expand Down

0 comments on commit f064c2e

Please sign in to comment.