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] improve height calculation #5447

1 change: 1 addition & 0 deletions src/components/datagrid/__mocks__/row_height_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const mockRowHeightUtils = ({
getLineCount: jest.fn(actual.getLineCount),
calculateHeightForLineCount: jest.fn(() => 50),
isRowHeightOverride: jest.fn(actual.isRowHeightOverride),
setRerenderGridBody: jest.fn(),
} as unknown) as ActualRowHeightUtils;

export const RowHeightUtils = jest.fn(() => mockRowHeightUtils);
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ exports[`EuiDataGridCell renders 1`] = `
"isRowHeightOverride": [MockFunction],
"pruneHiddenColumnHeights": [MockFunction],
"setGrid": [MockFunction],
"setRerenderGridBody": [MockFunction],
"setRowHeight": [MockFunction],
}
}
Expand Down Expand Up @@ -98,6 +99,7 @@ exports[`EuiDataGridCell renders 1`] = `
"isRowHeightOverride": [MockFunction],
"pruneHiddenColumnHeights": [MockFunction],
"setGrid": [MockFunction],
"setRerenderGridBody": [MockFunction],
"setRowHeight": [MockFunction],
}
}
Expand Down
80 changes: 74 additions & 6 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 { DEFAULT_ROW_HEIGHT } from '../row_height_utils';
import { DEFAULT_ROW_HEIGHT, RowHeightUtils } from '../row_height_utils';
import { EuiDataGridCell } from './data_grid_cell';
import {
DataGridSortingContext,
Expand All @@ -44,10 +44,12 @@ import {
import {
EuiDataGridBodyProps,
EuiDataGridInMemoryValues,
EuiDataGridRowHeightsOptions,
EuiDataGridRowManager,
EuiDataGridSchemaDetector,
} from '../data_grid_types';
import { makeRowManager } from './data_grid_row_manager';
import { useForceRender } from '../../../services/hooks/useForceRender';

export const VIRTUALIZED_CONTAINER_CLASS = 'euiDataGrid__virtualized';

Expand Down Expand Up @@ -253,6 +255,67 @@ export function getParentCellContent(_element: Node | HTMLElement) {
return element;
}

// computes the unconstrained (total possible) height of a grid
const useUnconstrainedHeight = ({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire commit + set of comments is amazing and really helps me understand what this PR is doing. Thanks for taking the time to do it!! 😍

In the future, I might poke a bit more at splitting up even more of data_grid_body's logic into separate utils/hooks like this for organization - I personally think it's super useful for dev experience and unit testing. Thanks for the awesome headstart!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking out into more hooks would be great; likely a great time to pair as well

rowHeightUtils,
startRow,
endRow,
getCorrectRowIndex,
rowHeightsOptions,
defaultHeight,
headerRowHeight,
footerRowHeight,
}: {
rowHeightUtils: RowHeightUtils;
startRow: number;
endRow: number;
getCorrectRowIndex: (rowIndex: number) => number;
rowHeightsOptions?: EuiDataGridRowHeightsOptions;
defaultHeight: number;
headerRowHeight: number;
footerRowHeight: number;
}) => {
// when a row height is updated, force a re-render of the grid body to update the unconstrained height
const forceRender = useForceRender();
useEffect(() => {
rowHeightUtils.setRerenderGridBody(forceRender);
}, [rowHeightUtils, forceRender]);

let knownHeight = 0; // tracks the pixel height of rows we know the size of
let knownRowCount = 0; // how many rows we know the size of
for (let i = startRow; i < endRow; i++) {
const correctRowIndex = getCorrectRowIndex(i); // map visible row to logical row

// lookup the height configuration of this row
const rowHeightOption = rowHeightUtils.getRowHeightOption(
correctRowIndex,
rowHeightsOptions
);

if (rowHeightOption) {
// this row's height is known
knownRowCount++;
knownHeight += rowHeightUtils.getCalculatedHeight(
rowHeightOption,
defaultHeight,
correctRowIndex,
rowHeightUtils.isRowHeightOverride(correctRowIndex, rowHeightsOptions)
);
}
}

// how many rows to provide space for on the screen
const rowCountToAffordFor = endRow - startRow;

const unconstrainedHeight =
defaultHeight * (rowCountToAffordFor - knownRowCount) + // guess how much space is required for unknown rows
knownHeight + // computed pixel height of the known rows
headerRowHeight + // account for header
footerRowHeight; // account for footer

return unconstrainedHeight;
};

export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
props
) => {
Expand Down Expand Up @@ -573,11 +636,16 @@ export const EuiDataGridBody: FunctionComponent<EuiDataGridBodyProps> = (
}
}, [getRowHeight]);

