From 6c9ff96b94deb1ca280ce3160ee03857b87f2403 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 5 Aug 2024 07:09:05 -0700 Subject: [PATCH 01/38] [Lens][Table] Add color by terms with color mappings --- .../categorical_color_mapping.tsx | 26 ++- .../color_mapping/color/rule_matching.ts | 2 +- .../components/palette_selector/scale.tsx | 1 + .../config/default_color_mapping.ts | 17 ++ .../shared_components/color_mapping/index.ts | 3 +- .../common/color_categories.ts | 6 +- .../public/helpers/color_assignment.ts | 1 - .../public/helpers/data_layers.tsx | 8 +- .../common/expressions/datatable/datatable.ts | 4 +- .../expressions/datatable/datatable_column.ts | 37 ++-- .../common/expressions/datatable/summary.ts | 10 +- .../datatable/transpose_helpers.ts | 20 +- .../coloring/color_mapping_accessor.ts | 32 +++ .../coloring/color_mapping_by_terms.tsx | 143 +++++++++++++ .../coloring/color_mapping_by_values.tsx | 79 +++++++ .../coloring/get_cell_color_fn.ts | 64 ++++++ .../shared_components/coloring/utils.ts | 7 +- .../shared_components/palette_picker.tsx | 14 +- .../datatable/components/cell_value.test.tsx | 4 +- .../datatable/components/cell_value.tsx | 103 +++------- .../datatable/components/columns.tsx | 4 +- .../datatable/components/dimension_editor.tsx | 134 +++++++----- .../components/table_actions.test.ts | 4 +- .../datatable/components/table_actions.ts | 16 +- .../datatable/components/table_basic.tsx | 73 +++++-- .../datatable/components/types.ts | 6 - .../public/visualizations/datatable/index.ts | 2 +- .../datatable/visualization.tsx | 59 +++--- .../partition/dimension_editor.tsx | 4 +- .../tagcloud/tags_dimension_editor.tsx | 4 +- .../visualizations/xy/visualization.tsx | 4 +- .../xy/xy_config_panel/dimension_editor.tsx | 194 +++++------------- .../xy_config_panel/xy_config_panel.test.tsx | 10 +- 33 files changed, 688 insertions(+), 407 deletions(-) create mode 100644 x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts create mode 100644 x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx create mode 100644 x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx create mode 100644 x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/categorical_color_mapping.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/categorical_color_mapping.tsx index 290c549684f90..a9249d703d747 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/categorical_color_mapping.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/categorical_color_mapping.tsx @@ -15,22 +15,26 @@ import { Container } from './components/container/container'; import { ColorMapping } from './config'; import { uiReducer } from './state/ui'; +export interface ColorMappingInputCategoricalData { + type: 'categories'; + /** an ORDERED array of categories rendered in the visualization */ + categories: Array; +} + +export interface ColorMappingInputContinuousData { + type: 'ranges'; + min: number; + max: number; + bins: number; +} + /** * A configuration object that is required to populate correctly the visible categories * or the ranges in the CategoricalColorMapping component */ export type ColorMappingInputData = - | { - type: 'categories'; - /** an ORDERED array of categories rendered in the visualization */ - categories: Array; - } - | { - type: 'ranges'; - min: number; - max: number; - bins: number; - }; + | ColorMappingInputCategoricalData + | ColorMappingInputContinuousData; /** * The props of the CategoricalColorMapping component diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts b/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts index a157c7927747c..61b15e4fb3516 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts @@ -41,7 +41,7 @@ export function rangeMatch(rule: ColorMapping.RuleRange, value: number) { } // TODO: move in some data/table related package -export const SPECIAL_TOKENS_STRING_CONVERTION = new Map([ +export const SPECIAL_TOKENS_STRING_CONVERSION = new Map([ [ '__other__', i18n.translate('coloring.colorMapping.terms.otherBucketLabel', { diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/palette_selector/scale.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/palette_selector/scale.tsx index 056db47157c60..c186aaf1ebf72 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/palette_selector/scale.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/palette_selector/scale.tsx @@ -107,6 +107,7 @@ export function ScaleMode({ getPaletteFn }: { getPaletteFn: ReturnType ( + paletteService: PaletteRegistry, + isDarkMode: boolean, + palette?: PaletteOutput, + colorMapping?: ColorMapping.Config +): string[] { + return colorMapping + ? getColorsFromMapping(isDarkMode, colorMapping) + : paletteService + .get(palette?.name || DEFAULT_FALLBACK_PALETTE) + .getCategoricalColors(10, palette); +} + export function getPaletteColors( isDarkMode: boolean, colorMappings?: ColorMapping.Config diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts index 7484eabe816ab..570f98cb5ef1a 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts @@ -11,10 +11,11 @@ export type { ColorMappingInputData } from './categorical_color_mapping'; export type { ColorMapping } from './config'; export * from './palettes'; export * from './color/color_handling'; -export { SPECIAL_TOKENS_STRING_CONVERTION } from './color/rule_matching'; +export { SPECIAL_TOKENS_STRING_CONVERSION } from './color/rule_matching'; export { DEFAULT_COLOR_MAPPING_CONFIG, DEFAULT_OTHER_ASSIGNMENT_INDEX, getPaletteColors, getColorsFromMapping, + getColorStops, } from './config/default_color_mapping'; diff --git a/src/plugins/chart_expressions/common/color_categories.ts b/src/plugins/chart_expressions/common/color_categories.ts index 0bb8811f2701a..145f34bb2883e 100644 --- a/src/plugins/chart_expressions/common/color_categories.ts +++ b/src/plugins/chart_expressions/common/color_categories.ts @@ -15,13 +15,15 @@ import { isMultiFieldKey } from '@kbn/data-plugin/common'; */ export function getColorCategories( rows: DatatableRow[], - accessor?: string + accessor?: string, + exclude?: any[] ): Array { return accessor ? rows.reduce<{ keys: Set; categories: Array }>( (acc, r) => { const value = r[accessor]; - if (value === undefined) { + + if (value === undefined || exclude?.includes(value)) { return acc; } // The categories needs to be stringified in their unformatted version. diff --git a/src/plugins/chart_expressions/expression_xy/public/helpers/color_assignment.ts b/src/plugins/chart_expressions/expression_xy/public/helpers/color_assignment.ts index 990d1ab93a1bc..0dd7ca05a2c9d 100644 --- a/src/plugins/chart_expressions/expression_xy/public/helpers/color_assignment.ts +++ b/src/plugins/chart_expressions/expression_xy/public/helpers/color_assignment.ts @@ -98,7 +98,6 @@ export const getAllSeries = ( /** * This function joins every data series name available on each layer by the same color palette. * The returned function `getRank` should return the position of a series name in this unified list by palette. - * */ export function getColorAssignments( layers: CommonXYLayerConfig[], diff --git a/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx b/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx index b3684217d137f..20315d0f59460 100644 --- a/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx @@ -28,7 +28,7 @@ import { getPalette, AVAILABLE_PALETTES, NeutralPalette, - SPECIAL_TOKENS_STRING_CONVERTION, + SPECIAL_TOKENS_STRING_CONVERSION, } from '@kbn/coloring'; import { getColorCategories } from '@kbn/chart-expressions-common'; import { isDataLayer } from '../../common/utils/layer_types_guards'; @@ -333,7 +333,7 @@ const getColor: GetColorFn = ( const name = getSeriesNameFn(series)?.toString() || ''; - const overwriteColors: Record = uiState?.get ? uiState.get('vis.colors', {}) : {}; + const overwriteColors: Record = uiState?.get?.('vis.colors', {}) ?? {}; if (Object.keys(overwriteColors).includes(name)) { return overwriteColors[name]; @@ -493,7 +493,7 @@ export const getSeriesProps: GetSeriesPropsFn = ({ // if colorMapping exist then we can apply it, if not let's use the legacy coloring method layer.colorMapping && splitColumnIds.length > 0 ? getColorSeriesAccessorFn( - JSON.parse(layer.colorMapping), // the color mapping is at this point just a strinfigied JSON + JSON.parse(layer.colorMapping), // the color mapping is at this point just a stringified JSON getPalette(AVAILABLE_PALETTES, NeutralPalette), isDarkMode, { @@ -501,7 +501,7 @@ export const getSeriesProps: GetSeriesPropsFn = ({ categories: getColorCategories(table.rows, splitColumnIds[0]), }, splitColumnIds[0], - SPECIAL_TOKENS_STRING_CONVERTION + SPECIAL_TOKENS_STRING_CONVERSION ) : (series) => getColor( diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable.ts index 6b73b63d30c63..cf70473770217 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/datatable.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/datatable.ts @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n'; import type { ExecutionContext } from '@kbn/expressions-plugin/common'; import type { FormatFactory, RowHeightMode } from '../../types'; -import type { ColumnConfigArg } from './datatable_column'; +import type { DatatableColumnConfigArgs } from './datatable_column'; import type { DatatableExpressionFunction } from './types'; export interface SortingState { @@ -24,7 +24,7 @@ export interface PagingState { export interface DatatableArgs { title: string; description?: string; - columns: ColumnConfigArg[]; + columns: DatatableColumnConfigArgs[]; sortingColumnId: SortingState['columnId']; sortingDirection: SortingState['direction']; fitRowToContent?: boolean; diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts index 267671a062655..7dc607d8b32c9 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts @@ -6,23 +6,25 @@ */ import type { Direction } from '@elastic/eui'; -import type { PaletteOutput, CustomPaletteParams } from '@kbn/coloring'; +import type { PaletteOutput, CustomPaletteParams, ColorMapping } from '@kbn/coloring'; import type { CustomPaletteState } from '@kbn/charts-plugin/common'; import type { ExpressionFunctionDefinition, DatatableColumn } from '@kbn/expressions-plugin/common'; import type { SortingHint } from '../../types'; import { CollapseFunction } from '../collapse'; +const LENS_DATATABLE_COLUMN = 'lens_datatable_column'; + export type LensGridDirection = 'none' | Direction; -export interface ColumnConfig { - columns: ColumnConfigArg[]; +export interface DatatableColumnConfig { + columns: DatatableColumnConfigArgs[]; sortingColumnId: string | undefined; sortingDirection: LensGridDirection; } -export type ColumnConfigArg = Omit & { - type: 'lens_datatable_column'; +export type DatatableColumnConfigArgs = Omit & { palette?: PaletteOutput; + colorMapping?: string; summaryRowValue?: unknown; sortingHint?: SortingHint; }; @@ -41,6 +43,7 @@ export interface ColumnState { bucketValues?: Array<{ originalBucketColumn: DatatableColumn; value: unknown }>; alignment?: 'left' | 'right' | 'center'; palette?: PaletteOutput; + colorMapping?: ColorMapping.Config; colorMode?: 'none' | 'cell' | 'text'; summaryRow?: 'none' | 'sum' | 'avg' | 'count' | 'min' | 'max'; summaryLabel?: string; @@ -48,18 +51,20 @@ export interface ColumnState { isMetric?: boolean; } -export type DatatableColumnResult = ColumnState & { type: 'lens_datatable_column' }; -export type DatatableColumnFunction = ExpressionFunctionDefinition< - 'lens_datatable_column', +export type DatatableColumnResult = DatatableColumnConfigArgs & { + type: typeof LENS_DATATABLE_COLUMN; +}; +export type DatatableColumnFn = ExpressionFunctionDefinition< + typeof LENS_DATATABLE_COLUMN, null, - ColumnState & { sortingHint?: SortingHint }, + DatatableColumnConfigArgs, DatatableColumnResult >; -export const datatableColumn: DatatableColumnFunction = { - name: 'lens_datatable_column', +export const datatableColumn: DatatableColumnFn = { + name: LENS_DATATABLE_COLUMN, aliases: [], - type: 'lens_datatable_column', + type: LENS_DATATABLE_COLUMN, help: '', inputTypes: ['null'], args: { @@ -76,12 +81,16 @@ export const datatableColumn: DatatableColumnFunction = { types: ['palette'], help: '', }, + colorMapping: { + types: ['string'], + help: '', + }, summaryRow: { types: ['string'], help: '' }, summaryLabel: { types: ['string'], help: '' }, }, - fn: function fn(input: unknown, args: ColumnState) { + fn: function fn(input, args) { return { - type: 'lens_datatable_column', + type: LENS_DATATABLE_COLUMN, ...args, }; }, diff --git a/x-pack/plugins/lens/common/expressions/datatable/summary.ts b/x-pack/plugins/lens/common/expressions/datatable/summary.ts index 4516abfabb06c..4088a0f7f903d 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/summary.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/summary.ts @@ -8,15 +8,15 @@ import { i18n } from '@kbn/i18n'; import type { FieldFormat } from '@kbn/field-formats-plugin/common'; import type { Datatable } from '@kbn/expressions-plugin/common'; -import { ColumnConfigArg } from './datatable_column'; +import { DatatableColumnConfigArgs } from './datatable_column'; import { getOriginalId } from './transpose_helpers'; import { isNumericFieldForDatatable } from './utils'; -type SummaryRowType = Extract; +type SummaryRowType = Extract; export function getFinalSummaryConfiguration( columnId: string, - columnArgs: Pick | undefined, + columnArgs: Pick | undefined, table: Datatable | undefined ) { const isNumeric = isNumericFieldForDatatable(table, columnId); @@ -87,7 +87,7 @@ export function getSummaryRowOptions(): Array<{ /** @internal **/ export function computeSummaryRowForColumn( - columnArgs: ColumnConfigArg, + columnArgs: DatatableColumnConfigArgs, table: Datatable, formatters: Record, defaultFormatter: FieldFormat @@ -101,7 +101,7 @@ export function computeSummaryRowForColumn( } function computeFinalValue( - type: ColumnConfigArg['summaryRow'], + type: DatatableColumnConfigArgs['summaryRow'], columnId: string, rows: Datatable['rows'] ) { diff --git a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts index ef07a87cfb9e1..3daad373640d3 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts @@ -8,7 +8,7 @@ import type { Datatable, DatatableColumn, DatatableRow } from '@kbn/expressions-plugin/common'; import type { FieldFormat } from '@kbn/field-formats-plugin/common'; import type { DatatableArgs } from './datatable'; -import type { ColumnConfig, ColumnConfigArg } from './datatable_column'; +import type { DatatableColumnConfig, DatatableColumnConfigArgs } from './datatable_column'; const TRANSPOSE_SEPARATOR = '---'; @@ -87,11 +87,11 @@ export function transposeTable( function transposeRows( firstTable: Datatable, - bucketsColumnArgs: ColumnConfigArg[], + bucketsColumnArgs: DatatableColumnConfigArgs[], formatters: Record, transposedColumnFormatter: FieldFormat, transposedColumnId: string, - metricsColumnArgs: ColumnConfigArg[] + metricsColumnArgs: DatatableColumnConfigArgs[] ) { const rowsByBucketColumns: Record = groupRowsByBucketColumns( firstTable, @@ -113,8 +113,8 @@ function transposeRows( */ function updateColumnArgs( args: DatatableArgs, - bucketsColumnArgs: ColumnConfig['columns'], - transposedColumnGroups: Array + bucketsColumnArgs: DatatableColumnConfig['columns'], + transposedColumnGroups: Array ) { args.columns = [...bucketsColumnArgs]; // add first column from each group, then add second column for each group, ... @@ -151,8 +151,8 @@ function getUniqueValues(table: Datatable, formatter: FieldFormat, columnId: str */ function transposeColumns( args: DatatableArgs, - bucketsColumnArgs: ColumnConfig['columns'], - metricColumns: ColumnConfig['columns'], + bucketsColumnArgs: DatatableColumnConfig['columns'], + metricColumns: DatatableColumnConfig['columns'], firstTable: Datatable, uniqueValues: string[], uniqueRawValues: unknown[], @@ -196,10 +196,10 @@ function transposeColumns( */ function mergeRowGroups( rowsByBucketColumns: Record, - bucketColumns: ColumnConfigArg[], + bucketColumns: DatatableColumnConfigArgs[], formatter: FieldFormat, transposedColumnId: string, - metricColumns: ColumnConfigArg[] + metricColumns: DatatableColumnConfigArgs[] ) { return Object.values(rowsByBucketColumns).map((rows) => { const mergedRow: DatatableRow = {}; @@ -222,7 +222,7 @@ function mergeRowGroups( */ function groupRowsByBucketColumns( firstTable: Datatable, - bucketColumns: ColumnConfigArg[], + bucketColumns: DatatableColumnConfigArgs[], formatters: Record ) { const rowsByBucketColumns: Record = {}; diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts new file mode 100644 index 0000000000000..61ccee03e4091 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { AVAILABLE_PALETTES, getColorFactory, getPalette, NeutralPalette } from '@kbn/coloring'; +import { ColorMappingInputCategoricalData } from '@kbn/coloring/src/shared_components/color_mapping/categorical_color_mapping'; +import { CellColorFn } from './get_cell_color_fn'; + +/** + * Return a color accessor function for XY charts depending on the split accessors received. + */ +export function getColorAccessorFn( + colorMapping: string, + data: ColorMappingInputCategoricalData, + isDarkMode: boolean +): CellColorFn { + const getColor = getColorFactory( + JSON.parse(colorMapping), + getPalette(AVAILABLE_PALETTES, NeutralPalette), + isDarkMode, + data + ); + + return (value) => { + if (value === undefined || value === null || typeof value === 'number') return null; + + return getColor(value); + }; +} diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx new file mode 100644 index 0000000000000..68f9400d490a6 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx @@ -0,0 +1,143 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React, { MutableRefObject, useState } from 'react'; + +import { + EuiBadge, + EuiFlexGroup, + EuiFlexItem, + EuiFormRow, + EuiSpacer, + EuiSwitch, + EuiText, +} from '@elastic/eui'; +import { + ColorMapping, + DEFAULT_COLOR_MAPPING_CONFIG, + CategoricalColorMapping, + SPECIAL_TOKENS_STRING_CONVERSION, + AVAILABLE_PALETTES, + PaletteOutput, + PaletteRegistry, + CustomPaletteParams, + getColorStops, +} from '@kbn/coloring'; +import { i18n } from '@kbn/i18n'; +import { trackUiCounterEvents } from '../../lens_ui_telemetry'; +import { PalettePicker } from '../palette_picker'; +import { PalettePanelContainer } from './palette_panel_container'; + +interface ColorMappingByTermsProps { + label?: string; + isDarkMode: boolean; + colorMapping?: ColorMapping.Config; + palette?: PaletteOutput; + isInlineEditing?: boolean; + setPalette: (palette: PaletteOutput) => void; + setColorMapping: (colorMapping?: ColorMapping.Config) => void; + paletteService: PaletteRegistry; + panelRef: MutableRefObject; + categories: Array; +} + +export function ColorMappingByTerms({ + label, + isDarkMode, + colorMapping, + palette, + isInlineEditing, + setPalette, + setColorMapping, + paletteService, + panelRef, + categories, +}: ColorMappingByTermsProps) { + const [useNewColorMapping, setUseNewColorMapping] = useState(Boolean(colorMapping)); + + return ( + + +
+ + + + + {i18n.translate('xpack.lens.colorMapping.tryLabel', { + defaultMessage: 'Use the new Color Mapping feature', + })}{' '} + + {i18n.translate('xpack.lens.colorMapping.techPreviewLabel', { + defaultMessage: 'Tech preview', + })} + + + + } + data-test-subj="lns_colorMappingOrLegacyPalette_switch" + compressed + checked={useNewColorMapping} + onChange={({ target: { checked } }) => { + trackUiCounterEvents(`color_mapping_switch_${checked ? 'enabled' : 'disabled'}`); + setColorMapping(checked ? { ...DEFAULT_COLOR_MAPPING_CONFIG } : undefined); + setUseNewColorMapping(checked); + }} + /> + + + + {useNewColorMapping ? ( + + ) : ( + + )} + + +
+ + + ); +} diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx new file mode 100644 index 0000000000000..b6d7bbdf64c13 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx @@ -0,0 +1,79 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React, { MutableRefObject } from 'react'; + +import { EuiFlexGroup, EuiFlexItem, EuiFormRow } from '@elastic/eui'; +import { + PaletteOutput, + PaletteRegistry, + CustomizablePalette, + DataBounds, + CustomPaletteParams, +} from '@kbn/coloring'; +import { i18n } from '@kbn/i18n'; +import { PalettePanelContainer } from './palette_panel_container'; + +interface ColorMappingByValuesProps { + label?: string; + palette: PaletteOutput; + isInlineEditing?: boolean; + setPalette: (palette: PaletteOutput) => void; + paletteService: PaletteRegistry; + panelRef: MutableRefObject; + dataBounds?: DataBounds; +} + +export function ColorMappingByValues({ + label, + palette, + isInlineEditing, + setPalette, + paletteService, + panelRef, + dataBounds, +}: ColorMappingByValuesProps) { + const colors = palette.params?.stops?.map(({ color }) => color) ?? []; + + return ( + + +
+ + + { + setPalette(p); + }} + /> + + +
+ + + ); +} diff --git a/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts b/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts new file mode 100644 index 0000000000000..a21572b3da97b --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { ColorMappingInputData, PaletteOutput, PaletteRegistry } from '@kbn/coloring'; +import { CustomPaletteState } from '@kbn/charts-plugin/common'; +import { getColorAccessorFn } from './color_mapping_accessor'; + +export type CellColorFn = (value?: number | string | null) => string | null; + +export function getCellColorFn( + paletteService: PaletteRegistry, + data: ColorMappingInputData, + isNumeric: boolean, + isDarkMode: boolean, + palette?: PaletteOutput, + colorMapping?: string +): CellColorFn { + if (isNumeric && palette && data.type === 'ranges') { + return (value) => { + if (value === null || value === undefined || typeof value !== 'number') return null; + + return ( + paletteService.get(palette.name).getColorForValue?.(value, palette.params, data) ?? null + ); + }; + } + + if (!isNumeric && data.type === 'categories') { + if (colorMapping) { + return getColorAccessorFn(colorMapping, data, isDarkMode); + } else if (palette) { + return (category) => { + if (category === undefined || category === null || typeof category === 'number') + return null; + + return paletteService.get(palette.name).getCategoricalColor( + [ + { + name: category, + rankAtDepth: Math.max( + data.categories.findIndex((v) => v === category), + 0 + ), + totalSeriesAtDepth: data.categories.length || 1, + }, + ], + { + maxDepth: 1, + totalSeries: data.categories.length || 1, + behindText: false, + syncColors: true, + }, + palette?.params ?? { colors: [] } + ); + }; + } + } + + return () => null; +} diff --git a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts index 9436dec74be04..aebca956a37a3 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts @@ -37,11 +37,8 @@ export function getContrastColor( return enforceColorContrast(color, backgroundColor) ? lightColor : darkColor; } -export function getNumericValue(rowValue: number | number[] | undefined) { - if (rowValue == null || Array.isArray(rowValue)) { - return; - } - return rowValue; +export function getNumericValue(rowValue?: unknown) { + return typeof rowValue === 'number' ? rowValue : undefined; } export function applyPaletteParams>( diff --git a/x-pack/plugins/lens/public/shared_components/palette_picker.tsx b/x-pack/plugins/lens/public/shared_components/palette_picker.tsx index 6796eee37a542..24c6b05e6b6e1 100644 --- a/x-pack/plugins/lens/public/shared_components/palette_picker.tsx +++ b/x-pack/plugins/lens/public/shared_components/palette_picker.tsx @@ -12,17 +12,14 @@ import { EuiColorPalettePicker, EuiColorPalettePickerPaletteProps } from '@elast import { EuiFormRow } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -export function PalettePicker({ - palettes, - activePalette, - setPalette, -}: { +interface PalettePickerProps { palettes: PaletteRegistry; - activePalette?: PaletteOutput; + activePalette?: PaletteOutput; setPalette: (palette: PaletteOutput) => void; -}) { - const paletteName = getActivePaletteName(activePalette?.name); +} +export function PalettePicker({ palettes, activePalette, setPalette }: PalettePickerProps) { + const paletteName = getActivePaletteName(activePalette?.name); const palettesToShow: EuiColorPalettePickerPaletteProps[] = palettes .getAll() .filter(({ internal }) => !internal) @@ -34,6 +31,7 @@ export function PalettePicker({ palette: getCategoricalColors(10, id === paletteName ? activePalette?.params : undefined), }; }); + return ( { }, }, type: 'lens_datatable_column', - } as ColumnConfigArg, + } as DatatableColumnConfigArgs, ], sortingColumnId: '', sortingDirection: 'none', diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index 921bbeaa8b757..5b467d04035d0 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -6,100 +6,57 @@ */ import React, { useContext, useEffect } from 'react'; -import useObservable from 'react-use/lib/useObservable'; -import { EuiDataGridCellValueElementProps, EuiLink } from '@elastic/eui'; -import type { CoreSetup } from '@kbn/core/public'; +import { EuiDataGridCellValueElementProps } from '@elastic/eui'; import classNames from 'classnames'; +import { PaletteOutput } from '@kbn/coloring'; +import { CustomPaletteState } from '@kbn/charts-plugin/common'; import type { FormatFactory } from '../../../../common/types'; import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; -import type { ColumnConfig } from '../../../../common/expressions'; +import type { DatatableColumnConfig } from '../../../../common/expressions'; import type { DataContextType } from './types'; -import { getContrastColor, getNumericValue } from '../../../shared_components/coloring/utils'; +import { getContrastColor } from '../../../shared_components/coloring/utils'; +import { CellColorFn } from '../../../shared_components/coloring/get_cell_color_fn'; + +const getParsedValue = (v: unknown) => + typeof v === 'number' ? v : v === null || v === undefined ? v : String(v); export const createGridCell = ( formatters: Record>, - columnConfig: ColumnConfig, + columnConfig: DatatableColumnConfig, DataContext: React.Context, - theme: CoreSetup['theme'], + isDarkMode: boolean, + getCellColor: ( + originalId: string, + palette?: PaletteOutput, + colorMapping?: string + ) => CellColorFn, fitRowToContent?: boolean ) => { return ({ rowIndex, columnId, setCellProps }: EuiDataGridCellValueElementProps) => { - const { table, alignments, minMaxByColumnId, getColorForValue, handleFilterClick } = - useContext(DataContext); - const IS_DARK_THEME: boolean = useObservable(theme.theme$, { darkMode: false }).darkMode; - - const rowValue = table?.rows[rowIndex]?.[columnId]; - + const { table, alignments, handleFilterClick } = useContext(DataContext); + const rowValue = getParsedValue(table?.rows[rowIndex]?.[columnId]); const colIndex = columnConfig.columns.findIndex(({ columnId: id }) => id === columnId); - const { colorMode, palette, oneClickFilter } = columnConfig.columns[colIndex] || {}; + const { oneClickFilter, colorMode, palette, colorMapping } = + columnConfig.columns[colIndex] ?? {}; const filterOnClick = oneClickFilter && handleFilterClick; - const content = formatters[columnId]?.convert(rowValue, filterOnClick ? 'text' : 'html'); const currentAlignment = alignments && alignments[columnId]; useEffect(() => { - const originalId = getOriginalId(columnId); - if (minMaxByColumnId?.[originalId]) { - if (colorMode !== 'none' && palette?.params && getColorForValue) { - // workout the bucket the value belongs to - const color = getColorForValue( - getNumericValue(rowValue), - palette.params, - minMaxByColumnId[originalId] - ); - if (color) { - const style = { [colorMode === 'cell' ? 'backgroundColor' : 'color']: color }; - if (colorMode === 'cell' && color) { - style.color = getContrastColor(color, IS_DARK_THEME); - } - setCellProps({ - style, - }); + if (colorMode !== 'none' && (palette || colorMapping)) { + const originalId = getOriginalId(columnId); // workout what bucket the value belongs to + const color = getCellColor(originalId, palette, colorMapping)(rowValue); + + if (color) { + const style = { [colorMode === 'cell' ? 'backgroundColor' : 'color']: color }; + if (colorMode === 'cell' && color) { + style.color = getContrastColor(color, isDarkMode); } + setCellProps({ style }); } } - // make sure to clean it up when something change - // this avoids cell's styling to stick forever - return () => { - if (minMaxByColumnId?.[originalId]) { - setCellProps({ - style: { - backgroundColor: undefined, - color: undefined, - }, - }); - } - }; - }, [ - rowValue, - columnId, - setCellProps, - colorMode, - palette, - minMaxByColumnId, - getColorForValue, - IS_DARK_THEME, - ]); + }, [rowValue, columnId, setCellProps, colorMode, palette, colorMapping]); - if (filterOnClick) { - return ( -
- { - handleFilterClick?.(columnId, rowValue, colIndex, rowIndex); - }} - > - {content} - -
- ); - } return (
void) | undefined, isReadOnly: boolean, - columnConfig: ColumnConfig, + columnConfig: DatatableColumnConfig, visibleColumns: string[], formatFactory: FormatFactory, onColumnResize: (eventData: { columnId: string; width: number | undefined }) => void, diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index 2ffe06144ff48..9f98ff06c178f 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -5,17 +5,18 @@ * 2.0. */ -import React from 'react'; +import React, { useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiFormRow, EuiSwitch, EuiButtonGroup, htmlIdGenerator } from '@elastic/eui'; -import { CustomizablePalette, PaletteRegistry } from '@kbn/coloring'; +import { PaletteRegistry } from '@kbn/coloring'; +import { getColorCategories } from '@kbn/chart-expressions-common'; +import { useDebouncedValue } from '@kbn/visualization-utils'; import type { VisualizationDimensionEditorProps } from '../../../types'; import type { DatatableVisualizationState } from '../visualization'; import { applyPaletteParams, defaultPaletteParams, - PalettePanelContainer, findMinMaxByColumnId, } from '../../../shared_components'; import { isNumericFieldForDatatable } from '../../../../common/expressions/datatable/utils'; @@ -23,19 +24,21 @@ import { getOriginalId } from '../../../../common/expressions/datatable/transpos 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'; const idPrefix = htmlIdGenerator()(); type ColumnType = DatatableVisualizationState['columns'][number]; -function updateColumnWith( +function updateColumn( state: DatatableVisualizationState, columnId: string, - newColumnProps: Partial + newColumn: Partial ) { return state.columns.map((currentColumn) => { if (currentColumn.columnId === columnId) { - return { ...currentColumn, ...newColumnProps }; + return { ...currentColumn, ...newColumn }; } else { return currentColumn; } @@ -45,15 +48,31 @@ function updateColumnWith( export function TableDimensionEditor( props: VisualizationDimensionEditorProps & { paletteService: PaletteRegistry; + isDarkMode: boolean; } ) { - const { state, setState, frame, accessor, isInlineEditing } = props; - const column = state.columns.find(({ columnId }) => accessor === columnId); + const { frame, accessor, isInlineEditing, isDarkMode } = props; + const column = props.state.columns.find(({ columnId }) => accessor === columnId); + const { inputValue: localState, handleInputChange: setLocalState } = + useDebouncedValue({ + value: props.state, + onChange: props.setState, + }); + + const updateColumnState = useCallback( + (columnId: string, newColumn: Partial) => { + setLocalState({ + ...localState, + columns: updateColumn(localState, columnId, newColumn), + }); + }, + [setLocalState, localState] + ); if (!column) return null; if (column.isTransposed) return null; - const currentData = frame.activeData?.[state.layerId]; + const currentData = frame.activeData?.[localState.layerId]; // either read config state or use same logic as chart itself const isNumeric = isNumericFieldForDatatable(currentData, accessor); @@ -61,14 +80,14 @@ export function TableDimensionEditor( const currentColorMode = column?.colorMode || 'none'; const hasDynamicColoring = currentColorMode !== 'none'; - const datasource = frame.datasourceLayers[state.layerId]; + const datasource = frame.datasourceLayers[localState.layerId]; const showDynamicColoringFeature = Boolean( - isNumeric && !datasource?.getOperationForColumnId(accessor)?.isBucketed + !datasource?.getOperationForColumnId(accessor)?.isBucketed ); - const visibleColumnsCount = state.columns.filter((c) => !c.hidden).length; + const visibleColumnsCount = localState.columns.filter((c) => !c.hidden).length; - const hasTransposedColumn = state.columns.some(({ isTransposed }) => isTransposed); + const hasTransposedColumn = localState.columns.some(({ isTransposed }) => isTransposed); const columnsToCheck = hasTransposedColumn ? currentData?.columns.filter(({ id }) => getOriginalId(id) === accessor).map(({ id }) => id) || [] @@ -76,12 +95,13 @@ export function TableDimensionEditor( const minMaxByColumnId = findMinMaxByColumnId(columnsToCheck, currentData, getOriginalId); const currentMinMax = minMaxByColumnId[accessor]; - const activePalette = column?.palette || { + const activePalette = column?.palette ?? { type: 'palette', name: defaultPaletteParams.name, }; // need to tell the helper that the colorStops are required to display const displayStops = applyPaletteParams(props.paletteService, activePalette, currentMinMax); + const categories = getColorCategories(currentData?.rows ?? [], accessor, [null]); return ( <> @@ -125,10 +145,7 @@ export function TableDimensionEditor( idSelected={`${idPrefix}${currentAlignment}`} onChange={(id) => { const newMode = id.replace(idPrefix, '') as ColumnType['alignment']; - setState({ - ...state, - columns: updateColumnWith(state, accessor, { alignment: newMode }), - }); + updateColumnState(accessor, { alignment: newMode }); }} /> @@ -188,44 +205,47 @@ export function TableDimensionEditor( }; } // clear up when switching to no coloring - if (column?.palette && newMode === 'none') { + if (newMode === 'none') { params.palette = undefined; + params.colorMapping = undefined; } - setState({ - ...state, - columns: updateColumnWith(state, accessor, params), - }); + updateColumnState(accessor, params); }} /> - {hasDynamicColoring && ( - - color)} - siblingRef={props.panelRef} + + {hasDynamicColoring && + (isNumeric ? ( + { + updateColumnState(accessor, { palette: newPalette }); + }} + paletteService={props.paletteService} + panelRef={props.panelRef} + dataBounds={currentMinMax} + /> + ) : ( + - { - setState({ - ...state, - columns: updateColumnWith(state, accessor, { palette: newPalette }), - }); - }} - /> - - - )} + setPalette={(palette) => { + updateColumnState(accessor, { palette }); + }} + setColorMapping={(colorMapping) => { + updateColumnState(accessor, { colorMapping }); + }} + paletteService={props.paletteService} + panelRef={props.panelRef} + categories={categories} + /> + ))} )} {!column.isTransposed && ( @@ -247,8 +267,8 @@ export function TableDimensionEditor( disabled={!column.hidden && visibleColumnsCount <= 1} onChange={() => { const newState = { - ...state, - columns: state.columns.map((currentColumn) => { + ...localState, + columns: localState.columns.map((currentColumn) => { if (currentColumn.columnId === accessor) { return { ...currentColumn, @@ -259,7 +279,7 @@ export function TableDimensionEditor( } }), }; - setState(newState); + setLocalState(newState); }} /> @@ -283,8 +303,8 @@ export function TableDimensionEditor( disabled={column.hidden} onChange={() => { const newState = { - ...state, - columns: state.columns.map((currentColumn) => { + ...localState, + columns: localState.columns.map((currentColumn) => { if (currentColumn.columnId === accessor) { return { ...currentColumn, @@ -295,7 +315,7 @@ export function TableDimensionEditor( } }), }; - setState(newState); + setLocalState(newState); }} /> @@ -323,7 +343,7 @@ export function TableDimensionDataExtraEditor( onChange={(collapseFn) => { setState({ ...state, - columns: updateColumnWith(state, accessor, { collapseFn }), + columns: updateColumn(state, accessor, { collapseFn }), }); }} /> diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.test.ts b/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.test.ts index 2a517acc33084..3b89f32b22ffc 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.test.ts +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.test.ts @@ -17,9 +17,9 @@ import { createGridHideHandler, createTransposeColumnFilterHandler, } from './table_actions'; -import type { LensGridDirection, ColumnConfig } from '../../../../common/expressions'; +import type { LensGridDirection, DatatableColumnConfig } from '../../../../common/expressions'; -function getDefaultConfig(): ColumnConfig { +function getDefaultConfig(): DatatableColumnConfig { return { columns: [ { columnId: 'a', type: 'lens_datatable_column' }, diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts b/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts index 8c1c56343b2f5..0cfd6289f5818 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts @@ -20,8 +20,8 @@ import { getSortingCriteria } from '@kbn/sort-predicates'; import { i18n } from '@kbn/i18n'; import type { LensResizeAction, LensSortAction, LensToggleAction } from './types'; import type { - ColumnConfig, - ColumnConfigArg, + DatatableColumnConfig, + DatatableColumnConfigArgs, LensGridDirection, } from '../../../../common/expressions'; import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; @@ -30,8 +30,8 @@ import { buildColumnsMetaLookup } from './helpers'; export const createGridResizeHandler = ( - columnConfig: ColumnConfig, - setColumnConfig: React.Dispatch>, + columnConfig: DatatableColumnConfig, + setColumnConfig: React.Dispatch>, onEditAction: (data: LensResizeAction['data']) => void ) => (eventData: { columnId: string; width: number | undefined }) => { @@ -59,8 +59,8 @@ export const createGridResizeHandler = export const createGridHideHandler = ( - columnConfig: ColumnConfig, - setColumnConfig: React.Dispatch>, + columnConfig: DatatableColumnConfig, + setColumnConfig: React.Dispatch>, onEditAction: (data: LensToggleAction['data']) => void ) => (eventData: { columnId: string }) => { @@ -177,7 +177,7 @@ function getColumnType({ columnId, lookup, }: { - columnConfig: ColumnConfig; + columnConfig: DatatableColumnConfig; columnId: string; lookup: Record< string, @@ -195,7 +195,7 @@ function getColumnType({ export const buildSchemaDetectors = ( columns: EuiDataGridColumn[], columnConfig: { - columns: ColumnConfigArg[]; + columns: DatatableColumnConfigArgs[]; sortingColumnId: string | undefined; sortingDirection: 'none' | 'asc' | 'desc'; }, diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx index 07c095ecc87ab..331da5cfcc992 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx @@ -6,7 +6,7 @@ */ import './table_basic.scss'; -import { CUSTOM_PALETTE } from '@kbn/coloring'; +import { ColorMappingInputData, PaletteOutput } from '@kbn/coloring'; import React, { useLayoutEffect, useCallback, @@ -27,9 +27,11 @@ import { EuiDataGridSorting, EuiDataGridStyle, } from '@elastic/eui'; -import { EmptyPlaceholder } from '@kbn/charts-plugin/public'; +import { CustomPaletteState, EmptyPlaceholder } from '@kbn/charts-plugin/public'; import { ClickTriggerEvent } from '@kbn/charts-plugin/public'; import { IconChartDatatable } from '@kbn/chart-icons'; +import useObservable from 'react-use/lib/useObservable'; +import { getColorCategories } from '@kbn/chart-expressions-common'; import type { LensTableRowContextMenuEvent } from '../../../types'; import type { FormatFactory } from '../../../../common/types'; import { RowHeightMode } from '../../../../common/types'; @@ -57,6 +59,8 @@ import { import { getFinalSummaryConfiguration } from '../../../../common/expressions/datatable/summary'; import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; import { DEFAULT_HEADER_ROW_HEIGHT, DEFAULT_HEADER_ROW_HEIGHT_LINES } from './constants'; +import { isNumericFieldForDatatable } from '../../../../common/expressions/datatable/utils'; +import { CellColorFn, getCellColorFn } from '../../../shared_components/coloring/get_cell_color_fn'; export const DataContext = React.createContext({}); @@ -72,6 +76,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const dataGridRef = useRef(null); const isInteractive = props.interactive; + const isDarkMode = useObservable(props.theme.theme$, { darkMode: false }).darkMode; const [columnConfig, setColumnConfig] = useState({ columns: props.args.columns, @@ -385,17 +390,58 @@ export const DatatableComponent = (props: DatatableRenderProps) => { isInteractive, ]); - const renderCellValue = useMemo( - () => - createGridCell( - formatters, - columnConfig, - DataContext, - props.theme, - props.args.fitRowToContent - ), - [formatters, columnConfig, props.theme, props.args.fitRowToContent] - ); + const renderCellValue = useMemo(() => { + const cellColorFnMap = new Map(); + const getCellColor = ( + originalId: string, + palette?: PaletteOutput, + colorMapping?: string + ): CellColorFn => { + if (cellColorFnMap.has(originalId)) { + return cellColorFnMap.get(originalId)!; + } + + const isNumeric = isNumericFieldForDatatable(firstLocalTable, originalId); + const data: ColorMappingInputData = isNumeric + ? { + type: 'ranges', + bins: 0, + ...minMaxByColumnId[originalId], + } + : { + type: 'categories', + categories: getColorCategories(firstLocalTable.rows, originalId, [null]), + }; + const colorFn = getCellColorFn( + props.paletteService, + data, + isNumeric, + isDarkMode, + palette, + colorMapping + ); + cellColorFnMap.set(originalId, colorFn); + + return colorFn; + }; + + return createGridCell( + formatters, + columnConfig, + DataContext, + isDarkMode, + getCellColor, + props.args.fitRowToContent + ); + }, [ + formatters, + columnConfig, + isDarkMode, + props.args.fitRowToContent, + props.paletteService, + firstLocalTable, + minMaxByColumnId, + ]); const columnVisibility = useMemo( () => ({ @@ -471,7 +517,6 @@ export const DatatableComponent = (props: DatatableRenderProps) => { rowHasRowClickTriggerActions: props.rowHasRowClickTriggerActions, alignments, minMaxByColumnId, - getColorForValue: props.paletteService.get(CUSTOM_PALETTE).getColorForValue!, handleFilterClick, }} > diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/types.ts b/x-pack/plugins/lens/public/visualizations/datatable/components/types.ts index 987d852161f2c..b884a2c716be9 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/types.ts +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/types.ts @@ -7,7 +7,6 @@ import { CoreSetup } from '@kbn/core/public'; import type { PaletteRegistry } from '@kbn/coloring'; -import { CustomPaletteState } from '@kbn/charts-plugin/public'; import type { IAggType } from '@kbn/data-plugin/public'; import type { Datatable, RenderMode } from '@kbn/expressions-plugin/common'; import type { @@ -82,9 +81,4 @@ export interface DataContextType { rowIndex: number, negate?: boolean ) => void; - getColorForValue?: ( - value: number | undefined, - state: CustomPaletteState, - minMax: { min: number; max: number } - ) => string | undefined; } diff --git a/x-pack/plugins/lens/public/visualizations/datatable/index.ts b/x-pack/plugins/lens/public/visualizations/datatable/index.ts index 78a8e6e43dfe8..f68f167ea5f02 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/index.ts +++ b/x-pack/plugins/lens/public/visualizations/datatable/index.ts @@ -44,7 +44,7 @@ export class DatatableVisualization { }) ); - return getDatatableVisualization({ paletteService: palettes, theme: core.theme }); + return getDatatableVisualization({ paletteService: palettes, kibanaTheme: core.theme }); }); } } diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index 534ece1785af4..bc892220e5e1a 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -8,13 +8,13 @@ import React from 'react'; import { Ast } from '@kbn/interpreter'; import { i18n } from '@kbn/i18n'; -import { PaletteRegistry, CUSTOM_PALETTE } from '@kbn/coloring'; +import { PaletteRegistry, CUSTOM_PALETTE, getColorStops } from '@kbn/coloring'; import { ThemeServiceStart } from '@kbn/core/public'; import { VIS_EVENT_TO_TRIGGER } from '@kbn/visualizations-plugin/public'; import { IconChartDatatable } from '@kbn/chart-icons'; import { LayerTypes } from '@kbn/expression-xy-plugin/public'; import { buildExpression, buildExpressionFunction } from '@kbn/expressions-plugin/common'; -import { isNumericFieldForDatatable } from '../../../common/expressions/datatable/utils'; +import useObservable from 'react-use/lib/useObservable'; import type { FormBasedPersistedState } from '../../datasources/form_based/types'; import type { SuggestionRequest, @@ -33,7 +33,7 @@ import type { SortingState, PagingState, CollapseExpressionFunction, - DatatableColumnFunction, + DatatableColumnFn, DatatableExpressionFunction, } from '../../../common/expressions'; import { DataTableToolbar } from './components/toolbar'; @@ -60,10 +60,10 @@ const visualizationLabel = i18n.translate('xpack.lens.datatable.label', { export const getDatatableVisualization = ({ paletteService, - theme, + kibanaTheme, }: { paletteService: PaletteRegistry; - theme: ThemeServiceStart; + kibanaTheme: ThemeServiceStart; }): Visualization => ({ id: 'lnsDatatable', @@ -197,13 +197,14 @@ export const getDatatableVisualization = ({ }, /* - Datatable works differently on text based datasource and form based - - Form based: It relies on the isBucketed flag to identify groups. It allows only numeric fields + Datatable works differently on text-based datasource and form-based + - Form-based: It relies on the isBucketed flag to identify groups. It allows only numeric fields on the Metrics dimension - - Text based: It relies on the isMetric flag to identify groups. It allows all type of fields + - Text-based: It relies on the isMetric flag to identify groups. It allows all type of fields on the Metric dimension in cases where there are no numeric columns **/ getConfiguration({ state, frame, layerId }) { + const isDarkMode = kibanaTheme.getTheme().darkMode; const { sortedColumns, datasource } = getDataSourceAndSortedColumns(state, frame.datasourceLayers, layerId) || {}; @@ -319,22 +320,14 @@ export const getDatatableVisualization = ({ return !operation?.isBucketed; }) .map((accessor) => { - const columnConfig = columnMap[accessor]; - const stops = columnConfig?.palette?.params?.stops; - const isNumeric = Boolean( - accessor && isNumericFieldForDatatable(frame.activeData?.[state.layerId], accessor) - ); - const hasColoring = Boolean(columnConfig?.colorMode !== 'none' && stops); + const { colorMode, palette, colorMapping, hidden } = columnMap[accessor] ?? {}; + const stops = getColorStops(paletteService, isDarkMode, palette, colorMapping); + const hasColoring = Boolean(colorMode !== 'none' && stops); return { columnId: accessor, - triggerIconType: columnConfig?.hidden - ? 'invisible' - : hasColoring && isNumeric - ? 'colorBy' - : undefined, - palette: - hasColoring && isNumeric && stops ? stops.map(({ color }) => color) : undefined, + triggerIconType: hidden ? 'invisible' : hasColoring ? 'colorBy' : undefined, + palette: hasColoring ? stops : undefined, }; }), supportsMoreColumns: true, @@ -386,7 +379,11 @@ export const getDatatableVisualization = ({ }; }, DimensionEditorComponent(props) { - return ; + const isDarkMode = useObservable(kibanaTheme.theme$, { darkMode: false }).darkMode; + + return ( + + ); }, DimensionEditorAdditionalSectionComponent(props) { @@ -482,12 +479,10 @@ export const getDatatableVisualization = ({ reverse: false, // managed at UI level }; const sortingHint = datasource!.getOperationForColumnId(column.columnId)!.sortingHint; - const hasNoSummaryRow = column.summaryRow == null || column.summaryRow === 'none'; - - const canColor = - datasource!.getOperationForColumnId(column.columnId)?.dataType === 'number'; - + const dataType = datasource!.getOperationForColumnId(column.columnId)?.dataType; + const canColor = dataType !== 'date'; + const isNumeric = dataType === 'number'; let isTransposable = !isTextBasedLanguage && !datasource!.getOperationForColumnId(column.columnId)?.isBucketed; @@ -497,7 +492,7 @@ export const getDatatableVisualization = ({ isTransposable = Boolean(column?.isMetric || operation?.inMetricDimension); } - const datatableColumnFn = buildExpressionFunction( + const datatableColumnFn = buildExpressionFunction( 'lens_datatable_column', { columnId: column.columnId, @@ -507,8 +502,12 @@ export const getDatatableVisualization = ({ isTransposed: column.isTransposed, transposable: isTransposable, alignment: column.alignment, - colorMode: canColor && column.colorMode ? column.colorMode : 'none', - palette: paletteService.get(CUSTOM_PALETTE).toExpression(paletteParams), + colorMode: canColor ? column.colorMode ?? 'none' : 'none', + palette: paletteService + // The palette for numeric values is a pseudo custom palette that is only custom from params level + .get(isNumeric ? CUSTOM_PALETTE : column.palette?.name || CUSTOM_PALETTE) + .toExpression(paletteParams), + colorMapping: column.colorMapping ? JSON.stringify(column.colorMapping) : undefined, summaryRow: hasNoSummaryRow ? undefined : column.summaryRow!, summaryLabel: hasNoSummaryRow ? undefined diff --git a/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx index a45faa1d5aaf1..88412b7675b17 100644 --- a/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx @@ -13,7 +13,7 @@ import { DEFAULT_COLOR_MAPPING_CONFIG, PaletteRegistry, ColorMapping, - SPECIAL_TOKENS_STRING_CONVERTION, + SPECIAL_TOKENS_STRING_CONVERSION, AVAILABLE_PALETTES, getColorsFromMapping, } from '@kbn/coloring'; @@ -188,7 +188,7 @@ export function DimensionEditor(props: DimensionEditorProps) { type: 'categories', categories: splitCategories, }} - specialTokens={SPECIAL_TOKENS_STRING_CONVERTION} + specialTokens={SPECIAL_TOKENS_STRING_CONVERSION} /> ) : ( ) : ( l.layerId === props.layerId)!; const dimensionEditor = isReferenceLayer(layer) ? ( ) : isAnnotationsLayer(layer) ? ( ) : ( - + ); return dimensionEditor; diff --git a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/dimension_editor.tsx index e53bc9164b0c3..c0916b823466e 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/dimension_editor.tsx @@ -5,81 +5,68 @@ * 2.0. */ -import React, { useCallback, useMemo, useState } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { i18n } from '@kbn/i18n'; import { useDebouncedValue } from '@kbn/visualization-utils'; import { ColorPicker } from '@kbn/visualization-ui-components'; -import { - EuiBadge, - EuiButtonGroup, - EuiFlexGroup, - EuiFlexItem, - EuiFormRow, - EuiSpacer, - EuiSwitch, - EuiText, - htmlIdGenerator, -} from '@elastic/eui'; -import { - PaletteRegistry, - ColorMapping, - DEFAULT_COLOR_MAPPING_CONFIG, - CategoricalColorMapping, - PaletteOutput, - SPECIAL_TOKENS_STRING_CONVERTION, - AVAILABLE_PALETTES, - getColorsFromMapping, -} from '@kbn/coloring'; +import { EuiButtonGroup, EuiFormRow, htmlIdGenerator } from '@elastic/eui'; +import { PaletteRegistry, ColorMapping, PaletteOutput } from '@kbn/coloring'; import { getColorCategories } from '@kbn/chart-expressions-common'; +import type { ValuesType } from 'utility-types'; import type { VisualizationDimensionEditorProps } from '../../../types'; import { State, XYState, XYDataLayerConfig, YConfig, YAxisMode } from '../types'; import { FormatFactory } from '../../../../common/types'; import { getSeriesColor, isHorizontalChart } from '../state_helpers'; -import { PalettePanelContainer, PalettePicker } from '../../../shared_components'; import { getDataLayers } from '../visualization_helpers'; import { CollapseSetting } from '../../../shared_components/collapse_setting'; import { getSortedAccessors } from '../to_expression'; import { getColorAssignments, getAssignedColorConfig } from '../color_assignment'; -import { trackUiCounterEvents } from '../../../lens_ui_telemetry'; +import { ColorMappingByTerms } from '../../../shared_components/coloring/color_mapping_by_terms'; -type UnwrapArray = T extends Array ? P : T; +export const idPrefix = htmlIdGenerator()(); -export function updateLayer( +function updateLayer( state: State, - layer: UnwrapArray, - index: number -): State { + index: number, + layer: ValuesType, + newLayer: Partial> +): State['layers'] { const newLayers = [...state.layers]; - newLayers[index] = layer; + newLayers[index] = { + ...layer, + ...newLayer, + } as ValuesType; - return { - ...state, - layers: newLayers, - }; + return newLayers; } -export const idPrefix = htmlIdGenerator()(); - export function DataDimensionEditor( props: VisualizationDimensionEditorProps & { formatFactory: FormatFactory; paletteService: PaletteRegistry; - darkMode: boolean; + isDarkMode: boolean; } ) { - const { state, layerId, accessor, darkMode, isInlineEditing } = props; + const { state, layerId, accessor, isDarkMode, isInlineEditing } = props; const index = state.layers.findIndex((l) => l.layerId === layerId); const layer = state.layers[index] as XYDataLayerConfig; - const canUseColorMapping = layer.colorMapping ? true : false; - - const [useNewColorMapping, setUseNewColorMapping] = useState(canUseColorMapping); const { inputValue: localState, handleInputChange: setLocalState } = useDebouncedValue({ value: props.state, onChange: props.setState, }); + const updateLayerState = useCallback( + (layerIndex: number, newLayer: Partial>) => { + setLocalState({ + ...localState, + layers: updateLayer(localState, layerIndex, layer, newLayer), + }); + }, + [layer, setLocalState, localState] + ); + const localYConfig = layer?.yConfig?.find((yAxisConfig) => yAxisConfig.forAccessor === accessor); const axisMode = localYConfig?.axisMode || 'auto'; @@ -100,22 +87,22 @@ export function DataDimensionEditor( ...yConfig, }); } - setLocalState(updateLayer(localState, { ...layer, yConfig: newYConfigs }, index)); + updateLayerState(index, { yConfig: newYConfigs }); }, - [accessor, index, localState, layer, setLocalState] + [layer.yConfig, updateLayerState, index, accessor] ); const setColorMapping = useCallback( (colorMapping?: ColorMapping.Config) => { - setLocalState(updateLayer(localState, { ...layer, colorMapping }, index)); + updateLayerState(index, { colorMapping }); }, - [index, localState, layer, setLocalState] + [updateLayerState, index] ); const setPalette = useCallback( (palette: PaletteOutput) => { - setLocalState(updateLayer(localState, { ...layer, palette }, index)); + updateLayerState(index, { palette }); }, - [index, localState, layer, setLocalState] + [updateLayerState, index] ); const overwriteColor = getSeriesColor(layer, accessor); @@ -143,105 +130,28 @@ export function DataDimensionEditor( ).color; }, [props.frame, props.paletteService, state.layers, accessor, props.formatFactory, layer]); - const localLayer: XYDataLayerConfig = layer; - - const colors = layer.colorMapping - ? getColorsFromMapping(props.darkMode, layer.colorMapping) - : props.paletteService - .get(layer.palette?.name || 'default') - .getCategoricalColors(10, layer.palette); - const table = props.frame.activeData?.[layer.layerId]; const { splitAccessor } = layer; const splitCategories = getColorCategories(table?.rows ?? [], splitAccessor); if (props.groupId === 'breakdown' && !layer.collapseFn) { return ( - - -
- - - - - {i18n.translate('xpack.lens.colorMapping.tryLabel', { - defaultMessage: 'Use the new Color Mapping feature', - })}{' '} - - {i18n.translate('xpack.lens.colorMapping.techPreviewLabel', { - defaultMessage: 'Tech preview', - })} - - - - } - data-test-subj="lns_colorMappingOrLegacyPalette_switch" - compressed - checked={useNewColorMapping} - onChange={({ target: { checked } }) => { - trackUiCounterEvents( - `color_mapping_switch_${checked ? 'enabled' : 'disabled'}` - ); - setColorMapping(checked ? { ...DEFAULT_COLOR_MAPPING_CONFIG } : undefined); - setUseNewColorMapping(checked); - }} - /> - - - - {canUseColorMapping || useNewColorMapping ? ( - setColorMapping(model)} - palettes={AVAILABLE_PALETTES} - data={{ - type: 'categories', - categories: splitCategories, - }} - specialTokens={SPECIAL_TOKENS_STRING_CONVERTION} - /> - ) : ( - { - setPalette(newPalette); - }} - /> - )} - - -
- - + ); } const isHorizontal = isHorizontalChart(state.layers); - const disabledMessage = Boolean(!localLayer.collapseFn && localLayer.splitAccessor) + const disabledMessage = Boolean(!layer.collapseFn && layer.splitAccessor) ? i18n.translate('xpack.lens.xyChart.colorPicker.tooltip.disabled', { defaultMessage: 'You are unable to apply custom colors to individual series when the layer includes a "Break down by" field.', @@ -329,13 +239,23 @@ export function DataDimensionEditorDataSectionExtra( onChange: props.setState, }); + const updateLayerState = useCallback( + (layerIndex: number, newLayer: Partial>) => { + setLocalState({ + ...localState, + layers: updateLayer(localState, layerIndex, layer, newLayer), + }); + }, + [layer, setLocalState, localState] + ); + if (props.groupId === 'breakdown') { return ( <> { - setLocalState(updateLayer(localState, { ...layer, collapseFn }, index)); + updateLayerState(index, { collapseFn }); }} /> diff --git a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/xy_config_panel.test.tsx b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/xy_config_panel.test.tsx index 9a98f5bae168b..b0f553969e058 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/xy_config_panel.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/xy_config_panel.test.tsx @@ -272,7 +272,7 @@ describe('XY Config panels', () => { addLayer={jest.fn()} removeLayer={jest.fn()} datasource={{} as DatasourcePublicAPI} - darkMode={false} + isDarkMode={false} /> ); @@ -300,7 +300,7 @@ describe('XY Config panels', () => { addLayer={jest.fn()} removeLayer={jest.fn()} datasource={{} as DatasourcePublicAPI} - darkMode={false} + isDarkMode={false} /> ); @@ -349,7 +349,7 @@ describe('XY Config panels', () => { addLayer={jest.fn()} removeLayer={jest.fn()} datasource={{} as DatasourcePublicAPI} - darkMode={false} + isDarkMode={false} /> ); @@ -395,7 +395,7 @@ describe('XY Config panels', () => { addLayer={jest.fn()} removeLayer={jest.fn()} datasource={{} as DatasourcePublicAPI} - darkMode={false} + isDarkMode={false} /> ); @@ -441,7 +441,7 @@ describe('XY Config panels', () => { addLayer={jest.fn()} removeLayer={jest.fn()} datasource={{} as DatasourcePublicAPI} - darkMode={false} + isDarkMode={false} /> ); From 1737e6da0152051c95f0a535aa224380e50a6d8a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 5 Aug 2024 07:40:10 -0700 Subject: [PATCH 02/38] fix: cell style updates --- .../datatable/components/cell_value.tsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index 5b467d04035d0..e9bc2c2959028 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -32,7 +32,7 @@ export const createGridCell = ( ) => CellColorFn, fitRowToContent?: boolean ) => { - return ({ rowIndex, columnId, setCellProps }: EuiDataGridCellValueElementProps) => { + return ({ rowIndex, columnId, setCellProps, isExpanded }: EuiDataGridCellValueElementProps) => { const { table, alignments, handleFilterClick } = useContext(DataContext); const rowValue = getParsedValue(table?.rows[rowIndex]?.[columnId]); const colIndex = columnConfig.columns.findIndex(({ columnId: id }) => id === columnId); @@ -55,7 +55,20 @@ export const createGridCell = ( setCellProps({ style }); } } - }, [rowValue, columnId, setCellProps, colorMode, palette, colorMapping]); + + // Clean up styles when something changes, this avoids cell's styling to stick forever + // Checks isExpanded to prevent clearing style after expanding cell + return () => { + if (colorMode !== 'none' && !isExpanded) { + setCellProps({ + style: { + backgroundColor: undefined, + color: undefined, + }, + }); + } + }; + }, [rowValue, columnId, setCellProps, colorMode, palette, colorMapping, isExpanded]); return (
Date: Mon, 5 Aug 2024 07:50:38 -0700 Subject: [PATCH 03/38] fix initial palette when adding new dimension --- .../lens/public/visualizations/datatable/visualization.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index bc892220e5e1a..7aa2726882afc 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -320,7 +320,12 @@ export const getDatatableVisualization = ({ return !operation?.isBucketed; }) .map((accessor) => { - const { colorMode, palette, colorMapping, hidden } = columnMap[accessor] ?? {}; + const { + colorMode = 'none', + palette, + colorMapping, + hidden, + } = columnMap[accessor] ?? {}; const stops = getColorStops(paletteService, isDarkMode, palette, colorMapping); const hasColoring = Boolean(colorMode !== 'none' && stops); From b81bb4eae91671bd70799cafb0915d5dd1c9981d Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 5 Aug 2024 11:09:07 -0700 Subject: [PATCH 04/38] Add color mapping to row dimensions --- .../datatable/components/dimension_editor.tsx | 6 ++-- .../datatable/visualization.tsx | 31 ++++++++++++++----- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index 9f98ff06c178f..938050d0b1128 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -81,9 +81,9 @@ export function TableDimensionEditor( const hasDynamicColoring = currentColorMode !== 'none'; const datasource = frame.datasourceLayers[localState.layerId]; - const showDynamicColoringFeature = Boolean( - !datasource?.getOperationForColumnId(accessor)?.isBucketed - ); + + const showDynamicColoringFeature = + datasource?.getOperationForColumnId(accessor)?.dataType !== 'date'; const visibleColumnsCount = localState.columns.filter((c) => !c.hidden).length; diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index 7aa2726882afc..edc5842b3090f 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -246,14 +246,29 @@ export const getDatatableVisualization = ({ } return datasource!.getOperationForColumnId(c)?.isBucketed && !column?.isTransposed; }) - .map((accessor) => ({ - columnId: accessor, - triggerIconType: columnMap[accessor].hidden - ? 'invisible' - : columnMap[accessor].collapseFn - ? 'aggregate' - : undefined, - })), + .map((accessor) => { + const { + colorMode = 'none', + palette, + colorMapping, + hidden, + collapseFn, + } = columnMap[accessor] ?? {}; + const stops = getColorStops(paletteService, isDarkMode, palette, colorMapping); + const hasColoring = Boolean(colorMode !== 'none' && stops); + + return { + columnId: accessor, + triggerIconType: hidden + ? 'invisible' + : hasColoring + ? 'colorBy' + : collapseFn + ? 'aggregate' + : undefined, + palette: hasColoring ? stops : undefined, + }; + }), supportsMoreColumns: true, filterOperations: (op) => op.isBucketed, dataTestSubj: 'lnsDatatable_rows', From 5471c2aef13173800aad20aa081112d6c9885bc5 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 5 Aug 2024 16:36:15 -0700 Subject: [PATCH 05/38] Style & copy tweaks to align with mockups --- .../color_mapping/components/assignment/match.tsx | 4 ++-- .../color_mapping/components/container/assigments.tsx | 4 ++-- .../color_mapping/components/container/container.tsx | 1 + .../form_based/dimension_panel/dimension_editor.tsx | 2 +- .../coloring/color_mapping_by_terms.tsx | 11 +++-------- .../coloring/color_mapping_by_values.tsx | 11 +++-------- .../datatable/components/dimension_editor.tsx | 3 --- .../public/visualizations/gauge/dimension_editor.tsx | 5 ++++- .../visualizations/legacy_metric/dimension_editor.tsx | 5 ++++- .../public/visualizations/metric/dimension_editor.tsx | 10 +++++----- .../visualizations/partition/dimension_editor.tsx | 2 +- .../visualizations/tagcloud/tags_dimension_editor.tsx | 2 +- x-pack/plugins/translations/translations/fr-FR.json | 2 +- x-pack/plugins/translations/translations/ja-JP.json | 2 +- x-pack/plugins/translations/translations/zh-CN.json | 2 +- 15 files changed, 30 insertions(+), 36 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/match.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/match.tsx index bfef7a270e1a0..c10b3c0194cf7 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/match.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/match.tsx @@ -73,6 +73,7 @@ export const Match: React.FC<{ return ( diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx index f525311b8afb3..96a7b9b7556d5 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx @@ -195,7 +195,7 @@ export function AssignmentsConfig({ 'coloring.colorMapping.container.mapValuesPromptDescription.mapValuesPromptDetail', { defaultMessage: - 'Add new assignments to begin associating terms in your data with specified colors.', + 'Add a new assignment to manually associate terms with specified colors.', } )}

@@ -214,7 +214,7 @@ export function AssignmentsConfig({ , diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/container.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/container.tsx index e3de3c0a24261..e8466e428ee88 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/container.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/container.tsx @@ -81,6 +81,7 @@ export function Container({ > )} diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx index 68f9400d490a6..82cd44091fc62 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx @@ -33,7 +33,6 @@ import { PalettePicker } from '../palette_picker'; import { PalettePanelContainer } from './palette_panel_container'; interface ColorMappingByTermsProps { - label?: string; isDarkMode: boolean; colorMapping?: ColorMapping.Config; palette?: PaletteOutput; @@ -46,7 +45,6 @@ interface ColorMappingByTermsProps { } export function ColorMappingByTerms({ - label, isDarkMode, colorMapping, palette, @@ -62,12 +60,9 @@ export function ColorMappingByTerms({ return ( diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx index b6d7bbdf64c13..ca6d92b451a64 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx @@ -19,7 +19,6 @@ import { i18n } from '@kbn/i18n'; import { PalettePanelContainer } from './palette_panel_container'; interface ColorMappingByValuesProps { - label?: string; palette: PaletteOutput; isInlineEditing?: boolean; setPalette: (palette: PaletteOutput) => void; @@ -29,7 +28,6 @@ interface ColorMappingByValuesProps { } export function ColorMappingByValues({ - label, palette, isInlineEditing, setPalette, @@ -42,12 +40,9 @@ export function ColorMappingByValues({ return ( diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index 938050d0b1128..c2d92d6b4ed7a 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -217,9 +217,6 @@ export function TableDimensionEditor( {hasDynamicColoring && (isNumeric ? ( { diff --git a/x-pack/plugins/lens/public/visualizations/gauge/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/gauge/dimension_editor.tsx index 86eaf39479ff5..b30e36cfefa37 100644 --- a/x-pack/plugins/lens/public/visualizations/gauge/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/gauge/dimension_editor.tsx @@ -110,13 +110,16 @@ export function GaugeDimensionEditor( display="columnCompressed" fullWidth label={i18n.translate('xpack.lens.paletteMetricGradient.label', { - defaultMessage: 'Color', + defaultMessage: 'Color mapping', })} > color)} siblingRef={props.panelRef} isInlineEditing={isInlineEditing} + title={i18n.translate('xpack.lens.paletteMetricGradient.label', { + defaultMessage: 'Color mapping', + })} > color)} siblingRef={props.panelRef} isInlineEditing={isInlineEditing} + title={i18n.translate('xpack.lens.paletteMetricGradient.label', { + defaultMessage: 'Color mapping', + })} > Date: Mon, 5 Aug 2024 23:24:59 -0700 Subject: [PATCH 06/38] derive categories across all matching transposed columns --- .../components/container/assigments.tsx | 1 + .../common/color_categories.ts | 43 +++++++++++-------- .../common/expressions/datatable/index.ts | 1 + .../datatable/transpose_helpers.ts | 4 ++ .../datatable/components/cell_value.tsx | 4 +- .../datatable/components/table_basic.tsx | 14 ++++-- 6 files changed, 43 insertions(+), 24 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx index 96a7b9b7556d5..8c4d3796249f8 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx @@ -251,6 +251,7 @@ export function AssignmentsConfig({ button={ { - return accessor - ? rows.reduce<{ keys: Set; categories: Array }>( - (acc, r) => { - const value = r[accessor]; + const ids = isTransposed + ? Object.keys(rows[0]).filter((key) => accessor && key.endsWith(accessor)) + : accessor + ? [accessor] + : []; - if (value === undefined || exclude?.includes(value)) { - return acc; - } + return rows + .flatMap((r) => + ids + .map((id) => r[id]) + .filter((v) => !(v === undefined || exclude?.includes(v))) + .map((v) => { // The categories needs to be stringified in their unformatted version. // We can't distinguish between a number and a string from a text input and the match should // work with both numeric field values and string values. - const key = (isMultiFieldKey(value) ? [...value.keys] : [value]).map(String); + const key = (isMultiFieldKey(v) ? [...v.keys] : [v]).map(String); const stringifiedKeys = key.join(','); - if (!acc.keys.has(stringifiedKeys)) { - acc.keys.add(stringifiedKeys); - acc.categories.push(key.length === 1 ? key[0] : key); - } - return acc; - }, - { keys: new Set(), categories: [] } - ).categories - : []; + return { key, stringifiedKeys }; + }) + ) + .reduce<{ keys: Set; categories: Array }>( + (acc, { key, stringifiedKeys }) => { + if (!acc.keys.has(stringifiedKeys)) { + acc.keys.add(stringifiedKeys); + acc.categories.push(key.length === 1 ? key[0] : key); + } + return acc; + }, + { keys: new Set(), categories: [] } + ).categories; } diff --git a/x-pack/plugins/lens/common/expressions/datatable/index.ts b/x-pack/plugins/lens/common/expressions/datatable/index.ts index 7003fd8d486b8..4b27aa90d5190 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/index.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/index.ts @@ -7,5 +7,6 @@ export * from './datatable_column'; export * from './datatable'; +export { isTransposeId, getOriginalId } from './transpose_helpers'; export type { DatatableProps, DatatableExpressionFunction } from './types'; diff --git a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts index 3daad373640d3..09e801b26f5e8 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts @@ -18,6 +18,10 @@ function getTransposeId(value: string, columnId: string) { return `${value}${TRANSPOSE_SEPARATOR}${columnId}`; } +export function isTransposeId(id: string): boolean { + return id.split(TRANSPOSE_SEPARATOR).length > 1; +} + export function getOriginalId(id: string) { if (id.includes(TRANSPOSE_SEPARATOR)) { const idParts = id.split(TRANSPOSE_SEPARATOR); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index e9bc2c2959028..dfdd4a9b4b371 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -11,7 +11,6 @@ import classNames from 'classnames'; import { PaletteOutput } from '@kbn/coloring'; import { CustomPaletteState } from '@kbn/charts-plugin/common'; import type { FormatFactory } from '../../../../common/types'; -import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; import type { DatatableColumnConfig } from '../../../../common/expressions'; import type { DataContextType } from './types'; import { getContrastColor } from '../../../shared_components/coloring/utils'; @@ -44,8 +43,7 @@ export const createGridCell = ( useEffect(() => { if (colorMode !== 'none' && (palette || colorMapping)) { - const originalId = getOriginalId(columnId); // workout what bucket the value belongs to - const color = getCellColor(originalId, palette, colorMapping)(rowValue); + const color = getCellColor(columnId, palette, colorMapping)(rowValue); if (color) { const style = { [colorMode === 'cell' ? 'backgroundColor' : 'color']: color }; diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx index 331da5cfcc992..fde2ebeb7a586 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx @@ -35,7 +35,7 @@ import { getColorCategories } from '@kbn/chart-expressions-common'; import type { LensTableRowContextMenuEvent } from '../../../types'; import type { FormatFactory } from '../../../../common/types'; import { RowHeightMode } from '../../../../common/types'; -import type { LensGridDirection } from '../../../../common/expressions'; +import { getOriginalId, isTransposeId, LensGridDirection } from '../../../../common/expressions'; import { VisualizationContainer } from '../../../visualization_container'; import { findMinMaxByColumnId } from '../../../shared_components'; import type { @@ -57,7 +57,6 @@ import { createTransposeColumnFilterHandler, } from './table_actions'; import { getFinalSummaryConfiguration } from '../../../../common/expressions/datatable/summary'; -import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; import { DEFAULT_HEADER_ROW_HEIGHT, DEFAULT_HEADER_ROW_HEIGHT_LINES } from './constants'; import { isNumericFieldForDatatable } from '../../../../common/expressions/datatable/utils'; import { CellColorFn, getCellColorFn } from '../../../shared_components/coloring/get_cell_color_fn'; @@ -393,10 +392,12 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const renderCellValue = useMemo(() => { const cellColorFnMap = new Map(); const getCellColor = ( - originalId: string, + columnId: string, palette?: PaletteOutput, colorMapping?: string ): CellColorFn => { + const originalId = getOriginalId(columnId); // workout what bucket the value belongs to + if (cellColorFnMap.has(originalId)) { return cellColorFnMap.get(originalId)!; } @@ -410,7 +411,12 @@ export const DatatableComponent = (props: DatatableRenderProps) => { } : { type: 'categories', - categories: getColorCategories(firstLocalTable.rows, originalId, [null]), + categories: getColorCategories( + firstLocalTable.rows, + originalId, + isTransposeId(columnId), + [null] + ), }; const colorFn = getCellColorFn( props.paletteService, From 5c2a577574ad03e4c8b5ac43bb607075555d2c9c Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 6 Aug 2024 18:31:31 -0700 Subject: [PATCH 07/38] fix i18n issues --- .../lens/public/visualizations/partition/dimension_editor.tsx | 2 +- .../public/visualizations/tagcloud/tags_dimension_editor.tsx | 2 +- x-pack/plugins/translations/translations/fr-FR.json | 3 --- x-pack/plugins/translations/translations/ja-JP.json | 3 --- x-pack/plugins/translations/translations/zh-CN.json | 3 --- 5 files changed, 2 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx index 9640bf59d6236..700dc31a25711 100644 --- a/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/partition/dimension_editor.tsx @@ -139,7 +139,7 @@ export function DimensionEditor(props: DimensionEditorProps) { title={ useNewColorMapping ? i18n.translate('xpack.lens.colorMapping.editColorMappingTitle', { - defaultMessage: 'Edit colors by term mapping', + defaultMessage: 'Assign colors to terms', }) : i18n.translate('xpack.lens.colorMapping.editColorsTitle', { defaultMessage: 'Edit colors', diff --git a/x-pack/plugins/lens/public/visualizations/tagcloud/tags_dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/tagcloud/tags_dimension_editor.tsx index b94ccda36c8c0..4e28701fd677a 100644 --- a/x-pack/plugins/lens/public/visualizations/tagcloud/tags_dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/tagcloud/tags_dimension_editor.tsx @@ -94,7 +94,7 @@ export function TagsDimensionEditor({ title={ useNewColorMapping ? i18n.translate('xpack.lens.colorMapping.editColorMappingTitle', { - defaultMessage: 'Edit colors by term mapping', + defaultMessage: 'Assign colors to terms', }) : i18n.translate('xpack.lens.colorMapping.editColorsTitle', { defaultMessage: 'Edit colors', diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index c1cc5d158197d..d401bb16f5704 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -23860,9 +23860,7 @@ "xpack.lens.metric.breakdownBy": "Répartir par", "xpack.lens.metric.color": "Couleur", "xpack.lens.metric.colorMode.dynamic": "Dynamique", - "xpack.lens.metric.colorMode.label": "Mode couleur", "xpack.lens.metric.colorMode.static": "Statique", - "xpack.lens.metric.dynamicColoring.label": "Mode couleur", "xpack.lens.metric.groupLabel": "Valeur d’objectif et unique", "xpack.lens.metric.headingLabel": "Valeur", "xpack.lens.metric.icon": "Décoration de l’icône", @@ -23918,7 +23916,6 @@ "xpack.lens.paletteHeatmapGradient.label": "Couleur", "xpack.lens.paletteMetricGradient.label": "Couleur", "xpack.lens.palettePicker.label": "Palette", - "xpack.lens.paletteTableGradient.label": "Couleur", "xpack.lens.pie.addLayer": "Visualisation", "xpack.lens.pie.arrayValues": "Les dimensions suivantes contiennent des valeurs de tableau : {label}. Le rendu de votre visualisation peut ne pas se présenter comme attendu.", "xpack.lens.pie.collapsedDimensionsDontCount": "(Les dimensions réduites ne sont pas concernées par cette limite.)", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index fe4e0d2155656..3be5106b0dba5 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -23784,9 +23784,7 @@ "xpack.lens.metric.breakdownBy": "内訳の基準", "xpack.lens.metric.color": "色", "xpack.lens.metric.colorMode.dynamic": "動的", - "xpack.lens.metric.colorMode.label": "色モード", "xpack.lens.metric.colorMode.static": "静的", - "xpack.lens.metric.dynamicColoring.label": "色モード", "xpack.lens.metric.groupLabel": "目標値と単一の値", "xpack.lens.metric.headingLabel": "値", "xpack.lens.metric.icon": "アイコン装飾", @@ -23842,7 +23840,6 @@ "xpack.lens.paletteHeatmapGradient.label": "色", "xpack.lens.paletteMetricGradient.label": "色", "xpack.lens.palettePicker.label": "パレット", - "xpack.lens.paletteTableGradient.label": "色", "xpack.lens.pie.addLayer": "ビジュアライゼーション", "xpack.lens.pie.arrayValues": "次のディメンションには配列値があります：{label}。可視化が想定通りに表示されない場合があります。", "xpack.lens.pie.collapsedDimensionsDontCount": "（折りたたまれたディメンションはこの制限に対してカウントされません。）", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 42b06be59ecbd..357dec4cb4e64 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -23893,9 +23893,7 @@ "xpack.lens.metric.breakdownBy": "细分方式", "xpack.lens.metric.color": "颜色", "xpack.lens.metric.colorMode.dynamic": "动态", - "xpack.lens.metric.colorMode.label": "颜色模式", "xpack.lens.metric.colorMode.static": "静态", - "xpack.lens.metric.dynamicColoring.label": "颜色模式", "xpack.lens.metric.groupLabel": "目标值和单值", "xpack.lens.metric.headingLabel": "值", "xpack.lens.metric.icon": "图标装饰", @@ -23951,7 +23949,6 @@ "xpack.lens.paletteHeatmapGradient.label": "颜色", "xpack.lens.paletteMetricGradient.label": "颜色", "xpack.lens.palettePicker.label": "调色板", - "xpack.lens.paletteTableGradient.label": "颜色", "xpack.lens.pie.addLayer": "可视化", "xpack.lens.pie.arrayValues": "以下维度包含数组值：{label}。您的可视化可能无法正常渲染。", "xpack.lens.pie.collapsedDimensionsDontCount": "（折叠的维度不计入此限制。）", From 020dd91a850f93c429b9fe3f5933ae6de92ee44c Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 8 Aug 2024 10:00:50 -0700 Subject: [PATCH 08/38] fix type errors in datatable expression --- .../lens/common/expressions/datatable/datatable.ts | 4 ++-- .../common/expressions/datatable/datatable_column.ts | 9 +++++---- .../lens/common/expressions/datatable/summary.ts | 10 +++++----- .../expressions/datatable/transpose_helpers.ts | 12 ++++++------ .../datatable/components/dimension_editor.tsx | 2 +- .../datatable/components/table_actions.ts | 12 ++---------- 6 files changed, 21 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable.ts index cf70473770217..11faf78dc3b4b 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/datatable.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/datatable.ts @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n'; import type { ExecutionContext } from '@kbn/expressions-plugin/common'; import type { FormatFactory, RowHeightMode } from '../../types'; -import type { DatatableColumnConfigArgs } from './datatable_column'; +import type { DatatableColumnResult } from './datatable_column'; import type { DatatableExpressionFunction } from './types'; export interface SortingState { @@ -24,7 +24,7 @@ export interface PagingState { export interface DatatableArgs { title: string; description?: string; - columns: DatatableColumnConfigArgs[]; + columns: DatatableColumnResult[]; sortingColumnId: SortingState['columnId']; sortingDirection: SortingState['direction']; fitRowToContent?: boolean; diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts index 7dc607d8b32c9..6f4702c8b32c1 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/datatable_column.ts @@ -17,12 +17,12 @@ const LENS_DATATABLE_COLUMN = 'lens_datatable_column'; export type LensGridDirection = 'none' | Direction; export interface DatatableColumnConfig { - columns: DatatableColumnConfigArgs[]; + columns: DatatableColumnResult[]; sortingColumnId: string | undefined; sortingDirection: LensGridDirection; } -export type DatatableColumnConfigArgs = Omit & { +export type DatatableColumnArgs = Omit & { palette?: PaletteOutput; colorMapping?: string; summaryRowValue?: unknown; @@ -51,13 +51,14 @@ export interface ColumnState { isMetric?: boolean; } -export type DatatableColumnResult = DatatableColumnConfigArgs & { +export type DatatableColumnResult = DatatableColumnArgs & { type: typeof LENS_DATATABLE_COLUMN; }; + export type DatatableColumnFn = ExpressionFunctionDefinition< typeof LENS_DATATABLE_COLUMN, null, - DatatableColumnConfigArgs, + DatatableColumnArgs, DatatableColumnResult >; diff --git a/x-pack/plugins/lens/common/expressions/datatable/summary.ts b/x-pack/plugins/lens/common/expressions/datatable/summary.ts index 4088a0f7f903d..a1727af7fd42a 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/summary.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/summary.ts @@ -8,15 +8,15 @@ import { i18n } from '@kbn/i18n'; import type { FieldFormat } from '@kbn/field-formats-plugin/common'; import type { Datatable } from '@kbn/expressions-plugin/common'; -import { DatatableColumnConfigArgs } from './datatable_column'; +import { DatatableColumnArgs } from './datatable_column'; import { getOriginalId } from './transpose_helpers'; import { isNumericFieldForDatatable } from './utils'; -type SummaryRowType = Extract; +type SummaryRowType = Extract; export function getFinalSummaryConfiguration( columnId: string, - columnArgs: Pick | undefined, + columnArgs: Pick | undefined, table: Datatable | undefined ) { const isNumeric = isNumericFieldForDatatable(table, columnId); @@ -87,7 +87,7 @@ export function getSummaryRowOptions(): Array<{ /** @internal **/ export function computeSummaryRowForColumn( - columnArgs: DatatableColumnConfigArgs, + columnArgs: DatatableColumnArgs, table: Datatable, formatters: Record, defaultFormatter: FieldFormat @@ -101,7 +101,7 @@ export function computeSummaryRowForColumn( } function computeFinalValue( - type: DatatableColumnConfigArgs['summaryRow'], + type: DatatableColumnArgs['summaryRow'], columnId: string, rows: Datatable['rows'] ) { diff --git a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts index 09e801b26f5e8..d0bbda9fa7934 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts @@ -8,7 +8,7 @@ import type { Datatable, DatatableColumn, DatatableRow } from '@kbn/expressions-plugin/common'; import type { FieldFormat } from '@kbn/field-formats-plugin/common'; import type { DatatableArgs } from './datatable'; -import type { DatatableColumnConfig, DatatableColumnConfigArgs } from './datatable_column'; +import type { DatatableColumnConfig, DatatableColumnArgs } from './datatable_column'; const TRANSPOSE_SEPARATOR = '---'; @@ -91,11 +91,11 @@ export function transposeTable( function transposeRows( firstTable: Datatable, - bucketsColumnArgs: DatatableColumnConfigArgs[], + bucketsColumnArgs: DatatableColumnArgs[], formatters: Record, transposedColumnFormatter: FieldFormat, transposedColumnId: string, - metricsColumnArgs: DatatableColumnConfigArgs[] + metricsColumnArgs: DatatableColumnArgs[] ) { const rowsByBucketColumns: Record = groupRowsByBucketColumns( firstTable, @@ -200,10 +200,10 @@ function transposeColumns( */ function mergeRowGroups( rowsByBucketColumns: Record, - bucketColumns: DatatableColumnConfigArgs[], + bucketColumns: DatatableColumnArgs[], formatter: FieldFormat, transposedColumnId: string, - metricColumns: DatatableColumnConfigArgs[] + metricColumns: DatatableColumnArgs[] ) { return Object.values(rowsByBucketColumns).map((rows) => { const mergedRow: DatatableRow = {}; @@ -226,7 +226,7 @@ function mergeRowGroups( */ function groupRowsByBucketColumns( firstTable: Datatable, - bucketColumns: DatatableColumnConfigArgs[], + bucketColumns: DatatableColumnArgs[], formatters: Record ) { const rowsByBucketColumns: Record = {}; diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index c2d92d6b4ed7a..ac2af9c2cd9a6 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -101,7 +101,7 @@ export function TableDimensionEditor( }; // need to tell the helper that the colorStops are required to display const displayStops = applyPaletteParams(props.paletteService, activePalette, currentMinMax); - const categories = getColorCategories(currentData?.rows ?? [], accessor, [null]); + const categories = getColorCategories(currentData?.rows ?? [], accessor, false, [null]); return ( <> diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts b/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts index 0cfd6289f5818..e53713069fb8f 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_actions.ts @@ -19,11 +19,7 @@ import { ClickTriggerEvent } from '@kbn/charts-plugin/public'; import { getSortingCriteria } from '@kbn/sort-predicates'; import { i18n } from '@kbn/i18n'; import type { LensResizeAction, LensSortAction, LensToggleAction } from './types'; -import type { - DatatableColumnConfig, - DatatableColumnConfigArgs, - LensGridDirection, -} from '../../../../common/expressions'; +import type { DatatableColumnConfig, LensGridDirection } from '../../../../common/expressions'; import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; import type { FormatFactory } from '../../../../common/types'; import { buildColumnsMetaLookup } from './helpers'; @@ -194,11 +190,7 @@ function getColumnType({ export const buildSchemaDetectors = ( columns: EuiDataGridColumn[], - columnConfig: { - columns: DatatableColumnConfigArgs[]; - sortingColumnId: string | undefined; - sortingDirection: 'none' | 'asc' | 'desc'; - }, + columnConfig: DatatableColumnConfig, table: Datatable, formatters: Record> ): EuiDataGridSchemaDetector[] => { From 361387039f0745ff0671e711c6928e47f59b2e0a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 12 Aug 2024 17:52:29 -0700 Subject: [PATCH 09/38] fix types and test errors --- .../expressions/datatable/summary.test.ts | 8 ++-- .../common/expressions/datatable/summary.ts | 2 +- .../datatable/components/cell_value.test.tsx | 48 +++++++++++-------- .../datatable/components/cell_value.tsx | 32 +++++++++++-- .../components/dimension_editor.test.tsx | 6 ++- .../datatable/visualization.test.tsx | 7 +-- .../test/functional/page_objects/lens_page.ts | 2 +- 7 files changed, 69 insertions(+), 36 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/datatable/summary.test.ts b/x-pack/plugins/lens/common/expressions/datatable/summary.test.ts index 207bf5779bbee..b772c3a51208c 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/summary.test.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/summary.test.ts @@ -72,7 +72,7 @@ describe('Summary row helpers', () => { it(`should return formatted value for a ${op} summary function`, () => { expect( computeSummaryRowForColumn( - { summaryRow: op, columnId: 'myColumn', type: 'lens_datatable_column' }, + { summaryRow: op, columnId: 'myColumn' }, mockNumericTable, { myColumn: customNumericFormatter, @@ -86,7 +86,7 @@ describe('Summary row helpers', () => { it('should ignore the column formatter, rather return the raw value for count operation', () => { expect( computeSummaryRowForColumn( - { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, + { summaryRow: 'count', columnId: 'myColumn' }, mockNumericTable, { myColumn: customNumericFormatter, @@ -99,7 +99,7 @@ describe('Summary row helpers', () => { it('should only count non-null/empty values', () => { expect( computeSummaryRowForColumn( - { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, + { summaryRow: 'count', columnId: 'myColumn' }, { ...mockNumericTable, rows: [...mockNumericTable.rows, { myColumn: null }] }, { myColumn: customNumericFormatter, @@ -112,7 +112,7 @@ describe('Summary row helpers', () => { it('should count numeric arrays as valid and distinct values', () => { expect( computeSummaryRowForColumn( - { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, + { summaryRow: 'count', columnId: 'myColumn' }, mockNumericTableWithArray, { myColumn: defaultFormatter, diff --git a/x-pack/plugins/lens/common/expressions/datatable/summary.ts b/x-pack/plugins/lens/common/expressions/datatable/summary.ts index a1727af7fd42a..f4ae186fc1d2f 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/summary.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/summary.ts @@ -93,7 +93,7 @@ export function computeSummaryRowForColumn( defaultFormatter: FieldFormat ) { const summaryValue = computeFinalValue(columnArgs.summaryRow, columnArgs.columnId, table.rows); - // ignore the coluymn formatter for the count case + // ignore the column formatter for the count case if (columnArgs.summaryRow === 'count') { return defaultFormatter.convert(summaryValue); } diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx index a0efcf04ec000..9636631104221 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx @@ -10,13 +10,15 @@ import { DataContext } from './table_basic'; import { createGridCell } from './cell_value'; import type { FieldFormat } from '@kbn/field-formats-plugin/common'; import { Datatable } from '@kbn/expressions-plugin/public'; -import { coreMock } from '@kbn/core/public/mocks'; -import { DatatableArgs, DatatableColumnConfigArgs } from '../../../../common/expressions'; +import { DatatableArgs } from '../../../../common/expressions'; import { DataContextType } from './types'; -import { chartPluginMock } from '@kbn/charts-plugin/public/mocks'; import { render, screen } from '@testing-library/react'; describe('datatable cell renderer', () => { + const innerCellColorFnMock = jest.fn().mockReturnValue('blue'); + const cellColorFnMock = jest.fn().mockReturnValue(innerCellColorFnMock); + const setCellProps = jest.fn(); + const table: Datatable = { type: 'datatable', columns: [ @@ -30,18 +32,16 @@ describe('datatable cell renderer', () => { ], rows: [{ a: 123 }], }; - const { theme: setUpMockTheme } = coreMock.createSetup(); const CellRenderer = createGridCell( { a: { convert: (x) => `formatted ${x}` } as FieldFormat, }, { columns: [], sortingColumnId: '', sortingDirection: 'none' }, DataContext, - setUpMockTheme + false, + cellColorFnMock ); - const setCellProps = jest.fn(); - afterEach(() => { jest.clearAllMocks(); }); @@ -101,7 +101,8 @@ describe('datatable cell renderer', () => { }, { columns: [], sortingColumnId: '', sortingDirection: 'none' }, DataContext, - setUpMockTheme, + false, + cellColorFnMock, true ); render( @@ -137,7 +138,8 @@ describe('datatable cell renderer', () => { sortingDirection: 'none', }, DataContext, - setUpMockTheme, + false, + cellColorFnMock, true ); render( @@ -156,9 +158,6 @@ describe('datatable cell renderer', () => { }); describe('dynamic coloring', () => { - const paletteRegistry = chartPluginMock.createPaletteRegistry(); - const customPalette = paletteRegistry.get('custom'); - function getCellRenderer(columnConfig: DatatableArgs) { return createGridCell( { @@ -166,7 +165,8 @@ describe('datatable cell renderer', () => { }, columnConfig, DataContext, - setUpMockTheme + false, + cellColorFnMock ); } function getColumnConfiguration(): DatatableArgs { @@ -189,7 +189,7 @@ describe('datatable cell renderer', () => { }, }, type: 'lens_datatable_column', - } as DatatableColumnConfigArgs, + }, ], sortingColumnId: '', sortingDirection: 'none', @@ -216,8 +216,7 @@ describe('datatable cell renderer', () => { { wrapper: DataContextProviderWrapper({ table, - minMaxByColumnId: { a: { min: 12, max: 155 /* > 123 */ } }, - getColorForValue: customPalette.getColorForValue, + minMaxByColumnId: { a: { min: 12, max: 155 } }, ...context, }), } @@ -252,14 +251,23 @@ describe('datatable cell renderer', () => { }); }); - it('should not color the cell when the value is an array', () => { + it('should not color the cell when color function returns null', () => { setCellProps.mockClear(); + innerCellColorFnMock.mockReturnValueOnce(null); const columnConfig = getColumnConfiguration(); columnConfig.columns[0].colorMode = 'cell'; - renderCellComponent(columnConfig, { - table: { ...table, rows: [{ a: [10, 123] }] }, - }); + renderCellComponent(columnConfig, {}); + + expect(setCellProps).not.toHaveBeenCalled(); + }); + + it('should not color the cell when color function returns empty string', () => { + innerCellColorFnMock.mockReturnValueOnce(''); + const columnConfig = getColumnConfiguration(); + columnConfig.columns[0].colorMode = 'cell'; + + renderCellComponent(columnConfig, {}); expect(setCellProps).not.toHaveBeenCalled(); }); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index dfdd4a9b4b371..c0809e1197318 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -6,7 +6,7 @@ */ import React, { useContext, useEffect } from 'react'; -import { EuiDataGridCellValueElementProps } from '@elastic/eui'; +import { EuiDataGridCellValueElementProps, EuiLink } from '@elastic/eui'; import classNames from 'classnames'; import { PaletteOutput } from '@kbn/coloring'; import { CustomPaletteState } from '@kbn/charts-plugin/common'; @@ -42,6 +42,7 @@ export const createGridCell = ( const currentAlignment = alignments && alignments[columnId]; useEffect(() => { + let colorSet = false; if (colorMode !== 'none' && (palette || colorMapping)) { const color = getCellColor(columnId, palette, colorMapping)(rowValue); @@ -50,24 +51,45 @@ export const createGridCell = ( if (colorMode === 'cell' && color) { style.color = getContrastColor(color, isDarkMode); } + colorSet = true; setCellProps({ style }); } } // Clean up styles when something changes, this avoids cell's styling to stick forever // Checks isExpanded to prevent clearing style after expanding cell - return () => { - if (colorMode !== 'none' && !isExpanded) { + if (colorMode !== 'none' && colorSet && !isExpanded) { + return () => { setCellProps({ style: { backgroundColor: undefined, color: undefined, }, }); - } - }; + }; + } }, [rowValue, columnId, setCellProps, colorMode, palette, colorMapping, isExpanded]); + if (filterOnClick) { + return ( +
+ { + handleFilterClick?.(columnId, rowValue, colIndex, rowIndex); + }} + > + {content} + +
+ ); + } + return (
{ let setState: (newState: DatatableVisualizationState) => void; let props: VisualizationDimensionEditorProps & { paletteService: PaletteRegistry; + isDarkMode: boolean; }; function testState(): DatatableVisualizationState { @@ -72,6 +73,7 @@ describe('data table dimension editor', () => { layerId: 'first', state, setState, + isDarkMode: false, paletteService: chartPluginMock.createPaletteRegistry(), panelRef: React.createRef(), addLayer: jest.fn(), @@ -200,8 +202,8 @@ describe('data table dimension editor', () => { expect(screen.getByTestId('lns-palettePanelFlyout')).toBeInTheDocument(); }); - it('should not show the dynamic coloring option for a bucketed operation', () => { - frame.activeData!.first.columns[0].meta.type = 'number'; + it('should not show the dynamic coloring option for a dates', () => { + frame.activeData!.first.columns[0].meta.type = 'date'; const datasourceLayers = frame.datasourceLayers as Record; datasourceLayers.first.getOperationForColumnId = jest.fn( () => ({ isBucketed: true } as OperationDescriptor) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx index 877fed8a4016e..d8346ecf9e318 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx @@ -37,7 +37,7 @@ function mockFrame(): FramePublicAPI { const datatableVisualization = getDatatableVisualization({ paletteService: chartPluginMock.createPaletteRegistry(), - theme: themeServiceMock.createStartContract(), + kibanaTheme: themeServiceMock.createStartContract(), }); describe('Datatable Visualization', () => { @@ -431,7 +431,7 @@ describe('Datatable Visualization', () => { columnId: 'b', palette: { type: 'palette' as const, - name: '', + name: 'custom', params: { stops: [{ color: 'blue', stop: 0 }] }, }, }, @@ -453,8 +453,9 @@ describe('Datatable Visualization', () => { }); it('does include palette for accessor config if the values are numeric and palette exists', () => { + params.state.columns[0].colorMode = 'cell'; expect(datatableVisualization.getConfiguration(params).groups[2].accessors).toEqual([ - { columnId: 'b', palette: ['blue'], triggerIconType: 'colorBy' }, + { columnId: 'b', palette: ['blue', 'yellow'], triggerIconType: 'colorBy' }, ]); }); it('does not include palette for accessor config if the values are not numeric and palette exists', () => { diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index cd9a3ed385b73..d477146ae3f81 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -173,7 +173,7 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont }, /** - * Changes the specified dimension to the specified operation and (optinally) field. + * Changes the specified dimension to the specified operation and optionally the field. * * @param opts.dimension - the selector of the dimension being changed * @param opts.operation - the desired operation ID for the dimension From 369e02066d779b95eb638527ac7c26346f6efa7e Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 12 Aug 2024 19:22:02 -0700 Subject: [PATCH 10/38] style: remove empty color when color is applied --- src/plugins/field_formats/public/lib/converters/_string.scss | 4 ++++ .../public/visualizations/datatable/components/cell_value.tsx | 1 + 2 files changed, 5 insertions(+) diff --git a/src/plugins/field_formats/public/lib/converters/_string.scss b/src/plugins/field_formats/public/lib/converters/_string.scss index 9d97f0195780c..dd27bded25083 100644 --- a/src/plugins/field_formats/public/lib/converters/_string.scss +++ b/src/plugins/field_formats/public/lib/converters/_string.scss @@ -1,3 +1,7 @@ .ffString__emptyValue { color: $euiColorDarkShade; } + +.lnsTableCell--colored .ffString__emptyValue { + color: unset; +} diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index c0809e1197318..01b6118563586 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -100,6 +100,7 @@ export const createGridCell = ( data-test-subj="lnsTableCellContent" className={classNames({ 'lnsTableCell--multiline': fitRowToContent, + 'lnsTableCell--colored': colorMode !== 'none', [`lnsTableCell--${currentAlignment}`]: true, })} /> From fd5a75402eb8e08dd607038f87b784f24da5da18 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 13 Aug 2024 17:00:21 -0700 Subject: [PATCH 11/38] fix legacy color sync on table palettes --- .../color_mapping/color/rule_matching.ts | 6 +++ .../shared_components/color_mapping/index.ts | 2 +- .../public/components/data_layers.tsx | 4 +- .../public/helpers/data_layers.tsx | 8 +-- .../component/settings/settings_flyout.tsx | 49 ++++++++++++++++--- .../expressions/datatable/datatable_fn.ts | 1 + .../common/expressions/datatable/types.ts | 1 + .../coloring/get_cell_color_fn.ts | 12 +++-- .../datatable/components/table_basic.tsx | 4 +- .../visualizations/datatable/expression.tsx | 1 + 10 files changed, 71 insertions(+), 17 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts b/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts index 61b15e4fb3516..7bfc22691bb72 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/color/rule_matching.ts @@ -55,3 +55,9 @@ export const SPECIAL_TOKENS_STRING_CONVERSION = new Map([ }), ], ]); + +/** + * Returns special string for sake of color mapping/syncing + */ +export const getSpecialString = (value: string) => + SPECIAL_TOKENS_STRING_CONVERSION.get(value) ?? value; diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts index 570f98cb5ef1a..047de63982215 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts @@ -11,7 +11,7 @@ export type { ColorMappingInputData } from './categorical_color_mapping'; export type { ColorMapping } from './config'; export * from './palettes'; export * from './color/color_handling'; -export { SPECIAL_TOKENS_STRING_CONVERSION } from './color/rule_matching'; +export { SPECIAL_TOKENS_STRING_CONVERSION, getSpecialString } from './color/rule_matching'; export { DEFAULT_COLOR_MAPPING_CONFIG, DEFAULT_OTHER_ASSIGNMENT_INDEX, diff --git a/src/plugins/chart_expressions/expression_xy/public/components/data_layers.tsx b/src/plugins/chart_expressions/expression_xy/public/components/data_layers.tsx index c5223a270eadd..ffdb868ecff53 100644 --- a/src/plugins/chart_expressions/expression_xy/public/components/data_layers.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/components/data_layers.tsx @@ -48,8 +48,8 @@ interface Props { endValue?: EndValue | undefined; paletteService: PaletteRegistry; formattedDatatables: DatatablesWithFormatInfo; - syncColors?: boolean; - timeZone?: string; + syncColors: boolean; + timeZone: string; emphasizeFitting?: boolean; fillOpacity?: number; minBarHeight: number; diff --git a/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx b/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx index 20315d0f59460..406c498c15bba 100644 --- a/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx +++ b/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx @@ -53,10 +53,10 @@ type GetSeriesPropsFn = (config: { colorAssignments: ColorAssignments; columnToLabelMap: Record; paletteService: PaletteRegistry; - syncColors?: boolean; yAxis?: GroupsConfiguration[number]; xAxis?: GroupsConfiguration[number]; - timeZone?: string; + syncColors: boolean; + timeZone: string; emphasizeFitting?: boolean; fillOpacity?: number; formattedDatatableInfo: DatatableWithFormatInfo; @@ -324,7 +324,7 @@ const getColor: GetColorFn = ( series, { layer, colorAssignments, paletteService, syncColors, getSeriesNameFn }, uiState, - singleTable + isSingleTable ) => { const overwriteColor = getSeriesColor(layer, series.yAccessor as string); if (overwriteColor !== null) { @@ -344,7 +344,7 @@ const getColor: GetColorFn = ( { name, totalSeriesAtDepth: colorAssignment.totalSeriesCount, - rankAtDepth: colorAssignment.getRank(singleTable ? 'commonLayerId' : layer.layerId, name), + rankAtDepth: colorAssignment.getRank(isSingleTable ? 'commonLayerId' : layer.layerId, name), }, ]; return paletteService.get(layer.palette.name).getCategoricalColor( diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 339d4d4bda10b..435caf5c48750 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -24,6 +24,8 @@ import { EuiTitle, EuiCallOut, EuiSwitch, + EuiText, + EuiIconTip, } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n-react'; import { DashboardContainerInput } from '../../../../common'; @@ -269,12 +271,47 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { <> + {i18n.translate( + 'dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchLabel', + { + defaultMessage: 'Sync color palettes across panels', + } + )}{' '} + + {i18n.translate('charts.palettes.defaultPaletteLabel', { + defaultMessage: 'Default', + })} + + ), + compatibility: ( + + {i18n.translate('charts.palettes.kibanaPaletteLabel', { + defaultMessage: 'Compatibility', + })} + + ), + }} + /> + } + iconProps={{ + className: 'eui-alignTop', + }} + position="top" + size="s" + type="questionInCircle" + /> + + } checked={dashboardSettingsState.syncColors} onChange={(event) => updateDashboardSetting({ syncColors: event.target.checked })} data-test-subj="dashboardSyncColorsCheckbox" diff --git a/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts b/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts index a5b8ef9a24013..b528bde76e220 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/datatable_fn.ts @@ -72,6 +72,7 @@ export const datatableFn = value: { data: table, untransposedData, + syncColors: context.isSyncColorsEnabled?.() ?? false, args: { ...args, title: (context.variables.embeddableTitle as string) ?? args.title, diff --git a/x-pack/plugins/lens/common/expressions/datatable/types.ts b/x-pack/plugins/lens/common/expressions/datatable/types.ts index 7de3bdde894dc..7f03a1f4fb19b 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/types.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/types.ts @@ -10,6 +10,7 @@ import type { DatatableArgs } from './datatable'; export interface DatatableProps { data: Datatable; + syncColors: boolean; untransposedData?: Datatable; args: DatatableArgs; } diff --git a/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts b/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts index a21572b3da97b..d2e55bf5dd61a 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts @@ -5,7 +5,12 @@ * 2.0. */ -import { ColorMappingInputData, PaletteOutput, PaletteRegistry } from '@kbn/coloring'; +import { + ColorMappingInputData, + PaletteOutput, + PaletteRegistry, + getSpecialString, +} from '@kbn/coloring'; import { CustomPaletteState } from '@kbn/charts-plugin/common'; import { getColorAccessorFn } from './color_mapping_accessor'; @@ -16,6 +21,7 @@ export function getCellColorFn( data: ColorMappingInputData, isNumeric: boolean, isDarkMode: boolean, + syncColors: boolean, palette?: PaletteOutput, colorMapping?: string ): CellColorFn { @@ -40,7 +46,7 @@ export function getCellColorFn( return paletteService.get(palette.name).getCategoricalColor( [ { - name: category, + name: getSpecialString(category), // needed to sync special categories (i.e. '') rankAtDepth: Math.max( data.categories.findIndex((v) => v === category), 0 @@ -52,7 +58,7 @@ export function getCellColorFn( maxDepth: 1, totalSeries: data.categories.length || 1, behindText: false, - syncColors: true, + syncColors, }, palette?.params ?? { colors: [] } ); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx index fde2ebeb7a586..3f51303957cb6 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx @@ -148,7 +148,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const hasAtLeastOneRowClickAction = props.rowHasRowClickTriggerActions?.some((x) => x); - const { getType, dispatchEvent, renderMode, formatFactory } = props; + const { getType, dispatchEvent, renderMode, formatFactory, syncColors } = props; const formatters: Record> = useMemo( () => @@ -423,6 +423,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { data, isNumeric, isDarkMode, + syncColors, palette, colorMapping ); @@ -447,6 +448,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { props.paletteService, firstLocalTable, minMaxByColumnId, + syncColors, ]); const columnVisibility = useMemo( diff --git a/x-pack/plugins/lens/public/visualizations/datatable/expression.tsx b/x-pack/plugins/lens/public/visualizations/datatable/expression.tsx index d58f0cbb04208..652abec75695e 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/expression.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/expression.tsx @@ -168,6 +168,7 @@ export const getDatatableRenderer = (dependencies: { interactive={isInteractive()} theme={dependencies.core.theme} renderComplete={renderComplete} + syncColors={config.syncColors} /> , domNode From eb2cf042bf6daf6cfe4b74687b85b28eea371fa8 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 15 Aug 2024 21:02:47 -0700 Subject: [PATCH 12/38] test: add and fix unit tests --- .../common/color_categories.test.ts | 71 +++++ .../charts/public/services/palettes/mock.ts | 7 +- .../datatable/transpose_helpers.ts | 2 +- .../common/expressions/datatable/utils.ts | 11 +- .../coloring/color_mapping_accessor.test.ts | 34 ++ .../datatable/components/cell_value.test.tsx | 24 +- .../components/dimension_editor.test.tsx | 173 ++++++---- .../datatable/components/dimension_editor.tsx | 15 +- ...mension_editor_additional_section.test.tsx | 55 +++- .../dimension_editor_addtional_section.tsx | 2 - .../datatable/components/table_basic.test.tsx | 167 +++++++--- .../datatable/visualization.test.tsx | 300 ++++++++++++++---- .../datatable/visualization.tsx | 17 +- 13 files changed, 673 insertions(+), 205 deletions(-) create mode 100644 src/plugins/chart_expressions/common/color_categories.test.ts create mode 100644 x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts diff --git a/src/plugins/chart_expressions/common/color_categories.test.ts b/src/plugins/chart_expressions/common/color_categories.test.ts new file mode 100644 index 0000000000000..cbd02e149ecdc --- /dev/null +++ b/src/plugins/chart_expressions/common/color_categories.test.ts @@ -0,0 +1,71 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { DatatableRow } from '@kbn/expressions-plugin/common'; +import { getColorCategories } from './color_categories'; + +const extensions = ['gz', 'css', '', 'rpm', 'deb', 'zip', null]; +const getExtension = (i: number) => extensions[i % extensions.length]; + +const basicDatatableRows: DatatableRow[] = Array.from({ length: 30 }).map((_, i) => ({ + count: i, + extension: getExtension(i), +})); + +const isTransposedDatatableRows: DatatableRow[] = Array.from({ length: 30 }).map((_, i) => ({ + count: i, + ['safari---extension']: getExtension(i), + ['chrome---extension']: getExtension(i + 1), + ['firefox---extension']: getExtension(i + 2), +})); + +describe('getColorCategories', () => { + it('should return all categories from datatable rows', () => { + expect(getColorCategories(basicDatatableRows, 'extension')).toEqual([ + 'gz', + 'css', + '', + 'rpm', + 'deb', + 'zip', + 'null', + ]); + }); + + it('should exclude selected categories from datatable rows', () => { + expect(getColorCategories(basicDatatableRows, 'extension', false, ['', null])).toEqual([ + 'gz', + 'css', + 'rpm', + 'deb', + 'zip', + ]); + }); + + it('should return categories across all transpose columns of datatable rows', () => { + expect(getColorCategories(isTransposedDatatableRows, 'extension', true)).toEqual([ + 'gz', + 'css', + '', + 'rpm', + 'deb', + 'zip', + 'null', + ]); + }); + + it('should exclude selected categories across all transpose columns of datatable rows', () => { + expect(getColorCategories(isTransposedDatatableRows, 'extension', true, ['', null])).toEqual([ + 'gz', + 'css', + 'rpm', + 'deb', + 'zip', + ]); + }); +}); diff --git a/src/plugins/charts/public/services/palettes/mock.ts b/src/plugins/charts/public/services/palettes/mock.ts index 8d7007f96529b..2d1ee87e385ca 100644 --- a/src/plugins/charts/public/services/palettes/mock.ts +++ b/src/plugins/charts/public/services/palettes/mock.ts @@ -74,9 +74,10 @@ export const getPaletteRegistry = () => { }; return { - get: (name: string) => - name === 'custom' ? mockPalette3 : name !== 'default' ? mockPalette2 : mockPalette1, - getAll: () => [mockPalette1, mockPalette2, mockPalette3], + get: jest.fn((name: string) => + name === 'custom' ? mockPalette3 : name !== 'default' ? mockPalette2 : mockPalette1 + ), + getAll: jest.fn(() => [mockPalette1, mockPalette2, mockPalette3]), }; }; diff --git a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts index d0bbda9fa7934..7f7e4d467f250 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts @@ -14,7 +14,7 @@ const TRANSPOSE_SEPARATOR = '---'; const TRANSPOSE_VISUAL_SEPARATOR = '›'; -function getTransposeId(value: string, columnId: string) { +export function getTransposeId(value: string, columnId: string) { return `${value}${TRANSPOSE_SEPARATOR}${columnId}`; } diff --git a/x-pack/plugins/lens/common/expressions/datatable/utils.ts b/x-pack/plugins/lens/common/expressions/datatable/utils.ts index 3b60b7e4834fe..71c3d92126b33 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/utils.ts +++ b/x-pack/plugins/lens/common/expressions/datatable/utils.ts @@ -8,10 +8,11 @@ import type { Datatable } from '@kbn/expressions-plugin/common'; import { getOriginalId } from './transpose_helpers'; -export function isNumericFieldForDatatable(currentData: Datatable | undefined, accessor: string) { - const column = currentData?.columns.find( - (col) => col.id === accessor || getOriginalId(col.id) === accessor - ); +export function isNumericFieldForDatatable(table: Datatable | undefined, accessor: string) { + return getFieldTypeFromDatatable(table, accessor) === 'number'; +} - return column?.meta.type === 'number'; +export function getFieldTypeFromDatatable(table: Datatable | undefined, accessor: string) { + return table?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor) + ?.meta.type; } diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts new file mode 100644 index 0000000000000..c121892b1c373 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { getColorAccessorFn } from './color_mapping_accessor'; + +jest.mock('@kbn/coloring', () => ({ + ...jest.requireActual('@kbn/coloring'), + getColorFactory: jest.fn().mockReturnValue(() => 'red'), +})); + +describe('getColorAccessorFn', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const getColorAccessor = getColorAccessorFn('{}', {} as any, false); + + it('should return null for null values', () => { + expect(getColorAccessor(null)).toBe(null); + }); + + it('should return null for undefined values', () => { + expect(getColorAccessor(undefined)).toBe(null); + }); + + it('should return null for numbers values', () => { + expect(getColorAccessor(123)).toBe(null); + }); + + it('should return color for string values', () => { + expect(getColorAccessor('testing')).toBe('red'); + }); +}); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx index 9636631104221..76b8fc7b61740 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.test.tsx @@ -13,6 +13,7 @@ import { Datatable } from '@kbn/expressions-plugin/public'; import { DatatableArgs } from '../../../../common/expressions'; import { DataContextType } from './types'; import { render, screen } from '@testing-library/react'; +import { getTransposeId } from '../../../../common/expressions/datatable/transpose_helpers'; describe('datatable cell renderer', () => { const innerCellColorFnMock = jest.fn().mockReturnValue('blue'); @@ -207,7 +208,7 @@ describe('datatable cell renderer', () => { { }); }); + it('should call getCellColor with full columnId of transpose column', () => { + const columnId = getTransposeId('test', 'a'); + const columnConfig = getColumnConfiguration(); + columnConfig.columns[0].colorMode = 'cell'; + columnConfig.columns[0].columnId = columnId; + + renderCellComponent(columnConfig, { + table: { + ...table, + columns: [ + { + ...table.columns[0], + id: columnId, + }, + ], + }, + }); + + expect(cellColorFnMock.mock.calls[0][0]).toBe(columnId); + }); + it('should set the coloring of the text when enabled', () => { const columnConfig = getColumnConfiguration(); columnConfig.columns[0].colorMode = 'text'; diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx index 775f93e1ab829..0a29f5c355033 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx @@ -6,12 +6,12 @@ */ import React from 'react'; -import type { PaletteRegistry } from '@kbn/coloring'; -import { render, screen } from '@testing-library/react'; +import { DEFAULT_COLOR_MAPPING_CONFIG, type PaletteRegistry } from '@kbn/coloring'; +import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { chartPluginMock } from '@kbn/charts-plugin/public/mocks'; import { LayerTypes } from '@kbn/expression-xy-plugin/public'; -import { getSelectedButtonInGroup } from '@kbn/test-eui-helpers'; +import { EuiButtonGroupTestHarness } from '@kbn/test-eui-helpers'; import { FramePublicAPI, OperationDescriptor, @@ -21,11 +21,18 @@ import { import { DatatableVisualizationState } from '../visualization'; import { createMockDatasource, createMockFramePublicAPI } from '../../../mocks'; import { TableDimensionEditor } from './dimension_editor'; +import { ColumnState } from '../../../../common/expressions'; +import { capitalize } from 'lodash'; +import { DatatableColumnType } from '@kbn/expressions-plugin/common'; describe('data table dimension editor', () => { let frame: FramePublicAPI; let state: DatatableVisualizationState; - let setState: (newState: DatatableVisualizationState) => void; + let btnGroups: { + colorMode: EuiButtonGroupTestHarness; + alignment: EuiButtonGroupTestHarness; + }; + let mockOperationForFirstColumn: (overrides?: Partial) => void; let props: VisualizationDimensionEditorProps & { paletteService: PaletteRegistry; isDarkMode: boolean; @@ -43,7 +50,19 @@ describe('data table dimension editor', () => { }; } + beforeAll(() => { + jest.useFakeTimers(); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + beforeEach(() => { + btnGroups = { + colorMode: new EuiButtonGroupTestHarness('lnsDatatable_dynamicColoring_groups'), + alignment: new EuiButtonGroupTestHarness('lnsDatatable_alignment_groups'), + }; state = testState(); frame = createMockFramePublicAPI(); frame.datasourceLayers = { @@ -64,15 +83,13 @@ describe('data table dimension editor', () => { rows: [], }, }; - setState = jest.fn(); - props = { accessor: 'foo', frame, groupId: 'columns', layerId: 'first', state, - setState, + setState: jest.fn(), isDarkMode: false, paletteService: chartPluginMock.createPaletteRegistry(), panelRef: React.createRef(), @@ -80,6 +97,18 @@ describe('data table dimension editor', () => { removeLayer: jest.fn(), datasource: {} as DatasourcePublicAPI, }; + + mockOperationForFirstColumn = (overrides: Partial = {}) => { + frame!.datasourceLayers!.first!.getOperationForColumnId = jest.fn().mockReturnValue({ + label: 'label', + isBucketed: false, + dataType: 'string', + hasTimeShift: false, + hasReducedTimeRange: false, + ...overrides, + } satisfies OperationDescriptor); + }; + mockOperationForFirstColumn(); }); const renderTableDimensionEditor = ( @@ -101,19 +130,19 @@ describe('data table dimension editor', () => { it('should render default alignment', () => { renderTableDimensionEditor(); - expect(getSelectedButtonInGroup('lnsDatatable_alignment_groups')()).toHaveTextContent('Left'); + expect(btnGroups.alignment.selected).toHaveTextContent('Left'); }); it('should render default alignment for number', () => { frame.activeData!.first.columns[0].meta.type = 'number'; renderTableDimensionEditor(); - expect(getSelectedButtonInGroup('lnsDatatable_alignment_groups')()).toHaveTextContent('Right'); + expect(btnGroups.alignment.selected).toHaveTextContent('Right'); }); it('should render specific alignment', () => { state.columns[0].alignment = 'center'; renderTableDimensionEditor(); - expect(getSelectedButtonInGroup('lnsDatatable_alignment_groups')()).toHaveTextContent('Center'); + expect(btnGroups.alignment.selected).toHaveTextContent('Center'); }); it('should set state for the right column', () => { @@ -127,7 +156,8 @@ describe('data table dimension editor', () => { ]; renderTableDimensionEditor(); userEvent.click(screen.getByRole('button', { name: 'Center' })); - expect(setState).toHaveBeenCalledWith({ + jest.advanceTimersByTime(256); + expect(props.setState).toHaveBeenCalledWith({ ...state, columns: [ { @@ -141,44 +171,49 @@ describe('data table dimension editor', () => { }); }); - it('should not show the dynamic coloring option for non numeric columns', () => { - renderTableDimensionEditor(); - expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).not.toBeInTheDocument(); - expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument(); - }); - it('should set the dynamic coloring default to "none"', () => { - frame.activeData!.first.columns[0].meta.type = 'number'; + state.columns[0].colorMode = undefined; renderTableDimensionEditor(); - expect(getSelectedButtonInGroup('lnsDatatable_dynamicColoring_groups')()).toHaveTextContent( - 'None' - ); + expect(btnGroups.colorMode.selected).toHaveTextContent('None'); expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument(); }); - it('should show the dynamic palette display ony when colorMode is different from "none"', () => { - frame.activeData!.first.columns[0].meta.type = 'number'; - state.columns[0].colorMode = 'text'; - renderTableDimensionEditor(); - expect(getSelectedButtonInGroup('lnsDatatable_dynamicColoring_groups')()).toHaveTextContent( - 'Text' - ); - expect(screen.getByTestId('lns_dynamicColoring_edit')).toBeInTheDocument(); - }); + it.each(['date'])( + 'should not show the dynamic coloring option for "%s" columns', + (dataType) => { + frame.activeData!.first.columns[0].meta.type = dataType; + renderTableDimensionEditor(); + expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).not.toBeInTheDocument(); + expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument(); + } + ); + + it.each(['cell', 'text'])( + 'should show the palette options ony when colorMode is "%s"', + (colorMode) => { + state.columns[0].colorMode = colorMode; + renderTableDimensionEditor(); + expect(btnGroups.colorMode.selected).toHaveTextContent(capitalize(colorMode)); + expect(screen.getByTestId('lns_dynamicColoring_edit')).toBeInTheDocument(); + } + ); + + it.each(['none', undefined])( + 'should not show the palette options when colorMode is "%s"', + (colorMode) => { + state.columns[0].colorMode = colorMode; + renderTableDimensionEditor(); + expect(btnGroups.colorMode.selected).toHaveTextContent(capitalize(colorMode ?? 'none')); + expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument(); + } + ); it('should set the coloring mode to the right column', () => { - frame.activeData!.first.columns[0].meta.type = 'number'; - state.columns = [ - { - columnId: 'foo', - }, - { - columnId: 'bar', - }, - ]; + state.columns = [{ columnId: 'foo' }, { columnId: 'bar' }]; renderTableDimensionEditor(); userEvent.click(screen.getByRole('button', { name: 'Cell' })); - expect(setState).toHaveBeenCalledWith({ + jest.advanceTimersByTime(256); + expect(props.setState).toHaveBeenCalledWith({ ...state, columns: [ { @@ -194,30 +229,62 @@ describe('data table dimension editor', () => { }); it('should open the palette panel when "Settings" link is clicked in the palette input', () => { - frame.activeData!.first.columns[0].meta.type = 'number'; state.columns[0].colorMode = 'cell'; - renderTableDimensionEditor(); userEvent.click(screen.getByLabelText('Edit colors')); expect(screen.getByTestId('lns-palettePanelFlyout')).toBeInTheDocument(); }); - it('should not show the dynamic coloring option for a dates', () => { - frame.activeData!.first.columns[0].meta.type = 'date'; - const datasourceLayers = frame.datasourceLayers as Record; - datasourceLayers.first.getOperationForColumnId = jest.fn( - () => ({ isBucketed: true } as OperationDescriptor) - ); + it('should show the dynamic coloring option for a bucketed operation', () => { state.columns[0].colorMode = 'cell'; + mockOperationForFirstColumn({ isBucketed: true }); renderTableDimensionEditor(); - expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).not.toBeInTheDocument(); - expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument(); + expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).toBeInTheDocument(); + expect(screen.queryByTestId('lns_dynamicColoring_edit')).toBeInTheDocument(); }); - it('should not show the summary field for non numeric columns', () => { + it('should clear palette and colorMapping when colorMode is set to "none"', () => { + state.columns[0].colorMode = 'cell'; + state.columns[0].palette = { + type: 'palette', + name: 'default', + }; + state.columns[0].colorMapping = DEFAULT_COLOR_MAPPING_CONFIG; + renderTableDimensionEditor(); - expect(screen.queryByTestId('lnsDatatable_summaryrow_function')).not.toBeInTheDocument(); - expect(screen.queryByTestId('lnsDatatable_summaryrow_label')).not.toBeInTheDocument(); + + act(() => { + // this throws an error about state update even in act() + btnGroups.colorMode.select('None'); + }); + + jest.advanceTimersByTime(256); + expect(props.setState).toBeCalledWith({ + ...state, + columns: [ + expect.objectContaining({ + colorMode: 'none', + palette: undefined, + colorMapping: undefined, + }), + ], + }); + }); + + [true, false].forEach((isTransposed) => { + it(`should${isTransposed ? ' not' : ''} show hidden switch when column is${ + !isTransposed ? ' not' : '' + } transposed`, () => { + state.columns[0].isTransposed = isTransposed; + renderTableDimensionEditor(); + + const hiddenSwitch = screen.queryByTestId('lns-table-column-hidden'); + if (isTransposed) { + expect(hiddenSwitch).not.toBeInTheDocument(); + } else { + expect(hiddenSwitch).toBeInTheDocument(); + } + }); }); }); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index ac2af9c2cd9a6..b67aa376d2810 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -19,7 +19,7 @@ import { defaultPaletteParams, findMinMaxByColumnId, } from '../../../shared_components'; -import { isNumericFieldForDatatable } from '../../../../common/expressions/datatable/utils'; +import { getFieldTypeFromDatatable } from '../../../../common/expressions/datatable/utils'; import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; import './dimension_editor.scss'; @@ -73,18 +73,12 @@ export function TableDimensionEditor( if (column.isTransposed) return null; const currentData = frame.activeData?.[localState.layerId]; - - // either read config state or use same logic as chart itself - const isNumeric = isNumericFieldForDatatable(currentData, accessor); + const dataType = getFieldTypeFromDatatable(currentData, accessor); + const isNumeric = dataType === 'number'; const currentAlignment = column?.alignment || (isNumeric ? 'right' : 'left'); const currentColorMode = column?.colorMode || 'none'; const hasDynamicColoring = currentColorMode !== 'none'; - - const datasource = frame.datasourceLayers[localState.layerId]; - - const showDynamicColoringFeature = - datasource?.getOperationForColumnId(accessor)?.dataType !== 'date'; - + const showDynamicColoringFeature = dataType !== 'date'; const visibleColumnsCount = localState.columns.filter((c) => !c.hidden).length; const hasTransposedColumn = localState.columns.some(({ isTransposed }) => isTransposed); @@ -204,6 +198,7 @@ export function TableDimensionEditor( }, }; } + // clear up when switching to no coloring if (newMode === 'none') { params.palette = undefined; diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_additional_section.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_additional_section.test.tsx index b55f637f3cbb3..a12e10d4585c2 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_additional_section.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_additional_section.test.tsx @@ -18,11 +18,11 @@ import { import { DatatableVisualizationState } from '../visualization'; import { createMockDatasource, createMockFramePublicAPI } from '../../../mocks'; import { TableDimensionEditorAdditionalSection } from './dimension_editor_addtional_section'; +import { ColumnState } from '../../../../common/expressions'; describe('data table dimension editor additional section', () => { let frame: FramePublicAPI; let state: DatatableVisualizationState; - let setState: (newState: DatatableVisualizationState) => void; let props: VisualizationDimensionEditorProps & { paletteService: PaletteRegistry; }; @@ -34,6 +34,7 @@ describe('data table dimension editor additional section', () => { columns: [ { columnId: 'foo', + summaryRow: undefined, }, ], }; @@ -53,21 +54,20 @@ describe('data table dimension editor additional section', () => { id: 'foo', name: 'foo', meta: { - type: 'string', + type: 'number', }, }, ], rows: [], }, }; - setState = jest.fn(); props = { accessor: 'foo', frame, groupId: 'columns', layerId: 'first', state, - setState, + setState: jest.fn(), paletteService: chartPluginMock.createPaletteRegistry(), panelRef: React.createRef(), addLayer: jest.fn(), @@ -76,27 +76,50 @@ describe('data table dimension editor additional section', () => { }; }); - it('should set the summary row function default to "none"', () => { - frame.activeData!.first.columns[0].meta.type = 'number'; - render(); + const renderComponent = ( + overrideProps?: Partial< + VisualizationDimensionEditorProps & { + paletteService: PaletteRegistry; + } + > + ) => { + return render(); + }; + + it('should set the summary row fn default to "none"', () => { + state.columns[0].summaryRow = undefined; + renderComponent(); expect(screen.getByRole('combobox')).toHaveValue('None'); expect(screen.queryByTestId('lnsDatatable_summaryrow_label')).not.toBeInTheDocument(); }); - it('should show the summary row label input ony when summary row is different from "none"', () => { - frame.activeData!.first.columns[0].meta.type = 'number'; - state.columns[0].summaryRow = 'sum'; - render(); - expect(screen.getByRole('combobox')).toHaveValue('Sum'); - expect(screen.getByTestId('lnsDatatable_summaryrow_label')).toHaveValue('Sum'); - }); + it.each<[summaryRow: ColumnState['summaryRow'], label: string]>([ + ['sum', 'Sum'], + ['avg', 'Average'], + ['count', 'Value count'], + ['min', 'Minimum'], + ['max', 'Maximum'], + ])( + 'should show the summary row label input ony when summary row fn is "%s"', + (summaryRow, label) => { + state.columns[0].summaryRow = summaryRow; + renderComponent(); + expect(screen.getByRole('combobox')).toHaveValue(label); + expect(screen.getByTestId('lnsDatatable_summaryrow_label')).toHaveValue(label); + } + ); it("should show the correct summary row name when user's changes summary label", () => { - frame.activeData!.first.columns[0].meta.type = 'number'; state.columns[0].summaryRow = 'sum'; state.columns[0].summaryLabel = 'MySum'; - render(); + renderComponent(); expect(screen.getByRole('combobox')).toHaveValue('Sum'); expect(screen.getByTestId('lnsDatatable_summaryrow_label')).toHaveValue('MySum'); }); + + it('should not show the summary field for non numeric columns', () => { + frame.activeData!.first.columns[0].meta.type = 'string'; + expect(screen.queryByTestId('lnsDatatable_summaryrow_function')).not.toBeInTheDocument(); + expect(screen.queryByTestId('lnsDatatable_summaryrow_label')).not.toBeInTheDocument(); + }); }); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_addtional_section.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_addtional_section.tsx index 62e038d71090c..92268d052cd44 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_addtional_section.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor_addtional_section.tsx @@ -21,7 +21,6 @@ import { getFinalSummaryConfiguration, getSummaryRowOptions, } from '../../../../common/expressions/datatable/summary'; - import { isNumericFieldForDatatable } from '../../../../common/expressions/datatable/utils'; import './dimension_editor.scss'; @@ -76,7 +75,6 @@ export function TableDimensionEditorAdditionalSection( const currentData = frame.activeData?.[state.layerId]; - // either read config state or use same logic as chart itself const isNumeric = isNumericFieldForDatatable(currentData, accessor); // when switching from one operation to another, make sure to keep the configuration consistent const { summaryRow, summaryLabel: fallbackSummaryLabel } = getFinalSummaryConfiguration( diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.test.tsx index 8f3060c037808..3c0243e1b980e 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.test.tsx @@ -20,6 +20,18 @@ import { DatatableComponent } from './table_basic'; import type { DatatableProps } from '../../../../common/expressions'; import { LENS_EDIT_PAGESIZE_ACTION } from './constants'; import { DatatableRenderProps } from './types'; +import { PaletteOutput } from '@kbn/coloring'; +import { CustomPaletteState } from '@kbn/charts-plugin/common'; +import { getCellColorFn } from '../../../shared_components/coloring/get_cell_color_fn'; +import { getTransposeId } from '../../../../common/expressions/datatable/transpose_helpers'; + +jest.mock('../../../shared_components/coloring/get_cell_color_fn', () => { + const mod = jest.requireActual('../../../shared_components/coloring/get_cell_color_fn'); + return { + ...mod, + getCellColorFn: jest.fn(mod.getCellColorFn), + }; +}); const { theme: setUpMockTheme } = coreMock.createSetup(); @@ -97,8 +109,12 @@ describe('DatatableComponent', () => { args = sample.args; }); + afterEach(() => { + jest.clearAllMocks(); + }); + const renderDatatableComponent = (propsOverrides: Partial = {}) => { - const props = { + const props: DatatableRenderProps = { data, args, formatFactory: () => ({ convert: (x) => x } as IFieldFormat), @@ -108,6 +124,7 @@ describe('DatatableComponent', () => { theme: setUpMockTheme, renderMode: 'edit' as const, interactive: true, + syncColors: false, renderComplete, ...propsOverrides, }; @@ -345,9 +362,9 @@ describe('DatatableComponent', () => { args: { ...args, columns: [ - { columnId: 'a', alignment: 'center', type: 'lens_datatable_column' }, - { columnId: 'b', type: 'lens_datatable_column' }, - { columnId: 'c', type: 'lens_datatable_column' }, + { columnId: 'a', alignment: 'center', type: 'lens_datatable_column', colorMode: 'none' }, + { columnId: 'b', type: 'lens_datatable_column', colorMode: 'none' }, + { columnId: 'c', type: 'lens_datatable_column', colorMode: 'none' }, ], sortingColumnId: 'b', sortingDirection: 'desc', @@ -358,44 +375,10 @@ describe('DatatableComponent', () => { .map((cell) => cell.className); expect(alignmentsClassNames).toEqual([ - // set via args - 'lnsTableCell--center', - // default for date - 'lnsTableCell--left', - // default for number - 'lnsTableCell--right', + 'lnsTableCell--center', // set via args + 'lnsTableCell--left', // default for date + 'lnsTableCell--right', // default for number ]); - // ({ convert: (x) => x } as IFieldFormat)} - // dispatchEvent={onDispatchEvent} - // getType={jest.fn()} - // renderMode="view" - // paletteService={chartPluginMock.createPaletteRegistry()} - // theme={setUpMockTheme} - // interactive - // renderComplete={renderComplete} - // /> - // ); - - // expect(wrapper.find(DataContext.Provider).prop('value').alignments).toEqual({ - // // set via args - // a: 'center', - // // default for date - // b: 'left', - // // default for number - // c: 'right', - // }); }); test('it should refresh the table header when the datatable data changes', () => { @@ -633,4 +616,106 @@ describe('DatatableComponent', () => { ); }); }); + + describe('renderCellValue', () => { + describe('getCellColor', () => { + const palette: PaletteOutput = { + type: 'palette', + name: 'default', + params: { + colors: [], + gradient: false, + stops: [], + range: 'number', + rangeMin: 0, + rangeMax: 100, + }, + }; + + describe('caching', () => { + test('caches getCellColorFn by columnId', () => { + args.columns[0].palette = palette; + args.columns[0].colorMode = 'cell'; + data.rows.push( + ...[ + { a: 'pants', b: 1588024800000, c: 4 }, + { a: 'hat', b: 1588024800000, c: 5 }, + { a: 'bag', b: 1588024800000, c: 6 }, + ] + ); + + renderDatatableComponent(); + + expect(getCellColorFn).toBeCalledTimes(2); // 2 initial renders of table + }); + + test('caches getCellColorFn by columnId with transpose columns', () => { + const columnId1 = getTransposeId('a', 'test'); + const columnId2 = getTransposeId('b', 'test'); + + renderDatatableComponent({ + data: { + ...data, + rows: [{ [columnId1]: 'shoe', [columnId2]: 'hat' }], + columns: [columnId1, columnId2].map((id) => ({ + ...data.columns[0], + id, + })), + }, + args: { + ...args, + columns: [columnId1, columnId2].map((columnId) => ({ + ...args.columns[0], + palette, + colorMode: 'cell', + columnId, + })), + }, + }); + + expect(getCellColorFn).toBeCalledTimes(2); // 2 initial renders of table + }); + }); + + const color = 'red'; + + test('should correctly color numerical values', () => { + args.columns[0].palette = palette; + args.columns[0].colorMode = 'cell'; + + (getCellColorFn as jest.Mock).mockReturnValue(() => color); + + renderDatatableComponent(); + + const cellColors = screen + .queryAllByRole('gridcell') + .map((cell) => [cell.textContent, cell.style.backgroundColor]); + + expect(cellColors).toEqual([ + ['shoes- a, column 1, row 1', 'red'], + ['1588024800000- b, column 2, row 1', ''], + ['3- c, column 3, row 1', ''], + ]); + }); + + test('should correctly color string values', () => { + args.columns[2].palette = palette; + args.columns[2].colorMode = 'cell'; + + (getCellColorFn as jest.Mock).mockReturnValue(() => color); + + renderDatatableComponent(); + + const cellColors = screen + .queryAllByRole('gridcell') + .map((cell) => [cell.textContent, cell.style.backgroundColor]); + + expect(cellColors).toEqual([ + ['shoes- a, column 1, row 1', ''], + ['1588024800000- b, column 2, row 1', ''], + ['3- c, column 3, row 1', 'red'], + ]); + }); + }); + }); }); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx index d8346ecf9e318..9f9cda08c2442 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx @@ -7,13 +7,8 @@ import { Ast } from '@kbn/interpreter'; import { buildExpression } from '@kbn/expressions-plugin/public'; -import { - createMockDatasource, - createMockFramePublicAPI, - DatasourceMock, - generateActiveData, -} from '../../mocks'; -import faker from 'faker'; +import { getColorStops } from '@kbn/coloring'; +import { createMockDatasource, createMockFramePublicAPI, DatasourceMock } from '../../mocks'; import { DatatableVisualizationState, getDatatableVisualization } from './visualization'; import { Operation, @@ -27,6 +22,19 @@ import { RowHeightMode } from '../../../common/types'; import { chartPluginMock } from '@kbn/charts-plugin/public/mocks'; import { LayerTypes } from '@kbn/expression-xy-plugin/public'; import { themeServiceMock } from '@kbn/core/public/mocks'; +import { ColorMapping, CUSTOM_PALETTE, CustomPaletteParams, PaletteOutput } from '@kbn/coloring'; +import { + ColumnState, + DatatableColumnFn, + DatatableExpressionFunction, +} from '../../../common/expressions'; + +jest.mock('@kbn/coloring', () => { + return { + ...jest.requireActual('@kbn/coloring'), + getColorStops: jest.fn().mockReturnValue([]), + }; +}); function mockFrame(): FramePublicAPI { return { @@ -35,12 +43,18 @@ function mockFrame(): FramePublicAPI { }; } -const datatableVisualization = getDatatableVisualization({ +const mockServices = { paletteService: chartPluginMock.createPaletteRegistry(), kibanaTheme: themeServiceMock.createStartContract(), -}); +}; + +const datatableVisualization = getDatatableVisualization(mockServices); describe('Datatable Visualization', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + describe('#initialize', () => { it('should initialize from the empty state', () => { expect(datatableVisualization.initialize(() => 'aaa', undefined)).toEqual({ @@ -417,65 +431,122 @@ describe('Datatable Visualization', () => { }); describe('with palette', () => { + const mockStops = ['red', 'white', 'blue']; + const datasource = createMockDatasource('test'); let params: VisualizationConfigProps; + beforeEach(() => { - const datasource = createMockDatasource('test'); - datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'b', fields: [] }]); - params = { - layerId: 'a', - state: { - layerId: 'a', - layerType: LayerTypes.DATA, - columns: [ - { - columnId: 'b', - palette: { - type: 'palette' as const, - name: 'custom', - params: { stops: [{ color: 'blue', stop: 0 }] }, - }, - }, - ], - }, - frame: { - ...mockFrame(), - activeData: generateActiveData([ - { - id: 'a', - rows: Array(3).fill({ - b: faker.random.number(), - }), - }, - ]), - datasourceLayers: { a: datasource.publicAPIMock }, - }, - }; + (getColorStops as jest.Mock).mockReturnValue(mockStops); }); - it('does include palette for accessor config if the values are numeric and palette exists', () => { - params.state.columns[0].colorMode = 'cell'; - expect(datatableVisualization.getConfiguration(params).groups[2].accessors).toEqual([ - { columnId: 'b', palette: ['blue', 'yellow'], triggerIconType: 'colorBy' }, - ]); - }); - it('does not include palette for accessor config if the values are not numeric and palette exists', () => { - params.frame.activeData = generateActiveData([ - { - id: 'a', - rows: Array(3).fill({ - b: faker.random.word(), - }), - }, - ]); - expect(datatableVisualization.getConfiguration(params).groups[2].accessors).toEqual([ - { columnId: 'b' }, - ]); + describe('rows', () => { + beforeEach(() => { + datasource.publicAPIMock.getOperationForColumnId.mockReturnValueOnce({ + dataType: 'string', + isBucketed: true, + label: 'label', + isStaticValue: false, + hasTimeShift: false, + hasReducedTimeRange: false, + }); + datasource.publicAPIMock.getTableSpec.mockReturnValue([ + { columnId: 'b', fields: [] }, + { columnId: 'c', fields: [] }, + ]); + + params = { + layerId: 'a', + state: { + layerId: 'a', + layerType: LayerTypes.DATA, + columns: [{ columnId: 'b' }, { columnId: 'c' }], + }, + frame: { + ...mockFrame(), + datasourceLayers: { a: datasource.publicAPIMock }, + }, + }; + }); + + it.each(['cell', 'text'])( + 'should include palette if colorMode is %s and has stops', + (colorMode) => { + params.state.columns[0].colorMode = colorMode; + expect(datatableVisualization.getConfiguration(params).groups[0].accessors).toEqual([ + { columnId: 'b', palette: mockStops, triggerIconType: 'colorBy' }, + ]); + } + ); + + it.each(['cell', 'text'])( + 'should not include palette if colorMode is %s but stops is empty', + (colorMode) => { + (getColorStops as jest.Mock).mockReturnValue([]); + params.state.columns[0].colorMode = colorMode; + expect(datatableVisualization.getConfiguration(params).groups[0].accessors).toEqual([ + { columnId: 'b' }, + ]); + } + ); + + it.each(['none', undefined])( + 'should not include palette if colorMode is %s even if stops exist', + (colorMode) => { + params.state.columns[0].colorMode = colorMode; + expect(datatableVisualization.getConfiguration(params).groups[0].accessors).toEqual([ + { columnId: 'b' }, + ]); + } + ); }); - it('does not include palette for accessor config if the values are numeric but palette exists', () => { - params.state.columns[0].palette = undefined; - expect(datatableVisualization.getConfiguration(params).groups[2].accessors).toEqual([ - { columnId: 'b' }, - ]); + + describe('metrics', () => { + beforeEach(() => { + datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'b', fields: [] }]); + params = { + layerId: 'a', + state: { + layerId: 'a', + layerType: LayerTypes.DATA, + columns: [{ columnId: 'b' }], + }, + frame: { + ...mockFrame(), + datasourceLayers: { a: datasource.publicAPIMock }, + }, + }; + }); + + it.each(['cell', 'text'])( + 'should include palette if colorMode is %s and has stops', + (colorMode) => { + params.state.columns[0].colorMode = colorMode; + expect(datatableVisualization.getConfiguration(params).groups[2].accessors).toEqual([ + { columnId: 'b', palette: mockStops, triggerIconType: 'colorBy' }, + ]); + } + ); + + it.each(['cell', 'text'])( + 'should not include palette if colorMode is %s but stops is empty', + (colorMode) => { + (getColorStops as jest.Mock).mockReturnValue([]); + params.state.columns[0].colorMode = colorMode; + expect(datatableVisualization.getConfiguration(params).groups[2].accessors).toEqual([ + { columnId: 'b' }, + ]); + } + ); + + it.each(['none', undefined])( + 'should not include palette if colorMode is %s even if stops exist', + (colorMode) => { + params.state.columns[0].colorMode = colorMode; + expect(datatableVisualization.getConfiguration(params).groups[2].accessors).toEqual([ + { columnId: 'b' }, + ]); + } + ); }); }); @@ -630,13 +701,12 @@ describe('Datatable Visualization', () => { datatableVisualization.toExpression( state, frame.datasourceLayers, - {}, { '1': { type: 'expression', chain: [] } } ) as Ast - ).findFunction('lens_datatable')[0].arguments; + ).findFunction('lens_datatable')[0].arguments; - const defaultExpressionTableState = { + const defaultExpressionTableState: DatatableVisualizationState = { layerId: 'a', layerType: LayerTypes.DATA, columns: [{ columnId: 'b' }, { columnId: 'c' }], @@ -882,6 +952,104 @@ describe('Datatable Visualization', () => { }) ); }); + + describe('palette/colorMapping/colorMode', () => { + const colorMapping: ColorMapping.Config = { + paletteId: 'default', + colorMode: { type: 'categorical' }, + assignments: [], + specialAssignments: [], + }; + const palette: PaletteOutput = { + type: 'palette', + name: 'default', + }; + const colorExpressionTableState = ( + colorMode?: 'cell' | 'text' | 'none' + ): DatatableVisualizationState => ({ + ...defaultExpressionTableState, + columns: [{ columnId: 'b', colorMapping, palette, colorMode }], + }); + + beforeEach(() => { + datasource.publicAPIMock.getTableSpec.mockReturnValue([{ columnId: 'b', fields: [] }]); + }); + + it.each<[DataType, string]>([ + ['string', palette.name], + ['number', CUSTOM_PALETTE], // required to property handle toExpression + ])( + 'should call paletteService.get with correct palette name for %s dataType', + (dataType, paletteName) => { + datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({ + dataType, + isBucketed: false, + label: 'label', + hasTimeShift: false, + hasReducedTimeRange: false, + }); + + getDatatableExpressionArgs(colorExpressionTableState()); + + expect(mockServices.paletteService.get).toBeCalledWith(paletteName); + } + ); + + describe.each<'cell' | 'text' | 'none' | undefined>(['cell', 'text', 'none', undefined])( + 'colorMode - %s', + (colorMode) => { + it.each<{ dataType: DataType; disallowed?: boolean }>([ + // allowed types + { dataType: 'document' }, + { dataType: 'ip' }, + { dataType: 'histogram' }, + { dataType: 'geo_point' }, + { dataType: 'geo_shape' }, + { dataType: 'counter' }, + { dataType: 'gauge' }, + { dataType: 'murmur3' }, + { dataType: 'string' }, + { dataType: 'number' }, + { dataType: 'boolean' }, + // disallowed types + { dataType: 'date', disallowed: true }, + ])( + 'should apply correct palette, colorMapping & colorMode for $dataType', + ({ dataType, disallowed = false }) => { + datasource.publicAPIMock.getOperationForColumnId.mockReturnValue({ + dataType, + isBucketed: false, + label: 'label', + hasTimeShift: false, + hasReducedTimeRange: false, + }); + + const expression = datatableVisualization.toExpression( + colorExpressionTableState(colorMode), + frame.datasourceLayers, + {}, + { '1': { type: 'expression', chain: [] } } + ) as Ast; + + const columnArgs = + buildExpression(expression).findFunction( + 'lens_datatable_column' + )[0].arguments; + + if (disallowed) { + expect(columnArgs.colorMode).toEqual(['none']); + expect(columnArgs.palette).toBeUndefined(); + expect(columnArgs.colorMapping).toBeUndefined(); + } else { + expect(columnArgs.colorMode).toEqual([colorMode ?? 'none']); + expect(columnArgs.palette).toEqual([expect.any(Object)]); + expect(columnArgs.colorMapping).toEqual([expect.any(String)]); + } + } + ); + } + ); + }); }); describe('#onEditAction', () => { diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index edc5842b3090f..5448c25812189 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -255,7 +255,7 @@ export const getDatatableVisualization = ({ collapseFn, } = columnMap[accessor] ?? {}; const stops = getColorStops(paletteService, isDarkMode, palette, colorMapping); - const hasColoring = Boolean(colorMode !== 'none' && stops); + const hasColoring = colorMode !== 'none' && stops.length > 0; return { columnId: accessor, @@ -342,7 +342,7 @@ export const getDatatableVisualization = ({ hidden, } = columnMap[accessor] ?? {}; const stops = getColorStops(paletteService, isDarkMode, palette, colorMapping); - const hasColoring = Boolean(colorMode !== 'none' && stops); + const hasColoring = colorMode !== 'none' && stops.length > 0; return { columnId: accessor, @@ -523,11 +523,14 @@ export const getDatatableVisualization = ({ transposable: isTransposable, alignment: column.alignment, colorMode: canColor ? column.colorMode ?? 'none' : 'none', - palette: paletteService - // The palette for numeric values is a pseudo custom palette that is only custom from params level - .get(isNumeric ? CUSTOM_PALETTE : column.palette?.name || CUSTOM_PALETTE) - .toExpression(paletteParams), - colorMapping: column.colorMapping ? JSON.stringify(column.colorMapping) : undefined, + palette: !canColor + ? undefined + : paletteService + // The palette for numeric values is a pseudo custom palette that is only custom from params level + .get(isNumeric ? CUSTOM_PALETTE : column.palette?.name || CUSTOM_PALETTE) + .toExpression(paletteParams), + colorMapping: + canColor && column.colorMapping ? JSON.stringify(column.colorMapping) : undefined, summaryRow: hasNoSummaryRow ? undefined : column.summaryRow!, summaryLabel: hasNoSummaryRow ? undefined From f4c1ab52427e336198dbae550fd62d044a445922 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 15 Aug 2024 21:16:07 -0700 Subject: [PATCH 13/38] cleanup cell value merged logic --- .../visualizations/datatable/components/cell_value.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index 01b6118563586..329dfaa626806 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -35,8 +35,12 @@ export const createGridCell = ( const { table, alignments, handleFilterClick } = useContext(DataContext); const rowValue = getParsedValue(table?.rows[rowIndex]?.[columnId]); const colIndex = columnConfig.columns.findIndex(({ columnId: id }) => id === columnId); - const { oneClickFilter, colorMode, palette, colorMapping } = - columnConfig.columns[colIndex] ?? {}; + const { + oneClickFilter, + colorMode = 'none', + palette, + colorMapping, + } = columnConfig.columns[colIndex] ?? {}; const filterOnClick = oneClickFilter && handleFilterClick; const content = formatters[columnId]?.convert(rowValue, filterOnClick ? 'text' : 'html'); const currentAlignment = alignments && alignments[columnId]; @@ -58,7 +62,7 @@ export const createGridCell = ( // Clean up styles when something changes, this avoids cell's styling to stick forever // Checks isExpanded to prevent clearing style after expanding cell - if (colorMode !== 'none' && colorSet && !isExpanded) { + if (colorSet && !isExpanded) { return () => { setCellProps({ style: { From a3ed882efcbe07c4c8a8a77b136878a971d9ba6a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 19 Aug 2024 07:40:10 -0700 Subject: [PATCH 14/38] fix i18n error --- .../component/settings/settings_flyout.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 435caf5c48750..60c416a8664fd 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -288,14 +288,14 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { values={{ default: ( - {i18n.translate('charts.palettes.defaultPaletteLabel', { + {i18n.translate('dashboard.palettes.defaultPaletteLabel', { defaultMessage: 'Default', })} ), compatibility: ( - {i18n.translate('charts.palettes.kibanaPaletteLabel', { + {i18n.translate('dashboard.palettes.kibanaPaletteLabel', { defaultMessage: 'Compatibility', })} From 01a70e37790f974c9b2197ad9045259a0f24f615 Mon Sep 17 00:00:00 2001 From: Nick Partridge Date: Tue, 20 Aug 2024 17:24:31 -0700 Subject: [PATCH 15/38] chore: simplify `color_categories.ts` logic Co-authored-by: Marco Liberati --- src/plugins/chart_expressions/common/color_categories.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/chart_expressions/common/color_categories.ts b/src/plugins/chart_expressions/common/color_categories.ts index b365358141112..3699c89d2c8ce 100644 --- a/src/plugins/chart_expressions/common/color_categories.ts +++ b/src/plugins/chart_expressions/common/color_categories.ts @@ -34,7 +34,7 @@ export function getColorCategories( // The categories needs to be stringified in their unformatted version. // We can't distinguish between a number and a string from a text input and the match should // work with both numeric field values and string values. - const key = (isMultiFieldKey(v) ? [...v.keys] : [v]).map(String); + const key = (isMultiFieldKey(v) ? v.keys : [v]).map(String); const stringifiedKeys = key.join(','); return { key, stringifiedKeys }; }) From 8517989826e198af6665a181367495737fd85330 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 20 Aug 2024 18:06:36 -0700 Subject: [PATCH 16/38] chore: color all bucketed columns by terms --- .../shared_components/coloring/utils.ts | 8 +++++ .../datatable/components/dimension_editor.tsx | 32 +++++++++---------- .../datatable/visualization.tsx | 14 ++++---- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts index aebca956a37a3..aaa048c8a7d9f 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts @@ -19,6 +19,14 @@ import { enforceColorContrast, } from '@kbn/coloring'; import { Datatable } from '@kbn/expressions-plugin/common'; +import { DataType } from '../../types'; + +/** + * Bucketed numerical columns should be treated as categorical + */ +export function shouldColorByTerms(dataType?: DataType, isBucketed?: boolean) { + return isBucketed || dataType !== 'number'; +} export function getContrastColor( color: string, diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index b67aa376d2810..b258e4b818769 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -19,7 +19,6 @@ import { defaultPaletteParams, findMinMaxByColumnId, } from '../../../shared_components'; -import { getFieldTypeFromDatatable } from '../../../../common/expressions/datatable/utils'; import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; import './dimension_editor.scss'; @@ -73,9 +72,10 @@ export function TableDimensionEditor( if (column.isTransposed) return null; const currentData = frame.activeData?.[localState.layerId]; - const dataType = getFieldTypeFromDatatable(currentData, accessor); - const isNumeric = dataType === 'number'; - const currentAlignment = column?.alignment || (isNumeric ? 'right' : 'left'); + 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 currentColorMode = column?.colorMode || 'none'; const hasDynamicColoring = currentColorMode !== 'none'; const showDynamicColoringFeature = dataType !== 'date'; @@ -210,18 +210,7 @@ export function TableDimensionEditor( {hasDynamicColoring && - (isNumeric ? ( - { - updateColumnState(accessor, { palette: newPalette }); - }} - paletteService={props.paletteService} - panelRef={props.panelRef} - dataBounds={currentMinMax} - /> - ) : ( + (showColorByTerms ? ( + ) : ( + { + updateColumnState(accessor, { palette: newPalette }); + }} + paletteService={props.paletteService} + panelRef={props.panelRef} + dataBounds={currentMinMax} + /> ))} )} diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index 5448c25812189..cda33b0ec850c 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -42,6 +42,7 @@ import { DEFAULT_HEADER_ROW_HEIGHT_LINES, DEFAULT_ROW_HEIGHT, } from './components/constants'; +import { shouldColorByTerms } from '../../shared_components'; export interface DatatableVisualizationState { columns: ColumnState[]; layerId: string; @@ -498,18 +499,17 @@ export const getDatatableVisualization = ({ : [], reverse: false, // managed at UI level }; - const sortingHint = datasource!.getOperationForColumnId(column.columnId)!.sortingHint; + const { dataType, isBucketed, sortingHint, inMetricDimension } = + datasource?.getOperationForColumnId(column.columnId) ?? {}; const hasNoSummaryRow = column.summaryRow == null || column.summaryRow === 'none'; - const dataType = datasource!.getOperationForColumnId(column.columnId)?.dataType; const canColor = dataType !== 'date'; - const isNumeric = dataType === 'number'; + const colorByTerms = shouldColorByTerms(dataType, isBucketed); let isTransposable = !isTextBasedLanguage && !datasource!.getOperationForColumnId(column.columnId)?.isBucketed; if (isTextBasedLanguage) { - const operation = datasource!.getOperationForColumnId(column.columnId); - isTransposable = Boolean(column?.isMetric || operation?.inMetricDimension); + isTransposable = Boolean(column?.isMetric || inMetricDimension); } const datatableColumnFn = buildExpressionFunction( @@ -526,8 +526,8 @@ export const getDatatableVisualization = ({ palette: !canColor ? undefined : paletteService - // The palette for numeric values is a pseudo custom palette that is only custom from params level - .get(isNumeric ? CUSTOM_PALETTE : column.palette?.name || CUSTOM_PALETTE) + // The by value palette is a pseudo custom palette that is only custom from params level + .get(colorByTerms ? column.palette?.name || CUSTOM_PALETTE : CUSTOM_PALETTE) .toExpression(paletteParams), colorMapping: canColor && column.colorMapping ? JSON.stringify(column.colorMapping) : undefined, From 3401c34fcaab4c5ee647cce7fb6fff33d749f5ad Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 20 Aug 2024 18:07:17 -0700 Subject: [PATCH 17/38] chore: set default palette based on color type --- .../visualizations/datatable/components/dimension_editor.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index b258e4b818769..be9fd09f4c119 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -91,7 +91,7 @@ export function TableDimensionEditor( const activePalette = column?.palette ?? { type: 'palette', - name: defaultPaletteParams.name, + name: showColorByTerms ? 'default' : defaultPaletteParams.name, }; // need to tell the helper that the colorStops are required to display const displayStops = applyPaletteParams(props.paletteService, activePalette, currentMinMax); From d01d4c33ce477f20a5df416ff30ac16f65c81a7a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 20 Aug 2024 20:48:23 -0700 Subject: [PATCH 18/38] fix: color bucketed numerics by term --- .../coloring/color_mapping_accessor.test.ts | 8 ++++--- .../coloring/color_mapping_accessor.ts | 4 ++-- .../coloring/get_cell_color_fn.ts | 15 ++++++------ .../shared_components/coloring/utils.ts | 7 ++++-- .../datatable/components/dimension_editor.tsx | 1 + .../datatable/components/table_basic.tsx | 24 +++++++++++-------- .../datatable/visualization.tsx | 10 ++++---- 7 files changed, 39 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts index c121892b1c373..139c6ced456bc 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.test.ts @@ -9,7 +9,9 @@ import { getColorAccessorFn } from './color_mapping_accessor'; jest.mock('@kbn/coloring', () => ({ ...jest.requireActual('@kbn/coloring'), - getColorFactory: jest.fn().mockReturnValue(() => 'red'), + getColorFactory: jest + .fn() + .mockReturnValue((v: string | number) => (v === '123' ? 'blue' : 'red')), })); describe('getColorAccessorFn', () => { @@ -24,8 +26,8 @@ describe('getColorAccessorFn', () => { expect(getColorAccessor(undefined)).toBe(null); }); - it('should return null for numbers values', () => { - expect(getColorAccessor(123)).toBe(null); + it('should return stringified value for numbers', () => { + expect(getColorAccessor(123)).toBe('blue'); }); it('should return color for string values', () => { diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts index 61ccee03e4091..cbbc07584f9b2 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts @@ -25,8 +25,8 @@ export function getColorAccessorFn( ); return (value) => { - if (value === undefined || value === null || typeof value === 'number') return null; + if (value === undefined || value === null) return null; - return getColor(value); + return getColor(String(value)); }; } diff --git a/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts b/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts index d2e55bf5dd61a..bb7e25220f793 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/get_cell_color_fn.ts @@ -19,13 +19,13 @@ export type CellColorFn = (value?: number | string | null) => string | null; export function getCellColorFn( paletteService: PaletteRegistry, data: ColorMappingInputData, - isNumeric: boolean, + colorByTerms: boolean, isDarkMode: boolean, syncColors: boolean, palette?: PaletteOutput, colorMapping?: string ): CellColorFn { - if (isNumeric && palette && data.type === 'ranges') { + if (!colorByTerms && palette && data.type === 'ranges') { return (value) => { if (value === null || value === undefined || typeof value !== 'number') return null; @@ -35,20 +35,21 @@ export function getCellColorFn( }; } - if (!isNumeric && data.type === 'categories') { + if (colorByTerms && data.type === 'categories') { if (colorMapping) { return getColorAccessorFn(colorMapping, data, isDarkMode); } else if (palette) { return (category) => { - if (category === undefined || category === null || typeof category === 'number') - return null; + if (category === undefined || category === null) return null; + + const strCategory = String(category); // can be a number as a string return paletteService.get(palette.name).getCategoricalColor( [ { - name: getSpecialString(category), // needed to sync special categories (i.e. '') + name: getSpecialString(strCategory), // needed to sync special categories (i.e. '') rankAtDepth: Math.max( - data.categories.findIndex((v) => v === category), + data.categories.findIndex((v) => v === strCategory), 0 ), totalSeriesAtDepth: data.categories.length || 1, diff --git a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts index aaa048c8a7d9f..f8f0dbe870aec 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts @@ -18,13 +18,16 @@ import { CUSTOM_PALETTE, enforceColorContrast, } from '@kbn/coloring'; -import { Datatable } from '@kbn/expressions-plugin/common'; +import { Datatable, DatatableColumnType } from '@kbn/expressions-plugin/common'; import { DataType } from '../../types'; /** * Bucketed numerical columns should be treated as categorical */ -export function shouldColorByTerms(dataType?: DataType, isBucketed?: boolean) { +export function shouldColorByTerms( + dataType?: DataType | DatatableColumnType, + isBucketed?: boolean +) { return isBucketed || dataType !== 'number'; } diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx index be9fd09f4c119..c1e097276cf3d 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.tsx @@ -18,6 +18,7 @@ import { applyPaletteParams, defaultPaletteParams, findMinMaxByColumnId, + shouldColorByTerms, } from '../../../shared_components'; import { getOriginalId } from '../../../../common/expressions/datatable/transpose_helpers'; diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx index 3f51303957cb6..d7df7f9086ece 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx @@ -37,7 +37,7 @@ import type { FormatFactory } from '../../../../common/types'; import { RowHeightMode } from '../../../../common/types'; import { getOriginalId, isTransposeId, LensGridDirection } from '../../../../common/expressions'; import { VisualizationContainer } from '../../../visualization_container'; -import { findMinMaxByColumnId } from '../../../shared_components'; +import { findMinMaxByColumnId, shouldColorByTerms } from '../../../shared_components'; import type { DataContextType, DatatableRenderProps, @@ -58,7 +58,7 @@ import { } from './table_actions'; import { getFinalSummaryConfiguration } from '../../../../common/expressions/datatable/summary'; import { DEFAULT_HEADER_ROW_HEIGHT, DEFAULT_HEADER_ROW_HEIGHT_LINES } from './constants'; -import { isNumericFieldForDatatable } from '../../../../common/expressions/datatable/utils'; +import { getFieldTypeFromDatatable } from '../../../../common/expressions/datatable/utils'; import { CellColorFn, getCellColorFn } from '../../../shared_components/coloring/get_cell_color_fn'; export const DataContext = React.createContext({}); @@ -402,14 +402,12 @@ export const DatatableComponent = (props: DatatableRenderProps) => { return cellColorFnMap.get(originalId)!; } - const isNumeric = isNumericFieldForDatatable(firstLocalTable, originalId); - const data: ColorMappingInputData = isNumeric + const dataType = getFieldTypeFromDatatable(firstLocalTable, originalId); + const isBucketed = bucketColumns.some((id) => columnId); + const colorByTerms = shouldColorByTerms(dataType, isBucketed); + + const data: ColorMappingInputData = colorByTerms ? { - type: 'ranges', - bins: 0, - ...minMaxByColumnId[originalId], - } - : { type: 'categories', categories: getColorCategories( firstLocalTable.rows, @@ -417,11 +415,16 @@ export const DatatableComponent = (props: DatatableRenderProps) => { isTransposeId(columnId), [null] ), + } + : { + type: 'ranges', + bins: 0, + ...minMaxByColumnId[originalId], }; const colorFn = getCellColorFn( props.paletteService, data, - isNumeric, + colorByTerms, isDarkMode, syncColors, palette, @@ -447,6 +450,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { props.args.fitRowToContent, props.paletteService, firstLocalTable, + bucketColumns, minMaxByColumnId, syncColors, ]); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index cda33b0ec850c..b554e95ff2569 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -207,7 +207,7 @@ export const getDatatableVisualization = ({ getConfiguration({ state, frame, layerId }) { const isDarkMode = kibanaTheme.getTheme().darkMode; const { sortedColumns, datasource } = - getDataSourceAndSortedColumns(state, frame.datasourceLayers, layerId) || {}; + getDataSourceAndSortedColumns(state, frame.datasourceLayers) || {}; const columnMap: Record = {}; state.columns.forEach((column) => { @@ -439,7 +439,7 @@ export const getDatatableVisualization = ({ datasourceExpressionsByLayers = {} ): Ast | null { const { sortedColumns, datasource } = - getDataSourceAndSortedColumns(state, datasourceLayers, state.layerId) || {}; + getDataSourceAndSortedColumns(state, datasourceLayers) || {}; const isTextBasedLanguage = datasource?.isTextBasedLanguage(); if ( @@ -664,8 +664,7 @@ export const getDatatableVisualization = ({ }, getSortedColumns(state, datasourceLayers) { - const { sortedColumns } = - getDataSourceAndSortedColumns(state, datasourceLayers || {}, state.layerId) || {}; + const { sortedColumns } = getDataSourceAndSortedColumns(state, datasourceLayers || {}) || {}; return sortedColumns; }, @@ -718,8 +717,7 @@ export const getDatatableVisualization = ({ function getDataSourceAndSortedColumns( state: DatatableVisualizationState, - datasourceLayers: DatasourceLayers, - layerId: string + datasourceLayers: DatasourceLayers ) { const datasource = datasourceLayers[state.layerId]; const originalOrder = datasource?.getTableSpec().map(({ columnId }) => columnId); From c9dcb25a8f04896637bc43b5682b0146d4d15beb Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2024 09:48:10 -0700 Subject: [PATCH 19/38] chore: update tests for new by terms logic --- .../coloring/color_mapping_by_terms.tsx | 5 +++- .../coloring/color_mapping_by_values.tsx | 5 +++- .../shared_components/coloring/utils.test.ts | 21 ++++++++++++- .../components/dimension_editor.test.tsx | 30 ++++++++++++------- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx index 82cd44091fc62..ad3de1541cdd8 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx @@ -80,7 +80,10 @@ export function ColorMappingByTerms({ } isInlineEditing={isInlineEditing} > -
+
({ })} isInlineEditing={isInlineEditing} > -
+
{ const paletteRegistry = chartPluginMock.createPaletteRegistry(); @@ -108,3 +113,17 @@ describe('findMinMaxByColumnId', () => { ).toEqual({ b: { min: 2, max: 53 } }); }); }); + +describe('shouldColorByTerms', () => { + it('should return true if bucketed regardless of value', () => { + expect(shouldColorByTerms('number', true)).toBe(true); + }); + + it('should return false if not bucketed and numeric', () => { + expect(shouldColorByTerms('number', false)).toBe(false); + }); + + it('should return true if not bucketed and non-numeric', () => { + expect(shouldColorByTerms('string', false)).toBe(true); + }); +}); diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx index 0a29f5c355033..f58b66dba20d3 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/dimension_editor.test.tsx @@ -17,13 +17,13 @@ import { OperationDescriptor, VisualizationDimensionEditorProps, DatasourcePublicAPI, + DataType, } from '../../../types'; import { DatatableVisualizationState } from '../visualization'; import { createMockDatasource, createMockFramePublicAPI } from '../../../mocks'; import { TableDimensionEditor } from './dimension_editor'; import { ColumnState } from '../../../../common/expressions'; import { capitalize } from 'lodash'; -import { DatatableColumnType } from '@kbn/expressions-plugin/common'; describe('data table dimension editor', () => { let frame: FramePublicAPI; @@ -134,7 +134,7 @@ 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'); }); @@ -178,10 +178,10 @@ describe('data table dimension editor', () => { expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument(); }); - it.each(['date'])( + it.each(['date'])( 'should not show the dynamic coloring option for "%s" columns', (dataType) => { - frame.activeData!.first.columns[0].meta.type = dataType; + mockOperationForFirstColumn({ dataType }); renderTableDimensionEditor(); expect(screen.queryByTestId('lnsDatatable_dynamicColoring_groups')).not.toBeInTheDocument(); expect(screen.queryByTestId('lns_dynamicColoring_edit')).not.toBeInTheDocument(); @@ -228,12 +228,22 @@ describe('data table dimension editor', () => { }); }); - it('should open the palette panel when "Settings" link is clicked in the palette input', () => { - state.columns[0].colorMode = 'cell'; - renderTableDimensionEditor(); - userEvent.click(screen.getByLabelText('Edit colors')); - expect(screen.getByTestId('lns-palettePanelFlyout')).toBeInTheDocument(); - }); + 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' }, + ])( + 'should show color by $flyout flyout when bucketing is $isBucketed with $dataType column', + ({ flyout, isBucketed, dataType }) => { + state.columns[0].colorMode = 'cell'; + mockOperationForFirstColumn({ isBucketed, dataType }); + renderTableDimensionEditor(); + + userEvent.click(screen.getByLabelText('Edit colors')); + + expect(screen.getByTestId(`lns-palettePanel-${flyout}`)).toBeInTheDocument(); + } + ); it('should show the dynamic coloring option for a bucketed operation', () => { state.columns[0].colorMode = 'cell'; From afda04d3315cd23d9cd90a3d33735f79e008ee46 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2024 18:03:39 -0700 Subject: [PATCH 20/38] test: fix flaky color sync tests - refactor to use by-value visualizations - adds datatable to test color sync - fixes horrible bug with setPalette not doing what it says - refactor sync tests to compare bucketed color mappings by term --- .../datatable/expression.test.tsx | 4 +- .../apps/dashboard/group2/sync_colors.ts | 88 +++++++++++++------ .../test/functional/page_objects/lens_page.ts | 21 ++++- 3 files changed, 81 insertions(+), 32 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx index e8d450fdb6b59..3e71b654f7213 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx @@ -18,7 +18,7 @@ const cellValueAction: LensCellValueAction = { iconType: 'test-icon', execute: () => {}, }; -function sampleArgs() { +function sampleArgs(): DatatableProps { const indexPatternId = 'indexPatternId'; const data: Datatable = { type: 'datatable', @@ -80,7 +80,7 @@ function sampleArgs() { sortingDirection: 'none', }; - return { data, args }; + return { data, args, syncColors: false }; } describe('datatable_expression', () => { diff --git a/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts b/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts index a388a4d90af3b..ecd5c86d22955 100644 --- a/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts +++ b/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts @@ -7,6 +7,7 @@ import { DebugState } from '@elastic/charts'; import expect from '@kbn/expect'; +import chroma from 'chroma-js'; import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ getService, getPageObjects }: FtrProviderContext) { @@ -51,73 +52,104 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await kibanaServer.savedObjects.cleanStandardList(); }); - it('should sync colors on dashboard by default', async function () { + it('should sync colors on dashboard for legacy default palette', async function () { await PageObjects.dashboard.navigateToApp(); await elasticChart.setNewChartUiDebugFlag(true); await PageObjects.dashboard.clickCreateDashboardPrompt(); + + // create non-filtered xy chart await dashboardAddPanel.clickCreateNewLink(); await PageObjects.lens.goToTimeRange(); - await PageObjects.lens.configureDimension({ dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', operation: 'count', field: 'Records', }); - await PageObjects.lens.configureDimension({ dimension: 'lnsXY_splitDimensionPanel > lns-empty-dimension', operation: 'terms', field: 'geo.src', palette: { mode: 'legacy', id: 'default' }, }); - - await PageObjects.lens.save('vis1', false, true); + await PageObjects.lens.saveAndReturn(); await PageObjects.header.waitUntilLoadingHasFinished(); - await dashboardAddPanel.clickCreateNewLink(); + // create filtered xy chart + await dashboardAddPanel.clickCreateNewLink(); await PageObjects.lens.configureDimension({ dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', operation: 'count', field: 'Records', }); - await PageObjects.lens.configureDimension({ dimension: 'lnsXY_splitDimensionPanel > lns-empty-dimension', operation: 'terms', field: 'geo.src', palette: { mode: 'legacy', id: 'default' }, }); - await filterBar.addFilter({ field: 'geo.src', operation: 'is not', value: 'CN' }); + await PageObjects.lens.saveAndReturn(); + await PageObjects.header.waitUntilLoadingHasFinished(); - await PageObjects.lens.save('vis2', false, true); + // create datatable vis + await dashboardAddPanel.clickCreateNewLink(); + await PageObjects.lens.switchToVisualization('lnsDatatable'); + await PageObjects.lens.configureDimension({ + dimension: 'lnsDatatable_rows > lns-empty-dimension', + operation: 'terms', + field: 'geo.src', + keepOpen: true, + }); + await PageObjects.lens.setTermsNumberOfValues(5); + await PageObjects.lens.setTableDynamicColoring('cell'); + await PageObjects.lens.setPalette('default', true); + await PageObjects.lens.closeDimensionEditor(); + await PageObjects.lens.configureDimension({ + dimension: 'lnsDatatable_metrics > lns-empty-dimension', + operation: 'count', + field: 'Records', + }); + await PageObjects.lens.saveAndReturn(); + + // Set dashboard to sync colors await PageObjects.dashboard.openSettingsFlyout(); await dashboardSettings.toggleSyncColors(true); await dashboardSettings.clickApplyButton(); await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.dashboard.waitForRenderComplete(); - const colorMapping1 = getColorMapping(await PageObjects.dashboard.getPanelChartDebugState(0)); - const colorMapping2 = getColorMapping(await PageObjects.dashboard.getPanelChartDebugState(1)); + const colorMappings1 = Object.entries( + getColorMapping(await PageObjects.dashboard.getPanelChartDebugState(0)) + ); + const colorMappings2 = Object.entries( + getColorMapping(await PageObjects.dashboard.getPanelChartDebugState(1)) + ); - expect(Object.keys(colorMapping1)).to.have.length(6); - expect(Object.keys(colorMapping1)).to.have.length(6); - const panel1Keys = ['CN']; - const panel2Keys = ['PK']; - const sharedKeys = ['IN', 'US', 'ID', 'BR', 'Other']; - // colors for keys exclusive to panel 1 should not occur in panel 2 - panel1Keys.forEach((panel1Key) => { - const assignedColor = colorMapping1[panel1Key]; - expect(Object.values(colorMapping2)).not.to.contain(assignedColor); - }); - // colors for keys exclusive to panel 2 should not occur in panel 1 - panel2Keys.forEach((panel2Key) => { - const assignedColor = colorMapping2[panel2Key]; - expect(Object.values(colorMapping1)).not.to.contain(assignedColor); + const els = await PageObjects.lens.getDatatableCellsByColumn(0); + const colorMappings3 = await Promise.all( + els.map(async (el) => [ + await el.getVisibleText(), + chroma((await PageObjects.lens.getStylesFromCell(el))['background-color']).hex(), // eui converts hex to rgb + ]) + ); + + expect(colorMappings1).to.have.length(6); + expect(colorMappings2).to.have.length(6); + expect(colorMappings3).to.have.length(6); + + const mergedColorAssignments = new Map>(); + + [...colorMappings1, ...colorMappings2, ...colorMappings3].forEach(([key, color]) => { + if (!mergedColorAssignments.has(key)) mergedColorAssignments.set(key, new Set()); + mergedColorAssignments.get(key)?.add(color); }); - // colors for keys used in both panels should be synced - sharedKeys.forEach((sharedKey) => { - expect(colorMapping1[sharedKey]).to.eql(colorMapping2[sharedKey]); + + // Each key should have only been assigned one color across all 3 visualizations + mergedColorAssignments.forEach((colors, key) => { + expect(colors.size).eql( + 1, + `Key "${key}" was assigned multiple colors: ${JSON.stringify([...colors])}` + ); }); }); diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index d477146ae3f81..f1a46ba903c8a 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -1129,8 +1129,7 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont return el.getVisibleText(); }, - async getDatatableCellStyle(rowIndex = 0, colIndex = 0) { - const el = await this.getDatatableCell(rowIndex, colIndex); + async getStylesFromCell(el: WebElementWrapper) { const styleString = (await el.getAttribute('style')) ?? ''; return styleString.split(';').reduce>((memo, cssLine) => { const [prop, value] = cssLine.split(':'); @@ -1141,6 +1140,11 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont }, {}); }, + async getDatatableCellStyle(rowIndex = 0, colIndex = 0) { + const el = await this.getDatatableCell(rowIndex, colIndex); + return this.getStylesFromCell(el); + }, + async getDatatableCellSpanStyle(rowIndex = 0, colIndex = 0) { const el = await (await this.getDatatableCell(rowIndex, colIndex)).findByCssSelector('span'); const styleString = (await el.getAttribute('style')) ?? ''; @@ -1174,6 +1178,12 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont ); }, + async getDatatableCellsByColumn(colIndex = 0) { + return await find.allByCssSelector( + `[data-test-subj="lnsDataTable"] [data-test-subj="dataGridRowCell"][data-gridcell-column-index="${colIndex}"]` + ); + }, + async isDatatableHeaderSorted(index = 0) { return find.existsByCssSelector( `[data-test-subj="lnsDataTable"] [data-test-subj="dataGridHeader"] [role=columnheader]:nth-child(${ @@ -1240,10 +1250,15 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont async setPalette(paletteId: string, isLegacy: boolean) { await testSubjects.click('lns_colorEditing_trigger'); + // This action needs to be slowed WAY down, otherwise it will not correctly set the palette + await PageObjects.common.sleep(200); await testSubjects.setEuiSwitch( 'lns_colorMappingOrLegacyPalette_switch', isLegacy ? 'uncheck' : 'check' ); + + await PageObjects.common.sleep(200); + if (isLegacy) { await testSubjects.click('lns-palettePicker'); await find.clickByCssSelector(`#${paletteId}`); @@ -1251,6 +1266,8 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont await testSubjects.click('kbnColoring_ColorMapping_PalettePicker'); await testSubjects.click(`kbnColoring_ColorMapping_Palette-${paletteId}`); } + await PageObjects.common.sleep(200); + await this.closePaletteEditor(); }, From c4ad8ad884c9b1a06f904cdbb51f5a470bf3d063 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2024 18:14:16 -0700 Subject: [PATCH 21/38] test: fix jest tests due to changes --- .../__snapshots__/xy_chart.test.tsx.snap | 40 +++++++++---------- .../datatable/expression.test.tsx | 4 +- .../metric/dimension_editor.test.tsx | 3 +- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap b/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap index b983bc6047077..789878ac673fa 100644 --- a/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap +++ b/src/plugins/chart_expressions/expression_xy/public/components/__snapshots__/xy_chart.test.tsx.snap @@ -1747,8 +1747,8 @@ exports[`XYChart component it renders area 1`] = ` minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -3302,8 +3302,8 @@ exports[`XYChart component it renders bar 1`] = ` minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -4857,8 +4857,8 @@ exports[`XYChart component it renders horizontal bar 1`] = ` minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -6412,8 +6412,8 @@ exports[`XYChart component it renders line 1`] = ` minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -7967,8 +7967,8 @@ exports[`XYChart component it renders stacked area 1`] = ` minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -9522,8 +9522,8 @@ exports[`XYChart component it renders stacked bar 1`] = ` minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -11077,8 +11077,8 @@ exports[`XYChart component it renders stacked horizontal bar 1`] = ` minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -12858,8 +12858,8 @@ exports[`XYChart component split chart should render split chart if both, splitR minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -14646,8 +14646,8 @@ exports[`XYChart component split chart should render split chart if splitColumnA minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} @@ -16432,8 +16432,8 @@ exports[`XYChart component split chart should render split chart if splitRowAcce minBarHeight={1} paletteService={ Object { - "get": [Function], - "getAll": [Function], + "get": [MockFunction], + "getAll": [MockFunction], } } shouldShowValueLabels={true} diff --git a/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx index 3e71b654f7213..715cfdf526aed 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/expression.test.tsx @@ -86,7 +86,7 @@ function sampleArgs(): DatatableProps { describe('datatable_expression', () => { describe('datatable renders', () => { test('it renders with the specified data and args', async () => { - const { data, args } = sampleArgs(); + const { data, args, ...rest } = sampleArgs(); const result = await getDatatable(() => Promise.resolve((() => {}) as FormatFactory)).fn( data, args, @@ -96,7 +96,7 @@ describe('datatable_expression', () => { expect(result).toEqual({ type: 'render', as: 'lens_datatable_renderer', - value: { data, args }, + value: { data, args, ...rest }, }); }); }); diff --git a/x-pack/plugins/lens/public/visualizations/metric/dimension_editor.test.tsx b/x-pack/plugins/lens/public/visualizations/metric/dimension_editor.test.tsx index 0b34d4453b2f1..721f3e8e7f730 100644 --- a/x-pack/plugins/lens/public/visualizations/metric/dimension_editor.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/metric/dimension_editor.test.tsx @@ -136,7 +136,7 @@ describe('dimension editor', () => { /> ); - const colorModeGroup = screen.queryByRole('group', { name: /color mode/i }); + const colorModeGroup = screen.queryByRole('group', { name: /Color by value/i }); const staticColorPicker = screen.queryByTestId(SELECTORS.COLOR_PICKER); const typeColor = (color: string) => { @@ -170,6 +170,7 @@ describe('dimension editor', () => { expect(screen.queryByTestId(SELECTORS.MAX_EDITOR)).not.toBeInTheDocument(); expect(screen.queryByTestId(SELECTORS.BREAKDOWN_EDITOR)).not.toBeInTheDocument(); }); + it('Color mode switch is shown when the primary metric is numeric', () => { const { colorModeGroup } = renderPrimaryMetricEditor(); expect(colorModeGroup).toBeInTheDocument(); From 46abc0aa8e60bfefbcc44cdc95fa3e6ed243f8ea Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2024 19:02:17 -0700 Subject: [PATCH 22/38] test: move getColorStops to lens utils, fix color stops for custom palette --- .../config/default_color_mapping.ts | 17 --------------- .../shared_components/color_mapping/index.ts | 1 - .../coloring/color_mapping_by_terms.tsx | 2 +- .../shared_components/coloring/utils.ts | 21 +++++++++++++++++++ .../datatable/visualization.tsx | 4 ++-- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/config/default_color_mapping.ts b/packages/kbn-coloring/src/shared_components/color_mapping/config/default_color_mapping.ts index 22a8c512f80e9..8a6ae646b7b6b 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/config/default_color_mapping.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/config/default_color_mapping.ts @@ -11,7 +11,6 @@ import { AVAILABLE_PALETTES, getPalette } from '../palettes'; import { EUIAmsterdamColorBlindPalette } from '../palettes/eui_amsterdam'; import { NeutralPalette } from '../palettes/neutral'; import { getColor, getGradientColorScale } from '../color/color_handling'; -import { DEFAULT_FALLBACK_PALETTE, PaletteOutput, PaletteRegistry } from '../../../palettes'; export const DEFAULT_NEUTRAL_PALETTE_INDEX = 1; export const DEFAULT_OTHER_ASSIGNMENT_INDEX = 0; @@ -38,22 +37,6 @@ export const DEFAULT_COLOR_MAPPING_CONFIG: ColorMapping.Config = { }, }; -/** - * Returns array of colors for provided colorMapping or palette - */ -export function getColorStops( - paletteService: PaletteRegistry, - isDarkMode: boolean, - palette?: PaletteOutput, - colorMapping?: ColorMapping.Config -): string[] { - return colorMapping - ? getColorsFromMapping(isDarkMode, colorMapping) - : paletteService - .get(palette?.name || DEFAULT_FALLBACK_PALETTE) - .getCategoricalColors(10, palette); -} - export function getPaletteColors( isDarkMode: boolean, colorMappings?: ColorMapping.Config diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts index 047de63982215..467296b67e3bf 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts @@ -17,5 +17,4 @@ export { DEFAULT_OTHER_ASSIGNMENT_INDEX, getPaletteColors, getColorsFromMapping, - getColorStops, } from './config/default_color_mapping'; diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx index ad3de1541cdd8..1eec5a4093275 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_terms.tsx @@ -25,12 +25,12 @@ import { PaletteOutput, PaletteRegistry, CustomPaletteParams, - getColorStops, } from '@kbn/coloring'; import { i18n } from '@kbn/i18n'; import { trackUiCounterEvents } from '../../lens_ui_telemetry'; import { PalettePicker } from '../palette_picker'; import { PalettePanelContainer } from './palette_panel_container'; +import { getColorStops } from './utils'; interface ColorMappingByTermsProps { isDarkMode: boolean; diff --git a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts index f8f0dbe870aec..211628a096189 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/utils.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/utils.ts @@ -17,10 +17,31 @@ import { getPaletteStops, CUSTOM_PALETTE, enforceColorContrast, + ColorMapping, + getColorsFromMapping, + DEFAULT_FALLBACK_PALETTE, } from '@kbn/coloring'; import { Datatable, DatatableColumnType } from '@kbn/expressions-plugin/common'; import { DataType } from '../../types'; +/** + * Returns array of colors for provided palette or colorMapping + */ +export function getColorStops( + paletteService: PaletteRegistry, + isDarkMode: boolean, + palette?: PaletteOutput, + colorMapping?: ColorMapping.Config +): string[] { + return colorMapping + ? getColorsFromMapping(isDarkMode, colorMapping) + : palette?.name === CUSTOM_PALETTE + ? palette?.params?.stops?.map(({ color }) => color) ?? [] + : paletteService + .get(palette?.name || DEFAULT_FALLBACK_PALETTE) + .getCategoricalColors(10, palette); +} + /** * Bucketed numerical columns should be treated as categorical */ diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index b554e95ff2569..19b1de3b1c9e3 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { Ast } from '@kbn/interpreter'; import { i18n } from '@kbn/i18n'; -import { PaletteRegistry, CUSTOM_PALETTE, getColorStops } from '@kbn/coloring'; +import { PaletteRegistry, CUSTOM_PALETTE } from '@kbn/coloring'; import { ThemeServiceStart } from '@kbn/core/public'; import { VIS_EVENT_TO_TRIGGER } from '@kbn/visualizations-plugin/public'; import { IconChartDatatable } from '@kbn/chart-icons'; @@ -42,7 +42,7 @@ import { DEFAULT_HEADER_ROW_HEIGHT_LINES, DEFAULT_ROW_HEIGHT, } from './components/constants'; -import { shouldColorByTerms } from '../../shared_components'; +import { getColorStops, shouldColorByTerms } from '../../shared_components'; export interface DatatableVisualizationState { columns: ColumnState[]; layerId: string; From 70a19392f2391d9ec8b035a612cc774b517275c6 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2024 21:55:10 -0700 Subject: [PATCH 23/38] fix: jest test/type error --- .../public/visualizations/datatable/visualization.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx index 9f9cda08c2442..c6670d933e729 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.test.tsx @@ -7,7 +7,6 @@ import { Ast } from '@kbn/interpreter'; import { buildExpression } from '@kbn/expressions-plugin/public'; -import { getColorStops } from '@kbn/coloring'; import { createMockDatasource, createMockFramePublicAPI, DatasourceMock } from '../../mocks'; import { DatatableVisualizationState, getDatatableVisualization } from './visualization'; import { @@ -28,10 +27,11 @@ import { DatatableColumnFn, DatatableExpressionFunction, } from '../../../common/expressions'; +import { getColorStops } from '../../shared_components/coloring'; -jest.mock('@kbn/coloring', () => { +jest.mock('../../shared_components/coloring', () => { return { - ...jest.requireActual('@kbn/coloring'), + ...jest.requireActual('../../shared_components/coloring'), getColorStops: jest.fn().mockReturnValue([]), }; }); From 1a8b94df47c98bd4e562fcd92e75c2648487c48c Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Wed, 21 Aug 2024 21:55:52 -0700 Subject: [PATCH 24/38] fix: bad search predicate --- .../datatable/components/table_basic.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx index d7df7f9086ece..9c3823c434a21 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/table_basic.tsx @@ -224,7 +224,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { [onClickValue, untransposedDataRef, isInteractive] ); - const bucketColumns = useMemo( + const bucketedColumns = useMemo( () => columnConfig.columns .filter((_col, index) => { @@ -240,8 +240,8 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const isEmpty = firstLocalTable.rows.length === 0 || - (bucketColumns.length && - props.data.rows.every((row) => bucketColumns.every((col) => row[col] == null))); + (bucketedColumns.length && + props.data.rows.every((row) => bucketedColumns.every((col) => row[col] == null))); const visibleColumns = useMemo( () => @@ -306,7 +306,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const columns: EuiDataGridColumn[] = useMemo( () => createGridColumns( - bucketColumns, + bucketedColumns, firstLocalTable, handleFilterClick, handleTransposedColumnClick, @@ -324,7 +324,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { props.columnFilterable ), [ - bucketColumns, + bucketedColumns, firstLocalTable, handleFilterClick, handleTransposedColumnClick, @@ -403,7 +403,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { } const dataType = getFieldTypeFromDatatable(firstLocalTable, originalId); - const isBucketed = bucketColumns.some((id) => columnId); + const isBucketed = bucketedColumns.some((id) => id === columnId); const colorByTerms = shouldColorByTerms(dataType, isBucketed); const data: ColorMappingInputData = colorByTerms @@ -450,7 +450,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { props.args.fitRowToContent, props.paletteService, firstLocalTable, - bucketColumns, + bucketedColumns, minMaxByColumnId, syncColors, ]); From aead73b904aac40d9d9616ae30d6794e397aecd4 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2024 08:48:58 -0700 Subject: [PATCH 25/38] chore: remove old nick --- x-pack/test/functional/apps/dashboard/group2/panel_titles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts b/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts index 1eb629fdf0d35..33297ac310630 100644 --- a/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts +++ b/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts @@ -88,7 +88,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); }); - describe('nick by reference', () => { + describe('by reference', () => { const VIS_LIBRARY_DESCRIPTION = 'Vis library description'; let count = 0; From c059e28e8e43e7b007f41fbcbe126ad5a461f268 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2024 12:28:19 -0700 Subject: [PATCH 26/38] fix: range column formatting --- .../color_mapping/color/color_handling.ts | 2 +- .../shared_components/color_mapping/index.ts | 7 +++++- .../coloring/color_mapping_accessor.ts | 9 ++++++-- x-pack/plugins/lens/public/utils.test.ts | 22 ++++++++++++++++++- x-pack/plugins/lens/public/utils.ts | 12 ++++++++++ .../datatable/components/cell_value.tsx | 20 ++++++++++++----- 6 files changed, 62 insertions(+), 10 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/color/color_handling.ts b/packages/kbn-coloring/src/shared_components/color_mapping/color/color_handling.ts index 8867b07572308..77aec76e68a4e 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/color/color_handling.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/color/color_handling.ts @@ -74,7 +74,7 @@ export function getColorFactory( }) : []; - // find all categories that doesn't match with an assignment + // find all categories that don't match with an assignment const notAssignedCategories = data.type === 'categories' ? data.categories.filter((category) => { diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts index 467296b67e3bf..9e111a3742930 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/index.ts +++ b/packages/kbn-coloring/src/shared_components/color_mapping/index.ts @@ -6,7 +6,12 @@ * Side Public License, v 1. */ -export { CategoricalColorMapping, type ColorMappingProps } from './categorical_color_mapping'; +export { + CategoricalColorMapping, + type ColorMappingProps, + type ColorMappingInputCategoricalData, + type ColorMappingInputContinuousData, +} from './categorical_color_mapping'; export type { ColorMappingInputData } from './categorical_color_mapping'; export type { ColorMapping } from './config'; export * from './palettes'; diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts index cbbc07584f9b2..116b21bb2245d 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_accessor.ts @@ -5,8 +5,13 @@ * 2.0. */ -import { AVAILABLE_PALETTES, getColorFactory, getPalette, NeutralPalette } from '@kbn/coloring'; -import { ColorMappingInputCategoricalData } from '@kbn/coloring/src/shared_components/color_mapping/categorical_color_mapping'; +import { + AVAILABLE_PALETTES, + getColorFactory, + getPalette, + NeutralPalette, + ColorMappingInputCategoricalData, +} from '@kbn/coloring'; import { CellColorFn } from './get_cell_color_fn'; /** diff --git a/x-pack/plugins/lens/public/utils.test.ts b/x-pack/plugins/lens/public/utils.test.ts index e775059586aff..58366d38161b0 100644 --- a/x-pack/plugins/lens/public/utils.test.ts +++ b/x-pack/plugins/lens/public/utils.test.ts @@ -7,7 +7,7 @@ import { createDatatableUtilitiesMock } from '@kbn/data-plugin/common/mocks'; import { Datatable } from '@kbn/expressions-plugin/public'; -import { getUniqueLabelGenerator, inferTimeField, renewIDs } from './utils'; +import { getUniqueLabelGenerator, inferTimeField, isLensRange, renewIDs } from './utils'; const datatableUtilities = createDatatableUtilitiesMock(); @@ -187,4 +187,24 @@ describe('utils', () => { expect([' ', ' '].map(labelGenerator)).toEqual(['[Untitled]', '[Untitled] [1]']); }); }); + + describe('isRange', () => { + it.each<[expected: boolean, input: unknown]>([ + [true, { from: 0, to: 100, label: '' }], + [true, { from: 0, to: null, label: '' }], + [true, { from: null, to: 100, label: '' }], + [false, { from: 0, to: 100 }], + [false, { from: 0, to: null }], + [false, { from: null, to: 100 }], + [false, { from: 0 }], + [false, { to: 100 }], + [false, null], + [false, undefined], + [false, 123], + [false, 'string'], + [false, {}], + ])('should return %s for %j', (expected, input) => { + expect(isLensRange(input)).toBe(expected); + }); + }); }); diff --git a/x-pack/plugins/lens/public/utils.ts b/x-pack/plugins/lens/public/utils.ts index 83d0b841d0b2a..43129161adde1 100644 --- a/x-pack/plugins/lens/public/utils.ts +++ b/x-pack/plugins/lens/public/utils.ts @@ -39,6 +39,7 @@ import { import type { DatasourceStates, VisualizationState } from './state_management'; import type { IndexPatternServiceAPI } from './data_views_service/service'; import { COLOR_MAPPING_OFF_BY_DEFAULT } from '../common/constants'; +import type { RangeTypeLens } from './datasources/form_based/operations/definitions/ranges'; export function getVisualizeGeoFieldMessage(fieldType: string) { return i18n.translate('xpack.lens.visualizeGeoFieldMessage', { @@ -47,6 +48,17 @@ export function getVisualizeGeoFieldMessage(fieldType: string) { }); } +export const isLensRange = (range: unknown = {}): range is RangeTypeLens => { + if (!range || typeof range !== 'object') return false; + const { from, to, label } = range as RangeTypeLens; + + return ( + label !== undefined && + (typeof from === 'number' || from === null) && + (typeof to === 'number' || to === null) + ); +}; + export const getResolvedDateRange = function (timefilter: TimefilterContract) { const { from, to } = timefilter.getTime(); return { fromDate: from, toDate: to }; diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index 329dfaa626806..9849aac738a20 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -16,8 +16,17 @@ import type { DataContextType } from './types'; import { getContrastColor } from '../../../shared_components/coloring/utils'; import { CellColorFn } from '../../../shared_components/coloring/get_cell_color_fn'; -const getParsedValue = (v: unknown) => - typeof v === 'number' ? v : v === null || v === undefined ? v : String(v); +import { isLensRange } from '../../../utils'; + +const getParsedValue = (v: unknown) => { + return typeof v === 'number' + ? v + : isLensRange(v) + ? v.toString() + : v === null || v === undefined + ? v + : String(v); +}; export const createGridCell = ( formatters: Record>, @@ -33,7 +42,8 @@ export const createGridCell = ( ) => { return ({ rowIndex, columnId, setCellProps, isExpanded }: EuiDataGridCellValueElementProps) => { const { table, alignments, handleFilterClick } = useContext(DataContext); - const rowValue = getParsedValue(table?.rows[rowIndex]?.[columnId]); + const rawRowValue = table?.rows[rowIndex]?.[columnId]; + const rowValue = getParsedValue(rawRowValue); const colIndex = columnConfig.columns.findIndex(({ columnId: id }) => id === columnId); const { oneClickFilter, @@ -42,7 +52,7 @@ export const createGridCell = ( colorMapping, } = columnConfig.columns[colIndex] ?? {}; const filterOnClick = oneClickFilter && handleFilterClick; - const content = formatters[columnId]?.convert(rowValue, filterOnClick ? 'text' : 'html'); + const content = formatters[columnId]?.convert(rawRowValue, filterOnClick ? 'text' : 'html'); const currentAlignment = alignments && alignments[columnId]; useEffect(() => { @@ -85,7 +95,7 @@ export const createGridCell = ( > { - handleFilterClick?.(columnId, rowValue, colIndex, rowIndex); + handleFilterClick?.(columnId, rawRowValue, colIndex, rowIndex); }} > {content} From 320f1601a55142f8675be8f47c9c2ab9217cca85 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2024 12:29:36 -0700 Subject: [PATCH 27/38] fix: setting switch font size --- .../dashboard_container/component/settings/settings_flyout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 60c416a8664fd..750056691cfb5 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -272,7 +272,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { + {i18n.translate( 'dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchLabel', { From 60d44bcdaff06a9d3e64b7461882f02414942b96 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2024 15:43:21 -0700 Subject: [PATCH 28/38] style: revert text style on EuiButtonEmpty --- .../color_mapping/components/container/assigments.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx index 8c4d3796249f8..d76db505647a4 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/container/assigments.tsx @@ -214,7 +214,6 @@ export function AssignmentsConfig({ , From b6f475419203ef8e8f247b2631ab0ad612a14909 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2024 15:47:44 -0700 Subject: [PATCH 29/38] chore: remove unneeded flex group --- .../coloring/color_mapping_by_values.tsx | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx index 52bdec4f89656..c113ef1a28192 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx @@ -58,18 +58,14 @@ export function ColorMappingByValues({ data-test-subj="lns-palettePanel-values" className="lnsPalettePanel__section lnsPalettePanel__section--shaded lnsIndexPatternDimensionEditor--padded" > - - - { - setPalette(p); - }} - /> - - + { + setPalette(p); + }} + />
From 0fb22f691b4b5419c3f61205c1ef91d09623c776 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 22 Aug 2024 17:21:43 -0700 Subject: [PATCH 30/38] style: minor UI tweaks --- .../components/assignment/assignment.tsx | 2 +- .../components/color_picker/color_picker.tsx | 29 ++++---------- .../components/color_picker/color_swatch.tsx | 15 ++++--- .../color_picker/palette_colors.tsx | 39 ++++++++++--------- .../components/container/assigments.tsx | 25 ++++++------ 5 files changed, 50 insertions(+), 60 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/assignment.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/assignment.tsx index 89c4375d4bc10..7e9e6869849c6 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/assignment.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/assignment.tsx @@ -121,7 +121,7 @@ export function Assignment({ css={ !disableDelete ? css` - color: ${euiThemeVars.euiTextSubduedColor}; + color: ${euiThemeVars.euiTextColor}; transition: ${euiThemeVars.euiAnimSpeedFast} ease-in-out; transition-property: color; &:hover, diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_picker.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_picker.tsx index f576daa2096cc..ebce7b9749830 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_picker.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_picker.tsx @@ -7,14 +7,7 @@ */ import React, { useState } from 'react'; -import { - EuiButtonEmpty, - EuiPopoverTitle, - EuiTab, - EuiTabs, - EuiTitle, - EuiHorizontalRule, -} from '@elastic/eui'; +import { EuiButtonEmpty, EuiPopoverTitle, EuiTab, EuiTabs, EuiHorizontalRule } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { ColorMapping } from '../../config'; import { getPalette } from '../../palettes'; @@ -56,22 +49,14 @@ export function ColorPicker({ > setTab('palette')} isSelected={tab === 'palette'}> - - - {i18n.translate('coloring.colorMapping.colorPicker.paletteTabLabel', { - defaultMessage: 'Colors', - })} - - + {i18n.translate('coloring.colorMapping.colorPicker.paletteTabLabel', { + defaultMessage: 'Colors', + })} setTab('custom')} isSelected={tab === 'custom'}> - - - {i18n.translate('coloring.colorMapping.colorPicker.customTabLabel', { - defaultMessage: 'Custom', - })} - - + {i18n.translate('coloring.colorMapping.colorPicker.customTabLabel', { + defaultMessage: 'Custom', + })} diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx index 34ffbefeca30f..a601e57fb2655 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx @@ -124,15 +124,14 @@ export const ColorSwatch = ({ css={css` &::after { content: ''; - width: 0; - height: 0; - border-left: 3px solid transparent; - border-right: 3px solid transparent; - border-top: 4px solid ${colorIsDark ? 'white' : 'black'}; - margin: 0; - bottom: 2px; + color: ${colorIsDark ? 'white' : 'black'}; + // custom arrowDown svg + background-image: url('data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI3IiBoZWlnaHQ9IjQiIHZpZXdCb3g9IjAgMCA3IDQiPgogIDxwYXRoIGQ9Ik0uMTQ2LjE0N2EuNS41IDAgMCAxIC43MDggMEwzLjUgMi43OTQgNi4xNDYuMTQ3YS41LjUgMCAxIDEgLjcwOC43MDhsLTMgM2EuNS41IDAgMCAxLS43MDggMGwtMy0zYS41LjUgMCAwIDEgMC0uNzA4WiIvPgo8L3N2Zz4K'); + height: 4px; + width: 7px; + bottom: 6px; + right: 4px; position: absolute; - right: 2px; } `} /> diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx index 77a8273654b89..d45d4741a565d 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx @@ -12,7 +12,7 @@ import { EuiFlexItem, EuiHorizontalRule, EuiIcon, - EuiText, + EuiTitle, EuiToolTip, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -50,13 +50,13 @@ export function PaletteColors({ <> - - + +
{i18n.translate('coloring.colorMapping.colorPicker.paletteColorsLabel', { defaultMessage: 'Palette colors', })} - - +
+ - - + +
{i18n.translate('coloring.colorMapping.colorPicker.themeAwareColorsLabel', { defaultMessage: 'Neutral colors', })} - - - - - + + + +
+
{assignments.length > 0 && } -
- {assignments.length > 0 && ( + + {assignments.length > 0 && ( +
{i18n.translate( 'coloring.colorMapping.container.clearAllAssignmentsButtonLabel', @@ -322,8 +325,8 @@ export function AssignmentsConfig({ )} - )} -
+
+ )} ); } From a94a5716910cba2ac22ff7114eb1f71155b1e34a Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Fri, 23 Aug 2024 08:22:14 -0700 Subject: [PATCH 31/38] update limits due to increased size of `@kbn/coloring` --- packages/kbn-optimizer/limits.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-optimizer/limits.yml b/packages/kbn-optimizer/limits.yml index a33156e376972..6fbcf4164aebe 100644 --- a/packages/kbn-optimizer/limits.yml +++ b/packages/kbn-optimizer/limits.yml @@ -55,7 +55,7 @@ pageLoadAssetSize: expressionLegacyMetricVis: 23121 expressionMetric: 22238 expressionMetricVis: 23121 - expressionPartitionVis: 29700 + expressionPartitionVis: 44888 expressionRepeatImage: 22341 expressionRevealImage: 25675 expressions: 140958 From df481413445d8cfe16d69b7a1d08e6921e06b021 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Fri, 23 Aug 2024 08:39:34 -0700 Subject: [PATCH 32/38] chore: add `colorMapping` telemetry --- .../public/visualizations/datatable/visualization.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index 19b1de3b1c9e3..ccc9433d60f03 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -43,6 +43,7 @@ import { DEFAULT_ROW_HEIGHT, } from './components/constants'; import { getColorStops, shouldColorByTerms } from '../../shared_components'; +import { getColorMappingTelemetryEvents } from '../../lens_ui_telemetry/color_telemetry_helpers'; export interface DatatableVisualizationState { columns: ColumnState[]; layerId: string; @@ -559,6 +560,15 @@ export const getDatatableVisualization = ({ }; }, + getTelemetryEventsOnSave(state, prevState) { + const colorMappingEvents = state.columns.flatMap((col) => { + const prevColumn = prevState?.columns?.find((prevCol) => prevCol.columnId === col.columnId); + return getColorMappingTelemetryEvents(col.colorMapping, prevColumn?.colorMapping); + }); + + return colorMappingEvents; + }, + getRenderEventCounters(state) { const events = { color_by_value: false, From 5f0b622ff1969d61af5d07cfdca3d4d2e1d5e48e Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 23 Aug 2024 16:47:27 +0000 Subject: [PATCH 33/38] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../shared_components/coloring/color_mapping_by_values.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx index c113ef1a28192..9e18f6ae149a8 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_mapping_by_values.tsx @@ -7,7 +7,7 @@ import React, { MutableRefObject } from 'react'; -import { EuiFlexGroup, EuiFlexItem, EuiFormRow } from '@elastic/eui'; +import { EuiFormRow } from '@elastic/eui'; import { PaletteOutput, PaletteRegistry, From 034334acf706b26b2d0af700c96ec8824397095f Mon Sep 17 00:00:00 2001 From: Nick Partridge Date: Mon, 26 Aug 2024 13:24:00 -0500 Subject: [PATCH 34/38] chore: apply code review changes Co-authored-by: Marco Liberati --- .../datatable/components/cell_value.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index 9849aac738a20..44bfb29f63512 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -19,13 +19,13 @@ import { CellColorFn } from '../../../shared_components/coloring/get_cell_color_ import { isLensRange } from '../../../utils'; const getParsedValue = (v: unknown) => { - return typeof v === 'number' - ? v - : isLensRange(v) - ? v.toString() - : v === null || v === undefined - ? v - : String(v); +if(v == null || typeof v === 'number'){ + return v; +} +if(isLensRange(v)){ + return v.toString(); +} +return String(v); }; export const createGridCell = ( From f0f0f4774466fd1ea4544793f7fa1b74c128e0a6 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Mon, 26 Aug 2024 13:34:32 -0500 Subject: [PATCH 35/38] chore: fix linting errors --- .../datatable/components/cell_value.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx index 44bfb29f63512..0761c7904e75f 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/components/cell_value.tsx @@ -19,13 +19,13 @@ import { CellColorFn } from '../../../shared_components/coloring/get_cell_color_ import { isLensRange } from '../../../utils'; const getParsedValue = (v: unknown) => { -if(v == null || typeof v === 'number'){ - return v; -} -if(isLensRange(v)){ - return v.toString(); -} -return String(v); + if (v == null || typeof v === 'number') { + return v; + } + if (isLensRange(v)) { + return v.toString(); + } + return String(v); }; export const createGridCell = ( From 77cd4094d32b80394bcbef984e927b9840257816 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 27 Aug 2024 16:47:53 -0500 Subject: [PATCH 36/38] fix: palette issue on datasource update --- .../coloring/palette_picker.tsx | 6 +- .../public/state_management/lens_slice.ts | 23 +++--- x-pack/plugins/lens/public/types.ts | 2 + .../datatable/visualization.tsx | 77 ++++++++++++++++--- 4 files changed, 86 insertions(+), 22 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx b/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx index 5cb2466d48421..d672c76f2124f 100644 --- a/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx +++ b/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx @@ -77,9 +77,9 @@ export function PalettePicker({ }) { const palettesToShow: EuiColorPalettePickerPaletteProps[] = palettes .getAll() - .filter(({ internal, canDynamicColoring }) => - showDynamicColorOnly ? canDynamicColoring : !internal - ) + .filter(({ internal, canDynamicColoring }) => { + return showDynamicColorOnly ? canDynamicColoring && !internal : !internal; + }) .map(({ id, title, getCategoricalColors }) => { const colors = getCategoricalColors( activePalette?.params?.steps || DEFAULT_COLOR_STEPS, diff --git a/x-pack/plugins/lens/public/state_management/lens_slice.ts b/x-pack/plugins/lens/public/state_management/lens_slice.ts index 3146f61d3c5d0..20c727734aa93 100644 --- a/x-pack/plugins/lens/public/state_management/lens_slice.ts +++ b/x-pack/plugins/lens/public/state_management/lens_slice.ts @@ -620,6 +620,7 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { const { datasourceState: syncedDatasourceState, visualizationState: syncedVisualizationState, + frame, } = syncLinkedDimensions( currentState, visualizationMap, @@ -627,7 +628,11 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { payload.datasourceId ); - state.visualization.state = syncedVisualizationState; + const visualization = visualizationMap[state.visualization.activeId!]; + + state.visualization.state = + visualization.onDatasourceUpdate?.(syncedVisualizationState, frame) ?? + syncedVisualizationState; state.datasourceStates[payload.datasourceId].state = syncedDatasourceState; }) .addCase(updateVisualizationState, (state, { payload }) => { @@ -1227,14 +1232,14 @@ function syncLinkedDimensions( const linkedDimensions = activeVisualization.getLinkedDimensions?.(visualizationState); const frame = selectFramePublicAPI({ lens: state }, datasourceMap); - const getDimensionGroups = (layerId: string) => - activeVisualization.getConfiguration({ - state: visualizationState, - layerId, - frame, - }).groups; - if (linkedDimensions) { + const getDimensionGroups = (layerId: string) => + activeVisualization.getConfiguration({ + state: visualizationState, + layerId, + frame, + }).groups; + const idAssuredLinks = linkedDimensions.map((link) => ({ ...link, to: { ...link.to, columnId: link.to.columnId ?? generateId() }, @@ -1276,5 +1281,5 @@ function syncLinkedDimensions( }); } - return { datasourceState, visualizationState }; + return { datasourceState, visualizationState, frame }; } diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 6af614357b89b..65a27f0afea25 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -1317,6 +1317,8 @@ export interface Visualization) => T; + onDatasourceUpdate?: (state: T, frame?: FramePublicAPI) => T; + /** * Some visualization track indexPattern changes (i.e. annotations) * This method makes it aware of the change and produces a new updated state diff --git a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx index ccc9433d60f03..d5c8684b3f2e0 100644 --- a/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { Ast } from '@kbn/interpreter'; import { i18n } from '@kbn/i18n'; -import { PaletteRegistry, CUSTOM_PALETTE } from '@kbn/coloring'; +import { PaletteRegistry, CUSTOM_PALETTE, PaletteOutput, CustomPaletteParams } from '@kbn/coloring'; import { ThemeServiceStart } from '@kbn/core/public'; import { VIS_EVENT_TO_TRIGGER } from '@kbn/visualizations-plugin/public'; import { IconChartDatatable } from '@kbn/chart-icons'; @@ -28,13 +28,14 @@ import { TableDimensionEditorAdditionalSection } from './components/dimension_ed import type { LayerType } from '../../../common/types'; import { RowHeightMode } from '../../../common/types'; import { getDefaultSummaryLabel } from '../../../common/expressions/datatable/summary'; -import type { - ColumnState, - SortingState, - PagingState, - CollapseExpressionFunction, - DatatableColumnFn, - DatatableExpressionFunction, +import { + type ColumnState, + type SortingState, + type PagingState, + type CollapseExpressionFunction, + type DatatableColumnFn, + type DatatableExpressionFunction, + getOriginalId, } from '../../../common/expressions'; import { DataTableToolbar } from './components/toolbar'; import { @@ -42,7 +43,13 @@ import { DEFAULT_HEADER_ROW_HEIGHT_LINES, DEFAULT_ROW_HEIGHT, } from './components/constants'; -import { getColorStops, shouldColorByTerms } from '../../shared_components'; +import { + applyPaletteParams, + defaultPaletteParams, + findMinMaxByColumnId, + getColorStops, + shouldColorByTerms, +} from '../../shared_components'; import { getColorMappingTelemetryEvents } from '../../lens_ui_telemetry/color_telemetry_helpers'; export interface DatatableVisualizationState { columns: ColumnState[]; @@ -117,6 +124,56 @@ export const getDatatableVisualization = ({ ); }, + onDatasourceUpdate(state, frame) { + const datasource = frame?.datasourceLayers?.[state.layerId]; + const paletteMap = new Map( + paletteService + .getAll() + .filter((p) => !p.internal) + .map((p) => [p.id, p]) + ); + + const hasTransposedColumn = state.columns.some(({ isTransposed }) => isTransposed); + const columns = state.columns.map((column) => { + if (column.palette) { + const accessor = column.columnId; + 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 + .filter(({ id }) => getOriginalId(id) === accessor) + .map(({ id }) => id) || [] + : [accessor]; + const minMaxByColumnId = findMinMaxByColumnId(columnsToCheck, currentData, getOriginalId); + + if (palette && !showColorByTerms && !palette?.canDynamicColoring) { + const newPalette: PaletteOutput = { + type: 'palette', + name: showColorByTerms ? 'default' : defaultPaletteParams.name, + }; + return { + ...column, + palette: { + ...newPalette, + params: { + stops: applyPaletteParams(paletteService, newPalette, minMaxByColumnId[accessor]), + }, + }, + }; + } + } + + return column; + }); + + return { + ...state, + columns, + }; + }, + getSuggestions({ table, state, @@ -205,7 +262,7 @@ export const getDatatableVisualization = ({ - Text-based: It relies on the isMetric flag to identify groups. It allows all type of fields on the Metric dimension in cases where there are no numeric columns **/ - getConfiguration({ state, frame, layerId }) { + getConfiguration({ state, frame }) { const isDarkMode = kibanaTheme.getTheme().darkMode; const { sortedColumns, datasource } = getDataSourceAndSortedColumns(state, frame.datasourceLayers) || {}; From fed30897746622e2f6becd36f6609ab193588802 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Tue, 27 Aug 2024 17:12:55 -0500 Subject: [PATCH 37/38] fix: design styles for padding and icon coloring --- .../color_mapping/components/color_picker/color_swatch.tsx | 4 ++-- .../color_mapping/components/color_picker/palette_colors.tsx | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx index a601e57fb2655..9b466810b069c 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/color_swatch.tsx @@ -124,9 +124,9 @@ export const ColorSwatch = ({ css={css` &::after { content: ''; - color: ${colorIsDark ? 'white' : 'black'}; + background-color: ${colorIsDark ? 'white' : 'black'}; // custom arrowDown svg - background-image: url('data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI3IiBoZWlnaHQ9IjQiIHZpZXdCb3g9IjAgMCA3IDQiPgogIDxwYXRoIGQ9Ik0uMTQ2LjE0N2EuNS41IDAgMCAxIC43MDggMEwzLjUgMi43OTQgNi4xNDYuMTQ3YS41LjUgMCAxIDEgLjcwOC43MDhsLTMgM2EuNS41IDAgMCAxLS43MDggMGwtMy0zYS41LjUgMCAwIDEgMC0uNzA4WiIvPgo8L3N2Zz4K'); + mask-image: url('data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI3IiBoZWlnaHQ9IjQiIHZpZXdCb3g9IjAgMCA3IDQiPgogIDxwYXRoIGQ9Ik0uMTQ2LjE0N2EuNS41IDAgMCAxIC43MDggMEwzLjUgMi43OTQgNi4xNDYuMTQ3YS41LjUgMCAxIDEgLjcwOC43MDhsLTMgM2EuNS41IDAgMCAxLS43MDggMGwtMy0zYS41LjUgMCAwIDEgMC0uNzA4WiIvPgo8L3N2Zz4K'); height: 4px; width: 7px; bottom: 6px; diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx index d45d4741a565d..48473fc191f25 100644 --- a/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx +++ b/packages/kbn-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx @@ -14,6 +14,7 @@ import { EuiIcon, EuiTitle, EuiToolTip, + EuiSpacer, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { ColorMapping } from '../../config'; @@ -57,6 +58,7 @@ export function PaletteColors({ })} + + Date: Tue, 27 Aug 2024 23:15:12 +0000 Subject: [PATCH 38/38] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../src/shared_components/coloring/palette_picker.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx b/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx index d672c76f2124f..ca7116142bfde 100644 --- a/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx +++ b/packages/kbn-coloring/src/shared_components/coloring/palette_picker.tsx @@ -77,9 +77,9 @@ export function PalettePicker({ }) { const palettesToShow: EuiColorPalettePickerPaletteProps[] = palettes .getAll() - .filter(({ internal, canDynamicColoring }) => { - return showDynamicColorOnly ? canDynamicColoring && !internal : !internal; - }) + .filter(({ internal, canDynamicColoring }) => { + return showDynamicColorOnly ? canDynamicColoring && !internal : !internal; + }) .map(({ id, title, getCategoricalColors }) => { const colors = getCategoricalColors( activePalette?.params?.steps || DEFAULT_COLOR_STEPS,