Skip to content

Commit

Permalink
[EuiDataGrid] Refactor row height calculations to no longer need comp…
Browse files Browse the repository at this point in the history
…uted styles on grid init (#5284)

* Initial attempt using static maps

NB: Saving this commit for historical purposes; but the issue with this solution is that lineHeight & fontSize change between legacy and Amsterdam theme and thus the calculations are wrong on the legacy theme

- Switch to static padding/fontSize/lineHeights maps instead of classes

- Undefined line height will NaN without euiDataGridRow CSS

- Remove this.fakeCell class instance since it's only used in one place

* Misc rowHeightUtils fixes/refactors

- Export single default/initial row height const which data_grid_body imports, instead of declaring 2 slightly different heights in 2 different locations

- fix rowIndex check that would fail on a rowIndex of 0

* Refactor isDefinedHeight and several rowHeightsOptions logic checks into a getRowHeightOption helper

- this helper DRYs out returning either a specific row's height config or the defaultHeight - we had this logic essentially repeated in multiple places

* Clean up shared cell props

- Pull out shared/same props into a single obj instead of repeating it 3x & making it less clear which props are different depending on the cell type

- remove getRowHeight fn from being passed down; it's not being used

- pass setRowHeight only to first column as a performance optimization; this will be used for lineCount heights and since lineCount heights are all the same, we really only need to run it once

- clean up footer row cells as well while we're here

* Convert lineCount rows to use this.props.setRowHeight state from data_grid_body

- IIRC this is what the state was originally used for; might as well go back to using it for lineCount

* Update computeStylesForGridCell to remove fontSize & lineHeight calcs

- Those were only being used for lineCount calculations; otherwise only padding is needed

+ remove numberFromPx in favor of a map with existing numbers

* Rename computeStylesForGridCell now that it's no longer computing styles & remove getComputedCellStyles

- re: getComputedCellStyles - I'm not seeing this affect either numeric, lineCount, or auto heights when changing grid density, hence me removing this

* Remove unused CSS classes

Now that we aren't using this CSS for computed styles any longer, we can remove them

* Remove now-unused cell wrapper observer

- we're exclusively watching the cell content now for both auto and line count heights

* Rename recalculateRowHeight to recalculateAutoHeight

- to make it more specific and clearer from the fn name what row height behavior this belongs to

* Unit tests + mock improvements

- add unit tests for recalculateLineCountHeight,

- mockRowHeightUtils - allow some generic methods to return their actual fns for expedience

- Remove jest.mock from data_grid.test.tsx - the __mocks__ folder automatically mocks it

- update snapshots/returns accordingly

* Add changelog entry

* Fix crashing eui.d.ts

* Fix changelog after release
  • Loading branch information
Constance authored Nov 3, 2021
1 parent d797498 commit 95e15fa
Show file tree
Hide file tree
Showing 11 changed files with 202 additions and 276 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## [`master`](https://github.com/elastic/eui/tree/master)

No public interface changes since `41.0.0`.
**Bug fixes**

- Fixed an `EuiDataGrid` race condition where grid rows had incorrect heights if loaded in before CSS ([#5284](https://github.com/elastic/eui/pull/5284))

## [`41.0.0`](https://github.com/elastic/eui/tree/v41.0.0)

Expand Down
33 changes: 9 additions & 24 deletions src/components/datagrid/__mocks__/row_height_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,41 +6,26 @@
* Side Public License, v 1.
*/

import { EuiDataGridRowHeightOption } from '../data_grid_types';
import { RowHeightUtils as ActualRowHeightUtils } from '../row_height_utils';

import { RowHeightUtils } from '../row_height_utils';
const actual = new ActualRowHeightUtils();

export const mockRowHeightUtils = ({
computeStylesForGridCell: jest.fn(),
cacheStyles: jest.fn(),
setGrid: jest.fn(),
getStylesForCell: jest.fn(() => ({
wordWrap: 'break-word',
wordBreak: 'break-word',
flexGrow: 1,
})),
isDefinedHeight: jest.fn(() => true),
isAutoHeight: jest.fn(() => false),
setRowHeight: jest.fn(),
pruneHiddenColumnHeights: jest.fn(),
getRowHeight: jest.fn(() => 32),
getComputedCellStyles: jest.fn(() => {}),
getCalculatedHeight: jest.fn(
(heightOption: EuiDataGridRowHeightOption, defaultHeight: number) => {
if (typeof heightOption === 'object') {
if (heightOption.lineCount) {
return heightOption.lineCount;
}
getRowHeightOption: jest.fn(actual.getRowHeightOption),
getCalculatedHeight: jest.fn(actual.getCalculatedHeight),
getLineCount: jest.fn(actual.getLineCount),
calculateHeightForLineCount: jest.fn(() => 50),
} as unknown) as ActualRowHeightUtils;

if (heightOption.height) {
return heightOption.height;
}
}

if (heightOption) {
return heightOption;
}

return defaultHeight;
}
),
} as unknown) as RowHeightUtils;
export const RowHeightUtils = jest.fn(() => mockRowHeightUtils);
24 changes: 0 additions & 24 deletions src/components/datagrid/_data_grid_data_row.scss
Original file line number Diff line number Diff line change
Expand Up @@ -235,49 +235,25 @@
}
}

