Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[tech debt] Address RowHeightUtils cell padding TODO
Browse files Browse the repository at this point in the history
- we can now obtain cell padding sizes from dynamic themes, hooray
cee-chen committed Sep 5, 2024

Verified

This commit was signed with the committer’s verified signature.
pdabelf5 Paul Abel
1 parent 45d61f1 commit 637a2ba
Showing 2 changed files with 48 additions and 28 deletions.
39 changes: 27 additions & 12 deletions packages/eui/src/components/datagrid/utils/row_heights.test.ts
Original file line number Diff line number Diff line change
@@ -12,7 +12,6 @@ import { renderHook } from '../../../test/rtl';
import { startingStyles } from '../controls';
import type { ImperativeGridApi } from '../data_grid_types';
import {
cellPaddingsMap,
RowHeightUtils,
RowHeightVirtualizationUtils,
useRowHeightUtils,
@@ -162,20 +161,35 @@ describe('RowHeightUtils', () => {

describe('styles utils', () => {
describe('cacheStyles', () => {
const mockCellPaddingsMap = {
s: '4px',
m: '6px',
l: '8px',
};

it('stores a styles instance variable based on the grid density', () => {
Object.entries(cellPaddingsMap).forEach(([densitySize, padding]) => {
rowHeightUtils.cacheStyles({ cellPadding: densitySize as any });

// @ts-ignore this var is private, but we're inspecting it for the sake of the unit test
expect(rowHeightUtils.styles).toEqual({
paddingTop: padding,
paddingBottom: padding,
});
});
Object.entries(mockCellPaddingsMap).forEach(
([densitySize, paddingStyle]) => {
const expectedPadding = parseFloat(paddingStyle);

rowHeightUtils.cacheStyles(
{ cellPadding: densitySize as any },
mockCellPaddingsMap
);
// @ts-ignore this var is private, but we're inspecting it for the sake of the unit test
expect(rowHeightUtils.styles).toEqual({
paddingTop: expectedPadding,
paddingBottom: expectedPadding,
});
}
);
});

it('falls back to m-sized cellPadding if gridStyle.cellPadding is undefined', () => {
rowHeightUtils.cacheStyles({ cellPadding: undefined });
rowHeightUtils.cacheStyles(
{ cellPadding: undefined },
mockCellPaddingsMap
);

// @ts-ignore this var is private, but we're inspecting it for the sake of the unit test
expect(rowHeightUtils.styles).toEqual({
@@ -220,9 +234,10 @@ describe('RowHeightUtils', () => {
describe('calculateHeightForLineCount', () => {
let getComputedStyleSpy: jest.SpyInstance;
const cell = document.createElement('div');
const mockCellPaddingsMap = { s: '', m: '6px', l: '' };

beforeEach(() => {
rowHeightUtils.cacheStyles({ cellPadding: 'm' });
rowHeightUtils.cacheStyles({ cellPadding: 'm' }, mockCellPaddingsMap);
getComputedStyleSpy = jest
.spyOn(window, 'getComputedStyle')
.mockReturnValue({ lineHeight: '24px' } as CSSStyleDeclaration);
37 changes: 21 additions & 16 deletions packages/eui/src/components/datagrid/utils/row_heights.ts
Original file line number Diff line number Diff line change
@@ -15,26 +15,23 @@ import {
useState,
} from 'react';
import { GridOnItemsRenderedProps } from 'react-window';
import { useForceRender, useLatest } from '../../../services';
import {
useForceRender,
useLatest,
useEuiMemoizedStyles,
} from '../../../services';
import { isNumber, isObject } from '../../../services/predicate';
import {
EuiDataGridColumn,
EuiDataGridRowHeightOption,
EuiDataGridRowHeightsOptions,
EuiDataGridScrollAnchorRow,
EuiDataGridStyle,
EuiDataGridStyleCellPaddings,
ImperativeGridApi,
} from '../data_grid_types';
import { euiDataGridVariables } from '../data_grid.styles';
import { DataGridSortedContext } from './sorting';

// TODO: Once JS variables are available, use them here instead of hard-coded maps
export const cellPaddingsMap: Record<EuiDataGridStyleCellPaddings, number> = {
s: 4,
m: 6,
l: 8,
};

export const AUTO_HEIGHT = 'auto';
export const DEFAULT_ROW_HEIGHT = 34;

@@ -99,10 +96,16 @@ export class RowHeightUtils {
paddingBottom: 0,
};

cacheStyles(gridStyles: EuiDataGridStyle) {
cacheStyles(
gridStyles: EuiDataGridStyle,
cellPaddingsMap: ReturnType<typeof euiDataGridVariables>['cellPadding']
) {
const paddingSize = gridStyles.cellPadding ?? 'm';
const padding = parseFloat(cellPaddingsMap[paddingSize]);

this.styles = {
paddingTop: cellPaddingsMap[gridStyles.cellPadding || 'm'],
paddingBottom: cellPaddingsMap[gridStyles.cellPadding || 'm'],
paddingTop: padding,
paddingBottom: padding,
};
}

@@ -378,11 +381,13 @@ export const useRowHeightUtils = ({
]);

// Re-cache styles whenever grid density changes
const styleVars = useEuiMemoizedStyles(euiDataGridVariables);
useEffect(() => {
rowHeightUtils.cacheStyles({
cellPadding: gridStyles.cellPadding,
});
}, [gridStyles.cellPadding, rowHeightUtils]);
rowHeightUtils.cacheStyles(
{ cellPadding: gridStyles.cellPadding },
styleVars.cellPadding
);
}, [gridStyles.cellPadding, rowHeightUtils, styleVars.cellPadding]);

// Update row heights map to remove hidden columns whenever orderedVisibleColumns change
useEffect(() => {

0 comments on commit 637a2ba

Please sign in to comment.