Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiDataGrid] Fix cell popovers not closing on outside grid click #7387

Merged
merged 4 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/components/datagrid/_data_grid_data_row.scss
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@
&:focus,
&:focus-within,
// Always make the cell actions visible when the cell popover is open
&.euiDataGridRowCell--open {
&.euiDataGridRowCell--open,
// Prevents the animation from replaying when keyboard focus is moved from the popover back to the cell
&[data-keyboard-closing] {
.euiDataGridRowCell__actions {
animation-duration: $euiAnimSpeedExtraFast;
animation-name: euiDataGridCellActionsSlideIn;
Expand All @@ -67,7 +69,7 @@
}

// if a cell is not hovered nor focused nor open via popover, don't show buttons in general
&:not(:hover):not(:focus):not(.euiDataGridRowCell--open) {
&:not(:hover):not(:focus):not(.euiDataGridRowCell--open):not([data-keyboard-closing]) {
.euiDataGridRowCell__actions {
display: none;
}
Expand Down
4 changes: 0 additions & 4 deletions src/components/datagrid/body/data_grid_cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,6 @@ export class EuiDataGridCell extends Component<
this.setState({ disableCellTabIndex: true });
}
}, 0);
// Close the cell popover if the popover was open and the user clicked the cell
if (this.props.popoverContext.popoverIsOpen) {
this.props.popoverContext.closeCellPopover();
}
}
};

Expand Down
81 changes: 73 additions & 8 deletions src/components/datagrid/body/data_grid_cell_popover.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,83 @@ describe('EuiDataGridCellPopover', () => {
});
});

it('closes the cell popover when the originating cell is clicked', () => {
cy.realMount(<EuiDataGrid {...baseProps} />);
cy.get(
'[data-gridcell-row-index="0"][data-gridcell-column-index="0"]'
).realClick();
describe('closes the cell popover', () => {
// Mount and open the first cell popover
beforeEach(() => {
cy.realMount(<EuiDataGrid {...baseProps} />);
cy.get(
'[data-gridcell-row-index="0"][data-gridcell-column-index="0"]'
).click();
cy.realPress('Enter');
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist');
});

it('when the originating cell is clicked', () => {
cy.get(
'[data-gridcell-row-index="0"][data-gridcell-column-index="0"]'
).realClick({ position: 'right' });
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should(
'not.exist'
);
});

it('when the cell expand action button is clicked', () => {
cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click();
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should(
'not.exist'
);
});

it('when anywhere outside the grid is clicked', () => {
cy.get('body').realClick({ position: 'bottomRight' });
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should(
'not.exist'
);
});
});

it('does not close the cell popover when other cell actions are clicked', () => {
const cellActions = [
({ Component }) => (
<Component
iconType="plusInCircle"
aria-label="A"
data-test-subj="cellActionA"
/>
),
({ Component }) => (
<Component
iconType="minusInCircle"
aria-label="B"
data-test-subj="cellActionB"
/>
),
];
cy.realMount(
<EuiDataGrid {...baseProps} columns={[{ id: 'A', cellActions }]} />
);
cy.get('[data-test-subj="dataGridRowCell"]').first().click();
cy.realPress('Enter');
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist');

cy.get('[data-test-subj="cellActionA"]').first().realClick();
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist');

// Close and re-open the cell popover by clicking
cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click();
cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click();

cy.get('[data-test-subj="cellActionB"]').first().realClick();
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist');

cy.get(
'[data-gridcell-row-index="0"][data-gridcell-column-index="0"]'
).realClick({ position: 'right' });
// Clicking the cell actions outside the popover should not have disabled the focus trap
cy.repeatRealPress('Tab', 3);
cy.focused().should(
'have.attr',
'data-test-subj',
'euiDataGridExpansionPopover'
);
cy.get('body').realClick({ position: 'bottomRight' });
cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should(
'not.exist'
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ describe('useCellPopover', () => {
display="block"
focusTrapProps={
Object {
"clickOutsideDisables": false,
"onClickOutside": [Function],
}
}
Expand Down
20 changes: 14 additions & 6 deletions src/components/datagrid/body/data_grid_cell_popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ export const useCellPopover = (): {
// clicking the expansion cell action triggers an outside click
const onClickOutside = useCallback(
(event: Event) => {
if (!popoverAnchor) return;
const cellActions = popoverAnchor.previousElementSibling;
if (cellActions?.contains(event.target as Node) === false) {
const cellActions =
popoverAnchor?.parentElement?.parentElement?.previousElementSibling;
if (!cellActions?.contains(event.target as Node)) {
closeCellPopover();
}
},
Expand All @@ -107,7 +107,7 @@ export const useCellPopover = (): {
anchorPosition={popoverAnchorPosition}
repositionToCrossAxis={false}
{...cellPopoverProps}
focusTrapProps={{ onClickOutside }}
focusTrapProps={{ onClickOutside, clickOutsideDisables: false }}
panelProps={{
'data-test-subj': 'euiDataGridExpansionPopover',
...(cellPopoverProps.panelProps || {}),
Expand All @@ -128,8 +128,16 @@ export const useCellPopover = (): {
event.preventDefault();
event.stopPropagation();
closeCellPopover();
// Ensure focus is returned to the parent cell
requestAnimationFrame(() => popoverAnchor.parentElement!.focus());
const cell =
popoverAnchor.parentElement?.parentElement?.parentElement;

// Prevent cell animation flash while focus is being shifted between popover and cell
cell?.setAttribute('data-keyboard-closing', 'true');
// Ensure focus is returned to the parent cell, and remove animation stopgap
requestAnimationFrame(() => {
popoverAnchor.parentElement!.focus();
cell?.removeAttribute('data-keyboard-closing');
});
}
}}
button={popoverAnchor}
Expand Down
Loading