diff --git a/src/components/datagrid/body/data_grid_body.tsx b/src/components/datagrid/body/data_grid_body.tsx index 64ffe016ca2..e84e497240a 100644 --- a/src/components/datagrid/body/data_grid_body.tsx +++ b/src/components/datagrid/body/data_grid_body.tsx @@ -391,6 +391,7 @@ export const EuiDataGridBody: FunctionComponent = ( gridRef: gridRef.current, gridStyles, columns, + rowHeightsOptions, }); const { defaultRowHeight, setRowHeight, getRowHeight } = useDefaultRowHeight({ diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 2f03c240189..4d5bd581306 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -452,6 +452,11 @@ function moveColumnToIndex( } describe('EuiDataGrid', () => { + // Mock requestAnimationFrame to run immediately + jest + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((cb: any) => cb()); + describe('rendering', () => { const getBoundingClientRect = window.Element.prototype.getBoundingClientRect; diff --git a/src/components/datagrid/utils/grid_height_width.ts b/src/components/datagrid/utils/grid_height_width.ts index 0b901720515..db87f45e5a7 100644 --- a/src/components/datagrid/utils/grid_height_width.ts +++ b/src/components/datagrid/utils/grid_height_width.ts @@ -99,12 +99,6 @@ export const useUnconstrainedHeight = ({ }) => { const { getCorrectRowIndex } = useContext(DataGridSortingContext); - // 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++) { @@ -137,6 +131,7 @@ export const useUnconstrainedHeight = ({ innerGridRef.current, 'width' ); + const forceRender = useForceRender(); useUpdateEffect(forceRender, [innerWidth]); const unconstrainedHeight = diff --git a/src/components/datagrid/utils/row_heights.test.ts b/src/components/datagrid/utils/row_heights.test.ts index 7661a662516..53328d8cfc3 100644 --- a/src/components/datagrid/utils/row_heights.test.ts +++ b/src/components/datagrid/utils/row_heights.test.ts @@ -6,7 +6,15 @@ * Side Public License, v 1. */ -import { RowHeightUtils, cellPaddingsMap } from './row_heights'; +import { act } from 'react-dom/test-utils'; +import { testCustomHook } from '../../../test/test_custom_hook.test_helper'; +import { startingStyles } from '../controls'; + +import { + RowHeightUtils, + cellPaddingsMap, + useRowHeightUtils, +} from './row_heights'; describe('RowHeightUtils', () => { const rowHeightUtils = new RowHeightUtils(); @@ -147,6 +155,16 @@ describe('RowHeightUtils', () => { }); }); }); + + it('falls back to m-sized cellPadding if gridStyle.cellPadding is undefined', () => { + rowHeightUtils.cacheStyles({ cellPadding: undefined }); + + // @ts-ignore this var is private, but we're inspecting it for the sake of the unit test + expect(rowHeightUtils.styles).toEqual({ + paddingTop: 6, + paddingBottom: 6, + }); + }); }); describe('getStylesForCell (returns inline CSS styles based on height config)', () => { @@ -371,3 +389,92 @@ describe('RowHeightUtils', () => { }); }); }); + +describe('useRowHeightUtils', () => { + const mockArgs = { + gridRef: null, + gridStyles: startingStyles, + columns: [{ id: 'A' }, { id: 'B' }], + rowHeightOptions: undefined, + }; + + it('instantiates and returns an instance of RowHeightUtils', () => { + const { return: rowHeightUtils } = testCustomHook(() => + useRowHeightUtils(mockArgs) + ); + expect(rowHeightUtils).toBeInstanceOf(RowHeightUtils); + }); + + it('populates internal RowHeightUtils vars from outside dependencies', () => { + const args = { ...mockArgs, gridRef: {} as any }; + const { return: rowHeightUtils } = testCustomHook(() => + useRowHeightUtils(args) + ); + // @ts-ignore - intentionally inspecting private var for test + expect(rowHeightUtils.grid).toEqual(args.gridRef); + // @ts-ignore - intentionally inspecting private var for test + expect(rowHeightUtils.rerenderGridBody).toBeInstanceOf(Function); + }); + + it('forces a rerender every time rowHeightsOptions changes', () => { + const requestAnimationFrameSpy = jest + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((cb: any) => cb()); + + const { updateHookArgs } = testCustomHook(useRowHeightUtils, mockArgs); + expect(requestAnimationFrameSpy).toHaveBeenCalledTimes(1); + + updateHookArgs({ rowHeightsOptions: { defaultHeight: 300 } }); + expect(requestAnimationFrameSpy).toHaveBeenCalledTimes(2); + + updateHookArgs({ + rowHeightsOptions: { defaultHeight: 300, rowHeights: { 0: 200 } }, + }); + expect(requestAnimationFrameSpy).toHaveBeenCalledTimes(3); + }); + + it('updates internal cached styles whenever gridStyle.cellPadding changes', () => { + const { return: rowHeightUtils, updateHookArgs } = testCustomHook( + useRowHeightUtils, + mockArgs + ); + + updateHookArgs({ gridStyles: { ...startingStyles, cellPadding: 's' } }); + // @ts-ignore - intentionally inspecting private var for test + expect(rowHeightUtils.styles).toEqual({ + paddingTop: 4, + paddingBottom: 4, + }); + }); + + it('prunes columns from the row heights cache if a column is hidden', () => { + const { return: rowHeightUtils, updateHookArgs } = testCustomHook< + RowHeightUtils + >(useRowHeightUtils, mockArgs); + + act(() => { + rowHeightUtils.setRowHeight(0, 'A', 30, 0); + rowHeightUtils.setRowHeight(0, 'B', 50, 0); + }); + // @ts-ignore - intentionally inspecting private var for test + expect(rowHeightUtils.heightsCache).toMatchInlineSnapshot(` + Map { + 0 => Map { + "A" => 42, + "B" => 62, + }, + } + `); + + updateHookArgs({ columns: [{ id: 'A' }] }); // Hiding column B + + // @ts-ignore - intentionally inspecting private var for test + expect(rowHeightUtils.heightsCache).toMatchInlineSnapshot(` + Map { + 0 => Map { + "A" => 42, + }, + } + `); + }); +}); diff --git a/src/components/datagrid/utils/row_heights.ts b/src/components/datagrid/utils/row_heights.ts index 4dc44269a3e..6d313fcf4a4 100644 --- a/src/components/datagrid/utils/row_heights.ts +++ b/src/components/datagrid/utils/row_heights.ts @@ -16,6 +16,7 @@ import { } from 'react'; import type { VariableSizeGrid as Grid } from 'react-window'; import { isObject, isNumber } from '../../../services/predicate'; +import { useForceRender } from '../../../services'; import { EuiDataGridStyleCellPaddings, EuiDataGridStyle, @@ -96,8 +97,8 @@ export class RowHeightUtils { cacheStyles(gridStyles: EuiDataGridStyle) { this.styles = { - paddingTop: cellPaddingsMap[gridStyles.cellPadding!], - paddingBottom: cellPaddingsMap[gridStyles.cellPadding!], + paddingTop: cellPaddingsMap[gridStyles.cellPadding || 'm'], + paddingBottom: cellPaddingsMap[gridStyles.cellPadding || 'm'], }; } @@ -201,6 +202,9 @@ export class RowHeightUtils { rowHeights.set(colId, adaptedHeight); this.heightsCache.set(rowIndex, rowHeights); this.resetRow(visibleRowIndex); + + // When an auto row height is updated, force a re-render + // of the grid body to update the unconstrained height this.rerenderGridBody(); } @@ -246,24 +250,41 @@ export class RowHeightUtils { } /** - * Hook for instantiating RowHeightUtils, and also updating - * internal vars from outside props via useEffects + * Hook for instantiating RowHeightUtils, setting internal class vars, + * and setting up various row-height-related side effects */ export const useRowHeightUtils = ({ gridRef, gridStyles, columns, + rowHeightsOptions, }: { gridRef: Grid | null; gridStyles: EuiDataGridStyle; columns: EuiDataGridColumn[]; + rowHeightsOptions?: EuiDataGridRowHeightsOptions; }) => { const rowHeightUtils = useMemo(() => new RowHeightUtils(), []); - // Update rowHeightUtils with grid ref + // Update rowHeightUtils with internal vars from outside dependencies + const forceRender = useForceRender(); useEffect(() => { if (gridRef) rowHeightUtils.setGrid(gridRef); - }, [gridRef, rowHeightUtils]); + rowHeightUtils.setRerenderGridBody(forceRender); + }, [gridRef, forceRender, rowHeightUtils]); + + // Forces a rerender whenever the row heights change, as this can cause the + // grid to change height/have scrollbars. Without this, grid rerendering is stale + useEffect(() => { + requestAnimationFrame(forceRender); + }, [ + // Effects that should cause rerendering + rowHeightsOptions?.defaultHeight, + rowHeightsOptions?.rowHeights, + // Dependencies + rowHeightUtils, + forceRender, + ]); // Re-cache styles whenever grid density changes useEffect(() => { diff --git a/upcoming_changelogs/5712.md b/upcoming_changelogs/5712.md new file mode 100644 index 00000000000..e9bb7cd7fed --- /dev/null +++ b/upcoming_changelogs/5712.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed EuiDataGrid not rerendering correctly on row heights change