.euiDataGridRowCell--fontSizeSmall {
@include euiFontSizeXS;
}

@include euiDataGridStyles(fontSizeLarge) {
@include euiDataGridRowCell {
@include euiFontSize;
}
}

.euiDataGridRowCell--fontSizeLarge {
@include euiFontSize;
}

// Padding alternates
@include euiDataGridStyles(paddingSmall) {
@include euiDataGridRowCell {
padding: $euiDataGridCellPaddingS;
}
}

.euiDataGridRowCell--paddingSmall {
padding: $euiDataGridCellPaddingS;

.euiDataGridRowCell__contentByHeight + .euiDataGridRowCell__expandButton {
padding: 0;
}
}

@include euiDataGridStyles(paddingLarge) {
@include euiDataGridRowCell {
padding: $euiDataGridCellPaddingL;
}
}

.euiDataGridRowCell--paddingLarge {
padding: $euiDataGridCellPaddingL;

.euiDataGridRowCell__contentByHeight + .euiDataGridRowCell__expandButton {
padding: $euiDataGridCellPaddingL 0;
}
}

@keyframes euiDataGridCellButtonSlideIn {
from {
margin-left: 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,27 @@ exports[`EuiDataGridCell renders 1`] = `
renderCellValue={[Function]}
rowHeightUtils={
Object {
"computeStylesForGridCell": [MockFunction],
"cacheStyles": [MockFunction],
"calculateHeightForLineCount": [MockFunction],
"getCalculatedHeight": [MockFunction],
"getComputedCellStyles": [MockFunction],
"getLineCount": [MockFunction],
"getRowHeight": [MockFunction],
"getRowHeightOption": [MockFunction] {
"calls": Array [
Array [
0,
undefined,
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
},
"getStylesForCell": [MockFunction],
"isAutoHeight": [MockFunction],
"isDefinedHeight": [MockFunction],
"pruneHiddenColumnHeights": [MockFunction],
"setGrid": [MockFunction],
"setRowHeight": [MockFunction],
Expand Down Expand Up @@ -57,13 +71,27 @@ exports[`EuiDataGridCell renders 1`] = `
renderCellValue={[Function]}
rowHeightUtils={
Object {
"computeStylesForGridCell": [MockFunction],
"cacheStyles": [MockFunction],
"calculateHeightForLineCount": [MockFunction],
"getCalculatedHeight": [MockFunction],
"getComputedCellStyles": [MockFunction],
"getLineCount": [MockFunction],
"getRowHeight": [MockFunction],
"getRowHeightOption": [MockFunction] {
"calls": Array [
Array [
0,
undefined,
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
},
"getStylesForCell": [MockFunction],
"isAutoHeight": [MockFunction],
"isDefinedHeight": [MockFunction],
"pruneHiddenColumnHeights": [MockFunction],
"setGrid": [MockFunction],
"setRowHeight": [MockFunction],
Expand Down
96 changes: 31 additions & 65 deletions src/components/datagrid/body/data_grid_body.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
useMutationObserver,
} from '../../observer/mutation_observer';
import { useResizeObserver } from '../../observer/resize_observer';
import { AUTO_HEIGHT } from '../row_height_utils';
import { DEFAULT_ROW_HEIGHT } from '../row_height_utils';
import { EuiDataGridCell } from './data_grid_cell';
import {
DataGridSortingContext,
Expand Down Expand Up @@ -70,7 +70,6 @@ export const Cell: FunctionComponent<GridChildComponentProps> = ({
setRowHeight,
schemaDetectors,
rowHeightsOptions,
getRowHeight,
rowHeightUtils,
} = data;

Expand Down Expand Up @@ -119,30 +118,33 @@ export const Cell: FunctionComponent<GridChildComponentProps> = ({
[`euiDataGridRowCell--${textTransform}`]: textTransform,
});

const sharedCellProps = {
rowIndex,
visibleRowIndex,
colIndex: columnIndex,
interactiveCellId,
className: classes,
style: {
...style,
top: `${parseFloat(style.top as string) + headerRowHeight}px`,
},
rowHeightsOptions,
rowHeightUtils,
setRowHeight: isFirstColumn ? setRowHeight : undefined,
};

if (isLeadingControlColumn) {
const leadingColumn = leadingControlColumns[columnIndex];
const { id, rowCellRender } = leadingColumn;

cellContent = (
<EuiDataGridCell
rowIndex={rowIndex}
visibleRowIndex={visibleRowIndex}
colIndex={columnIndex}
{...sharedCellProps}
columnId={id}
popoverContent={DefaultColumnFormatter}
width={leadingColumn.width}
renderCellValue={rowCellRender}
interactiveCellId={interactiveCellId}
isExpandable={false}
className={classes}
setRowHeight={setRowHeight}
getRowHeight={getRowHeight}
rowHeightsOptions={rowHeightsOptions}
rowHeightUtils={rowHeightUtils}
style={{
...style,
top: `${parseFloat(style.top as string) + headerRowHeight}px`,
}}
/>
);
} else if (isTrailingControlColumn) {
Expand All @@ -153,23 +155,12 @@ export const Cell: FunctionComponent<GridChildComponentProps> = ({

cellContent = (
<EuiDataGridCell
rowIndex={rowIndex}
visibleRowIndex={visibleRowIndex}
colIndex={columnIndex}
{...sharedCellProps}
columnId={id}
popoverContent={DefaultColumnFormatter}
width={trailingColumn.width}
renderCellValue={rowCellRender}
interactiveCellId={interactiveCellId}
isExpandable={false}
className={classes}
rowHeightsOptions={rowHeightsOptions}
getRowHeight={getRowHeight}
rowHeightUtils={rowHeightUtils}
style={{
...style,
top: `${parseFloat(style.top as string) + headerRowHeight}px`,
}}
/>
);
} else {
Expand All @@ -188,9 +179,7 @@ export const Cell: FunctionComponent<GridChildComponentProps> = ({

cellContent = (
<EuiDataGridCell
rowIndex={rowIndex}
visibleRowIndex={visibleRowIndex}
colIndex={columnIndex}
{...sharedCellProps}
columnId={columnId}
column={column}
columnType={columnType}
Expand All @@ -199,14 +188,6 @@ export const Cell: FunctionComponent<GridChildComponentProps> = ({
renderCellValue={renderCellValue}
interactiveCellId={interactiveCellId}
isExpandable={isExpandable}
className={classes}
rowHeightsOptions={rowHeightsOptions}
getRowHeight={getRowHeight}
rowHeightUtils={rowHeightUtils}
style={{
...style,
top: `${parseFloat(style.top as string) + headerRowHeight}px`,
}}
/>
);
}
Expand Down Expand Up @@ -240,7 +221,6 @@ const InnerElement: VariableSizeGridProps['innerElementType'] = forwardRef<
});
InnerElement.displayName = 'EuiDataGridInnerElement';

const INITIAL_ROW_HEIGHT = 34;
const IS_JEST_ENVIRONMENT = global.hasOwnProperty('_isJest');

/**
Expand Down Expand Up @@ -524,45 +504,32 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
[rowHeightUtils]
);

const [minRowHeight, setRowHeight] = useState(INITIAL_ROW_HEIGHT);

const computedCellStyles = rowHeightUtils.getComputedCellStyles();
const [minRowHeight, setRowHeight] = useState(DEFAULT_ROW_HEIGHT);

const defaultHeight = useMemo(() => {
// @ts-ignore we need to re-run this when computedCellStyles changes,
// but it isn't used directly; so let's make the hooks lint rule see
// that it is used, but we need to tell eslint to ignore
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const _computedCellStyles = computedCellStyles;
return rowHeightsOptions?.defaultHeight
? rowHeightUtils.getCalculatedHeight(
rowHeightsOptions.defaultHeight,
minRowHeight
)
: minRowHeight;
}, [rowHeightsOptions, minRowHeight, rowHeightUtils, computedCellStyles]);
}, [rowHeightsOptions, minRowHeight, rowHeightUtils]);

const getRowHeight = useCallback(
(rowIndex) => {
const correctRowIndex = getCorrectRowIndex(rowIndex);
let height;

if (rowHeightsOptions) {
if (rowHeightsOptions.rowHeights) {
const initialHeight = rowHeightsOptions.rowHeights[correctRowIndex];

if (initialHeight) {
height = rowHeightUtils.getCalculatedHeight(
initialHeight,
minRowHeight,
correctRowIndex
);
}
}

if (!height && rowHeightsOptions.defaultHeight === AUTO_HEIGHT) {
height = rowHeightUtils.getRowHeight(correctRowIndex);
}
const rowHeightOption = rowHeightUtils.getRowHeightOption(
correctRowIndex,
rowHeightsOptions
);
if (rowHeightOption) {
height = rowHeightUtils.getCalculatedHeight(
rowHeightOption,
minRowHeight,
correctRowIndex
);
}

return height || defaultHeight;
Expand Down Expand Up @@ -707,7 +674,6 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
itemData={{
schemaDetectors,
setRowHeight,
getRowHeight,
getCorrectRowIndex,
rowMap,
rowOffset: pagination
Expand Down
Loading

0 comments on commit 95e15fa

Please sign in to comment.