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.

Sign up for GitHub

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] Add rowHeightSwitcher logic + API change #5372

Merged
merged 31 commits into from
Nov 24, 2021
Merged

[EuiDataGrid] Add rowHeightSwitcher logic + API change #5372

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
916c1c7
Rename styleSelector to displaySelector + update types & docs
cee-chen Nov 9, 2021
79c878d
Refactor out nested object helper
cee-chen Nov 9, 2021
58fe94f
Update displaySelector with conditional density/row height options
cee-chen Nov 9, 2021
37aecd6
Add rowHeightsOptions controls to popover
cee-chen Nov 10, 2021
129b2bb
Add conditional lineCount row and logic
cee-chen Nov 10, 2021
742051d
Fix styles not applying correctly for rowHeightsOptions that have und…
cee-chen Nov 11, 2021
be59b08
Fix multiline content not top-aligning correctly when in single/undef…
cee-chen Nov 10, 2021
75e933a
Fix single/undefined row heights to account for passed lineHeight API…
cee-chen Nov 11, 2021
39aa176
Add changelog entry
cee-chen Nov 11, 2021
8580dc8
[PR feedback] Increase default lineCount to 2
cee-chen Nov 16, 2021
0a4eae8
Merge branch 'feat/datagrid/toolbar-reorg-and-row-height-switcher' in…
cee-chen Nov 17, 2021
7e34537
Fix changelog
cee-chen Nov 17, 2021
d31a66d
Fix control column appearance by switching them back to vertical cent…
cee-chen Nov 17, 2021
670dc2c
Tweak various height alignments on compressed grid settings
cee-chen Nov 17, 2021
c3f237d
Fix expand action button background color mismatch for auto/lineCount…
cee-chen Nov 17, 2021
4dac79e
PR feedback: convert cell actions CSS selectors to mixin
cee-chen Nov 18, 2021
cb51f97
Fix sasslint issues
cee-chen Nov 18, 2021
b70aa3d
PR feedback: Change line count number to EuiRange
cee-chen Nov 18, 2021
52d70e3
PR feedback - remove height tweaks for compressed auto fit
cee-chen Nov 18, 2021
3891366
[PR feedback] Remove `showStyleSelector`
cee-chen Nov 18, 2021
cab4a06
Remove unused unit tests
cee-chen Nov 18, 2021
571be25
[PR feedback] Intelligently disable toolbar control if all nested opt…
cee-chen Nov 18, 2021
9348b1d
Merge branch 'feat/datagrid/toolbar-reorg-and-row-height-switcher' in…
cee-chen Nov 19, 2021
3dd2d3f
Do not fall back to undefined/single for static heights
cee-chen Nov 23, 2021
0ab5aac
PR changelog feedback
constancecchen Nov 23, 2021
f226ad7
Update grid density to also intelligently detect initial state
cee-chen Nov 23, 2021
be0db10
Merge branch 'feat/datagrid/toolbar-reorg-and-row-height-switcher' in…
cee-chen Nov 23, 2021
fc9f65c
[PR feedback] Improve typing to avoid any usage
cee-chen Nov 24, 2021
4250a73
[PR feedback] Document recommendation for disabling row height switch…
cee-chen Nov 24, 2021
fc4105c
[Pr feedback] Simplify getNestedObjectOptions
cee-chen Nov 24, 2021
0b9ca21
Add workaround for failing Cypress tests
cee-chen Nov 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/components/datagrid/body/data_grid_cell.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,12 @@ describe('EuiDataGridCell', () => {
});
});

describe('recalculateLineCountHeight', () => {
describe('recalculateLineHeight', () => {
const setRowHeight = jest.fn();
beforeEach(() => setRowHeight.mockClear());

const callMethod = (component: ReactWrapper) =>
(component.instance() as any).recalculateLineCountHeight();
(component.instance() as any).recalculateLineHeight();

it('observes the first cell for size changes and calls this.props.setRowHeight on change', () => {
const component = mountEuiDataGridCellWithContext({
Expand All @@ -234,7 +234,20 @@ describe('EuiDataGridCell', () => {
expect(setRowHeight).toHaveBeenCalled();
});

it('does nothing if cell height is not set to lineCount', () => {
it('calculates undefined heights as single rows with a lineCount of 1', () => {
const component = mountEuiDataGridCellWithContext({
rowHeightsOptions: { defaultHeight: undefined },
setRowHeight,
});

callMethod(component);
expect(
mockRowHeightUtils.calculateHeightForLineCount
).toHaveBeenCalledWith(expect.any(HTMLElement), 1);
expect(setRowHeight).toHaveBeenCalled();
});

it('does nothing if cell height is not lineCount or undefined', () => {
const component = mountEuiDataGridCellWithContext({
rowHeightsOptions: { defaultHeight: 34 },
setRowHeight,
Expand Down
16 changes: 9 additions & 7 deletions src/components/datagrid/body/data_grid_cell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export class EuiDataGridCell extends Component<
}
};

recalculateLineCountHeight = () => {
recalculateLineHeight = () => {
if (!this.props.setRowHeight) return; // setRowHeight is only passed by data_grid_body into one cell per row
if (!this.cellContentsRef) return;

Expand All @@ -193,7 +193,10 @@ export class EuiDataGridCell extends Component<
rowIndex,
rowHeightsOptions
);
const lineCount = rowHeightUtils?.getLineCount(rowHeightOption);
const isSingleLine = rowHeightOption == null; // Undefined rowHeightsOptions default to a single line
const lineCount = isSingleLine
? 1
: rowHeightUtils?.getLineCount(rowHeightOption);
Comment on lines +196 to +199
Copy link
Contributor Author

@cee-chen cee-chen Nov 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This fix/workaround essentially has the single/undefined row heights act as lineCount of 1, which allows single rows to take advantage of lineCount's ability to get cell heights/font sizes/line heights and dynamically respond to density changes.

Before:

The non-dynamic height changing is even more noticeable now that single row heights are no longer center aligned (per previous commit/fix) and now that you can compare lineCount directly against undefined, which does correctly reset the minimum row height based on density.

before

After:

after

I initially played with an approach that used a static minRowHeight map depending on density and exporting that from the display selector - but I think this approach is much simpler/better because it's using the logic we already have, and also accounts for row/line height differences between the legacy theme and Amsterdam theme.

This also fixes an issue that occurs when switching from lineCount to undefined heights - the undefined heights' minRowHeight was not being correctly reset, so this approach basically kills two birds with one stone.


if (lineCount) {
const height = rowHeightUtils!.calculateHeightForLineCount(
Expand Down Expand Up @@ -231,11 +234,10 @@ export class EuiDataGridCell extends Component<
this.recalculateAutoHeight();

if (
// @ts-ignore - optional chaining operator handles types & cases that aren't lineCount
this.props.rowHeightsOptions?.defaultHeight?.lineCount !== // @ts-ignore - see above
prevProps.rowHeightsOptions?.defaultHeight?.lineCount
this.props.rowHeightsOptions?.defaultHeight !==
prevProps.rowHeightsOptions?.defaultHeight
) {
this.recalculateLineCountHeight();
this.recalculateLineHeight();
}

if (this.props.columnId !== prevProps.columnId) {
Expand Down Expand Up @@ -292,7 +294,7 @@ export class EuiDataGridCell extends Component<
if (ref && hasResizeObserver) {
this.contentObserver = new (window as any).ResizeObserver(() => {
this.recalculateAutoHeight();
this.recalculateLineCountHeight();
this.recalculateLineHeight();
});
this.contentObserver.observe(ref);
} else if (this.contentObserver) {
Expand Down