const rowCountToAffordFor = pagination
? pagination.pageSize
: visibleRowIndices.length;
const unconstrainedHeight =
defaultHeight * rowCountToAffordFor + headerRowHeight + footerRowHeight;
const unconstrainedHeight = useUnconstrainedHeight({
rowHeightUtils,
startRow,
endRow,
getCorrectRowIndex,
rowHeightsOptions,
defaultHeight,
headerRowHeight,
footerRowHeight,
});

// unable to determine this until the container's size is known anyway
const unconstrainedWidth = 0;
Expand Down
29 changes: 29 additions & 0 deletions src/components/datagrid/data_grid.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,35 @@ describe('EuiDataGrid', () => {
.should('have.lengthOf', 0);
});
});

describe('height calculation', async () => {
it('computes a new unconstrained height when switching to auto height', () => {
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
const renderCellValue: EuiDataGridProps['renderCellValue'] = ({
rowIndex,
columnId,
}) => (
<>
row {rowIndex}
<br />
column {columnId}
</>
);

mount(<EuiDataGrid {...baseProps} renderCellValue={renderCellValue} />);

getGridData();
cy.get('[data-test-subj=euiDataGridBody]')
.invoke('outerHeight')
.then((firstHeight) => {
cy.get('[data-test-subj=dataGridDisplaySelectorPopover]').click();
cy.get('[data-text="Auto fit"]').click();

cy.get('[data-test-subj=euiDataGridBody]')
.invoke('outerHeight')
.should('be.greaterThan', firstHeight);
});
});
});
});

function getGridData() {
Expand Down
8 changes: 8 additions & 0 deletions src/components/datagrid/row_height_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ describe('RowHeightUtils', () => {

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

it('calls rerenderGridBody', () => {
const rerenderGridBody = jest.fn();
rowHeightUtils.setRerenderGridBody(rerenderGridBody);
expect(rerenderGridBody).toHaveBeenCalledTimes(0);
rowHeightUtils.setRowHeight(1, 'a', 34, 1);
expect(rerenderGridBody).toHaveBeenCalledTimes(1);
});
});

describe('getRowHeight', () => {
Expand Down
6 changes: 6 additions & 0 deletions src/components/datagrid/row_height_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ export class RowHeightUtils {
private timerId?: number;
private grid?: Grid;
private lastUpdatedRow: number = Infinity;
private rerenderGridBody: Function = () => {};

isAutoHeight(
rowIndex: number,
Expand Down Expand Up @@ -192,6 +193,7 @@ export class RowHeightUtils {
rowHeights.set(colId, adaptedHeight);
this.heightsCache.set(rowIndex, rowHeights);
this.resetRow(visibleRowIndex);
this.rerenderGridBody();
}

pruneHiddenColumnHeights(visibleColumns: EuiDataGridColumn[]) {
Expand Down Expand Up @@ -229,4 +231,8 @@ export class RowHeightUtils {
setGrid(grid: Grid) {
this.grid = grid;
}

setRerenderGridBody(rerenderGridBody: Function) {
this.rerenderGridBody = rerenderGridBody;
}
}
5 changes: 3 additions & 2 deletions src/services/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
* Side Public License, v 1.
*/

export * from './useCombinedRefs';
export * from './useUpdateEffect';
export * from './useDependentState';
export * from './useCombinedRefs';
export * from './useForceRender';
export * from './useIsWithinBreakpoints';
export * from './useMouseMove';
export * from './useUpdateEffect';
47 changes: 47 additions & 0 deletions src/services/hooks/useForceRender.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import React, { useImperativeHandle, createRef, forwardRef } from 'react';
import { act } from 'react-dom/test-utils';
import { mount } from 'enzyme';
import { useForceRender } from './useForceRender';

interface MockRefShape {
render: () => void;
}

describe('useForceRender', () => {
const renderTracker = jest.fn();

// eslint-disable-next-line local/forward-ref
const MockComponent = forwardRef<MockRefShape>((props, ref) => {
const render = useForceRender();

renderTracker();

// expose the render function on the component's ref
useImperativeHandle(ref, () => ({ render }), [render]);

return null;
});

beforeEach(() => {
jest.clearAllMocks();
});

it('causes the component to re-render', () => {
const ref = createRef<MockRefShape>();
mount(<MockComponent ref={ref} />);

expect(renderTracker).toHaveBeenCalledTimes(1);
act(() => {
ref.current!.render();
});
expect(renderTracker).toHaveBeenCalledTimes(2);
});
});
16 changes: 16 additions & 0 deletions src/services/hooks/useForceRender.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { useState, useCallback } from 'react';

export const useForceRender = () => {
const [, setRenderCount] = useState(0);
return useCallback(() => {
setRenderCount((x) => x + 1);
}, [setRenderCount]);
chandlerprall marked this conversation as resolved.
Show resolved Hide resolved
};