Skip to content

Commit

Permalink
[EuiDataGrid] Various row height fixes (#8025)
Browse files Browse the repository at this point in the history
  • Loading branch information
cee-chen authored Sep 19, 2024
1 parent b768c93 commit 84aa840
Show file tree
Hide file tree
Showing 63 changed files with 251 additions and 195 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions packages/eui/changelogs/upcoming/8025.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
**Bug fixes**

- Fixed several `EuiDataGrid` row height bugs:
- Fixed row heights not recalculating when `rowHeightOptions.lineHeight`, `gridStyles.fontSize`, or `gridStyles.cellPadding` changed
- Fixed incorrect height calculations for `rowHeightOptions.rowHeights` with `lineCount`s
- Fixed control column content to align better with multi-line row heights, as well as custom line-heights
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="A"
>
A
Expand Down Expand Up @@ -653,7 +653,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="B"
>
B
Expand Down Expand Up @@ -989,7 +989,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="A"
>
A
Expand Down Expand Up @@ -1031,7 +1031,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="B"
>
B
Expand Down Expand Up @@ -1085,7 +1085,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
tabindex="-1"
>
<div
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
data-datagrid-cellcontent="true"
>
0
Expand Down Expand Up @@ -1142,7 +1142,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
tabindex="-1"
>
<div
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
data-datagrid-cellcontent="true"
>
0
Expand All @@ -1161,7 +1161,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
tabindex="-1"
>
<div
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
data-datagrid-cellcontent="true"
>
1
Expand Down Expand Up @@ -1218,7 +1218,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
tabindex="-1"
>
<div
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
data-datagrid-cellcontent="true"
>
1
Expand All @@ -1237,7 +1237,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
tabindex="-1"
>
<div
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
data-datagrid-cellcontent="true"
>
2
Expand Down Expand Up @@ -1294,7 +1294,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
tabindex="-1"
>
<div
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
data-datagrid-cellcontent="true"
>
2
Expand Down Expand Up @@ -1478,7 +1478,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="A"
>
Column A
Expand Down Expand Up @@ -1520,7 +1520,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="B"
>
<div>
Expand Down Expand Up @@ -1840,7 +1840,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="A"
>
A
Expand Down Expand Up @@ -1882,7 +1882,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="B"
>
B
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="columnA"
>
columnA
Expand Down Expand Up @@ -68,7 +68,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="columnB"
>
columnB
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="columnA"
>
columnA
Expand Down Expand Up @@ -72,7 +72,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = `
data-test-subj="dataGridColumnResizer"
/>
<div
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
title="columnB"
>
columnB
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export const euiDataGridCellOutlineSelectors = (parentSelector = '&') => {
};

export const euiDataGridRowCellStyles = (euiThemeContext: UseEuiTheme) => {
const { euiTheme } = euiThemeContext;
const cellOutline = euiDataGridCellOutlineStyles(euiThemeContext);
const { outline: outlineSelectors } = euiDataGridCellOutlineSelectors();

Expand Down Expand Up @@ -146,17 +147,45 @@ export const euiDataGridRowCellStyles = (euiThemeContext: UseEuiTheme) => {
euiDataGridRowCell__content: css`
overflow: hidden;
`,
controlColumn: css`
${logicalCSS('max-height', '100%')}
display: flex;
align-items: center;
`,
autoHeight: css`
${logicalCSS('height', 'auto')}
`,
defaultHeight: css`
${logicalCSS('height', '100%')}
`,
// Control columns should be vertically centered with the *first line* of text
// for both single and multi-line heights (see https://github.com/elastic/eui/issues/7897)
controlColumn: css`
display: inline-flex;
align-items: start;
gap: ${euiTheme.size.xxs}; /* EuiButtonIcons */
/* FF and webkit browsers have more centered vertical alignment behind undocumented browser prefixes */
vertical-align: -webkit-baseline-middle;
vertical-align: -moz-middle-with-baseline;
/* Compact sizing affordance for EuiButtonIcons */
.euiDataGrid--fontSizeSmall
&:where(.euiDataGridRowCell__content--defaultHeight) {
${logicalCSS('height', '100%')}
align-items: center;
}
/* Line up single EuiCheckboxes a bit better (insert Peter Griffin blinds gif) */
.euiCheckbox:not(:has(label)) {
display: inline;
.euiCheckbox__square {
display: inline-flex;
vertical-align: text-bottom;
/* sub alignment looks better on Firefox, text-bottom looks better on webkit 💀 */
@supports (vertical-align: -moz-middle-with-baseline) {
vertical-align: sub;
}
}
}
`,
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('EuiDataGridCell', () => {
),
popoverContext: mockPopoverContext,
rowHeightUtils: mockRowHeightUtils,
gridStyles: {},
};

beforeEach(() => jest.clearAllMocks());
Expand Down Expand Up @@ -202,6 +203,12 @@ describe('EuiDataGridCell', () => {
it('rowHeightsOptions', () => {
component.setProps({ rowHeightsOptions: { defaultHeight: 'auto' } });
});
it('gridStyles.fontSize', () => {
component.setProps({ gridStyles: { fontSize: 's' } });
});
it('gridStyles.cellPadding', () => {
component.setProps({ gridStyles: { cellPadding: 'l' } });
});
it('renderCellValue', () => {
component.setProps({ renderCellValue: () => <div>test</div> });
});
Expand Down Expand Up @@ -254,7 +261,10 @@ describe('EuiDataGridCell', () => {
});

it('should not update for prop/state changes not specified above', () => {
component.setProps({ className: 'test' });
component.setProps({
className: 'test',
gridStyles: { header: 'underline' },
});
expect(shouldComponentUpdate).toHaveReturnedWith(false);
});
});
Expand Down Expand Up @@ -625,7 +635,7 @@ describe('EuiDataGridCell', () => {
callMethod(component);
expect(
mockRowHeightUtils.calculateHeightForLineCount
).toHaveBeenCalledWith(expect.any(HTMLElement), 3, false);
).toHaveBeenCalledWith(expect.any(HTMLElement), 3);
expect(setRowHeight).toHaveBeenCalled();
});
});
Expand All @@ -647,25 +657,76 @@ describe('EuiDataGridCell', () => {
callMethod(component);
expect(
mockRowHeightUtils.calculateHeightForLineCount
).toHaveBeenCalledWith(expect.any(HTMLElement), 10, true);
).toHaveBeenCalledWith(expect.any(HTMLElement), 10);
expect(mockRowHeightUtils.setRowHeight).toHaveBeenCalled();
expect(setRowHeight).not.toHaveBeenCalled();
});

it('recalculates when the override for the row changes', () => {
const component = mount(
<EuiDataGridCell {...requiredProps} setRowHeight={setRowHeight} />
);

component.setProps({
rowHeightsOptions: {
rowHeights: {
0: { lineCount: 2 },
},
},
});
expect(mockRowHeightUtils.setRowHeight).toHaveBeenCalledTimes(1);

// Handle row index changes as well
component.setProps({
rowHeightsOptions: {
rowHeights: {
0: { lineCount: 2 },
2: { lineCount: 4 },
},
},
rowIndex: 2,
});
expect(mockRowHeightUtils.setRowHeight).toHaveBeenCalledTimes(2);

expect(setRowHeight).not.toHaveBeenCalled();
});
});

it('recalculates when rowHeightsOptions.defaultHeight.lineCount changes', () => {
it('recalculates when props that affect row/line height change', () => {
const component = mount(
<EuiDataGridCell
{...requiredProps}
rowHeightsOptions={{ defaultHeight: { lineCount: 7 } }}
rowHeightsOptions={{ defaultHeight: { lineCount: 4 } }}
setRowHeight={setRowHeight}
/>
);
component.setProps({
rowHeightsOptions: { defaultHeight: { lineCount: 2 } },
});
expect(setRowHeight).toHaveBeenCalledTimes(1);

// Other props that can affect row heights

const rowHeightsOptionsWithLineHeight = {
defaultHeight: { lineCount: 2 },
lineHeight: '3',
};
component.setProps({
rowHeightsOptions: { defaultHeight: { lineCount: 6 } },
rowHeightsOptions: rowHeightsOptionsWithLineHeight,
});
expect(setRowHeight).toHaveBeenCalled();
expect(setRowHeight).toHaveBeenCalledTimes(2);

component.setProps({
rowHeightsOptions: rowHeightsOptionsWithLineHeight,
gridStyles: { cellPadding: 'l' },
});
expect(setRowHeight).toHaveBeenCalledTimes(3);

component.setProps({
rowHeightsOptions: rowHeightsOptionsWithLineHeight,
gridStyles: { cellPadding: 'l', fontSize: 'l' },
});
expect(setRowHeight).toHaveBeenCalledTimes(4);
});

it('calculates undefined heights as single rows with a lineCount of 1', () => {
Expand All @@ -680,7 +741,7 @@ describe('EuiDataGridCell', () => {
callMethod(component);
expect(
mockRowHeightUtils.calculateHeightForLineCount
).toHaveBeenCalledWith(expect.any(HTMLElement), 1, false);
).toHaveBeenCalledWith(expect.any(HTMLElement), 1);
expect(setRowHeight).toHaveBeenCalled();
});

Expand Down
Loading

0 comments on commit 84aa840

Please sign in to comment.