[EuiDataGrid] Various row height fixes (#8025)

packages/eui/changelogs/upcoming/8025.md

@@ -0,0 +1,6 @@
+**Bug fixes**
+
+- Fixed several `EuiDataGrid` row height bugs:
+  - Fixed row heights not recalculating when `rowHeightOptions.lineHeight`, `gridStyles.fontSize`, or `gridStyles.cellPadding` changed
+  - Fixed incorrect height calculations for `rowHeightOptions.rowHeights` with `lineCount`s
+  - Fixed control column content to align better with multi-line row heights, as well as custom line-heights

packages/eui/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap

@@ -611,7 +611,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="A"
                 >
                   A
@@ -653,7 +653,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="B"
                 >
                   B
@@ -989,7 +989,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="A"
                 >
                   A
@@ -1031,7 +1031,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="B"
                 >
                   B
@@ -1085,7 +1085,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
               tabindex="-1"
             >
               <div
-                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
+                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
                 data-datagrid-cellcontent="true"
               >
                 0
@@ -1142,7 +1142,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
               tabindex="-1"
             >
               <div
-                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
+                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
                 data-datagrid-cellcontent="true"
               >
                 0
@@ -1161,7 +1161,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
               tabindex="-1"
             >
               <div
-                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
+                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
                 data-datagrid-cellcontent="true"
               >
                 1
@@ -1218,7 +1218,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
               tabindex="-1"
             >
               <div
-                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
+                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
                 data-datagrid-cellcontent="true"
               >
                 1
@@ -1237,7 +1237,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
               tabindex="-1"
             >
               <div
-                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
+                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
                 data-datagrid-cellcontent="true"
               >
                 2
@@ -1294,7 +1294,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
               tabindex="-1"
             >
               <div
-                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-defaultHeight"
+                class="euiDataGridRowCell__content euiDataGridRowCell__content--defaultHeight emotion-euiDataGridRowCell__content-controlColumn-autoHeight"
                 data-datagrid-cellcontent="true"
               >
                 2
@@ -1478,7 +1478,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="A"
                 >
                   Column A
@@ -1520,7 +1520,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="B"
                 >
                   <div>
@@ -1840,7 +1840,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="A"
                 >
                   A
@@ -1882,7 +1882,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = `
                   data-test-subj="dataGridColumnResizer"
                 />
                 <div
-                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+                  class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
                   title="B"
                 >
                   B

packages/eui/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap

@@ -26,7 +26,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render
         data-test-subj="dataGridColumnResizer"
       />
       <div
-        class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+        class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
         title="columnA"
       >
         columnA
@@ -68,7 +68,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render
         data-test-subj="dataGridColumnResizer"
       />
       <div
-        class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+        class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
         title="columnB"
       >
         columnB

packages/eui/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap

@@ -30,7 +30,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = `
           data-test-subj="dataGridColumnResizer"
         />
         <div
-          class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+          class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
           title="columnA"
         >
           columnA
@@ -72,7 +72,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = `
           data-test-subj="dataGridColumnResizer"
         />
         <div
-          class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content-left"
+          class="euiDataGridHeaderCell__content emotion-euiDataGridHeaderCell__content"
           title="columnB"
         >
           columnB

packages/eui/src/components/datagrid/body/cell/data_grid_cell.styles.ts

@@ -101,6 +101,7 @@ export const euiDataGridCellOutlineSelectors = (parentSelector = '&') => {
 };
 
 export const euiDataGridRowCellStyles = (euiThemeContext: UseEuiTheme) => {
+  const { euiTheme } = euiThemeContext;
   const cellOutline = euiDataGridCellOutlineStyles(euiThemeContext);
   const { outline: outlineSelectors } = euiDataGridCellOutlineSelectors();
 
@@ -146,17 +147,45 @@ export const euiDataGridRowCellStyles = (euiThemeContext: UseEuiTheme) => {
       euiDataGridRowCell__content: css`
         overflow: hidden;
       `,
-      controlColumn: css`
-        ${logicalCSS('max-height', '100%')}
-        display: flex;
-        align-items: center;
-      `,
       autoHeight: css`
         ${logicalCSS('height', 'auto')}
       `,
       defaultHeight: css`
         ${logicalCSS('height', '100%')}
       `,
+      // Control columns should be vertically centered with the *first line* of text
+      // for both single and multi-line heights (see https://github.com/elastic/eui/issues/7897)
+      controlColumn: css`
+        display: inline-flex;
+        align-items: start;
+        gap: ${euiTheme.size.xxs}; /* EuiButtonIcons */
+
+        /* FF and webkit browsers have more centered vertical alignment behind undocumented browser prefixes */
+        vertical-align: -webkit-baseline-middle;
+        vertical-align: -moz-middle-with-baseline;
+
+        /* Compact sizing affordance for EuiButtonIcons */
+        .euiDataGrid--fontSizeSmall
+          &:where(.euiDataGridRowCell__content--defaultHeight) {
+          ${logicalCSS('height', '100%')}
+          align-items: center;
+        }
+
+        /* Line up single EuiCheckboxes a bit better (insert Peter Griffin blinds gif) */
+        .euiCheckbox:not(:has(label)) {
+          display: inline;
+
+          .euiCheckbox__square {
+            display: inline-flex;
+            vertical-align: text-bottom;
+
+            /* sub alignment looks better on Firefox, text-bottom looks better on webkit 💀 */
+            @supports (vertical-align: -moz-middle-with-baseline) {
+              vertical-align: sub;
+            }
+          }
+        }
+      `,
     },
   };
 };

packages/eui/src/components/datagrid/body/cell/data_grid_cell.test.tsx

@@ -45,6 +45,7 @@ describe('EuiDataGridCell', () => {
     ),
     popoverContext: mockPopoverContext,
     rowHeightUtils: mockRowHeightUtils,
+    gridStyles: {},
   };
 
   beforeEach(() => jest.clearAllMocks());
@@ -202,6 +203,12 @@ describe('EuiDataGridCell', () => {
         it('rowHeightsOptions', () => {
           component.setProps({ rowHeightsOptions: { defaultHeight: 'auto' } });
         });
+        it('gridStyles.fontSize', () => {
+          component.setProps({ gridStyles: { fontSize: 's' } });
+        });
+        it('gridStyles.cellPadding', () => {
+          component.setProps({ gridStyles: { cellPadding: 'l' } });
+        });
         it('renderCellValue', () => {
           component.setProps({ renderCellValue: () => <div>test</div> });
         });
@@ -254,7 +261,10 @@ describe('EuiDataGridCell', () => {
     });
 
     it('should not update for prop/state changes not specified above', () => {
-      component.setProps({ className: 'test' });
+      component.setProps({
+        className: 'test',
+        gridStyles: { header: 'underline' },
+      });
       expect(shouldComponentUpdate).toHaveReturnedWith(false);
     });
   });
@@ -625,7 +635,7 @@ describe('EuiDataGridCell', () => {
           callMethod(component);
           expect(
             mockRowHeightUtils.calculateHeightForLineCount
-          ).toHaveBeenCalledWith(expect.any(HTMLElement), 3, false);
+          ).toHaveBeenCalledWith(expect.any(HTMLElement), 3);
           expect(setRowHeight).toHaveBeenCalled();
         });
       });
@@ -647,25 +657,76 @@ describe('EuiDataGridCell', () => {
           callMethod(component);
           expect(
             mockRowHeightUtils.calculateHeightForLineCount
-          ).toHaveBeenCalledWith(expect.any(HTMLElement), 10, true);
+          ).toHaveBeenCalledWith(expect.any(HTMLElement), 10);
           expect(mockRowHeightUtils.setRowHeight).toHaveBeenCalled();
           expect(setRowHeight).not.toHaveBeenCalled();
         });
+
+        it('recalculates when the override for the row changes', () => {
+          const component = mount(
+            <EuiDataGridCell {...requiredProps} setRowHeight={setRowHeight} />
+          );
+
+          component.setProps({
+            rowHeightsOptions: {
+              rowHeights: {
+                0: { lineCount: 2 },
+              },
+            },
+          });
+          expect(mockRowHeightUtils.setRowHeight).toHaveBeenCalledTimes(1);
+
+          // Handle row index changes as well
+          component.setProps({
+            rowHeightsOptions: {
+              rowHeights: {
+                0: { lineCount: 2 },
+                2: { lineCount: 4 },
+              },
+            },
+            rowIndex: 2,
+          });
+          expect(mockRowHeightUtils.setRowHeight).toHaveBeenCalledTimes(2);
+
+          expect(setRowHeight).not.toHaveBeenCalled();
+        });
       });
 
-      it('recalculates when rowHeightsOptions.defaultHeight.lineCount changes', () => {
+      it('recalculates when props that affect row/line height change', () => {
         const component = mount(
           <EuiDataGridCell
             {...requiredProps}
-            rowHeightsOptions={{ defaultHeight: { lineCount: 7 } }}
+            rowHeightsOptions={{ defaultHeight: { lineCount: 4 } }}
             setRowHeight={setRowHeight}
           />
         );
+        component.setProps({
+          rowHeightsOptions: { defaultHeight: { lineCount: 2 } },
+        });
+        expect(setRowHeight).toHaveBeenCalledTimes(1);
+
+        // Other props that can affect row heights
 
+        const rowHeightsOptionsWithLineHeight = {
+          defaultHeight: { lineCount: 2 },
+          lineHeight: '3',
+        };
         component.setProps({
-          rowHeightsOptions: { defaultHeight: { lineCount: 6 } },
+          rowHeightsOptions: rowHeightsOptionsWithLineHeight,
         });
-        expect(setRowHeight).toHaveBeenCalled();
+        expect(setRowHeight).toHaveBeenCalledTimes(2);
+
+        component.setProps({
+          rowHeightsOptions: rowHeightsOptionsWithLineHeight,
+          gridStyles: { cellPadding: 'l' },
+        });
+        expect(setRowHeight).toHaveBeenCalledTimes(3);
+
+        component.setProps({
+          rowHeightsOptions: rowHeightsOptionsWithLineHeight,
+          gridStyles: { cellPadding: 'l', fontSize: 'l' },
+        });
+        expect(setRowHeight).toHaveBeenCalledTimes(4);
       });
 
       it('calculates undefined heights as single rows with a lineCount of 1', () => {
@@ -680,7 +741,7 @@ describe('EuiDataGridCell', () => {
         callMethod(component);
         expect(
           mockRowHeightUtils.calculateHeightForLineCount
-        ).toHaveBeenCalledWith(expect.any(HTMLElement), 1, false);
+        ).toHaveBeenCalledWith(expect.any(HTMLElement), 1);
         expect(setRowHeight).toHaveBeenCalled();
       });