Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
import { getOriginalId } from '@kbn/transpose-utils';
import { Datatable, DatatableColumnType } from '@kbn/expressions-plugin/common';
import { KbnPalettes } from '@kbn/palettes';
import { DataType } from '../../types';
import { DataType, DatasourcePublicAPI } from '../../types';

/**
* Returns array of colors for provided palette or colorMapping
Expand All @@ -45,8 +45,33 @@ export function getPaletteDisplayColors(
.getCategoricalColors(palette?.params?.steps || 10, palette);
}

/**
* Analyze the column from the datasource prospective (formal check)
* to know whether it's a numeric type or not
* Note: to be used for Lens UI only
*/
export function getAccessorType(
datasource: DatasourcePublicAPI | undefined,
accessor: string | undefined
) {
// No accessor means it's not a numeric type by default
if (!accessor || !datasource) {
return { isNumeric: false, isCategory: false };
}
const operation = datasource.getOperationForColumnId(accessor);
const isNumericTypeFromOperation = Boolean(
!operation?.isBucketed && operation?.dataType === 'number' && !operation.hasArraySupport
);
const isBucketableTypeFromOperationType = Boolean(
operation?.isBucketed ||
(!['number', 'date'].includes(operation?.dataType || '') && !operation?.hasArraySupport)
);
return { isNumeric: isNumericTypeFromOperation, isCategory: isBucketableTypeFromOperationType };
}

/**
* Bucketed numerical columns should be treated as categorical
* Note: to be used within expression renderer scope only
*/
export function shouldColorByTerms(
dataType?: DataType | DatatableColumnType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ 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, DatasourcePublicAPI, OperationDescriptor } from '../../../types';
import { FramePublicAPI, DatasourcePublicAPI, OperationDescriptor, DataType } from '../../../types';
import { DatatableVisualizationState } from '../visualization';
import { createMockDatasource, createMockFramePublicAPI } from '../../../mocks';
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';
import { getKbnPalettes } from '@kbn/palettes';

