[Lens] Datatable - make 3 lines the default setting for header row he…

…ight (#180505) ## Summary Fixes #172698 The default setting for the header row height is custom, 3 lines (so if user doesn't modify the header row height). It doesn't visually affect datatables with short labels. <img width="765" alt="Screenshot 2024-04-10 at 18 39 51" src="https://github.com/elastic/kibana/assets/4283304/bdec216b-36e4-4ca4-8135-85008b7f311d"> <img width="739" alt="Screenshot 2024-04-10 at 18 39 25" src="https://github.com/elastic/kibana/assets/4283304/e5e6c4d5-f9c6-4270-85ba-f0ada2fbea69"> To code reviewers - this PR in itself is quite small but I rewrote all the datatable tests to testing library and made some code style improvements.
elastic · Apr 16, 2024 · 057da1e · 057da1e
1 parent e9266f3
commit 057da1e
Show file tree

Hide file tree

Showing 27 changed files with 800 additions and 2,304 deletions.
diff --git a/...ges/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx b/...ges/kbn-unified-data-table/src/components/data_table_additional_display_settings.test.tsx
@@ -17,6 +17,7 @@ import {
 import lodash from 'lodash';
 import { fireEvent, render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
+import { RowHeightMode } from './row_height_settings';
 
 jest.spyOn(lodash, 'debounce').mockImplementation((fn: any) => fn);
 
@@ -26,9 +27,9 @@ const renderDisplaySettings = (
   return render(
     <UnifiedDataTableAdditionalDisplaySettings
       sampleSize={10}
-      rowHeight="custom"
+      rowHeight={RowHeightMode.custom}
       rowHeightLines={10}
-      headerRowHeight="custom"
+      headerRowHeight={RowHeightMode.custom}
       headerRowHeightLines={5}
       {...props}
     />
@@ -44,9 +45,9 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () {
         <UnifiedDataTableAdditionalDisplaySettings
           sampleSize={10}
           onChangeSampleSize={onChangeSampleSizeMock}
-          rowHeight="custom"
+          rowHeight={RowHeightMode.custom}
           rowHeightLines={10}
-          headerRowHeight="custom"
+          headerRowHeight={RowHeightMode.custom}
           headerRowHeightLines={5}
         />
       );
@@ -80,9 +81,9 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () {
           maxAllowedSampleSize={500}
           sampleSize={50}
           onChangeSampleSize={onChangeSampleSizeMock}
-          rowHeight="custom"
+          rowHeight={RowHeightMode.custom}
           rowHeightLines={10}
-          headerRowHeight="custom"
+          headerRowHeight={RowHeightMode.custom}
           headerRowHeightLines={5}
         />
       );
@@ -114,9 +115,9 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () {
         <UnifiedDataTableAdditionalDisplaySettings
           sampleSize={200}
           onChangeSampleSize={onChangeSampleSizeMock}
-          rowHeight="custom"
+          rowHeight={RowHeightMode.custom}
           rowHeightLines={10}
-          headerRowHeight="custom"
+          headerRowHeight={RowHeightMode.custom}
           headerRowHeightLines={5}
         />
       );
@@ -159,7 +160,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () {
       const onChangeRowHeight = jest.fn();
       const onChangeRowHeightLines = jest.fn();
       renderDisplaySettings({
-        rowHeight: 'custom',
+        rowHeight: RowHeightMode.custom,
         onChangeRowHeight,
         onChangeRowHeightLines,
       });
@@ -188,7 +189,7 @@ describe('UnifiedDataTableAdditionalDisplaySettings', function () {
       const onChangeHeaderRowHeight = jest.fn();
       const onChangeHeaderRowHeightLines = jest.fn();
       renderDisplaySettings({
-        headerRowHeight: 'custom',
+        headerRowHeight: RowHeightMode.custom,
         onChangeHeaderRowHeight,
         onChangeHeaderRowHeightLines,
       });

diff --git a/packages/kbn-unified-data-table/src/components/row_height_settings.tsx b/packages/kbn-unified-data-table/src/components/row_height_settings.tsx
@@ -11,13 +11,18 @@ import { i18n } from '@kbn/i18n';
 import { EuiButtonGroup, EuiFormRow, EuiRange, htmlIdGenerator } from '@elastic/eui';
 import { euiThemeVars } from '@kbn/ui-theme';
 
+export enum RowHeightMode {
+  auto = 'auto',
+  single = 'single',
+  custom = 'custom',
+}
 export interface RowHeightSettingsProps {
-  rowHeight?: 'auto' | 'single' | 'custom';
+  rowHeight?: RowHeightMode;
   rowHeightLines?: number;
   maxRowHeight?: number;
   label: string;
   compressed?: boolean;
-  onChangeRowHeight: (newHeightMode: 'auto' | 'single' | 'custom' | undefined) => void;
+  onChangeRowHeight: (newHeightMode: RowHeightMode | undefined) => void;
   onChangeRowHeightLines: (newRowHeightLines: number) => void;
   'data-test-subj'?: string;
 }
@@ -36,25 +41,25 @@ export function RowHeightSettings({
 }: RowHeightSettingsProps) {
   const rowHeightModeOptions = [
     {
-      id: `${idPrefix}single`,
+      id: `${idPrefix}${RowHeightMode.single}`,
       label: i18n.translate('unifiedDataTable.rowHeight.single', {
         defaultMessage: 'Single',
       }),
-      'data-test-subj': `${dataTestSubj}_rowHeight_single`,
+      'data-test-subj': `${dataTestSubj}_rowHeight_${RowHeightMode.single}`,
     },
     {
-      id: `${idPrefix}auto`,
+      id: `${idPrefix}${RowHeightMode.auto}`,
       label: i18n.translate('unifiedDataTable.rowHeight.auto', {
         defaultMessage: 'Auto fit',
       }),
-      'data-test-subj': `${dataTestSubj}_rowHeight_auto`,
+      'data-test-subj': `${dataTestSubj}_rowHeight_${RowHeightMode.auto}`,
     },
     {
-      id: `${idPrefix}custom`,
+      id: `${idPrefix}${RowHeightMode.custom}`,
       label: i18n.translate('unifiedDataTable.rowHeight.custom', {
         defaultMessage: 'Custom',
       }),
-      'data-test-subj': `${dataTestSubj}_rowHeight_custom`,
+      'data-test-subj': `${dataTestSubj}_rowHeight_${RowHeightMode.custom}`,
     },
   ];
 
@@ -67,14 +72,14 @@ export function RowHeightSettings({
             legend={label}
             buttonSize="compressed"
             options={rowHeightModeOptions}
-            idSelected={`${idPrefix}${rowHeight ?? 'single'}`}
+            idSelected={`${idPrefix}${rowHeight ?? RowHeightMode.single}`}
             onChange={(optionId) => {
               const newMode = optionId.replace(idPrefix, '') as RowHeightSettingsProps['rowHeight'];
               onChangeRowHeight(newMode);
             }}
             data-test-subj={`${dataTestSubj}_rowHeightButtonGroup`}
           />
-          {rowHeight === 'custom' ? (
+          {rowHeight === RowHeightMode.custom ? (
             <EuiRange
               compressed
               fullWidth

diff --git a/packages/kbn-unified-data-table/src/hooks/use_row_height.test.tsx b/packages/kbn-unified-data-table/src/hooks/use_row_height.test.tsx
@@ -9,6 +9,7 @@
 import { renderHook } from '@testing-library/react-hooks';
 import { createLocalStorageMock } from '../../__mocks__/local_storage_mock';
 import { useRowHeight } from './use_row_height';
+import { RowHeightMode } from '../components/row_height_settings';
 
 const CONFIG_ROW_HEIGHT = 3;
 
@@ -51,15 +52,15 @@ describe('useRowHeightsOptions', () => {
     const {
       hook: { result },
     } = renderRowHeightHook({ rowHeightState: 2 });
-    expect(result.current.rowHeight).toEqual('custom');
+    expect(result.current.rowHeight).toEqual(RowHeightMode.custom);
     expect(result.current.rowHeightLines).toEqual(2);
   });
 
   it('should apply rowHeight from local storage', () => {
     const {
       hook: { result },
     } = renderRowHeightHook();
-    expect(result.current.rowHeight).toEqual('custom');
+    expect(result.current.rowHeight).toEqual(RowHeightMode.custom);
     expect(result.current.rowHeightLines).toEqual(5);
   });
 
@@ -70,7 +71,7 @@ describe('useRowHeightsOptions', () => {
       previousRowHeight: undefined,
       previousConfigRowHeight: undefined,
     });
-    expect(result.current.rowHeight).toEqual('custom');
+    expect(result.current.rowHeight).toEqual(RowHeightMode.custom);
     expect(result.current.rowHeightLines).toEqual(CONFIG_ROW_HEIGHT);
   });
 
@@ -80,7 +81,7 @@ describe('useRowHeightsOptions', () => {
     } = renderRowHeightHook({
       previousConfigRowHeight: 4,
     });
-    expect(result.current.rowHeight).toEqual('custom');
+    expect(result.current.rowHeight).toEqual(RowHeightMode.custom);
     expect(result.current.rowHeightLines).toEqual(3);
   });
 
@@ -105,19 +106,19 @@ describe('useRowHeightsOptions', () => {
       storage,
       hook: { result },
     } = renderRowHeightHook({ onUpdateRowHeight });
-    result.current.onChangeRowHeight?.('auto');
+    result.current.onChangeRowHeight?.(RowHeightMode.auto);
     expect(storage.get('discover:dataGridRowHeight')).toEqual({
       previousRowHeight: -1,
       previousConfigRowHeight: CONFIG_ROW_HEIGHT,
     });
     expect(onUpdateRowHeight).toHaveBeenLastCalledWith(-1);
-    result.current.onChangeRowHeight?.('single');
+    result.current.onChangeRowHeight?.(RowHeightMode.single);
     expect(storage.get('discover:dataGridRowHeight')).toEqual({
       previousRowHeight: 0,
       previousConfigRowHeight: CONFIG_ROW_HEIGHT,
     });
     expect(onUpdateRowHeight).toHaveBeenLastCalledWith(0);
-    result.current.onChangeRowHeight?.('custom');
+    result.current.onChangeRowHeight?.(RowHeightMode.custom);
     expect(storage.get('discover:dataGridRowHeight')).toEqual({
       previousRowHeight: CONFIG_ROW_HEIGHT,
       previousConfigRowHeight: CONFIG_ROW_HEIGHT,
@@ -141,13 +142,13 @@ describe('useRowHeightsOptions', () => {
 
   it('should convert provided rowHeightState to rowHeight and rowHeightLines', () => {
     const { hook, initialProps } = renderRowHeightHook({ rowHeightState: -1 });
-    expect(hook.result.current.rowHeight).toEqual('auto');
+    expect(hook.result.current.rowHeight).toEqual(RowHeightMode.auto);
     expect(hook.result.current.rowHeightLines).toEqual(-1);
     hook.rerender({ ...initialProps, rowHeightState: 0 });
-    expect(hook.result.current.rowHeight).toEqual('single');
+    expect(hook.result.current.rowHeight).toEqual(RowHeightMode.single);
     expect(hook.result.current.rowHeightLines).toEqual(0);
     hook.rerender({ ...initialProps, rowHeightState: 3 });
-    expect(hook.result.current.rowHeight).toEqual('custom');
+    expect(hook.result.current.rowHeight).toEqual(RowHeightMode.custom);
     expect(hook.result.current.rowHeightLines).toEqual(3);
   });
 });
diff --git a/packages/kbn-unified-data-table/src/hooks/use_row_height.ts b/packages/kbn-unified-data-table/src/hooks/use_row_height.ts
@@ -15,7 +15,7 @@ import {
   updateStoredRowHeight,
 } from '../utils/row_heights';
 import { ROWS_HEIGHT_OPTIONS } from '../constants';
-import { RowHeightSettingsProps } from '../components/row_height_settings';
+import { RowHeightMode, RowHeightSettingsProps } from '../components/row_height_settings';
 
 interface UseRowHeightProps {
   storage: Storage;
@@ -57,11 +57,11 @@ export const useRowHeight = ({
   const rowHeight = useMemo<RowHeightSettingsProps['rowHeight']>(() => {
     switch (rowHeightLines) {
       case ROWS_HEIGHT_OPTIONS.auto:
-        return 'auto';
+        return RowHeightMode.auto;
       case ROWS_HEIGHT_OPTIONS.single:
-        return 'single';
+        return RowHeightMode.single;
       default:
-        return 'custom';
+        return RowHeightMode.custom;
     }
   }, [rowHeightLines]);
 
@@ -70,10 +70,10 @@ export const useRowHeight = ({
       let newRowHeightLines: number;
 
       switch (newRowHeight) {
-        case 'auto':
+        case RowHeightMode.auto:
           newRowHeightLines = ROWS_HEIGHT_OPTIONS.auto;
           break;
-        case 'single':
+        case RowHeightMode.single:
           newRowHeightLines = ROWS_HEIGHT_OPTIONS.single;
           break;
         default:

diff --git a/src/plugins/vis_types/table/public/convert_to_lens/configurations/index.ts b/src/plugins/vis_types/table/public/convert_to_lens/configurations/index.ts
@@ -14,6 +14,12 @@ import {
 } from '@kbn/visualizations-plugin/common';
 import { TableVisParams } from '../../../common';
 
+enum RowHeightMode {
+  auto = 'auto',
+  single = 'single',
+  custom = 'custom',
+}
+
 const getColumns = (
   params: TableVisParams,
   metrics: string[],
@@ -48,8 +54,8 @@ const getRowHeight = (
 ): Pick<TableVisConfiguration, 'rowHeight' | 'headerRowHeight'> => {
   const { autoFitRowToContent } = params;
   return {
-    rowHeight: autoFitRowToContent ? 'auto' : 'single',
-    headerRowHeight: autoFitRowToContent ? 'auto' : 'single',
+    rowHeight: autoFitRowToContent ? RowHeightMode.auto : RowHeightMode.single,
+    headerRowHeight: autoFitRowToContent ? RowHeightMode.auto : RowHeightMode.single,
   };
 };
 

diff --git a/src/plugins/visualizations/common/convert_to_lens/types/configurations.ts b/src/plugins/visualizations/common/convert_to_lens/types/configurations.ts
@@ -190,13 +190,19 @@ export interface ColumnState {
   palette?: PaletteOutput<CustomPaletteParams>;
 }
 
+enum RowHeightMode {
+  auto = 'auto',
+  single = 'single',
+  custom = 'custom',
+}
+
 export interface TableVisConfiguration {
   columns: ColumnState[];
   layerId: string;
   layerType: 'data';
   sorting?: SortingState;
-  rowHeight?: 'auto' | 'single' | 'custom';
-  headerRowHeight?: 'auto' | 'single' | 'custom';
+  rowHeight?: RowHeightMode;
+  headerRowHeight?: RowHeightMode;
   rowHeightLines?: number;
   headerRowHeightLines?: number;
   paging?: PagingState;

diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable.ts
@@ -7,7 +7,7 @@
 
 import { i18n } from '@kbn/i18n';
 import type { ExecutionContext } from '@kbn/expressions-plugin/common';
-import type { FormatFactory } from '../../types';
+import type { FormatFactory, RowHeightMode } from '../../types';
 import type { ColumnConfigArg } from './datatable_column';
 import type { DatatableExpressionFunction } from './types';
 
@@ -29,7 +29,7 @@ export interface DatatableArgs {
   sortingDirection: SortingState['direction'];
   fitRowToContent?: boolean;
   rowHeightLines?: number;
-  headerRowHeight?: 'auto' | 'single' | 'custom';
+  headerRowHeight?: RowHeightMode;
   headerRowHeightLines?: number;
   pageSize?: PagingState['size'];
 }

diff --git a/x-pack/plugins/lens/common/types.ts b/x-pack/plugins/lens/common/types.ts
@@ -95,3 +95,9 @@ export interface LegacyMetricState {
   size?: string;
   textAlign?: 'left' | 'right' | 'center';
 }
+
+export enum RowHeightMode {
+  auto = 'auto',
+  single = 'single',
+  custom = 'custom',
+}
diff --git a/x-pack/plugins/lens/public/shared_components/coloring/palette_panel_container.tsx b/x-pack/plugins/lens/public/shared_components/coloring/palette_panel_container.tsx
@@ -21,6 +21,7 @@ export function PalettePanelContainer(props: {
   return (
     <SettingWithSiblingFlyout
       {...props}
+      dataTestSubj="lns-palettePanelFlyout"
       SettingTrigger={({ onClick }: { onClick: () => void }) => (
         <>
           <EuiFlexItem>

diff --git a/x-pack/plugins/lens/public/shared_components/coloring/utils.test.ts b/x-pack/plugins/lens/public/shared_components/coloring/utils.test.ts
@@ -6,7 +6,7 @@
  */
 
 import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
-import { applyPaletteParams, getContrastColor } from './utils';
+import { applyPaletteParams, findMinMaxByColumnId, getContrastColor } from './utils';
 
 describe('applyPaletteParams', () => {
   const paletteRegistry = chartPluginMock.createPaletteRegistry();
@@ -70,3 +70,41 @@ describe('getContrastColor', () => {
     expect(getContrastColor('rgba(255,255,255,0)', false)).toBe('#000000');
   });
 });
+
+describe('findMinMaxByColumnId', () => {
+  it('should return the min and max values for a column', () => {
+    expect(
+      findMinMaxByColumnId(['b'], {
+        type: 'datatable',
+        columns: [
+          {
+            id: 'a',
+            name: 'a',
+            meta: {
+              type: 'string',
+              source: 'esaggs',
+              field: 'a',
+              sourceParams: { type: 'terms', indexPatternId: 'indexPatternId' },
+            },
+          },
+          {
+            id: 'b',
+            name: 'b',
+            meta: {
+              type: 'number',
+              source: 'esaggs',
+              field: 'b',
+              sourceParams: { indexPatternId: 'indexPatternId', type: 'count' },
+            },
+          },
+        ],
+        rows: [
+          { a: 'shoes', b: 3 },
+          { a: 'shoes', b: 2 },
+          { a: 'shoes', b: 43 },
+          { a: 'shoes', b: 53 },
+        ],
+      })
+    ).toEqual({ b: { min: 2, max: 53 } });
+  });
+});