Skip to content

Commit

Permalink
[EuiDataGrid] Fix open cell popovers causing auto height rows to bug …
Browse files Browse the repository at this point in the history
…out + misc cleanup (#5622)

* Fix auto height rows breaking when cell popover opens

The `flex` display on the parent `.euiDataGridRowCell` is unnecessary, and what's causing this bug

* [cleanup] Remove unnecessary position relative on .euiDataGridRowCell

- it's being overridden by react-window's absolute positioning in any case

* [cleanup] Remove unnecessary `euiDataGridRowCell__expand` class from the popover anchor wrapper

- as far as I can tell, it's not doing anything

* [cleanup] Focus trap/popover hacks

- the width properties aren't doing anything, so remove them

- the height property is doing something but is superfluous with the inline height: 100%, so remove the inline style

- add an overflow hidden to preserve overflowing content being correctly cropped by the cell padding when the popover is open (the EuiWrappingPopover adds extra div wrappers)

* [misc cleanup] DRY out showCellActions if statement

- can be conditional JSX instead of repeating EuiDataGridCellContent again

* Add changelog entry

* Revert popover cutoff change, it's introducing too many side effects

+ was already in place in main since 40.0.0

* Fix broken focus cell positioning on footer cells

- footer cells aren't virtualized and don't have absolute positioning inline

* Fix more footer issues
+ move footer-specific CSS that virtualized data grid cells don't need to the footer row file
  • Loading branch information
Constance authored Feb 10, 2022
1 parent c1f5de4 commit 91368b5
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 34 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

- Fixed `EuiDataGrid` to correctly remove the cell expansion action button when a column sets both `cellActions` and `isExpandable` to false ([#5592](https://github.com/elastic/eui/pull/5592))
- Fixed `EuiDataGrid` re-playing the cell actions animation when hovering over an already-focused cell ([#5592](https://github.com/elastic/eui/pull/5592))
- Fixed `EuiDataGrid` auto row heights bugging out when cell popovers are opened ([#5622](https://github.com/elastic/eui/pull/5622))

**Breaking changes**

Expand Down
15 changes: 1 addition & 14 deletions src/components/datagrid/_data_grid_data_row.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
.euiDataGridRow {
// needed for footer cells to correctly position
display: flex;
background-color: $euiColorEmptyShade;
}

Expand All @@ -10,16 +8,10 @@
padding: $euiDataGridCellPaddingM;
border-right: $euiDataGridVerticalBorder;
border-bottom: $euiBorderThin;
flex: 0 0 auto;
position: relative;
align-items: center;
display: flex;
overflow: hidden;

// Hack to allow for all the focus guard stuff
// Hack to allow focus trap to still stretch to full row height on defined heights
> * {
max-width: 100%;
width: 100%;
height: 100%;
}

Expand Down Expand Up @@ -114,11 +106,6 @@
z-index: $euiZDataGridCellPopover !important;
}

.euiDataGridRowCell__expand {
width: 100%;
max-width: 100%;
}

.euiDataGridRowCell__expandFlex {
position: relative; // for positioning expand button
display: flex;
Expand Down
6 changes: 6 additions & 0 deletions src/components/datagrid/body/_data_grid_footer_row.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
.euiDataGridFooter {
display: flex;
}

@include euiDataGridFooterCell {
flex: 0 0 auto;
position: relative;
font-weight: $euiFontWeightBold;
}

Expand Down
27 changes: 9 additions & 18 deletions src/components/datagrid/body/data_grid_cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,6 @@ export class EuiDataGridCell extends Component<
onDeactivation={() => {
this.setState({ isEntered: false }, this.preventTabbing);
}}
style={isDefinedHeight ? { height: '100%' } : {}}
clickOutsideDisables={true}
>
<div className={anchorClass} ref={this.popoverAnchorRef}>
Expand All @@ -628,12 +627,12 @@ export class EuiDataGridCell extends Component<
);

if (hasCellActions) {
if (showCellActions) {
innerContent = (
<div className={anchorClass} ref={this.popoverAnchorRef}>
<div className={expandClass}>
<EuiDataGridCellContent {...cellContentProps} />
</div>
innerContent = (
<div className={anchorClass} ref={this.popoverAnchorRef}>
<div className={expandClass}>
<EuiDataGridCellContent {...cellContentProps} />
</div>
{showCellActions && (
<EuiDataGridCellActions
rowIndex={rowIndex}
colIndex={colIndex}
Expand All @@ -648,17 +647,9 @@ export class EuiDataGridCell extends Component<
}
}}
/>
</div>
);
} else {
innerContent = (
<div className={anchorClass} ref={this.popoverAnchorRef}>
<div className={expandClass}>
<EuiDataGridCellContent {...cellContentProps} />
</div>
</div>
);
}
)}
</div>
);
}

const content = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ describe('useCellPopover', () => {
populateCellPopover(cellPopoverContext);
expect(getUpdatedState().cellPopover).toMatchInlineSnapshot(`
<EuiWrappingPopover
anchorClassName="euiDataGridRowCell__expand"
button={<div />}
closePopover={[Function]}
display="block"
Expand Down
1 change: 0 additions & 1 deletion src/components/datagrid/body/data_grid_cell_popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ export const useCellPopover = (): {
const cellPopover = popoverIsOpen && popoverAnchor && (
<EuiWrappingPopover
hasArrow={false}
anchorClassName="euiDataGridRowCell__expand"
button={popoverAnchor}
isOpen={popoverIsOpen}
panelClassName="euiDataGridRowCell__popover"
Expand Down

0 comments on commit 91368b5

Please sign in to comment.