describe('data table dimension editor', () => {
Expand Down Expand Up @@ -127,14 +126,13 @@ describe('data table dimension editor', () => {
});

it('should render default alignment for number', () => {
frame.activeData!.first.columns[0].meta.type = 'number';
mockOperationForFirstColumn({ dataType: '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' };
mockOperationForFirstColumn({ isBucketed: true, dataType: 'number' });
renderTableDimensionEditor();
expect(btnGroups.alignment.selected).toHaveTextContent('Left');
});
Expand Down Expand Up @@ -178,10 +176,10 @@ describe('data table dimension editor', () => {
expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument();
});

it.each<DatatableColumnType>(['date'])(
it.each<DataType>(['date'])(
'should not show the dynamic coloring option for "%s" columns',
(type) => {
frame.activeData!.first.columns[0].meta.type = type;
mockOperationForFirstColumn({ dataType: type });

renderTableDimensionEditor();
expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).not.toBeInTheDocument();
Expand Down Expand Up @@ -229,16 +227,15 @@ describe('data table dimension editor', () => {
});
});

it.each<{ flyout: 'terms' | 'values'; isBucketed: boolean; type: DatatableColumnType }>([
it.each<{ flyout: 'terms' | 'values'; isBucketed: boolean; type: DataType }>([
{ 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 $type column',
async ({ flyout, isBucketed, type }) => {
state.columns[0].colorMode = 'cell';
frame.activeData!.first.columns[0].meta.type = type;
mockOperationForFirstColumn({ isBucketed });
mockOperationForFirstColumn({ isBucketed, dataType: type });
renderTableDimensionEditor();

await user.click(screen.getByLabelText('Edit colors'));
Expand All @@ -250,8 +247,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 });
mockOperationForFirstColumn({ isBucketed: true, dataType: 'string' });

renderTableDimensionEditor();
expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,14 @@ import type { DatatableVisualizationState } from '../visualization';
import {
defaultPaletteParams,
findMinMaxByColumnId,
shouldColorByTerms,
getAccessorType,
} from '../../../shared_components';

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';
import { DatatableInspectorTables } from '../../../../common/expressions/datatable/datatable_fn';

const idPrefix = htmlIdGenerator()();
Expand Down Expand Up @@ -90,13 +86,13 @@ export function TableDimensionEditor(props: TableDimensionEditorProps) {
const currentData =
frame.activeData?.[localState.layerId] ?? frame.activeData?.[DatatableInspectorTables.Default];
const datasource = frame.datasourceLayers?.[localState.layerId];
const { isBucketed } = datasource?.getOperationForColumnId(accessor) ?? {};
const meta = getFieldMetaFromDatatable(currentData, accessor);
const showColorByTerms = shouldColorByTerms(meta?.type, isBucketed);
const currentAlignment = getColumnAlignment(column, isNumericField(meta));

const { isNumeric, isCategory: isBucketable } = getAccessorType(datasource, accessor);
const showColorByTerms = isBucketable;
const showDynamicColoringFeature = isBucketable || isNumeric;
const currentAlignment = getColumnAlignment(column, isNumeric);
const currentColorMode = column?.colorMode || 'none';
const hasDynamicColoring = currentColorMode !== 'none';
const showDynamicColoringFeature = meta?.type !== 'date';
const visibleColumnsCount = localState.columns.filter((c) => !c.hidden).length;

const hasTransposedColumn = localState.columns.some(({ isTransposed }) => isTransposed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import {
defaultPaletteParams,
findMinMaxByColumnId,
getPaletteDisplayColors,
shouldColorByTerms,
getAccessorType,
} from '../../shared_components';
import { getColorMappingTelemetryEvents } from '../../lens_ui_telemetry/color_telemetry_helpers';
import { DatatableInspectorTables } from '../../../common/expressions/datatable/datatable_fn';
Expand Down Expand Up @@ -147,11 +147,11 @@ export const getDatatableVisualization = ({

const hasTransposedColumn = state.columns.some(({ isTransposed }) => isTransposed);
const columns = state.columns.map((column) => {
if (column.palette) {
const accessor = column.columnId;
const accessor = column.columnId;
const { isNumeric, isCategory: isBucketable } = getAccessorType(datasource, accessor);
if (column.palette && (isNumeric || isBucketable)) {
const showColorByTerms = isBucketable;
const currentData = frame?.activeData?.[state.layerId];
const { dataType, isBucketed } = datasource?.getOperationForColumnId(column.columnId) ?? {};
const showColorByTerms = shouldColorByTerms(dataType, isBucketed);
const palette = paletteMap.get(column.palette?.name ?? '');
const columnsToCheck = hasTransposedColumn
? currentData?.columns
Expand All @@ -163,7 +163,7 @@ export const getDatatableVisualization = ({
if (palette && !showColorByTerms && !palette?.canDynamicColoring && dataBounds) {
const newPalette: PaletteOutput<CustomPaletteParams> = {
type: 'palette',
name: showColorByTerms ? 'default' : defaultPaletteParams.name,
name: defaultPaletteParams.name,
};
return {
...column,
Expand Down Expand Up @@ -583,6 +583,10 @@ export const getDatatableVisualization = ({
columns: columns
.filter((c) => !c.collapseFn)
.map((column) => {
const { isNumeric, isCategory: isBucketable } = getAccessorType(
datasource,
column.columnId
);
const stops = getOverridePaletteStops(paletteService, column.palette);
const paletteParams = {
...column.palette?.params,
Expand All @@ -594,11 +598,11 @@ export const getDatatableVisualization = ({
: [],
reverse: false, // managed at UI level
};
const { dataType, isBucketed, sortingHint, inMetricDimension } =
const { sortingHint, inMetricDimension } =
datasource?.getOperationForColumnId(column.columnId) ?? {};
const hasNoSummaryRow = column.summaryRow == null || column.summaryRow === 'none';
const canColor = dataType !== 'date';
const colorByTerms = shouldColorByTerms(dataType, isBucketed);
const canColor = isNumeric || isBucketable;
const colorByTerms = isBucketable;
let isTransposable =
!isTextBasedLanguage &&
!datasource!.getOperationForColumnId(column.columnId)?.isBucketed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,13 @@ import { getColumnByAccessor } from '@kbn/visualizations-plugin/common/utils';
import { css } from '@emotion/react';
import { DebouncedInput, IconSelect } from '@kbn/visualization-ui-components';
import { useDebouncedValue } from '@kbn/visualization-utils';
import { PalettePanelContainer } from '../../shared_components';
import { PalettePanelContainer, getAccessorType } from '../../shared_components';
import type { VisualizationDimensionEditorProps } from '../../types';
import { defaultNumberPaletteParams, defaultPercentagePaletteParams } from './palette_config';
import { DEFAULT_MAX_COLUMNS, getDefaultColor, showingBar } from './visualization';
import { CollapseSetting } from '../../shared_components/collapse_setting';
import { MetricVisualizationState } from './types';
import { metricIconsSet } from '../../shared_components/icon_set';
import { isMetricNumericType } from './helpers';

export type SupportingVisType = 'none' | 'bar' | 'trendline';

Expand Down Expand Up @@ -217,7 +216,7 @@ function PrimaryMetricEditor(props: SubProps) {
return null;
}

const isMetricNumeric = isMetricNumericType(props.datasource, accessor);
const { isNumeric: isMetricNumeric } = getAccessorType(props.datasource, accessor);

const hasDynamicColoring = Boolean(isMetricNumeric && state.palette);

Expand Down Expand Up @@ -417,7 +416,7 @@ export function DimensionEditorAdditionalSection({
}: VisualizationDimensionEditorProps<MetricVisualizationState>) {
const { euiTheme } = useEuiTheme();

const isMetricNumeric = isMetricNumericType(datasource, accessor);
const { isNumeric: isMetricNumeric } = getAccessorType(datasource, accessor);
if (accessor !== state.metricAccessor || !isMetricNumeric) {
return null;
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { showingBar } from './metric_visualization';
import { DEFAULT_MAX_COLUMNS, getDefaultColor } from './visualization';
import { MetricVisualizationState } from './types';
import { metricStateDefaults } from './constants';
import { isMetricNumericType } from './helpers';
import { getAccessorType } from '../../shared_components';

// TODO - deduplicate with gauges?
function computePaletteParams(
Expand Down Expand Up @@ -105,7 +105,7 @@ export const toExpression = (
const datasource = datasourceLayers[state.layerId];
const datasourceExpression = datasourceExpressionsByLayers[state.layerId];

const isMetricNumeric = isMetricNumericType(datasource, state.metricAccessor);
const { isNumeric: isMetricNumeric } = getAccessorType(datasource, state.metricAccessor);
const maxPossibleTiles =
// if there's a collapse function, no need to calculate since we're dealing with a single tile
state.breakdownByAccessor && !state.collapseFn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { toExpression } from './to_expression';
import { nonNullable } from '../../utils';
import { METRIC_NUMERIC_MAX } from '../../user_messages_ids';
import { MetricVisualizationState } from './types';
import { isMetricNumericType } from './helpers';
import { getAccessorType } from '../../shared_components';

export const DEFAULT_MAX_COLUMNS = 3;

Expand Down Expand Up @@ -658,7 +658,7 @@ export const getMetricVisualization = ({
const hasStaticColoring = !!state.color;
const hasDynamicColoring = !!state.palette;

const isMetricNumeric = isMetricNumericType(
const { isNumeric: isMetricNumeric } = getAccessorType(
frame?.datasourceLayers[state.layerId],
state.metricAccessor
);
Expand Down