[Lens][Datatable] Fix non-numeric default cell text alignment

x-pack/plugins/lens/common/expressions/datatable/utils.ts

@@ -5,14 +5,25 @@
  * 2.0.
  */
 
-import type { Datatable } from '@kbn/expressions-plugin/common';
+import { type Datatable, type DatatableColumnMeta } from '@kbn/expressions-plugin/common';
 import { getOriginalId } from './transpose_helpers';
 
+/**
+ * Returns true for numerical fields, excluding ranges
+ */
+export function isNumericField(meta?: DatatableColumnMeta): boolean {
+  return meta?.type === 'number' && meta.params?.id !== 'range';
+}
+
+/**
+ * Returns true for numerical fields, excluding ranges
+ */
 export function isNumericFieldForDatatable(table: Datatable | undefined, accessor: string) {
-  return getFieldTypeFromDatatable(table, accessor) === 'number';
+  const meta = getFieldMetaFromDatatable(table, accessor);
+  return isNumericField(meta);
 }
 
-export function getFieldTypeFromDatatable(table: Datatable | undefined, accessor: string) {
+export function getFieldMetaFromDatatable(table: Datatable | undefined, accessor: string) {
   return table?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor)
-    ?.meta.type;
+    ?.meta;
 }

x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx

@@ -6,25 +6,20 @@
  */
 
 import React from 'react';
-import { DEFAULT_COLOR_MAPPING_CONFIG, type PaletteRegistry } from '@kbn/coloring';
+import { DEFAULT_COLOR_MAPPING_CONFIG } from '@kbn/coloring';
 import { act, render, screen } from '@testing-library/react';
 import userEvent, { type UserEvent } from '@testing-library/user-event';
 import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
 import { LayerTypes } from '@kbn/expression-xy-plugin/public';
 import { EuiButtonGroupTestHarness } from '@kbn/test-eui-helpers';
-import {
-  FramePublicAPI,
-  OperationDescriptor,
-  VisualizationDimensionEditorProps,
-  DatasourcePublicAPI,
-  DataType,
-} from '../../../types';
+import { FramePublicAPI, DatasourcePublicAPI, OperationDescriptor } from '../../../types';
 import { DatatableVisualizationState } from '../visualization';
 import { createMockDatasource, createMockFramePublicAPI } from '../../../mocks';
-import { TableDimensionEditor } from './dimension_editor';
+import { TableDimensionEditor, TableDimensionEditorProps } from './dimension_editor';
 import { ColumnState } from '../../../../common/expressions';
 import { capitalize } from 'lodash';
 import { I18nProvider } from '@kbn/i18n-react';
