From 8ec529f355103feb0010e09af1de1fec6b11f594 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 26 Feb 2024 11:55:25 +0100 Subject: [PATCH 01/13] [ESQL] add chart switch to esql --- .../components/chart_switch_trigger.tsx | 78 ++++++++ .../components/index.ts | 2 + .../kbn-visualization-ui-components/index.ts | 1 + .../tsconfig.json | 3 +- .../metric_vis_function.ts | 2 +- .../layer_configuration_section.tsx | 4 + .../lens_configuration_flyout.tsx | 47 ++--- .../shared/edit_on_the_fly/types.ts | 1 + .../datasources/form_based/datapanel.scss | 6 - .../form_based/form_based_suggestions.ts | 5 +- .../components/dimension_editor.tsx | 5 +- .../text_based/dnd/get_drop_props.tsx | 4 +- .../text_based/text_based_languages.test.ts | 137 ++++++++++---- .../text_based/text_based_languages.tsx | 169 +++++++++++++----- .../public/datasources/text_based/utils.ts | 9 +- .../config_panel/config_panel.tsx | 2 + .../config_panel/layer_panel.test.tsx | 3 + .../editor_frame/config_panel/layer_panel.tsx | 6 + .../config_panel/layer_settings.test.tsx | 12 +- .../config_panel/layer_settings.tsx | 24 ++- .../editor_frame/config_panel/types.ts | 3 + .../workspace_panel/chart_switch.tsx | 116 +++++------- .../lens/public/embeddable/embeddable.tsx | 2 +- x-pack/plugins/lens/public/types.ts | 2 +- .../legacy_metric/metric_suggestions.ts | 5 - .../xy/xy_config_panel/layer_header.tsx | 42 +---- 26 files changed, 466 insertions(+), 224 deletions(-) create mode 100644 packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx diff --git a/packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx b/packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx new file mode 100644 index 000000000000..22b47df0a454 --- /dev/null +++ b/packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx @@ -0,0 +1,78 @@ +/* + * 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 React from 'react'; +import { EuiIcon, EuiText, EuiFlexGroup, EuiFlexItem, IconType } from '@elastic/eui'; +import { ToolbarButton } from '@kbn/shared-ux-button-toolbar'; +import { css } from '@emotion/react'; + +export const ChartSwitchTrigger = function ({ + label, + icon, + onClick, + dataTestSubj, + size = 's', +}: { + label: string; + icon?: IconType; + onClick: () => void; + dataTestSubj?: string; + size?: 's' | 'm'; +}) { + return ( + + ) : ( + + ) + } + /> + ); +}; + +const ChartSwitchLabel = function ({ label, icon }: { label: string; icon?: IconType }) { + return ( + <> + {icon && } + {label} + + ); +}; + +const LayerChartSwitchLabel = function ({ label, icon }: { label: string; icon?: IconType }) { + return ( + + {icon && ( + + + + )} + + + + {label} + + + + ); +}; diff --git a/packages/kbn-visualization-ui-components/components/index.ts b/packages/kbn-visualization-ui-components/components/index.ts index 88fecd132a86..bdd2c4aa3504 100644 --- a/packages/kbn-visualization-ui-components/components/index.ts +++ b/packages/kbn-visualization-ui-components/components/index.ts @@ -30,6 +30,8 @@ export * from './line_style_settings'; export * from './text_decoration_setting'; +export * from './chart_switch_trigger'; + export type { AccessorConfig } from './dimension_buttons'; export type { FieldOptionValue, FieldOption, DataType } from './field_picker'; diff --git a/packages/kbn-visualization-ui-components/index.ts b/packages/kbn-visualization-ui-components/index.ts index 34c14599d6f0..5a51c27af5e7 100644 --- a/packages/kbn-visualization-ui-components/index.ts +++ b/packages/kbn-visualization-ui-components/index.ts @@ -29,6 +29,7 @@ export { LineStyleSettings, TextDecorationSetting, emptyTitleText, + ChartSwitchTrigger, } from './components'; export { isFieldLensCompatible } from './util'; diff --git a/packages/kbn-visualization-ui-components/tsconfig.json b/packages/kbn-visualization-ui-components/tsconfig.json index e5dcfa9c5c85..9a32aedb62a4 100644 --- a/packages/kbn-visualization-ui-components/tsconfig.json +++ b/packages/kbn-visualization-ui-components/tsconfig.json @@ -32,6 +32,7 @@ "@kbn/field-formats-plugin", "@kbn/field-utils", "@kbn/calculate-width-from-char-count", - "@kbn/visualization-utils" + "@kbn/visualization-utils", + "@kbn/shared-ux-button-toolbar" ], } diff --git a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts index 77f061e7a0ce..b66e43a94818 100644 --- a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts +++ b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts @@ -145,7 +145,7 @@ export const metricVisFunction = (): MetricVisExpressionFunctionDefinition => ({ } if (args.metric.length > 1 || input.rows.length > 1) { - throw new Error(errors.severalMetricsAndColorFullBackgroundSpecifiedError()); + // throw new Error(errors.severalMetricsAndColorFullBackgroundSpecifiedError()); } } diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx index 86f2cde79ee5..ec1b987ffff9 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx @@ -27,6 +27,7 @@ export function LayerConfiguration({ hasPadding, setIsInlineFlyoutVisible, getUserMessages, + shouldDisplayChartSwitch, }: LayerConfigurationProps) { const dispatch = useLensDispatch(); const { euiTheme } = useEuiTheme(); @@ -57,6 +58,8 @@ export function LayerConfiguration({ dataViews: startDependencies.dataViews, uiActions: startDependencies.uiActions, hideLayerHeader: datasourceId === 'textBased', + // TODO: remove this prop once we display the chart switch in Discover + shouldDisplayChartSwitch, indexPatternService, setIsInlineFlyoutVisible, getUserMessages, @@ -67,6 +70,7 @@ export function LayerConfiguration({ padding: ${hasPadding ? euiTheme.size.s : 0}; `} > + @@ -372,7 +373,7 @@ export function LensEditConfigurationFlyout({ visualizationMap={visualizationMap} datasourceMap={datasourceMap} datasourceId={datasourceId} - hasPadding={true} + hasPadding framePublicAPI={framePublicAPI} setIsInlineFlyoutVisible={setIsInlineFlyoutVisible} /> @@ -412,6 +413,8 @@ export function LensEditConfigurationFlyout({ ${euiScrollBarStyles(euiTheme)} padding-left: ${euiThemeVars.euiFormMaxWidth}; margin-left: -${euiThemeVars.euiFormMaxWidth}; + padding-right: ${euiThemeVars.euiSizeXS}; + .euiAccordion-isOpen & { block-size: auto !important; flex: 1; @@ -451,15 +454,15 @@ export function LensEditConfigurationFlyout({ } }} isDisabled={false} - allowQueryCancellation={true} + allowQueryCancellation /> )}
- {i18n.translate('xpack.lens.config.layerConfigurationLabel', { - defaultMessage: 'Layer configuration', + {i18n.translate('xpack.lens.config.chartConfigurationLabel', { + defaultMessage: 'Chart configuration', })}
@@ -495,17 +498,21 @@ export function LensEditConfigurationFlyout({ setIsLayerAccordionOpen(!isLayerAccordionOpen); }} > - + <> + + + @@ -515,8 +522,8 @@ export function LensEditConfigurationFlyout({ css={css` border-top: ${euiThemeVars.euiBorderThin}; border-bottom: ${euiThemeVars.euiBorderThin}; - padding-left: ${euiThemeVars.euiSize}; - padding-right: ${euiThemeVars.euiSize}; + padding-left: ${euiThemeVars.euiSizeS}; + padding-right: ${euiThemeVars.euiSizeS}; .euiAccordion__childWrapper { flex: ${isSuggestionsAccordionOpen ? 1 : 'none'} } diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/types.ts b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/types.ts index d974bfa63477..960e0459f8eb 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/types.ts +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/types.ts @@ -102,4 +102,5 @@ export interface LayerConfigurationProps { hasPadding?: boolean; setIsInlineFlyoutVisible: (flag: boolean) => void; getUserMessages: UserMessagesGetter; + shouldDisplayChartSwitch?: boolean; } diff --git a/x-pack/plugins/lens/public/datasources/form_based/datapanel.scss b/x-pack/plugins/lens/public/datasources/form_based/datapanel.scss index 65e9209c9562..0229e4c4fb6c 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/datapanel.scss +++ b/x-pack/plugins/lens/public/datasources/form_based/datapanel.scss @@ -17,9 +17,3 @@ .lnsChangeIndexPatternPopover__trigger { padding: 0 $euiSize; } - -.lnsLayerPanelChartSwitch_title { - font-weight: 600; - display: inline; - padding-left: 8px; -} diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts index 4bb6e5cdf8ec..d2057ea58f15 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts @@ -603,9 +603,12 @@ function createNewLayerWithMetricAggregation( export function getDatasourceSuggestionsFromCurrentState( state: FormBasedPrivateState, - indexPatterns: IndexPatternMap, + indexPatterns?: IndexPatternMap, filterLayers: (layerId: string) => boolean = () => true ): Array> { + if (!indexPatterns) { + return []; + } const layers = Object.entries(state.layers || {}).filter(([layerId]) => filterLayers(layerId)); if (layers.length > 1) { diff --git a/x-pack/plugins/lens/public/datasources/text_based/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datasources/text_based/components/dimension_editor.tsx index 8971696fc902..0f82a65fc1ff 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datasources/text_based/components/dimension_editor.tsx @@ -14,6 +14,7 @@ import type { DatasourceDimensionEditorProps, DataType } from '../../../types'; import { FieldSelect } from './field_select'; import type { TextBasedPrivateState } from '../types'; import { retrieveLayerColumnsFromCache, getColumnsFromCache } from '../fieldlist_cache'; +import { isNotNumeric, isNumeric } from '../utils'; export type TextBasedDimensionEditorProps = DatasourceDimensionEditorProps & { @@ -28,7 +29,7 @@ export function TextBasedDimensionEditor(props: TextBasedDimensionEditorProps) { query ); const allFields = query ? getColumnsFromCache(query) : []; - const hasNumberTypeColumns = allColumns?.some((c) => c?.meta?.type === 'number'); + const hasNumberTypeColumns = allColumns?.some(isNumeric); const fields = allFields.map((col) => { return { id: col.id, @@ -38,7 +39,7 @@ export function TextBasedDimensionEditor(props: TextBasedDimensionEditorProps) { props.isMetricDimension && hasNumberTypeColumns ? props.filterOperations({ dataType: col?.meta?.type as DataType, - isBucketed: Boolean(col?.meta?.type !== 'number'), + isBucketed: Boolean(isNotNumeric(col)), scale: 'ordinal', }) : true, diff --git a/x-pack/plugins/lens/public/datasources/text_based/dnd/get_drop_props.tsx b/x-pack/plugins/lens/public/datasources/text_based/dnd/get_drop_props.tsx index bbbf7869849c..c1d58c488c01 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/dnd/get_drop_props.tsx +++ b/x-pack/plugins/lens/public/datasources/text_based/dnd/get_drop_props.tsx @@ -10,7 +10,7 @@ import { isOperation } from '../../../types'; import type { TextBasedPrivateState } from '../types'; import type { GetDropPropsArgs } from '../../../types'; import { isDraggedField, isOperationFromTheSameGroup } from '../../../utils'; -import { canColumnBeDroppedInMetricDimension } from '../utils'; +import { canColumnBeDroppedInMetricDimension, isNotNumeric } from '../utils'; import { retrieveLayerColumnsFromCache } from '../fieldlist_cache'; export const getDropProps = ( @@ -28,7 +28,7 @@ export const getDropProps = ( if (isDraggedField(source)) { const nextLabel = source.humanData.label; - if (target?.isMetricDimension && sourceField?.meta?.type !== 'number') { + if (target?.isMetricDimension && sourceField && isNotNumeric(sourceField)) { return; } return { diff --git a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts index 22dac6643945..bc160ab4bcd5 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts +++ b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.test.ts @@ -80,6 +80,36 @@ const dateRange = { toDate: '2022-04-17T08:25:00.000Z', }; +const queryBaseState = { + layers: { + a: { + columns: [ + { + columnId: 'a', + fieldName: 'Test 1', + meta: { + type: 'number', + }, + }, + { + columnId: 'b', + fieldName: 'Test 2', + meta: { + type: 'number', + }, + }, + ], + query: { esql: 'FROM foo' }, + index: '1', + }, + }, + indexPatternRefs: [ + { id: '1', title: 'foo' }, + { id: '2', title: 'my-fake-restricted-pattern' }, + { id: '3', title: 'my-compatible-pattern' }, + ], +} as unknown as TextBasedPrivateState; + describe('Textbased Data Source', () => { let baseState: TextBasedPrivateState; let TextBasedDatasource: Datasource; @@ -677,36 +707,6 @@ describe('Textbased Data Source', () => { }); it('should generate an expression for an SQL query', async () => { - const queryBaseState = { - layers: { - a: { - columns: [ - { - columnId: 'a', - fieldName: 'Test 1', - meta: { - type: 'number', - }, - }, - { - columnId: 'b', - fieldName: 'Test 2', - meta: { - type: 'number', - }, - }, - ], - query: { esql: 'FROM foo' }, - index: '1', - }, - }, - indexPatternRefs: [ - { id: '1', title: 'foo' }, - { id: '2', title: 'my-fake-restricted-pattern' }, - { id: '3', title: 'my-compatible-pattern' }, - ], - } as unknown as TextBasedPrivateState; - expect( TextBasedDatasource.toExpression(queryBaseState, 'a', indexPatterns, dateRange, new Date()) ).toMatchInlineSnapshot(` @@ -843,4 +843,81 @@ describe('Textbased Data Source', () => { }); }); }); + describe('#getDatasourceSuggestionsFromCurrentState', () => { + test('should return unchanged suggestion only for one numeric column', () => { + const suggestions = TextBasedDatasource.getDatasourceSuggestionsFromCurrentState(baseState); + expect(suggestions.length).toEqual(1); + expect(suggestions[0].table.changeType).toEqual('unchanged'); + }); + test('should return unchanged suggestion and reduced suggestion for one bucketed and one numeric column', () => { + baseState.layers.a.columns.push({ + columnId: 'c', + fieldName: 'Test 3', + meta: { + type: 'string', + }, + }); + const suggestions = TextBasedDatasource.getDatasourceSuggestionsFromCurrentState(baseState); + expect(suggestions.length).toEqual(2); + expect(suggestions.map((s) => s.table.changeType)).toEqual(['unchanged', 'reduced']); + }); + test('should return unchanged suggestion and 2 reduced suggestions for two numeric columns (converting one to bucket)', () => { + const suggestions = + TextBasedDatasource.getDatasourceSuggestionsFromCurrentState(queryBaseState); + + expect(suggestions.length).toEqual(3); + expect(suggestions.map((s) => s.table.changeType)).toEqual([ + 'unchanged', + 'reduced', + 'reduced', + ]); + }); + test('should return unchanges suggestion and 3 reduced suggestions for many columns', () => { + baseState.layers.a.columns.push( + { + columnId: 'b', + fieldName: 'Test 3', + meta: { + type: 'string', + }, + }, + { + columnId: 'c', + fieldName: 'Test 4', + meta: { + type: 'string', + }, + }, + { + columnId: 'd', + fieldName: 'Test 5', + meta: { + type: 'string', + }, + }, + { + columnId: 'e', + fieldName: 'Test 6', + meta: { + type: 'string', + }, + }, + { + columnId: 'f', + fieldName: 'Test 7', + meta: { + type: 'string', + }, + } + ); + const suggestions = TextBasedDatasource.getDatasourceSuggestionsFromCurrentState(baseState); + expect(suggestions.length).toEqual(4); + expect(suggestions.map((s) => s.table.changeType)).toEqual([ + 'unchanged', + 'reduced', + 'reduced', + 'reduced', + ]); + }); + }); }); diff --git a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx index 48a62cdfb1d0..9f5b44e660bd 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx +++ b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx @@ -38,11 +38,16 @@ import type { TextBasedLayerColumn, TextBasedField, } from './types'; -import type { Datasource } from '../../types'; +import type { Datasource, DatasourceSuggestion } from '../../types'; import { getUniqueLabelGenerator, nonNullable } from '../../utils'; import { onDrop, getDropProps } from './dnd'; import { removeColumn } from './remove_column'; -import { canColumnBeUsedBeInMetricDimension, MAX_NUM_OF_COLUMNS } from './utils'; +import { + canColumnBeUsedBeInMetricDimension, + isNotNumeric, + isNumeric, + MAX_NUM_OF_COLUMNS, +} from './utils'; import { getColumnsFromCache, addColumnsToCache, @@ -65,6 +70,97 @@ const getSelectedFieldsFromColumns = memoizeOne( isEqual ); +const getUnchangedSuggestionTable = ( + state: TextBasedPrivateState, + allColumns: TextBasedLayerColumn[], + id: string +) => { + return { + state: { + ...state, + }, + table: { + changeType: 'unchanged' as TableChangeType, + isMultiRow: false, + layerId: id, + columns: + state.layers[id].columns?.map((f) => { + const inMetricDimension = canColumnBeUsedBeInMetricDimension(allColumns, f?.meta?.type); + return { + columnId: f.columnId, + operation: { + dataType: f?.meta?.type as DataType, + label: f.fieldName, + isBucketed: Boolean(isNotNumeric(f)), + // makes non-number fields to act as metrics, used for datatable suggestions + ...(inMetricDimension && { + inMetricDimension, + }), + }, + }; + }) ?? [], + }, + keptLayerIds: [id], + }; +}; + +const getSuggestionsByRules = ( + state: TextBasedPrivateState, + allColumns: TextBasedLayerColumn[], + id: string, + rules: Array<{ isBucketed: boolean; allowAll?: boolean }> +) => { + const columnsToKeep = rules.reduce((acc, rule) => { + const fn = rule.isBucketed ? isNotNumeric : isNumeric; + let column = state.layers[id].columns?.find( + (col) => fn(col) && !acc.some((c) => c.columnId === col.columnId) + ); + if (!column && rule.allowAll) { + column = state.layers[id].columns?.find( + (col) => !acc.some((c) => c.columnId === col.columnId) + ); + } + return column ? [...acc, column] : acc; + }, []); + + if (!columnsToKeep.length || columnsToKeep.length !== rules.length) { + return; + } + return { + state: { + ...state, + layers: { + [id]: { + ...state.layers[id], + columns: columnsToKeep, + }, + }, + }, + table: { + changeType: 'reduced' as TableChangeType, + isMultiRow: false, + layerId: id, + columns: + columnsToKeep?.map((f, i) => { + const inMetricDimension = canColumnBeUsedBeInMetricDimension(allColumns, f?.meta?.type); + return { + columnId: f.columnId, + operation: { + dataType: f?.meta?.type as DataType, + label: f.fieldName, + isBucketed: !!rules[i].isBucketed, + // makes non-number fields to act as metrics, used for datatable suggestions + ...(inMetricDimension && { + inMetricDimension, + }), + }, + }; + }) ?? [], + }, + keptLayerIds: [id], + }; +}; + export function getTextBasedDatasource({ core, storage, @@ -79,38 +175,31 @@ export function getTextBasedDatasource({ dataViews: DataViewsPublicPluginStart; }) { const getSuggestionsForState = (state: TextBasedPrivateState) => { - return Object.entries(state.layers)?.map(([id, layer]) => { + return Object.entries(state.layers)?.flatMap(([id, layer]) => { const allColumns = retrieveLayerColumnsFromCache(layer.columns, layer.query); - return { - state: { - ...state, - }, - table: { - changeType: 'unchanged' as TableChangeType, - isMultiRow: false, - layerId: id, - columns: - layer.columns?.map((f) => { - const inMetricDimension = canColumnBeUsedBeInMetricDimension( - allColumns, - f?.meta?.type - ); - return { - columnId: f.columnId, - operation: { - dataType: f?.meta?.type as DataType, - label: f.fieldName, - isBucketed: Boolean(f?.meta?.type !== 'number'), - // makes non-number fields to act as metrics, used for datatable suggestions - ...(inMetricDimension && { - inMetricDimension, - }), - }, - }; - }) ?? [], - }, - keptLayerIds: [id], - }; + + const unchangedSuggestionTable = getUnchangedSuggestionTable(state, allColumns, id); + + // we are trying here to cover the most common cases for the charts we offer + const metricTable = getSuggestionsByRules(state, allColumns, id, [{ isBucketed: false }]); + const metricBucketTable = getSuggestionsByRules(state, allColumns, id, [ + { isBucketed: false }, + { isBucketed: true, allowAll: true }, + ]); + const metricBucketBucketTable = getSuggestionsByRules(state, allColumns, id, [ + { isBucketed: false }, + { isBucketed: true, allowAll: true }, + { isBucketed: true, allowAll: true }, + ]); + + return [unchangedSuggestionTable, metricBucketBucketTable, metricBucketTable, metricTable] + .filter(nonNullable) + .reduce>>((acc, cur) => { + if (acc.find(({ table }) => isEqual(table.columns, cur.table.columns))) { + return acc; + } + return [...acc, cur]; + }, []); }); }; const getSuggestionsForVisualizeField = ( @@ -127,7 +216,7 @@ export function getTextBasedDatasource({ // Number fields are assigned automatically as metrics (!isBucketed). There are cases where the query // will not return number fields. In these cases we want to suggest a datatable // Datatable works differently in this case. On the metrics dimension can be all type of fields - const hasNumberTypeColumns = textBasedQueryColumns?.some((c) => c?.meta?.type === 'number'); + const hasNumberTypeColumns = textBasedQueryColumns?.some(isNumeric); const newColumns = textBasedQueryColumns.map((c) => { const inMetricDimension = canColumnBeUsedBeInMetricDimension( textBasedQueryColumns, @@ -190,7 +279,7 @@ export function getTextBasedDatasource({ operation: { dataType: f?.meta?.type as DataType, label: f.fieldName, - isBucketed: Boolean(f?.meta?.type !== 'number'), + isBucketed: Boolean(isNotNumeric(f)), }, }; }) ?? [], @@ -449,7 +538,7 @@ export function getTextBasedDatasource({ return { dataType: column?.meta?.type as DataType, label: columnLabelMap[columnId] ?? column?.fieldName, - isBucketed: Boolean(column?.meta?.type !== 'number'), + isBucketed: Boolean(isNotNumeric(column)), inMetricDimension: column.inMetricDimension, hasTimeShift: false, hasReducedTimeRange: false, @@ -516,7 +605,7 @@ export function getTextBasedDatasource({ operation: { dataType: f?.meta?.type as DataType, label: f.fieldName, - isBucketed: Boolean(f?.meta?.type !== 'number'), + isBucketed: Boolean(isNotNumeric(f)), }, }; }), @@ -525,7 +614,7 @@ export function getTextBasedDatasource({ operation: { dataType: field?.meta?.type as DataType, label: field?.name ?? '', - isBucketed: Boolean(field?.meta?.type !== 'number'), + isBucketed: Boolean(isNotNumeric(field)), }, }, ], @@ -558,11 +647,11 @@ export function getTextBasedDatasource({ const columns = Object.entries(layer.columns).map(([colId, col]) => { return { id: colId, - role: col.meta?.type !== 'number' ? ('split' as const) : ('metric' as const), + role: isNotNumeric(col) ? ('split' as const) : ('metric' as const), operation: { dataType: col?.meta?.type as DataType, label: col.fieldName, - isBucketed: Boolean(col?.meta?.type !== 'number'), + isBucketed: Boolean(isNotNumeric(col)), hasTimeShift: false, hasReducedTimeRange: false, fields: [col.fieldName], diff --git a/x-pack/plugins/lens/public/datasources/text_based/utils.ts b/x-pack/plugins/lens/public/datasources/text_based/utils.ts index 16f4f9b5cfcd..57cabae29e70 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/utils.ts +++ b/x-pack/plugins/lens/public/datasources/text_based/utils.ts @@ -142,12 +142,17 @@ export function getIndexPatternFromTextBasedQuery(query: AggregateQuery): string return indexPattern; } +export const isNumeric = (column: TextBasedLayerColumn | DatatableColumn) => + column?.meta?.type === 'number'; +export const isNotNumeric = (column: TextBasedLayerColumn | DatatableColumn) => + column?.meta?.type !== 'number'; + export function canColumnBeDroppedInMetricDimension( columns: TextBasedLayerColumn[] | DatatableColumn[], selectedColumnType?: string ): boolean { // check if at least one numeric field exists - const hasNumberTypeColumns = columns?.some((c) => c?.meta?.type === 'number'); + const hasNumberTypeColumns = columns?.some(isNumeric); return !hasNumberTypeColumns || (hasNumberTypeColumns && selectedColumnType === 'number'); } @@ -156,7 +161,7 @@ export function canColumnBeUsedBeInMetricDimension( selectedColumnType?: string ): boolean { // check if at least one numeric field exists - const hasNumberTypeColumns = columns?.some((c) => c?.meta?.type === 'number'); + const hasNumberTypeColumns = columns?.some(isNumeric); return ( !hasNumberTypeColumns || columns.length >= MAX_NUM_OF_COLUMNS || diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx index 8c31fb2f9d80..5a28b1ac421e 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx @@ -275,10 +275,12 @@ export function LayerPanels( layerId={layerId} layerIndex={layerIndex} visualizationState={visualization.state} + visualizationMap={props.visualizationMap} updateVisualization={setVisualizationState} updateDatasource={updateDatasource} updateDatasourceAsync={updateDatasourceAsync} displayLayerSettings={!props.hideLayerHeader} + shouldDisplayChartSwitch={props.shouldDisplayChartSwitch} onChangeIndexPattern={(args) => { onChangeIndexPattern(args); const layersToRemove = diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx index 71d29df9911e..08fb71bc5979 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx @@ -80,6 +80,9 @@ describe('LayerPanel', () => { function getDefaultProps() { return { layerId: 'first', + visualizationMap: { + testVis: mockVisualization, + }, activeVisualization: mockVisualization, dimensionGroups: mockVisualization.getConfiguration({} as VisualizationConfigProps).groups, datasourceMap: { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 0ed5945704a3..3c203cacff3b 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -61,6 +61,8 @@ export function LayerPanel(props: LayerPanelProps) { registerNewLayerRef, layerIndex, activeVisualization, + visualizationMap, + datasourceMap, updateVisualization, updateDatasource, toggleFullscreen, @@ -71,6 +73,7 @@ export function LayerPanel(props: LayerPanelProps) { core, onDropToDimension, setIsInlineFlyoutVisible, + shouldDisplayChartSwitch, } = props; const isSaveable = useLensSelector((state) => state.lens.isSaveable); @@ -375,6 +378,9 @@ export function LayerPanel(props: LayerPanelProps) { }), }} activeVisualization={activeVisualization} + visualizationMap={visualizationMap} + datasourceMap={datasourceMap} + shouldDisplayChartSwitch={shouldDisplayChartSwitch} /> {props.displayLayerSettings && ( diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_settings.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_settings.test.tsx index c2b7f2b5fa8d..22848703cb96 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_settings.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_settings.test.tsx @@ -8,7 +8,11 @@ import React from 'react'; import { screen } from '@testing-library/react'; import faker from 'faker'; -import { createMockFramePublicAPI, createMockVisualization } from '../../../mocks'; +import { + createMockDatasource, + createMockFramePublicAPI, + createMockVisualization, +} from '../../../mocks'; import { LayerSettings } from './layer_settings'; import { renderWithReduxStore } from '../../../mocks'; @@ -16,6 +20,12 @@ describe('LayerSettings', () => { const renderLayerSettings = (propsOverrides = {}) => { return renderWithReduxStore( + ); + } if (!activeVisualization.LayerHeaderComponent) { const description = activeVisualization.getDescription(layerConfigProps.state); return ; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts index 9d7499803326..557ba90045e1 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/types.ts @@ -35,11 +35,13 @@ export interface ConfigPanelWrapperProps { getUserMessages?: UserMessagesGetter; hideLayerHeader?: boolean; setIsInlineFlyoutVisible?: (status: boolean) => void; + shouldDisplayChartSwitch?: boolean; } export interface LayerPanelProps { visualizationState: unknown; datasourceMap: DatasourceMap; + visualizationMap: VisualizationMap; framePublicAPI: FramePublicAPI; core: DatasourceDimensionEditorProps['core']; activeVisualization: Visualization; @@ -82,6 +84,7 @@ export interface LayerPanelProps { getUserMessages?: UserMessagesGetter; displayLayerSettings: boolean; setIsInlineFlyoutVisible?: (status: boolean) => void; + shouldDisplayChartSwitch?: boolean; } export interface LayerDatasourceDropProps { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx index 2cb8fc75a675..a65b01040f06 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx @@ -20,7 +20,7 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; -import { ToolbarButton } from '@kbn/shared-ux-button-toolbar'; +import { ChartSwitchTrigger } from '@kbn/visualization-ui-components'; import { Visualization, FramePublicAPI, @@ -59,44 +59,11 @@ export interface ChartSwitchProps { framePublicAPI: FramePublicAPI; visualizationMap: VisualizationMap; datasourceMap: DatasourceMap; + size?: 's' | 'm'; } type SelectableEntry = EuiSelectableOption<{ value: string }>; -function VisualizationSummary({ - visualizationMap, - visualization, -}: { - visualizationMap: VisualizationMap; - visualization: { - activeId: string | null; - state: unknown; - }; -}) { - const activeVisualization = visualizationMap[visualization.activeId || '']; - - if (!activeVisualization) { - return ( - <> - {i18n.translate('xpack.lens.configPanel.selectVisualization', { - defaultMessage: 'Select a visualization', - })} - - ); - } - - const description = activeVisualization.getDescription(visualization.state); - - return ( - <> - {description.icon && ( - - )} - {description.label} - - ); -} - const MAX_LIST_HEIGHT = 380; const ENTRY_HEIGHT = 32; @@ -126,7 +93,12 @@ function getCurrentVisualizationId( ); } -export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { +export const ChartSwitch = memo(function ChartSwitch({ + framePublicAPI, + visualizationMap, + datasourceMap, + size = 'm', +}: ChartSwitchProps) { const [flyoutOpen, setFlyoutOpen] = useState(false); const dispatchLens = useLensDispatch(); const activeDatasourceId = useLensSelector(selectActiveDatasourceId); @@ -152,7 +124,7 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { dispatchLens( removeLayers({ visualizationId: visualization.activeId, - layerIds: Object.keys(props.framePublicAPI.datasourceLayers), + layerIds: Object.keys(framePublicAPI.datasourceLayers), }) ); } @@ -162,17 +134,17 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { visualizationId: string, subVisualizationId: string ): VisualizationSelection { - const newVisualization = props.visualizationMap[visualizationId]; + const newVisualization = visualizationMap[visualizationId]; const switchVisType = (type: string, state: unknown) => { - if (props.visualizationMap[visualizationId].switchVisualizationType) { + if (visualizationMap[visualizationId].switchVisualizationType) { return safeFnCall( - () => props.visualizationMap[visualizationId].switchVisualizationType!(type, state), + () => visualizationMap[visualizationId].switchVisualizationType!(type, state), state ); } return state; }; - const layers = Object.entries(props.framePublicAPI.datasourceLayers); + const layers = Object.entries(framePublicAPI.datasourceLayers); const containsData = layers.some( ([_layerId, datasource]) => datasource && datasource.getTableSpec().length > 0 ); @@ -189,14 +161,16 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { visualizationId, subVisualizationId, dataLoss: 'nothing', - keptLayerIds: Object.keys(props.framePublicAPI.datasourceLayers), + keptLayerIds: Object.keys(framePublicAPI.datasourceLayers), getVisualizationState: () => switchVisType(subVisualizationId, visualization.state), sameDatasources: true, }; } const topSuggestion = getTopSuggestion( - props, + visualizationMap, + datasourceMap, + framePublicAPI, visualizationId, datasourceStates, visualization, @@ -245,11 +219,8 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { newVisualization.initialize( addNewLayer, visualization.activeId === newVisualization.id ? visualization.state : undefined, - visualization.activeId && - props.visualizationMap[visualization.activeId].getMainPalette - ? props.visualizationMap[visualization.activeId].getMainPalette!( - visualization.state - ) + visualization.activeId && visualizationMap[visualization.activeId].getMainPalette + ? visualizationMap[visualization.activeId].getMainPalette!(visualization.state) : undefined ) ), @@ -268,11 +239,8 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { return { visualizationTypes: [], visualizationsLookup: {} }; } const subVisualizationId = - visualization.activeId && props.visualizationMap[visualization.activeId] - ? getCurrentVisualizationId( - props.visualizationMap[visualization.activeId], - visualization.state - ) + visualization.activeId && visualizationMap[visualization.activeId] + ? getCurrentVisualizationId(visualizationMap[visualization.activeId], visualization.state) : undefined; const lowercasedSearchTerm = searchTerm.toLowerCase(); // reorganize visualizations in groups @@ -296,7 +264,7 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { selection: VisualizationSelection; } > = {}; - Object.entries(props.visualizationMap).forEach(([visualizationId, v]) => { + Object.entries(visualizationMap).forEach(([visualizationId, v]) => { for (const visualizationType of v.visualizationTypes) { const isSearchMatch = visualizationType.label.toLowerCase().includes(lowercasedSearchTerm) || @@ -408,14 +376,18 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { // eslint-disable-next-line react-hooks/exhaustive-deps [ flyoutOpen, - props.visualizationMap, - props.framePublicAPI, + visualizationMap, + framePublicAPI, visualization.activeId, visualization.state, searchTerm, ] ); + const { icon, label } = visualizationMap[visualization.activeId || ''].getDescription( + visualization.state + ); + return (
setFlyoutOpen(!flyoutOpen)} - data-test-subj="lnsChartSwitchPopover" - fontWeight="bold" - label={ - - } /> } isOpen={flyoutOpen} @@ -494,7 +462,9 @@ export const ChartSwitch = memo(function ChartSwitch(props: ChartSwitchProps) { }); function getTopSuggestion( - props: ChartSwitchProps, + visualizationMap: VisualizationMap, + datasourceMap: DatasourceMap, + framePublicAPI: FramePublicAPI, visualizationId: string, datasourceStates: DatasourceStates, visualization: VisualizationState, @@ -503,23 +473,23 @@ function getTopSuggestion( ): Suggestion | undefined { const mainPalette = visualization.activeId && - props.visualizationMap[visualization.activeId] && - props.visualizationMap[visualization.activeId].getMainPalette - ? props.visualizationMap[visualization.activeId].getMainPalette!(visualization.state) + visualizationMap[visualization.activeId] && + visualizationMap[visualization.activeId].getMainPalette + ? visualizationMap[visualization.activeId].getMainPalette!(visualization.state) : undefined; const unfilteredSuggestions = getSuggestions({ - datasourceMap: props.datasourceMap, + datasourceMap, datasourceStates, visualizationMap: { [visualizationId]: newVisualization }, activeVisualization: visualization.activeId - ? props.visualizationMap[visualization.activeId] + ? visualizationMap[visualization.activeId] : undefined, visualizationState: visualization.state, subVisualizationId, - activeData: props.framePublicAPI.activeData, + activeData: framePublicAPI.activeData, mainPalette, - dataViews: props.framePublicAPI.dataViews, + dataViews: framePublicAPI.dataViews, }); const suggestions = unfilteredSuggestions.filter((suggestion) => { // don't use extended versions of current data table on switching between visualizations diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.tsx index 6fe8508d8630..00d5c79b6484 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx @@ -885,7 +885,7 @@ export class Embeddable navigateToLensEditor={ !this.isTextBasedLanguage() ? this.navigateToLensEditor.bind(this) : undefined } - displayFlyoutHeader={true} + displayFlyoutHeader canEditTextBasedQuery={this.isTextBasedLanguage()} isNewPanel={isNewPanel} deletePanel={deletePanel} diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index ee1fce5bdf1c..9e48aff73d26 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -442,7 +442,7 @@ export interface Datasource { ) => Array>; getDatasourceSuggestionsFromCurrentState: ( state: T, - indexPatterns: IndexPatternMap, + indexPatterns?: IndexPatternMap, filterFn?: (layerId: string) => boolean, activeData?: Record ) => Array>; diff --git a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts index 7e00322fedcd..a9ff9851ee4b 100644 --- a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts +++ b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts @@ -40,11 +40,6 @@ export function getSuggestions({ return []; } - // do not return the legacy metric vis for the textbased mode (i.e. ES|QL) - if (datasourceId === 'textBased') { - return []; - } - return [getSuggestion(table)]; } diff --git a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx index 30ffc3a32b1f..633d6a3ed280 100644 --- a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx +++ b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx @@ -11,14 +11,11 @@ import { EuiIcon, EuiPopover, EuiSelectable, - EuiText, EuiPopoverTitle, useEuiTheme, EuiIconTip, - EuiFlexGroup, - EuiFlexItem, } from '@elastic/eui'; -import { ToolbarButton } from '@kbn/shared-ux-button-toolbar'; +import { ChartSwitchTrigger } from '@kbn/visualization-ui-components'; import { IconChartBarReferenceLine, IconChartBarAnnotations } from '@kbn/chart-icons'; import { euiThemeVars } from '@kbn/ui-theme'; import { css } from '@emotion/react'; @@ -26,7 +23,6 @@ import { getIgnoreGlobalFilterIcon } from '../../../shared_components/ignore_glo import type { VisualizationLayerHeaderContentProps, VisualizationLayerWidgetProps, - VisualizationType, } from '../../../types'; import { State, visualizationTypes, SeriesType, XYAnnotationLayerConfig } from '../types'; import { @@ -171,9 +167,11 @@ function DataLayerHeader(props: VisualizationLayerWidgetProps) { return ( setPopoverIsOpen(!isPopoverOpen)} - currentVisType={currentVisType} /> } isOpen={isPopoverOpen} @@ -225,33 +223,3 @@ function DataLayerHeader(props: VisualizationLayerWidgetProps) { ); } - -const DataLayerHeaderTrigger = function ({ - currentVisType, - onClick, -}: { - currentVisType: VisualizationType; - onClick: () => void; -}) { - return ( - - - - - - - {currentVisType.fullLabel || currentVisType.label} - - - - } - /> - ); -}; From 3511e6bcec16361d3fd4f6513c5d33dd587b70a2 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Tue, 27 Feb 2024 13:15:32 +0100 Subject: [PATCH 02/13] colorFullBackground case --- .../__snapshots__/metric_vis_function.test.ts.snap | 4 ---- .../expression_functions/metric_vis_function.test.ts | 8 ++++---- .../common/expression_functions/metric_vis_function.ts | 7 ++----- .../editor_frame/workspace_panel/chart_switch.tsx | 10 +++++++--- .../legacy_metric/metric_suggestions.test.ts | 4 ++-- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/__snapshots__/metric_vis_function.test.ts.snap b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/__snapshots__/metric_vis_function.test.ts.snap index defaca87fad1..13f841aeb0af 100644 --- a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/__snapshots__/metric_vis_function.test.ts.snap +++ b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/__snapshots__/metric_vis_function.test.ts.snap @@ -104,7 +104,3 @@ Object { `; exports[`interpreter/functions#metric returns error if bucket and colorFullBackground specified 1`] = `"Full background coloring cannot be applied to visualizations that have a bucket specified."`; - -exports[`interpreter/functions#metric returns error if data includes several rows and colorFullBackground specified 1`] = `"Full background coloring cannot be applied to a visualization with multiple metrics."`; - -exports[`interpreter/functions#metric returns error if several metrics and colorFullBackground specified 1`] = `"Full background coloring cannot be applied to a visualization with multiple metrics."`; diff --git a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.test.ts b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.test.ts index ed5096452405..1de3a0bd804b 100644 --- a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.test.ts +++ b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.test.ts @@ -94,7 +94,7 @@ describe('interpreter/functions#metric', () => { expect(() => fn(context, args)).toThrowErrorMatchingSnapshot(); }); - it('returns error if several metrics and colorFullBackground specified', () => { + it('ignores colorFullBackground setting if several metrics are specified', () => { args.colorFullBackground = true; args.metric.push({ type: 'vis_dimension', @@ -105,13 +105,13 @@ describe('interpreter/functions#metric', () => { }, }); - expect(() => fn(context, args)).toThrowErrorMatchingSnapshot(); + expect(fn(context, args).value.visConfig.metric.colorFullBackground).toBeFalsy(); }); - it('returns error if data includes several rows and colorFullBackground specified', () => { + it('ignores colorFullBackground if data includes several rows', () => { args.colorFullBackground = true; context.rows.push({ 'col-0-1': 0 }); - expect(() => fn(context, args)).toThrowErrorMatchingSnapshot(); + expect(fn(context, args).value.visConfig.metric.colorFullBackground).toBeFalsy(); }); }); diff --git a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts index b66e43a94818..2599c9be8681 100644 --- a/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts +++ b/src/plugins/chart_expressions/expression_legacy_metric/common/expression_functions/metric_vis_function.ts @@ -143,10 +143,6 @@ export const metricVisFunction = (): MetricVisExpressionFunctionDefinition => ({ if (args.bucket) { throw new Error(errors.splitByBucketAndColorFullBackgroundSpecifiedError()); } - - if (args.metric.length > 1 || input.rows.length > 1) { - // throw new Error(errors.severalMetricsAndColorFullBackgroundSpecifiedError()); - } } args.metric.forEach((metric) => validateAccessor(metric, input.columns)); @@ -197,7 +193,8 @@ export const metricVisFunction = (): MetricVisExpressionFunctionDefinition => ({ ...args.labelFont, }, }, - colorFullBackground: args.colorFullBackground, + colorFullBackground: + args.metric.length > 1 || input.rows.length > 1 ? false : args.colorFullBackground, style: { bgColor: args.colorMode === ColorMode.Background, labelColor: args.colorMode === ColorMode.Labels, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx index a65b01040f06..137144f00e0d 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx @@ -384,9 +384,13 @@ export const ChartSwitch = memo(function ChartSwitch({ ] ); - const { icon, label } = visualizationMap[visualization.activeId || ''].getDescription( - visualization.state - ); + const { icon, label } = (visualization.activeId && + visualizationMap[visualization.activeId]?.getDescription(visualization.state)) || { + label: i18n.translate('xpack.lens.configPanel.selectVisualization', { + defaultMessage: 'Select a visualization', + }), + icon: undefined, + }; return (
diff --git a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts index e4b4dc187945..40b95058e823 100644 --- a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts +++ b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts @@ -135,7 +135,7 @@ describe('metric_suggestions', () => { expect(suggestion).toHaveLength(0); }); - test('does not suggest for text based languages', () => { + test('suggests for text based languages', () => { const col = { columnId: 'id', operation: { @@ -155,7 +155,7 @@ describe('metric_suggestions', () => { datasourceId: 'textBased', }); - expect(suggestion).toHaveLength(0); + expect(suggestion).toHaveLength(1); }); test('suggests a basic metric chart', () => { const [suggestion, ...rest] = getSuggestions({ From 405d57af94bae08167c0f04c12983da808de7cf6 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 7 Mar 2024 13:14:52 +0100 Subject: [PATCH 03/13] Marco's CR --- x-pack/plugins/lens/public/datasources/text_based/utils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/datasources/text_based/utils.ts b/x-pack/plugins/lens/public/datasources/text_based/utils.ts index 57cabae29e70..a9555713f8ed 100644 --- a/x-pack/plugins/lens/public/datasources/text_based/utils.ts +++ b/x-pack/plugins/lens/public/datasources/text_based/utils.ts @@ -144,8 +144,7 @@ export function getIndexPatternFromTextBasedQuery(query: AggregateQuery): string export const isNumeric = (column: TextBasedLayerColumn | DatatableColumn) => column?.meta?.type === 'number'; -export const isNotNumeric = (column: TextBasedLayerColumn | DatatableColumn) => - column?.meta?.type !== 'number'; +export const isNotNumeric = (column: TextBasedLayerColumn | DatatableColumn) => !isNumeric(column); export function canColumnBeDroppedInMetricDimension( columns: TextBasedLayerColumn[] | DatatableColumn[], From ef4f1716e21c10eb79e7ad4dff27b18bb2a0b10f Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 7 Mar 2024 13:18:49 +0100 Subject: [PATCH 04/13] Stratoula CR --- .../visualizations/legacy_metric/metric_suggestions.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts index a9ff9851ee4b..9dafb34364b8 100644 --- a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts +++ b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts @@ -40,16 +40,19 @@ export function getSuggestions({ return []; } - return [getSuggestion(table)]; + return [getSuggestion(table, datasourceId)]; } -function getSuggestion(table: TableSuggestion): VisualizationSuggestion { +function getSuggestion( + table: TableSuggestion, + datasourceId?: string +): VisualizationSuggestion { const col = table.columns[0]; const title = table.label || col.operation.label; return { title, - score: 0.1, + score: datasourceId === 'textBased' ? 0 : 0.1, previewIcon: IconChartMetric, state: { layerId: table.layerId, From acb269e744738989c2c15c7bd565770db5e1664b Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 7 Mar 2024 13:28:29 +0100 Subject: [PATCH 05/13] remove multi layers --- .../shared/edit_on_the_fly/lens_configuration_flyout.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx index fae8bd929077..bf5cf347b6fc 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx @@ -500,7 +500,8 @@ export function LensEditConfigurationFlyout({ > <> Date: Thu, 7 Mar 2024 13:45:49 +0100 Subject: [PATCH 06/13] copy + padding --- .../src/text_based_languages_editor.styles.ts | 1 - .../components/chart_switch_trigger.tsx | 3 ++- .../shared/edit_on_the_fly/lens_configuration_flyout.tsx | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/kbn-text-based-editor/src/text_based_languages_editor.styles.ts b/packages/kbn-text-based-editor/src/text_based_languages_editor.styles.ts index 706620d69db9..44874074e846 100644 --- a/packages/kbn-text-based-editor/src/text_based_languages_editor.styles.ts +++ b/packages/kbn-text-based-editor/src/text_based_languages_editor.styles.ts @@ -68,7 +68,6 @@ export const textBasedLanguagedEditorStyles = ( transform: 'translate(0, -50%)', }, bottomContainer: { - border: euiTheme.border.thin, borderLeft: editorIsInline ? 'none' : euiTheme.border.thin, borderRight: editorIsInline ? 'none' : euiTheme.border.thin, borderTop: diff --git a/packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx b/packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx index 22b47df0a454..183d1ee48574 100644 --- a/packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx +++ b/packages/kbn-visualization-ui-components/components/chart_switch_trigger.tsx @@ -9,6 +9,7 @@ import React from 'react'; import { EuiIcon, EuiText, EuiFlexGroup, EuiFlexItem, IconType } from '@elastic/eui'; import { ToolbarButton } from '@kbn/shared-ux-button-toolbar'; +import { euiThemeVars } from '@kbn/ui-theme'; import { css } from '@emotion/react'; export const ChartSwitchTrigger = function ({ @@ -67,7 +68,7 @@ const LayerChartSwitchLabel = function ({ label, icon }: { label: string; icon?: css={css` font-weight: 600; display: inline; - padding-left: 8px; + padding-left: ${euiThemeVars.euiSizeS}; `} > {label} diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx index bf5cf347b6fc..f7fb64a64e92 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx @@ -461,6 +461,7 @@ export function LensEditConfigurationFlyout({
- {i18n.translate('xpack.lens.config.chartConfigurationLabel', { - defaultMessage: 'Chart configuration', + {i18n.translate('xpack.lens.config.visualizationConfigurationLabel', { + defaultMessage: 'Visualization configuration', })}
From f419e4f00eda7e22899c49966c9e8119613c057e Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 7 Mar 2024 15:38:09 +0100 Subject: [PATCH 07/13] revert paddings --- .../shared/edit_on_the_fly/lens_configuration_flyout.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx index f7fb64a64e92..ca2fe628b454 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx @@ -413,7 +413,6 @@ export function LensEditConfigurationFlyout({ ${euiScrollBarStyles(euiTheme)} padding-left: ${euiThemeVars.euiFormMaxWidth}; margin-left: -${euiThemeVars.euiFormMaxWidth}; - padding-right: ${euiThemeVars.euiSizeXS}; .euiAccordion-isOpen & { block-size: auto !important; @@ -462,8 +461,8 @@ export function LensEditConfigurationFlyout({ grow={isLayerAccordionOpen ? 1 : false} css={css` position: relative; - padding-left: ${euiThemeVars.euiSizeS}; - padding-right: ${euiThemeVars.euiSizeS}; + padding-left: ${euiThemeVars.euiSize}; + padding-right: ${euiThemeVars.euiSize}; .euiAccordion__childWrapper { flex: ${isLayerAccordionOpen ? 1 : 'none'} } @@ -524,8 +523,8 @@ export function LensEditConfigurationFlyout({ css={css` border-top: ${euiThemeVars.euiBorderThin}; border-bottom: ${euiThemeVars.euiBorderThin}; - padding-left: ${euiThemeVars.euiSizeS}; - padding-right: ${euiThemeVars.euiSizeS}; + padding-left: ${euiThemeVars.euiSize}; + padding-right: ${euiThemeVars.euiSize}; .euiAccordion__childWrapper { flex: ${isSuggestionsAccordionOpen ? 1 : 'none'} } From 48e4a9c4abd0d7afe33db052821ff26c08346edc Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Thu, 7 Mar 2024 15:51:14 +0100 Subject: [PATCH 08/13] padding new impl --- .../layer_configuration_section.tsx | 23 ++++++++++++------- .../lens_configuration_flyout.tsx | 7 ++++-- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx index ec1b987ffff9..dae81eab843e 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/layer_configuration_section.tsx @@ -67,16 +67,23 @@ export function LayerConfiguration({ return (
- - - - +
+ + + + +
); } diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx index ca2fe628b454..4f789662869d 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx @@ -461,8 +461,6 @@ export function LensEditConfigurationFlyout({ grow={isLayerAccordionOpen ? 1 : false} css={css` position: relative; - padding-left: ${euiThemeVars.euiSize}; - padding-right: ${euiThemeVars.euiSize}; .euiAccordion__childWrapper { flex: ${isLayerAccordionOpen ? 1 : 'none'} } @@ -470,6 +468,11 @@ export function LensEditConfigurationFlyout({ `} > Date: Fri, 8 Mar 2024 11:48:09 +0100 Subject: [PATCH 09/13] fix location of subflyout --- .../shared/edit_on_the_fly/lens_configuration_flyout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx index 4f789662869d..08152e7618b2 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx @@ -460,9 +460,9 @@ export function LensEditConfigurationFlyout({ Date: Fri, 8 Mar 2024 12:32:19 +0100 Subject: [PATCH 10/13] hide legacy metric --- .../editor_frame/workspace_panel/chart_switch.tsx | 9 ++++++++- x-pack/plugins/lens/public/types.ts | 2 ++ .../visualizations/legacy_metric/metric_suggestions.ts | 3 ++- .../visualizations/legacy_metric/visualization.tsx | 7 ++++++- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx index 137144f00e0d..c9ae39160e39 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx @@ -95,7 +95,7 @@ function getCurrentVisualizationId( export const ChartSwitch = memo(function ChartSwitch({ framePublicAPI, - visualizationMap, + visualizationMap: completeVisualizationMap, datasourceMap, size = 'm', }: ChartSwitchProps) { @@ -105,6 +105,13 @@ export const ChartSwitch = memo(function ChartSwitch({ const visualization = useLensSelector(selectVisualization); const datasourceStates = useLensSelector(selectDatasourceStates); + const visualizationMap = { ...completeVisualizationMap }; + Object.keys(visualizationMap).forEach((key) => { + if (completeVisualizationMap[key]?.hideFromChartSwitch?.(framePublicAPI)) { + delete visualizationMap[key]; + } + }); + const commitSelection = (selection: VisualizationSelection) => { setFlyoutOpen(false); diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 9e48aff73d26..e2419c09dfef 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -1071,6 +1071,8 @@ export interface Visualization string; + + hideFromChartSwitch?: (frame: FramePublicAPI) => boolean; /** * If the visualization has subtypes, update the subtype in state. */ diff --git a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts index 9dafb34364b8..93e564cde841 100644 --- a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts +++ b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.ts @@ -52,7 +52,8 @@ function getSuggestion( return { title, - score: datasourceId === 'textBased' ? 0 : 0.1, + hide: datasourceId === 'textBased', + score: 0.1, previewIcon: IconChartMetric, state: { layerId: table.layerId, diff --git a/x-pack/plugins/lens/public/visualizations/legacy_metric/visualization.tsx b/x-pack/plugins/lens/public/visualizations/legacy_metric/visualization.tsx index 2f164ba3ed5b..2cbe3449f951 100644 --- a/x-pack/plugins/lens/public/visualizations/legacy_metric/visualization.tsx +++ b/x-pack/plugins/lens/public/visualizations/legacy_metric/visualization.tsx @@ -23,7 +23,7 @@ import { import { ExpressionFunctionVisDimension } from '@kbn/visualizations-plugin/common'; import type { MetricVisExpressionFunctionDefinition } from '@kbn/expression-legacy-metric-vis-plugin/common'; import { getSuggestions } from './metric_suggestions'; -import { Visualization, OperationMetadata, DatasourceLayers } from '../../types'; +import { Visualization, OperationMetadata, DatasourceLayers, FramePublicAPI } from '../../types'; import type { LegacyMetricState } from '../../../common/types'; import { MetricDimensionEditor } from './dimension_editor'; import { MetricToolbar } from './metric_config_panel'; @@ -169,6 +169,11 @@ export const getLegacyMetricVisualization = ({ }), }, ], + hideFromChartSwitch(frame: FramePublicAPI) { + return Object.values(frame.datasourceLayers).some( + (datasource) => datasource && datasource.datasourceId === 'textBased' + ); + }, getVisualizationTypeId() { return 'lnsLegacyMetric'; From 16abff50d3430fc7c4482588e89a7e1adcb8b7a7 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Fri, 8 Mar 2024 14:30:52 +0100 Subject: [PATCH 11/13] test --- .../editor_frame/workspace_panel/chart_switch.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx index a82da848dc4a..8284f4ad5ff9 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx @@ -64,6 +64,7 @@ describe('chart_switch', () => { * - Never switches to subvisC2 * - Allows a switch to subvisC3 * - Allows a switch to subvisC1 + * visD: is not visible because hideFromChartSwitch returns true */ function mockVisualizationMap() { return { @@ -114,6 +115,10 @@ describe('chart_switch', () => { ]; }), }, + visD: { + ...generateVisualization('visD'), + hideFromChartSwitch: () => true, + }, }; } @@ -545,7 +550,7 @@ describe('chart_switch', () => { }); }); - it('should show all visualization types', async () => { + it('should show all visualization types and subtypes except from one that are hidden (D)', async () => { const { openChartSwitch, getMenuItem } = renderChartSwitch(); openChartSwitch(); const allDisplayed = ['visA', 'visB', 'subvisC1', 'subvisC2', 'subvisC3'].every((subType) => From ff103c9aad43e112ea771ccdf68ea830b282d4da Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Fri, 8 Mar 2024 16:36:37 +0100 Subject: [PATCH 12/13] test --- .../visualizations/legacy_metric/metric_suggestions.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts index 40b95058e823..794c8cedf516 100644 --- a/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts +++ b/x-pack/plugins/lens/public/visualizations/legacy_metric/metric_suggestions.test.ts @@ -171,6 +171,7 @@ describe('metric_suggestions', () => { expect(rest).toHaveLength(0); expect(suggestion).toMatchInlineSnapshot(` Object { + "hide": false, "previewIcon": [Function], "score": 0.1, "state": Object { @@ -206,6 +207,7 @@ describe('metric_suggestions', () => { expect(rest).toHaveLength(0); expect(suggestion).toMatchInlineSnapshot(` Object { + "hide": false, "previewIcon": [Function], "score": 0.1, "state": Object { From b6ba485f23a51c526a188d8b2383402a636456e0 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Fri, 8 Mar 2024 17:11:26 +0100 Subject: [PATCH 13/13] revert jumpy accordion bad solution --- .../shared/edit_on_the_fly/lens_configuration_flyout.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx index 08152e7618b2..73c3797f3e7b 100644 --- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx +++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx @@ -462,7 +462,6 @@ export function LensEditConfigurationFlyout({ css={css` .euiAccordion__childWrapper { flex: ${isLayerAccordionOpen ? 1 : 'none'} - position: relative; } } `}