From 92a7d1eb811148468dd4c7f7b10e4ca45dc9ed2c Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 10:29:47 -0700 Subject: [PATCH 01/16] [Utils] Remove `getStylesForCell` - in favor of a more agnostic height-type util - We'll be replacing the line count styles with the new `EuiTextBlockTruncate` component - NOTE: the `height: 100%` style needs to be retained for `lineCount` and `numerical` height types, hence the new util that will be passed to a CSS class --- .../datagrid/utils/__mocks__/row_heights.ts | 6 +-- .../datagrid/utils/row_heights.test.ts | 42 +++++++------------ src/components/datagrid/utils/row_heights.ts | 35 ++++++---------- 3 files changed, 28 insertions(+), 55 deletions(-) diff --git a/src/components/datagrid/utils/__mocks__/row_heights.ts b/src/components/datagrid/utils/__mocks__/row_heights.ts index 0528c89d2cb..e06b4957d9d 100644 --- a/src/components/datagrid/utils/__mocks__/row_heights.ts +++ b/src/components/datagrid/utils/__mocks__/row_heights.ts @@ -24,11 +24,7 @@ export const RowHeightUtils = jest.fn().mockImplementation(() => { const rowHeightUtilsMock: RowHeightUtilsPublicAPI = { cacheStyles: jest.fn(), - getStylesForCell: jest.fn(() => ({ - wordWrap: 'break-word', - wordBreak: 'break-word', - flexGrow: 1, - })), + getHeightType: jest.fn(rowHeightUtils.getHeightType), isAutoHeight: jest.fn(() => false), setRowHeight: jest.fn(), pruneHiddenColumnHeights: jest.fn(), diff --git a/src/components/datagrid/utils/row_heights.test.ts b/src/components/datagrid/utils/row_heights.test.ts index 643ac31ef1a..a7b8c832a73 100644 --- a/src/components/datagrid/utils/row_heights.test.ts +++ b/src/components/datagrid/utils/row_heights.test.ts @@ -184,34 +184,22 @@ describe('RowHeightUtils', () => { }); }); }); + }); - describe('getStylesForCell (returns inline CSS styles based on height config)', () => { - describe('auto height', () => { - it('returns empty styles object', () => { - expect( - rowHeightUtils.getStylesForCell({ defaultHeight: 'auto' }, 0) - ).toEqual({}); - }); - }); - - describe('lineCount height', () => { - it('returns line-clamp CSS', () => { - expect( - rowHeightUtils.getStylesForCell( - { defaultHeight: { lineCount: 5 } }, - 0 - ) - ).toEqual(expect.objectContaining({ WebkitLineClamp: 5 })); - }); - }); - - describe('numeric heights', () => { - it('returns default CSS', () => { - expect( - rowHeightUtils.getStylesForCell({ defaultHeight: 34 }, 0) - ).toEqual({ height: '100%', overflow: 'hidden' }); - }); - }); + describe('getHeightType', () => { + it('returns a string enum based on rowHeightsOptions', () => { + expect(rowHeightUtils.getHeightType(undefined)).toEqual('default'); + expect(rowHeightUtils.getHeightType('auto')).toEqual('auto'); + expect(rowHeightUtils.getHeightType({ lineCount: 3 })).toEqual( + 'lineCount' + ); + expect(rowHeightUtils.getHeightType({ lineCount: 0 })).toEqual( + 'lineCount' + ); + expect(rowHeightUtils.getHeightType({ height: 100 })).toEqual( + 'numerical' + ); + expect(rowHeightUtils.getHeightType(100)).toEqual('numerical'); }); }); diff --git a/src/components/datagrid/utils/row_heights.ts b/src/components/datagrid/utils/row_heights.ts index 3aed6933c4b..0c82650e390 100644 --- a/src/components/datagrid/utils/row_heights.ts +++ b/src/components/datagrid/utils/row_heights.ts @@ -7,7 +7,6 @@ */ import { - CSSProperties, MutableRefObject, useCallback, useContext, @@ -107,31 +106,21 @@ export class RowHeightUtils { }; } - getStylesForCell = ( - rowHeightsOptions: EuiDataGridRowHeightsOptions, - rowIndex: number - ): CSSProperties => { - const height = this.getRowHeightOption(rowIndex, rowHeightsOptions); + /** + * Height types + */ - if (height === AUTO_HEIGHT) { - return {}; + getHeightType = (option?: EuiDataGridRowHeightOption) => { + if (option == null) { + return 'default'; } - - const lineCount = this.getLineCount(height); - if (lineCount) { - return { - WebkitLineClamp: lineCount, - display: '-webkit-box', - WebkitBoxOrient: 'vertical', - height: '100%', - overflow: 'hidden', - }; + if (option === AUTO_HEIGHT) { + return 'auto'; } - - return { - height: '100%', - overflow: 'hidden', - }; + if (this.getLineCount(option) != null) { + return 'lineCount'; + } + return 'numerical'; }; /** From 953e6f7a3387351a552aa8338f24e5cd0aa3bd1d Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 10:37:27 -0700 Subject: [PATCH 02/16] Convert height-specific styles to CSS - Remove unnecessary `flex-grow: 1` - isn't doing anything + start the process of moving out the text truncation/breaking CSS to their className utils --- .../datagrid/_data_grid_data_row.scss | 7 +++--- .../datagrid/body/data_grid_cell.tsx | 25 +++++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index cfaa2a9095b..6bb260d85b7 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -96,9 +96,10 @@ text-transform: capitalize; } - .euiDataGridRowCell__definedHeight { - @include euiTextBreakWord; - flex-grow: 1; + .euiDataGridRowCell__lineCountHeight, + .euiDataGridRowCell__numericalHeight { + height: 100%; + overflow: hidden; } // We only truncate if the cell is not a control column. diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 0cdb33969fa..1e145e8529e 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -69,21 +69,26 @@ const EuiDataGridCellContent: FunctionComponent< const CellElement = renderCellValue as JSXElementConstructor; + const rowHeightOption = rowHeightUtils?.getRowHeightOption( + rowIndex, + rowHeightsOptions + ); + const cellHeightType = rowHeightUtils?.getHeightType(rowHeightOption); + + const classes = classNames( + `euiDataGridRowCell__${cellHeightType}Height`, + { + 'eui-textBreakWord': cellHeightType !== 'default', + 'euiDataGridRowCell__truncate': cellHeightType === 'default', // TODO: Convert to .eui-textTruncate + } + ); + return ( <>
Date: Wed, 4 Oct 2023 10:41:21 -0700 Subject: [PATCH 03/16] Convert `isControlColumn` logic from CSS to JS - allows us to remove torturous selector and repeat CSS in favor of using explicit className utils - `isDefinedHeight` is no longer used, remove it --- src/components/datagrid/_data_grid_data_row.scss | 14 ++------------ src/components/datagrid/body/data_grid_cell.tsx | 12 +++++++----- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 6bb260d85b7..1cf024a4f77 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -101,18 +101,6 @@ height: 100%; overflow: hidden; } - - // We only truncate if the cell is not a control column. - &:not(.euiDataGridRowCell--controlColumn) { - .euiDataGridRowCell__content, - .euiDataGridRowCell__truncate, - &.euiDataGridRowCell__truncate, - .euiDataGridRowCell__expandContent { - @include euiTextTruncate; - overflow: hidden; - white-space: nowrap; - } - } } .euiDataGridRowCell__popover { @@ -144,6 +132,8 @@ .euiDataGridRowCell__expandContent { flex-grow: 1; + max-width: 100%; + overflow: hidden; } .euiDataGridRowCell__contentByHeight { diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 1e145e8529e..43a217a93f6 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -47,7 +47,7 @@ const EuiDataGridCellContent: FunctionComponent< setCellProps: EuiDataGridCellValueElementProps['setCellProps']; setCellContentsRef: EuiDataGridCell['setCellContentsRef']; isExpanded: boolean; - isDefinedHeight: boolean; + isControlColumn: boolean; isFocused: boolean; ariaRowIndex: number; } @@ -61,7 +61,7 @@ const EuiDataGridCellContent: FunctionComponent< colIndex, ariaRowIndex, rowHeightUtils, - isDefinedHeight, + isControlColumn, isFocused, ...rest }) => { @@ -77,9 +77,9 @@ const EuiDataGridCellContent: FunctionComponent< const classes = classNames( `euiDataGridRowCell__${cellHeightType}Height`, - { + !isControlColumn && { 'eui-textBreakWord': cellHeightType !== 'default', - 'euiDataGridRowCell__truncate': cellHeightType === 'default', // TODO: Convert to .eui-textTruncate + 'eui-textTruncate': cellHeightType === 'default', } ); @@ -673,7 +673,9 @@ export class EuiDataGridCell extends Component< setCellContentsRef: this.setCellContentsRef, rowHeightsOptions, rowHeightUtils, - isDefinedHeight, + isControlColumn: cellClasses.includes( + 'euiDataGridRowCell--controlColumn' + ), ariaRowIndex, }; From 5957ca22ed6fa33e55d70f239c9d2eeaecbc985e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 10:43:07 -0700 Subject: [PATCH 04/16] Update grid cells to dogfood `EuiTextBlockTruncate` --- .../datagrid/body/data_grid_cell.tsx | 73 ++++++++++++------- 1 file changed, 45 insertions(+), 28 deletions(-) diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 43a217a93f6..368b73ae2e2 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -24,6 +24,7 @@ import { keys } from '../../../services'; import { EuiScreenReaderOnly } from '../../accessibility'; import { EuiFocusTrap } from '../../focus_trap'; import { EuiI18n } from '../../i18n'; +import { EuiTextBlockTruncate } from '../../text_truncate'; import { hasResizeObserver } from '../../observer/resize_observer/resize_observer'; import { DataGridFocusContext } from '../utils/focus'; import { RowHeightVirtualizationUtils } from '../utils/row_heights'; @@ -83,36 +84,52 @@ const EuiDataGridCellContent: FunctionComponent< } ); + let cellContent = ( +
+ +
+ ); + if (cellHeightType === 'lineCount' && !isControlColumn) { + const lines = rowHeightUtils!.getLineCount(rowHeightOption)!; + cellContent = ( + + {cellContent} + + ); + } + + const screenReaderText = ( + + + + ); + return ( <> -
- -
- - - + {cellContent} + {screenReaderText} ); } From 7ed08d06400f253fac05c603931dd9998ce3c543 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 10:59:20 -0700 Subject: [PATCH 05/16] Update datagrid rendered tests & snapshots --- .../__snapshots__/data_grid.test.tsx.snap | 60 ++++++------ .../data_grid_body_custom.test.tsx.snap | 8 +- .../data_grid_body_virtualized.test.tsx.snap | 4 +- .../data_grid_cell.test.tsx.snap | 2 +- .../datagrid/body/data_grid_cell.test.tsx | 93 ++++++++++++++++--- 5 files changed, 117 insertions(+), 50 deletions(-) diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index 85841317fe0..a97458d2308 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -730,7 +730,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` class="euiDataGridRowCell__expandContent" >
0, A @@ -764,7 +764,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` class="euiDataGridRowCell__expandContent" >
0, B @@ -798,7 +798,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` class="euiDataGridRowCell__expandContent" >
1, A @@ -832,7 +832,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` class="euiDataGridRowCell__expandContent" >
1, B @@ -866,7 +866,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` class="euiDataGridRowCell__expandContent" >
2, A @@ -900,7 +900,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` class="euiDataGridRowCell__expandContent" >
2, B @@ -1257,7 +1257,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
0 @@ -1297,7 +1297,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
0, A @@ -1331,7 +1331,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
0, B @@ -1373,7 +1373,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
0 @@ -1421,7 +1421,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
1 @@ -1461,7 +1461,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
1, A @@ -1495,7 +1495,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
1, B @@ -1537,7 +1537,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
1 @@ -1585,7 +1585,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
2 @@ -1625,7 +1625,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
2, A @@ -1659,7 +1659,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
2, B @@ -1701,7 +1701,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` class="euiDataGridRowCell__expandContent" >
2 @@ -2017,7 +2017,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` class="euiDataGridRowCell__expandContent" >
0, A @@ -2051,7 +2051,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` class="euiDataGridRowCell__expandContent" >
0, B @@ -2085,7 +2085,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` class="euiDataGridRowCell__expandContent" >
1, A @@ -2119,7 +2119,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` class="euiDataGridRowCell__expandContent" >
1, B @@ -2153,7 +2153,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` class="euiDataGridRowCell__expandContent" >
2, A @@ -2187,7 +2187,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` class="euiDataGridRowCell__expandContent" >
2, B @@ -2495,7 +2495,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` class="euiDataGridRowCell__expandContent" >
0, A @@ -2529,7 +2529,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` class="euiDataGridRowCell__expandContent" >
0, B @@ -2563,7 +2563,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` class="euiDataGridRowCell__expandContent" >
1, A @@ -2597,7 +2597,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` class="euiDataGridRowCell__expandContent" >
1, B @@ -2631,7 +2631,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` class="euiDataGridRowCell__expandContent" >
2, A @@ -2665,7 +2665,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` class="euiDataGridRowCell__expandContent" >
2, B diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap index bce634a36b1..524806b2335 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap @@ -140,7 +140,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render class="euiDataGridRowCell__expandContent" >
hello @@ -174,7 +174,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render class="euiDataGridRowCell__expandContent" >
world @@ -212,7 +212,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render class="euiDataGridRowCell__expandContent" >
lorem @@ -246,7 +246,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render class="euiDataGridRowCell__expandContent" >
ipsum diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap index 2e2fa824dc3..1120e653d8c 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap @@ -141,7 +141,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = ` class="euiDataGridRowCell__expandContent" >
@@ -177,7 +177,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = ` class="euiDataGridRowCell__expandContent" >
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap index 3282d55d703..c6f97d7c98f 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap @@ -55,7 +55,7 @@ exports[`EuiDataGridCell renders 1`] = ` class="euiDataGridRowCell__expandContent" >
diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/data_grid_cell.test.tsx index 6aa83e8677b..c606a777676 100644 --- a/src/components/datagrid/body/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/data_grid_cell.test.tsx @@ -718,19 +718,86 @@ describe('EuiDataGridCell', () => { }); }); - it('renders certain classes/styles if rowHeightOptions is passed', () => { - const component = mount( - - ); + describe('renders certain classes/styles based on rowHeightOptions', () => { + const props = { ...requiredProps, renderCellValue: () => null }; - expect( - component.find('.euiDataGridRowCell__contentByHeight').exists() - ).toBe(true); + test('default', () => { + const component = mount( + + ); + + expect( + component.find('.euiDataGridRowCell__expandContent').exists() + ).toBe(true); + expect( + component.find('.euiDataGridRowCell__contentByHeight').exists() + ).not.toBe(true); + + expect(component.find('.euiDataGridRowCell__defaultHeight').render()) + .toMatchInlineSnapshot(` +
+ `); + }); + + test('auto', () => { + const component = mount( + + ); + + expect( + component.find('.euiDataGridRowCell__expandContent').exists() + ).not.toBe(true); + expect( + component.find('.euiDataGridRowCell__contentByHeight').exists() + ).toBe(true); + + expect(component.find('.euiDataGridRowCell__autoHeight').render()) + .toMatchInlineSnapshot(` +
+ `); + }); + + test('numerical', () => { + const component = mount( + + ); + + expect(component.find('.euiDataGridRowCell__numericalHeight').render()) + .toMatchInlineSnapshot(` +
+ `); + }); + + test('lineCount', () => { + const component = mount( + + ); + + expect(component.find('div.euiDataGridRowCell__lineCountHeight').render()) + .toMatchInlineSnapshot(` +
+ `); + }); }); }); From 362d480d212adb78230b49aadf8854e28a96a963 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 12:05:05 -0700 Subject: [PATCH 06/16] =?UTF-8?q?=F0=9F=94=A5=20Remove=20unnecssary=20CSS?= =?UTF-8?q?=20on=20lineCount?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `overflow` is already set by text truncate component - `height: 100%` doesn't appear to do anything or be useful. if for some reason it is needed in the future, we can always re-add it --- src/components/datagrid/_data_grid_data_row.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 1cf024a4f77..a826a935d88 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -96,7 +96,6 @@ text-transform: capitalize; } - .euiDataGridRowCell__lineCountHeight, .euiDataGridRowCell__numericalHeight { height: 100%; overflow: hidden; From ad6894b76ac80b0d08d4d7b91fb50536c2f381b8 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 14:39:11 -0700 Subject: [PATCH 07/16] =?UTF-8?q?=F0=9F=94=A5=20Remove=20convoluted=20mixi?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - in favor of passing condition/setting conditional styling logic directly in the cell actions component --- .../datagrid/_data_grid_data_row.scss | 21 ++++++++++--------- src/components/datagrid/_mixins.scss | 15 ------------- .../datagrid/body/data_grid_cell.tsx | 8 +++++++ .../body/data_grid_cell_actions.test.tsx | 17 +++++++++++++-- .../datagrid/body/data_grid_cell_actions.tsx | 14 ++++++++----- 5 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index a826a935d88..805e91f94a4 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -141,18 +141,18 @@ } // Cell actions -.euiDataGridRowCell__expandActions { +.euiDataGridRowCell__actions { display: flex; } -@include euiDataGridRowCellActions($definedHeight: false) { +.euiDataGridRowCell__actions--flex { flex-grow: 0; } -@include euiDataGridRowCellActions($definedHeight: true) { - background-color: $euiColorEmptyShade; +.euiDataGridRowCell__actions--overlay { position: absolute; right: 0; top: 0; padding: $euiDataGridCellPaddingM 0; + background-color: $euiColorEmptyShade; } .euiDataGridRowCell__actionButtonIcon { @@ -171,20 +171,20 @@ // Row stripes @include euiDataGridStyles(stripes) { .euiDataGridRow--striped { - @include euiDataGridRowCellActions($definedHeight: true) { + &, + .euiDataGridRowCell__actions--overlay { background-color: $euiColorLightestShade; } - background-color: $euiColorLightestShade; } } // Row highlights @include euiDataGridStyles(rowHoverHighlight) { .euiDataGridRow:hover { - @include euiDataGridRowCellActions($definedHeight: true) { + &, + .euiDataGridRowCell__actions--overlay { background-color: $euiColorHighlight; } - background-color: $euiColorHighlight; } } @@ -230,10 +230,11 @@ // Compressed density grids - height tweaks @include euiDataGridStyles(fontSizeSmall, paddingSmall) { - @include euiDataGridRowCellActions($definedHeight: true) { + .euiDataGridRowCell__actions--overlay { padding: ($euiDataGridCellPaddingS / 2) 0; } - @include euiDataGridRowCellActions($definedHeight: false) { + + .euiDataGridRowCell__actions--flex { transform: translateY(1px); } } diff --git a/src/components/datagrid/_mixins.scss b/src/components/datagrid/_mixins.scss index 1a70c21cc97..4303730770c 100644 --- a/src/components/datagrid/_mixins.scss +++ b/src/components/datagrid/_mixins.scss @@ -82,18 +82,3 @@ $euiDataGridStyles: ( @content; } } - -@mixin euiDataGridRowCellActions($definedHeight: false) { - @if $definedHeight { - // Defined heights are cells with row heights of auto, lineCount, or a static height - // that set the __contentByHeight class - .euiDataGridRowCell__contentByHeight + .euiDataGridRowCell__expandActions { - @content; - } - } @else { - // Otherwise, an undefined height (single flex row) will set __expandContent - .euiDataGridRowCell__expandContent + .euiDataGridRowCell__expandActions { - @content; - } - } -} diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 368b73ae2e2..1327a1fe15a 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -678,6 +678,13 @@ export class EuiDataGridCell extends Component< rowHeightsOptions ); + const rowHeightOption = rowHeightUtils?.getRowHeightOption( + rowIndex, + rowHeightsOptions + ); + const cellHeightType = + rowHeightUtils?.getHeightType(rowHeightOption) || 'default'; + const cellContentProps = { ...rest, setCellProps: this.setCellProps, @@ -729,6 +736,7 @@ export class EuiDataGridCell extends Component< rowIndex={rowIndex} colIndex={colIndex} column={column} + cellHeightType={cellHeightType} onExpandClick={() => { if (popoverIsOpen) { closeCellPopover(); diff --git a/src/components/datagrid/body/data_grid_cell_actions.test.tsx b/src/components/datagrid/body/data_grid_cell_actions.test.tsx index e14f235f60c..3b2a11b026b 100644 --- a/src/components/datagrid/body/data_grid_cell_actions.test.tsx +++ b/src/components/datagrid/body/data_grid_cell_actions.test.tsx @@ -8,6 +8,7 @@ import React from 'react'; import { shallow } from 'enzyme'; +import { render } from '../../../test/rtl'; import { EuiDataGridColumnCellAction } from '../data_grid_types'; import { @@ -24,6 +25,7 @@ describe('EuiDataGridCellActions', () => { onExpandClick: jest.fn(), rowIndex: 0, colIndex: 0, + cellHeightType: 'default', }; it('renders an expand button', () => { @@ -31,7 +33,7 @@ describe('EuiDataGridCellActions', () => { expect(component).toMatchInlineSnapshot(`
{ expect(component).toMatchInlineSnapshot(`
{ `); }); + it('renders with overlay positioning for non default height cells', () => { + const { container } = render( + + ); + + // TODO: Switch to `.toHaveStyle({ position: 'absolute' })` once on Emotion + expect(container.firstChild).toHaveClass( + 'euiDataGridRowCell__actions--overlay' + ); + }); + describe('visible cell actions limit', () => { it('by default, does not render more than the first two primary cell actions', () => { const component = shallow( diff --git a/src/components/datagrid/body/data_grid_cell_actions.tsx b/src/components/datagrid/body/data_grid_cell_actions.tsx index 21bac608c36..4e7d9830a9f 100644 --- a/src/components/datagrid/body/data_grid_cell_actions.tsx +++ b/src/components/datagrid/body/data_grid_cell_actions.tsx @@ -18,17 +18,20 @@ import { EuiButtonIcon, EuiButtonIconProps } from '../../button/button_icon'; import { EuiButtonEmpty, EuiButtonEmptyProps } from '../../button/button_empty'; import { EuiFlexGroup, EuiFlexItem } from '../../flex'; import { EuiPopoverFooter } from '../../popover'; +import classNames from 'classnames'; export const EuiDataGridCellActions = ({ onExpandClick, column, rowIndex, colIndex, + cellHeightType, }: { onExpandClick: () => void; column?: EuiDataGridColumn; rowIndex: number; colIndex: number; + cellHeightType: string; }) => { // Note: The cell expand button/expansion popover is *always* rendered if // column.cellActions is present (regardless of column.isExpandable). @@ -91,11 +94,12 @@ export const EuiDataGridCellActions = ({ ); }, [column, colIndex, rowIndex]); - return ( -
- {[...additionalButtons, expandButton]} -
- ); + const classes = classNames('euiDataGridRowCell__actions', { + 'euiDataGridRowCell__actions--flex': cellHeightType === 'default', + 'euiDataGridRowCell__actions--overlay': cellHeightType !== 'default', + }); + + return
{[...additionalButtons, expandButton]}
; }; export const EuiDataGridCellPopoverActions = ({ From 905766398481b2caf69e0c83310836c6abbf9e04 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 14:42:54 -0700 Subject: [PATCH 08/16] =?UTF-8?q?=F0=9F=94=A5=20DRY=20out=20`cellHeightsTy?= =?UTF-8?q?pe`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - pass var(s) instead of re-getting it twice --- .../datagrid/body/data_grid_cell.tsx | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 1327a1fe15a..c90131e90f5 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -35,6 +35,7 @@ import { EuiDataGridCellValueElementProps, EuiDataGridCellValueProps, EuiDataGridCellPopoverElementProps, + EuiDataGridRowHeightOption, } from '../data_grid_types'; import { EuiDataGridCellActions, @@ -51,31 +52,28 @@ const EuiDataGridCellContent: FunctionComponent< isControlColumn: boolean; isFocused: boolean; ariaRowIndex: number; + rowHeight?: EuiDataGridRowHeightOption; + cellHeightType: string; } > = memo( ({ renderCellValue, column, setCellContentsRef, - rowHeightsOptions, rowIndex, colIndex, ariaRowIndex, + rowHeight, rowHeightUtils, isControlColumn, isFocused, + cellHeightType, ...rest }) => { // React is more permissible than the TS types indicate const CellElement = renderCellValue as JSXElementConstructor; - const rowHeightOption = rowHeightUtils?.getRowHeightOption( - rowIndex, - rowHeightsOptions - ); - const cellHeightType = rowHeightUtils?.getHeightType(rowHeightOption); - const classes = classNames( `euiDataGridRowCell__${cellHeightType}Height`, !isControlColumn && { @@ -101,7 +99,7 @@ const EuiDataGridCellContent: FunctionComponent<
); if (cellHeightType === 'lineCount' && !isControlColumn) { - const lines = rowHeightUtils!.getLineCount(rowHeightOption)!; + const lines = rowHeightUtils!.getLineCount(rowHeight)!; cellContent = ( {cellContent} @@ -678,24 +676,25 @@ export class EuiDataGridCell extends Component< rowHeightsOptions ); - const rowHeightOption = rowHeightUtils?.getRowHeightOption( + const rowHeight = rowHeightUtils?.getRowHeightOption( rowIndex, rowHeightsOptions ); const cellHeightType = - rowHeightUtils?.getHeightType(rowHeightOption) || 'default'; + rowHeightUtils?.getHeightType(rowHeight) || 'default'; const cellContentProps = { ...rest, setCellProps: this.setCellProps, column, columnType, + cellHeightType, isExpandable, isExpanded: popoverIsOpen, isDetails: false, isFocused: this.state.isFocused, setCellContentsRef: this.setCellContentsRef, - rowHeightsOptions, + rowHeight, rowHeightUtils, isControlColumn: cellClasses.includes( 'euiDataGridRowCell--controlColumn' From 2f6a07bdc29d65cf110973066345f1fa62850412 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 15:06:31 -0700 Subject: [PATCH 09/16] =?UTF-8?q?=F0=9F=94=A5=20Clean=20up=20cell=20conten?= =?UTF-8?q?t=20DOM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - move all logic related to content to the `EuiDataGridCellContent` component, including the wrapper let the parent cell pass down any needed refs + actions --- .../datagrid/body/data_grid_cell.tsx | 70 +++++++++---------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index c90131e90f5..f9ee2df0815 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -17,6 +17,7 @@ import React, { KeyboardEvent, memo, MutableRefObject, + ReactNode, } from 'react'; import { createPortal } from 'react-dom'; import { tabbable } from 'tabbable'; @@ -48,18 +49,21 @@ const EuiDataGridCellContent: FunctionComponent< EuiDataGridCellValueProps & { setCellProps: EuiDataGridCellValueElementProps['setCellProps']; setCellContentsRef: EuiDataGridCell['setCellContentsRef']; + setPopoverAnchorRef: MutableRefObject; isExpanded: boolean; isControlColumn: boolean; isFocused: boolean; ariaRowIndex: number; rowHeight?: EuiDataGridRowHeightOption; cellHeightType: string; + cellActions?: ReactNode; } > = memo( ({ renderCellValue, column, setCellContentsRef, + setPopoverAnchorRef, rowIndex, colIndex, ariaRowIndex, @@ -68,12 +72,16 @@ const EuiDataGridCellContent: FunctionComponent< isControlColumn, isFocused, cellHeightType, + cellActions, ...rest }) => { // React is more permissible than the TS types indicate const CellElement = renderCellValue as JSXElementConstructor; + // TODO: Clean up expand/content by height shenanigans + const wrapperClasses = classNames(); + const classes = classNames( `euiDataGridRowCell__${cellHeightType}Height`, !isControlColumn && { @@ -125,10 +133,11 @@ const EuiDataGridCellContent: FunctionComponent< ); return ( - <> +
{cellContent} {screenReaderText} - + {cellActions} +
); } ); @@ -671,11 +680,6 @@ export class EuiDataGridCell extends Component< } }; - const isDefinedHeight = !!rowHeightUtils?.getRowHeightOption( - rowIndex, - rowHeightsOptions - ); - const rowHeight = rowHeightUtils?.getRowHeightOption( rowIndex, rowHeightsOptions @@ -694,6 +698,7 @@ export class EuiDataGridCell extends Component< isDetails: false, isFocused: this.state.isFocused, setCellContentsRef: this.setCellContentsRef, + setPopoverAnchorRef: this.popoverAnchorRef, rowHeight, rowHeightUtils, isControlColumn: cellClasses.includes( @@ -702,11 +707,6 @@ export class EuiDataGridCell extends Component< ariaRowIndex, }; - const anchorClass = 'euiDataGridRowCell__expandFlex'; - const expandClass = isDefinedHeight - ? 'euiDataGridRowCell__contentByHeight' - : 'euiDataGridRowCell__expandContent'; - let innerContent = ( -
-
- -
-
+ ); if (isExpandable) { innerContent = ( -
-
- -
- {showCellActions && ( - { - if (popoverIsOpen) { - closeCellPopover(); - } else { - openCellPopover({ rowIndex: visibleRowIndex, colIndex }); - } - }} - /> - )} -
+ { + if (popoverIsOpen) { + closeCellPopover(); + } else { + openCellPopover({ rowIndex: visibleRowIndex, colIndex }); + } + }} + /> + ) + } + /> ); } From 2247d9fec20521fbe1d5784cee0453a2816c8de5 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 15:09:59 -0700 Subject: [PATCH 10/16] =?UTF-8?q?=F0=9F=94=A5=20Clean=20up=20cell=20conten?= =?UTF-8?q?t=20CSS=20classes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remove extremely confusing `__expand` class names in favor of using the height type (that determines the kind of behavior the content has, e.g. flex vs absolutely positioned, etc) - Make sure only the heights that need certain CSS have that CSS (the non-default-height cells don't need display flex) --- .../datagrid/_data_grid_data_row.scss | 34 +++++------- .../datagrid/body/data_grid_cell.test.tsx | 55 +++++-------------- .../datagrid/body/data_grid_cell.tsx | 8 ++- 3 files changed, 34 insertions(+), 63 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 805e91f94a4..bf027b52bb7 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -46,7 +46,7 @@ * * TODO: Remove this workaround once https://bugs.webkit.org/show_bug.cgi?id=258539 is resolved */ - .euiDataGridRowCell__expandContent { + .euiDataGridRowCell__defaultHeight .euiDataGridRowCell__content { animation-name: euiDataGridCellActionsSafariWorkaround; animation-duration: 1000ms; // I don't know why the duration matters or why it being longer works more consistently 🥲 animation-delay: $euiAnimSpeedNormal + $euiAnimSpeedExtraFast; // Wait for above animation to finish @@ -95,11 +95,6 @@ &.euiDataGridRowCell--capitalize { text-transform: capitalize; } - - .euiDataGridRowCell__numericalHeight { - height: 100%; - overflow: hidden; - } } .euiDataGridRowCell__popover { @@ -118,28 +113,27 @@ @include euiBottomShadow; // TODO: Convert to euiShadowMedium() in Emotion } -.euiDataGridRowCell__expandFlex { - position: relative; // for positioning expand button +.euiDataGridRowCell__contentWrapper { + position: relative; // Needed for .euiDataGridRowCell__actions--overlay + height: 100%; + overflow: hidden; +} + +.euiDataGridRowCell__defaultHeight { display: flex; align-items: baseline; - height: 100%; + max-width: 100%; + + .euiDataGridRowCell__content { + flex-grow: 1; + } .euiDataGridRowCell--controlColumn & { + height: 100%; align-items: center; } } -.euiDataGridRowCell__expandContent { - flex-grow: 1; - max-width: 100%; - overflow: hidden; -} - -.euiDataGridRowCell__contentByHeight { - flex-grow: 1; - height: 100%; -} - // Cell actions .euiDataGridRowCell__actions { display: flex; diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/data_grid_cell.test.tsx index c606a777676..0271566e937 100644 --- a/src/components/datagrid/body/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/data_grid_cell.test.tsx @@ -727,19 +727,9 @@ describe('EuiDataGridCell', () => { ); expect( - component.find('.euiDataGridRowCell__expandContent').exists() + component.find('.euiDataGridRowCell__defaultHeight').exists() ).toBe(true); - expect( - component.find('.euiDataGridRowCell__contentByHeight').exists() - ).not.toBe(true); - - expect(component.find('.euiDataGridRowCell__defaultHeight').render()) - .toMatchInlineSnapshot(` -
- `); + expect(component.find('.eui-textTruncate').exists()).toBe(true); }); test('auto', () => { @@ -750,20 +740,10 @@ describe('EuiDataGridCell', () => { /> ); - expect( - component.find('.euiDataGridRowCell__expandContent').exists() - ).not.toBe(true); - expect( - component.find('.euiDataGridRowCell__contentByHeight').exists() - ).toBe(true); - - expect(component.find('.euiDataGridRowCell__autoHeight').render()) - .toMatchInlineSnapshot(` -
- `); + expect(component.find('.euiDataGridRowCell__autoHeight').exists()).toBe( + true + ); + expect(component.find('.eui-textBreakWord').exists()).toBe(true); }); test('numerical', () => { @@ -774,13 +754,10 @@ describe('EuiDataGridCell', () => { /> ); - expect(component.find('.euiDataGridRowCell__numericalHeight').render()) - .toMatchInlineSnapshot(` -
- `); + expect( + component.find('.euiDataGridRowCell__numericalHeight').exists() + ).toBe(true); + expect(component.find('.eui-textBreakWord').exists()).toBe(true); }); test('lineCount', () => { @@ -791,13 +768,11 @@ describe('EuiDataGridCell', () => { /> ); - expect(component.find('div.euiDataGridRowCell__lineCountHeight').render()) - .toMatchInlineSnapshot(` -
- `); + expect( + component.find('.euiDataGridRowCell__lineCountHeight').exists() + ).toBe(true); + expect(component.find('.eui-textBreakWord').exists()).toBe(true); + expect(component.find('.euiTextBlockTruncate').exists()).toBe(true); }); }); }); diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index f9ee2df0815..ca27c5cd855 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -79,11 +79,13 @@ const EuiDataGridCellContent: FunctionComponent< const CellElement = renderCellValue as JSXElementConstructor; - // TODO: Clean up expand/content by height shenanigans - const wrapperClasses = classNames(); + const wrapperClasses = classNames( + 'euiDataGridRowCell__contentWrapper', + `euiDataGridRowCell__${cellHeightType}Height` + ); const classes = classNames( - `euiDataGridRowCell__${cellHeightType}Height`, + 'euiDataGridRowCell__content', !isControlColumn && { 'eui-textBreakWord': cellHeightType !== 'default', 'eui-textTruncate': cellHeightType === 'default', From bae819e0e64d5949bf1872c930355bad538f7663 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 15:12:11 -0700 Subject: [PATCH 11/16] Update snapshots --- .../__snapshots__/data_grid.test.tsx.snap | 780 ++++++++---------- .../data_grid_body_custom.test.tsx.snap | 104 +-- .../data_grid_body_virtualized.test.tsx.snap | 60 +- .../data_grid_cell.test.tsx.snap | 50 +- 4 files changed, 423 insertions(+), 571 deletions(-) diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index a97458d2308..1955c97a301 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -724,25 +724,21 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` tabindex="-1" >
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
@@ -1251,25 +1227,21 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` data-focus-lock-disabled="disabled" >
-
- 0 -
- + 0
+
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 0 -
- + 0
+
-
- 1 -
- + 1
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 1 -
- + 1
+
-
- 2 -
- + 2
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
-
- 2 -
- + 2
+
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
@@ -2489,25 +2393,21 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` tabindex="-1" >
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap index 524806b2335..029ed6665a9 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap @@ -134,25 +134,21 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render tabindex="-1" >
-
- hello -
- + hello
+
-
- world -
- + world
+
@@ -206,25 +198,21 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render tabindex="-1" >
-
- lorem -
- + lorem
+
-
- ipsum -
- + ipsum
+
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap index 1120e653d8c..7251f9857ab 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap @@ -135,27 +135,23 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = ` tabindex="-1" >
-
- - cell content - -
- + + cell content +
+
-
- - cell content - -
- + + cell content +
+
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap index c6f97d7c98f..019cda14347 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap @@ -49,38 +49,34 @@ exports[`EuiDataGridCell renders 1`] = ` tabindex="-1" >
-
-
- - -
+
+ +
-
+
`; From 2a4329d18ec627a57a13c791921fc405d7fd3e6a Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 15:13:24 -0700 Subject: [PATCH 12/16] [nit] var naming and syntax cleanup --- .../datagrid/body/data_grid_cell.tsx | 53 ++++++++----------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index ca27c5cd855..6b4dbc7afb3 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -709,7 +709,25 @@ export class EuiDataGridCell extends Component< ariaRowIndex, }; - let innerContent = ( + const cellActions = showCellActions && ( + { + if (popoverIsOpen) { + closeCellPopover(); + } else { + openCellPopover({ rowIndex: visibleRowIndex, colIndex }); + } + }} + /> + ); + + const cellContent = isExpandable ? ( + + ) : ( ); - if (isExpandable) { - innerContent = ( - { - if (popoverIsOpen) { - closeCellPopover(); - } else { - openCellPopover({ rowIndex: visibleRowIndex, colIndex }); - } - }} - /> - ) - } - /> - ); - } - - const content = ( + const cell = (
- {innerContent} + {cellContent}
); return rowManager && !IS_JEST_ENVIRONMENT ? createPortal( - content, + cell, rowManager.getRow({ rowIndex, visibleRowIndex, @@ -786,6 +779,6 @@ export class EuiDataGridCell extends Component< height: style!.height as number, // comes in as an integer from react-window }) ) - : content; + : cell; } } From e291a7df11d757b10c2d83b01e102a6e8e57728a Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 16:25:29 -0700 Subject: [PATCH 13/16] =?UTF-8?q?=F0=9F=94=A5=20more=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - move flex CSS specific to default height cells to its selector, vs. the baseline actions component --- .../datagrid/_data_grid_data_row.scss | 24 ++++++++++--------- .../body/data_grid_cell_actions.test.tsx | 4 ++-- .../datagrid/body/data_grid_cell_actions.tsx | 1 - 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index bf027b52bb7..0c67342deff 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -128,6 +128,10 @@ flex-grow: 1; } + .euiDataGridRowCell__actions { + flex-grow: 0; + } + .euiDataGridRowCell--controlColumn & { height: 100%; align-items: center; @@ -137,16 +141,14 @@ // Cell actions .euiDataGridRowCell__actions { display: flex; -} -.euiDataGridRowCell__actions--flex { - flex-grow: 0; -} -.euiDataGridRowCell__actions--overlay { - position: absolute; - right: 0; - top: 0; - padding: $euiDataGridCellPaddingM 0; - background-color: $euiColorEmptyShade; + + &--overlay { + position: absolute; + right: 0; + top: 0; + padding: $euiDataGridCellPaddingM 0; + background-color: $euiColorEmptyShade; + } } .euiDataGridRowCell__actionButtonIcon { @@ -228,7 +230,7 @@ padding: ($euiDataGridCellPaddingS / 2) 0; } - .euiDataGridRowCell__actions--flex { + .euiDataGridRowCell__defaultHeight .euiDataGridRowCell__actions { transform: translateY(1px); } } diff --git a/src/components/datagrid/body/data_grid_cell_actions.test.tsx b/src/components/datagrid/body/data_grid_cell_actions.test.tsx index 3b2a11b026b..151f3003814 100644 --- a/src/components/datagrid/body/data_grid_cell_actions.test.tsx +++ b/src/components/datagrid/body/data_grid_cell_actions.test.tsx @@ -33,7 +33,7 @@ describe('EuiDataGridCellActions', () => { expect(component).toMatchInlineSnapshot(`
{ expect(component).toMatchInlineSnapshot(`
Date: Wed, 4 Oct 2023 16:16:03 -0700 Subject: [PATCH 14/16] =?UTF-8?q?=F0=9F=90=9B=20Fix=20bug=20in=20productio?= =?UTF-8?q?n=20w/=20cell=20popovers=20for=20cells=20with=20numeric=20heigh?= =?UTF-8?q?ts=20that=20have=20overflowing=20content?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit + Add Cypress regression tests to ensure popovers are rendering from the correct anchor/position --- .../datagrid/_data_grid_data_row.scss | 9 +++ .../datagrid/body/data_grid_cell.tsx | 15 +++- .../body/data_grid_cell_popover.spec.tsx | 73 +++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 0c67342deff..1841427b646 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -138,6 +138,15 @@ } } +.euiDataGridRowCell__numericalHeight { + // Without this rule, popover anchors content that overflows off the page + [data-euiportal], + .euiPopover, + .euiPopover__anchor { + height: 100%; + } +} + // Cell actions .euiDataGridRowCell__actions { display: flex; diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 6b4dbc7afb3..23ec939abb3 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -94,7 +94,18 @@ const EuiDataGridCellContent: FunctionComponent< let cellContent = (
{ + setCellContentsRef(el); + setPopoverAnchorRef.current = + cellHeightType === 'default' + ? // Default height cells need the popover to be anchored on the wrapper, + // in order for the popover to centered on the full cell width (as content + // width is affected by the width of cell actions) + (el?.parentElement as HTMLDivElement) + : // Numerical height cells need the popover anchor to be below the wrapper + // class, in order to set height: 100% on the portalled popover div wrappers + el; + }} data-datagrid-cellcontent className={classes} > @@ -135,7 +146,7 @@ const EuiDataGridCellContent: FunctionComponent< ); return ( -
+
{cellContent} {screenReaderText} {cellActions} diff --git a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx index deca8599bc1..9e58a654f1f 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx +++ b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx @@ -130,4 +130,77 @@ describe('EuiDataGridCellPopover', () => { cy.get('.euiDataGridRowCell__popover.hello.world').should('exist'); }); + + describe('popover anchor/positioning', () => { + const props = { + ...baseProps, + rowCount: 1, + renderCellValue: ({ columnId }) => { + if (columnId === 'A') { + return 'short text'; + } else { + return 'Very long text that should get cut off because it is so long'; + } + }, + }; + + const openCellPopover = (id: string) => { + cy.get( + `[data-gridcell-row-index="0"][data-gridcell-column-id="${id}"]` + ).realClick(); + cy.realPress('Enter'); + }; + + it('default row height', () => { + cy.realMount(); + + openCellPopover('B'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'left', '24.5px') + .should('have.css', 'top', '104px'); + }); + + it('lineCount row height', () => { + cy.realMount( + + ); + openCellPopover('B'); + + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'left', '24.5px') + .should('have.css', 'top', '127px'); + }); + + it('numerical row height', () => { + cy.realMount( + + ); + openCellPopover('B'); + + // Should not be anchored to the bottom of the overflowing text + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'left', '24.5px') + .should('have.css', 'top', '106px'); + }); + + it('auto row height', () => { + cy.realMount( + + ); + + openCellPopover('B'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'left', '24.5px') + .should('have.css', 'top', '151px'); + + // The shorter cell content should not have the same top position + openCellPopover('A'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'left', '19px') + .should('have.css', 'top', '103px'); + }); + }); }); From c5343af33c52a3f57538f517c27c193d89dc916e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 16:18:40 -0700 Subject: [PATCH 15/16] =?UTF-8?q?=F0=9F=94=A5=20Remove=20Safari=20animatio?= =?UTF-8?q?n=20workaround=20now=20that=20Safari=2017=20is=20out?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../datagrid/_data_grid_data_row.scss | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 1841427b646..5a6869fe66c 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -38,22 +38,6 @@ animation-delay: $euiAnimSpeedNormal; animation-fill-mode: forwards; } - /* - * For some incredibly bizarre reason, Safari doesn't correctly update the flex - * width of the content (when rows are an undefined height/single flex row), - * which causes the action icons to overlap & makes the content less readable. - * This workaround "animation" forces a rerender of the flex content width - * - * TODO: Remove this workaround once https://bugs.webkit.org/show_bug.cgi?id=258539 is resolved - */ - .euiDataGridRowCell__defaultHeight .euiDataGridRowCell__content { - animation-name: euiDataGridCellActionsSafariWorkaround; - animation-duration: 1000ms; // I don't know why the duration matters or why it being longer works more consistently 🥲 - animation-delay: $euiAnimSpeedNormal + $euiAnimSpeedExtraFast; // Wait for above animation to finish - animation-iteration-count: 1; - animation-fill-mode: forwards; - animation-timing-function: linear; - } } // On focus, directly show action buttons (without animation) @@ -255,14 +239,3 @@ width: $euiSizeM; } } -@keyframes euiDataGridCellActionsSafariWorkaround { - from { - width: 100%; - flex-basis: 100%; - } - - to { - width: auto; - flex-basis: auto; - } -} From a8c1af730414fc5b9493f231aaa5d9196544a64d Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 4 Oct 2023 18:28:08 -0700 Subject: [PATCH 16/16] =?UTF-8?q?=F0=9F=92=80=20CI=20pixel=20aliasing=20sh?= =?UTF-8?q?enanigans?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../datagrid/body/data_grid_cell_popover.spec.tsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx index 9e58a654f1f..0c4dd05fb04 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx +++ b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx @@ -157,7 +157,8 @@ describe('EuiDataGridCellPopover', () => { openCellPopover('B'); cy.get('[data-test-subj="euiDataGridExpansionPopover"]') .should('have.css', 'left', '24.5px') - .should('have.css', 'top', '104px'); + .should('have.css', 'top') + .and('match', /^(104|103)px/); // CI is off by 1 px }); it('lineCount row height', () => { @@ -171,7 +172,8 @@ describe('EuiDataGridCellPopover', () => { cy.get('[data-test-subj="euiDataGridExpansionPopover"]') .should('have.css', 'left', '24.5px') - .should('have.css', 'top', '127px'); + .should('have.css', 'top') + .and('match', /^(127|126)px/); // CI is off by 1 px }); it('numerical row height', () => { @@ -183,7 +185,8 @@ describe('EuiDataGridCellPopover', () => { // Should not be anchored to the bottom of the overflowing text cy.get('[data-test-subj="euiDataGridExpansionPopover"]') .should('have.css', 'left', '24.5px') - .should('have.css', 'top', '106px'); + .should('have.css', 'top') + .and('match', /^(106|105)px/); // CI is off by 1 px }); it('auto row height', () => { @@ -194,13 +197,15 @@ describe('EuiDataGridCellPopover', () => { openCellPopover('B'); cy.get('[data-test-subj="euiDataGridExpansionPopover"]') .should('have.css', 'left', '24.5px') - .should('have.css', 'top', '151px'); + .should('have.css', 'top') + .and('match', /^(151|150)px/); // CI is off by 1 px // The shorter cell content should not have the same top position openCellPopover('A'); cy.get('[data-test-subj="euiDataGridExpansionPopover"]') .should('have.css', 'left', '19px') - .should('have.css', 'top', '103px'); + .should('have.css', 'top') + .and('match', /^(103|102)px/); // CI is off by 1 px }); }); });