+import { DatatableColumnType } from '@kbn/expressions-plugin/common';
 
 describe('data table dimension editor', () => {
   let user: UserEvent;
@@ -35,10 +30,8 @@ describe('data table dimension editor', () => {
     alignment: EuiButtonGroupTestHarness;
   };
   let mockOperationForFirstColumn: (overrides?: Partial<OperationDescriptor>) => void;
-  let props: VisualizationDimensionEditorProps<DatatableVisualizationState> & {
-    paletteService: PaletteRegistry;
-    isDarkMode: boolean;
-  };
+
+  let props: TableDimensionEditorProps;
 
   function testState(): DatatableVisualizationState {
     return {
@@ -80,6 +73,7 @@ describe('data table dimension editor', () => {
             name: 'foo',
             meta: {
               type: 'string',
+              params: {},
             },
           },
         ],
@@ -114,13 +108,7 @@ describe('data table dimension editor', () => {
     mockOperationForFirstColumn();
   });
 
-  const renderTableDimensionEditor = (
-    overrideProps?: Partial<
-      VisualizationDimensionEditorProps<DatatableVisualizationState> & {
-        paletteService: PaletteRegistry;
-      }
-    >
-  ) => {
+  const renderTableDimensionEditor = (overrideProps?: Partial<TableDimensionEditorProps>) => {
     return render(<TableDimensionEditor {...props} {...overrideProps} />, {
       wrapper: ({ children }) => (
         <I18nProvider>
@@ -137,11 +125,18 @@ describe('data table dimension editor', () => {
   });
 
   it('should render default alignment for number', () => {
-    mockOperationForFirstColumn({ dataType: 'number' });
+    frame.activeData!.first.columns[0].meta.type = 'number';
     renderTableDimensionEditor();
     expect(btnGroups.alignment.selected).toHaveTextContent('Right');
   });
 
+  it('should render default alignment for ranges', () => {
+    frame.activeData!.first.columns[0].meta.type = 'number';
+    frame.activeData!.first.columns[0].meta.params = { id: 'range' };
+    renderTableDimensionEditor();
+    expect(btnGroups.alignment.selected).toHaveTextContent('Left');
+  });
+
   it('should render specific alignment', () => {
     state.columns[0].alignment = 'center';
     renderTableDimensionEditor();
@@ -181,10 +176,11 @@ describe('data table dimension editor', () => {
     expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument();
   });
 
-  it.each<DataType>(['date'])(
+  it.each<DatatableColumnType>(['date'])(
     'should not show the dynamic coloring option for "%s" columns',
-    (dataType) => {
-      mockOperationForFirstColumn({ dataType });
+    (type) => {
+      frame.activeData!.first.columns[0].meta.type = type;
+
       renderTableDimensionEditor();
       expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).not.toBeInTheDocument();
       expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument();
@@ -231,15 +227,16 @@ describe('data table dimension editor', () => {
     });
   });
 
-  it.each<{ flyout: 'terms' | 'values'; isBucketed: boolean; dataType: DataType }>([
-    { flyout: 'terms', isBucketed: true, dataType: 'number' },
-    { flyout: 'terms', isBucketed: false, dataType: 'string' },
-    { flyout: 'values', isBucketed: false, dataType: 'number' },
+  it.each<{ flyout: 'terms' | 'values'; isBucketed: boolean; type: DatatableColumnType }>([
+    { flyout: 'terms', isBucketed: true, type: 'number' },
+    { flyout: 'terms', isBucketed: false, type: 'string' },
+    { flyout: 'values', isBucketed: false, type: 'number' },
   ])(
-    'should show color by $flyout flyout when bucketing is $isBucketed with $dataType column',
-    async ({ flyout, isBucketed, dataType }) => {
+    'should show color by $flyout flyout when bucketing is $isBucketed with $type column',
+    async ({ flyout, isBucketed, type }) => {
       state.columns[0].colorMode = 'cell';
-      mockOperationForFirstColumn({ isBucketed, dataType });
+      frame.activeData!.first.columns[0].meta.type = type;
+      mockOperationForFirstColumn({ isBucketed });
       renderTableDimensionEditor();
 
       await user.click(screen.getByLabelText('Edit colors'));
@@ -251,6 +248,7 @@ describe('data table dimension editor', () => {
 
   it('should show the dynamic coloring option for a bucketed operation', () => {
     state.columns[0].colorMode = 'cell';
+    frame.activeData!.first.columns[0].meta.type = 'string';
     mockOperationForFirstColumn({ isBucketed: true });
 
     renderTableDimensionEditor();

x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx

@@ -26,6 +26,11 @@ import './dimension_editor.scss';
 import { CollapseSetting } from '../../../shared_components/collapse_setting';
 import { ColorMappingByValues } from '../../../shared_components/coloring/color_mapping_by_values';
 import { ColorMappingByTerms } from '../../../shared_components/coloring/color_mapping_by_terms';
+import { getColumnAlignment } from '../utils';
+import {
+  getFieldMetaFromDatatable,
+  isNumericField,
+} from '../../../../common/expressions/datatable/utils';
 
 const idPrefix = htmlIdGenerator()();
 
@@ -45,12 +50,13 @@ function updateColumn(
   });
 }
 
-export function TableDimensionEditor(
-  props: VisualizationDimensionEditorProps<DatatableVisualizationState> & {
+export type TableDimensionEditorProps =
+  VisualizationDimensionEditorProps<DatatableVisualizationState> & {
     paletteService: PaletteRegistry;
     isDarkMode: boolean;
-  }
-) {
+  };
+
+export function TableDimensionEditor(props: TableDimensionEditorProps) {
   const { frame, accessor, isInlineEditing, isDarkMode } = props;
   const column = props.state.columns.find(({ columnId }) => accessor === columnId);
   const { inputValue: localState, handleInputChange: setLocalState } =
@@ -74,12 +80,13 @@ export function TableDimensionEditor(
 
   const currentData = frame.activeData?.[localState.layerId];
   const datasource = frame.datasourceLayers?.[localState.layerId];
-  const { dataType, isBucketed } = datasource?.getOperationForColumnId(accessor) ?? {};
-  const showColorByTerms = shouldColorByTerms(dataType, isBucketed);
-  const currentAlignment = column?.alignment || (dataType === 'number' ? 'right' : 'left');
+  const { isBucketed } = datasource?.getOperationForColumnId(accessor) ?? {};
+  const meta = getFieldMetaFromDatatable(currentData, accessor);
+  const showColorByTerms = shouldColorByTerms(meta?.type, isBucketed);
+  const currentAlignment = getColumnAlignment(column, isNumericField(meta));
   const currentColorMode = column?.colorMode || 'none';
   const hasDynamicColoring = currentColorMode !== 'none';
-  const showDynamicColoringFeature = dataType !== 'date';
+  const showDynamicColoringFeature = meta?.type !== 'date';
   const visibleColumnsCount = localState.columns.filter((c) => !c.hidden).length;
 
   const hasTransposedColumn = localState.columns.some(({ isTransposed }) => isTransposed);

x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.test.tsx

@@ -11,7 +11,6 @@ import userEvent from '@testing-library/user-event';
 import { I18nProvider } from '@kbn/i18n-react';
 import faker from 'faker';
 import { act } from 'react-dom/test-utils';
-import { IAggType } from '@kbn/data-plugin/public';
 import { IFieldFormat } from '@kbn/field-formats-plugin/common';
 import { coreMock } from '@kbn/core/public/mocks';
 import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
@@ -73,6 +72,17 @@ function sampleArgs() {
           sourceParams: { indexPatternId, type: 'count' },
         },
       },
+      {
+        id: 'd',
+        name: 'd',
+        meta: {
+          type: 'number',
+          source: 'esaggs',
+          field: 'd',
+          params: { id: 'range' },
+          sourceParams: { indexPatternId, type: 'range' },
+        },
+      },
     ],
     rows: [{ a: 'shoes', b: 1588024800000, c: 3 }],
   };
@@ -119,7 +129,9 @@ describe('DatatableComponent', () => {
       args,
       formatFactory: () => ({ convert: (x) => x } as IFieldFormat),
       dispatchEvent: onDispatchEvent,
-      getType: jest.fn(() => ({ type: 'buckets' } as IAggType)),
+      getType: jest.fn().mockReturnValue({
+        type: 'buckets',
+      }),
       paletteService: chartPluginMock.createPaletteRegistry(),
       theme: setUpMockTheme,
       renderMode: 'edit' as const,
@@ -357,14 +369,39 @@ describe('DatatableComponent', () => {
     ]);
   });
 
-  test('it adds alignment data to context', () => {
+  test('it adds explicit alignment to context', () => {
     renderDatatableComponent({
       args: {
         ...args,
         columns: [
           { columnId: 'a', alignment: 'center', type: 'lens_datatable_column', colorMode: 'none' },
+          { columnId: 'b', alignment: 'center', type: 'lens_datatable_column', colorMode: 'none' },
+          { columnId: 'c', alignment: 'center', type: 'lens_datatable_column', colorMode: 'none' },
+          { columnId: 'd', alignment: 'center', type: 'lens_datatable_column', colorMode: 'none' },
+        ],
+      },
+    });
+    const alignmentsClassNames = screen
+      .getAllByTestId('lnsTableCellContent')
+      .map((cell) => cell.className);
+
+    expect(alignmentsClassNames).toEqual([
+      'lnsTableCell--center', // set via args
+      'lnsTableCell--center', // set via args
+      'lnsTableCell--center', // set via args
+      'lnsTableCell--center', // set via args
+    ]);
+  });
+
+  test('it adds default alignment data to context', () => {
+    renderDatatableComponent({
+      args: {
+        ...args,
+        columns: [
+          { columnId: 'a', type: 'lens_datatable_column', colorMode: 'none' },
           { columnId: 'b', type: 'lens_datatable_column', colorMode: 'none' },
           { columnId: 'c', type: 'lens_datatable_column', colorMode: 'none' },
+          { columnId: 'd', type: 'lens_datatable_column', colorMode: 'none' },
         ],
         sortingColumnId: 'b',
         sortingDirection: 'desc',
@@ -375,9 +412,10 @@ describe('DatatableComponent', () => {
       .map((cell) => cell.className);
 
     expect(alignmentsClassNames).toEqual([
-      'lnsTableCell--center', // set via args
+      'lnsTableCell--left', // default for string
       'lnsTableCell--left', // default for date
       'lnsTableCell--right', // default for number
+      'lnsTableCell--left', // default for range
     ]);
   });
 

x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx

@@ -58,8 +58,12 @@ import {
 } from './table_actions';
 import { getFinalSummaryConfiguration } from '../../../../common/expressions/datatable/summary';
 import { DEFAULT_HEADER_ROW_HEIGHT, DEFAULT_HEADER_ROW_HEIGHT_LINES } from './constants';
-import { getFieldTypeFromDatatable } from '../../../../common/expressions/datatable/utils';
+import {
+  getFieldMetaFromDatatable,
+  isNumericField,
+} from '../../../../common/expressions/datatable/utils';
 import { CellColorFn, getCellColorFn } from '../../../shared_components/coloring/get_cell_color_fn';
+import { getColumnAlignment } from '../utils';
 
 export const DataContext = React.createContext<DataContextType>({});
 
@@ -229,18 +233,15 @@ export const DatatableComponent = (props: DatatableRenderProps) => {
       columnConfig.columns
         .filter((_col, index) => {
           const col = firstTableRef.current.columns[index];
-          return (
-            col?.meta?.sourceParams?.type &&
-            getType(col.meta.sourceParams.type as string)?.type === 'buckets'
-          );
+          return getType(col?.meta)?.type === 'buckets';
         })
         .map((col) => col.columnId),
     [firstTableRef, columnConfig, getType]
   );
 
   const isEmpty =
     firstLocalTable.rows.length === 0 ||
-    (bucketedColumns.length &&
+    (bucketedColumns.length > 0 &&
       props.data.rows.every((row) => bucketedColumns.every((col) => row[col] == null)));
 
   const visibleColumns = useMemo(
@@ -271,24 +272,20 @@ export const DatatableComponent = (props: DatatableRenderProps) => {
       firstLocalTable.columns.reduce<Record<string, boolean>>(
         (map, column) => ({
           ...map,
-          [column.id]: column.meta.type === 'number',
+          [column.id]: isNumericField(column.meta),
         }),
         {}
       ),
-    [firstLocalTable]
+    [firstLocalTable.columns]
   );
 
   const alignments: Record<string, 'left' | 'right' | 'center'> = useMemo(() => {
     const alignmentMap: Record<string, 'left' | 'right' | 'center'> = {};
     columnConfig.columns.forEach((column) => {
-      if (column.alignment) {
-        alignmentMap[column.columnId] = column.alignment;
-      } else {
-        alignmentMap[column.columnId] = isNumericMap[column.columnId] ? 'right' : 'left';
-      }
+      alignmentMap[column.columnId] = getColumnAlignment(column, isNumericMap[column.columnId]);
     });
     return alignmentMap;
-  }, [columnConfig, isNumericMap]);
+  }, [columnConfig.columns, isNumericMap]);
 
   const minMaxByColumnId: Record<string, { min: number; max: number }> = useMemo(() => {
     return findMinMaxByColumnId(
@@ -402,7 +399,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => {
         return cellColorFnMap.get(originalId)!;
       }
 
-      const dataType = getFieldTypeFromDatatable(firstLocalTable, originalId);
+      const dataType = getFieldMetaFromDatatable(firstLocalTable, originalId)?.type;
       const isBucketed = bucketedColumns.some((id) => id === columnId);
       const colorByTerms = shouldColorByTerms(dataType, isBucketed);
 

x-pack/plugins/lens/public/visualizations/datatable/components/types.ts

@@ -8,7 +8,7 @@
 import { CoreSetup } from '@kbn/core/public';
 import type { PaletteRegistry } from '@kbn/coloring';
 import type { IAggType } from '@kbn/data-plugin/public';
-import type { Datatable, RenderMode } from '@kbn/expressions-plugin/common';
+import type { Datatable, DatatableColumnMeta, RenderMode } from '@kbn/expressions-plugin/common';
 import type {
   ILensInterpreterRenderHandlers,
   LensCellValueAction,
@@ -49,7 +49,7 @@ export type LensPagesizeAction = LensEditEvent<typeof LENS_EDIT_PAGESIZE_ACTION>
 export type DatatableRenderProps = DatatableProps & {
   formatFactory: FormatFactory;
   dispatchEvent: ILensInterpreterRenderHandlers['event'];
-  getType: (name: string) => IAggType | undefined;
+  getType: (meta?: DatatableColumnMeta) => IAggType | undefined;
   renderMode: RenderMode;
   paletteService: PaletteRegistry;
   theme: CoreSetup['theme'];