From 7ffafe993de04bad2eeb66e3e260b8645b2f2988 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Mon, 6 Dec 2021 14:53:17 +0300 Subject: [PATCH 1/3] [Lens] Mosaic - add show values in legend configuration property for Waffle chart type --- .../common/expressions/pie_chart/pie_chart.ts | 4 + .../common/expressions/pie_chart/types.ts | 1 + .../pie_visualization/render_function.tsx | 3 +- .../pie_visualization/render_helpers.ts | 8 +- .../public/pie_visualization/to_expression.ts | 10 +- .../lens/public/pie_visualization/toolbar.tsx | 129 ++++++++++-------- 6 files changed, 95 insertions(+), 60 deletions(-) diff --git a/x-pack/plugins/lens/common/expressions/pie_chart/pie_chart.ts b/x-pack/plugins/lens/common/expressions/pie_chart/pie_chart.ts index 053b46e480c7b..4f2736d739b11 100644 --- a/x-pack/plugins/lens/common/expressions/pie_chart/pie_chart.ts +++ b/x-pack/plugins/lens/common/expressions/pie_chart/pie_chart.ts @@ -83,6 +83,10 @@ export const pie: ExpressionFunctionDefinition< types: ['boolean'], help: '', }, + showValuesInLegend: { + types: ['boolean'], + help: '', + }, legendPosition: { types: ['string'], options: [Position.Top, Position.Right, Position.Bottom, Position.Left], diff --git a/x-pack/plugins/lens/common/expressions/pie_chart/types.ts b/x-pack/plugins/lens/common/expressions/pie_chart/types.ts index 8c9ec4e5a54e7..a7aa92369dce2 100644 --- a/x-pack/plugins/lens/common/expressions/pie_chart/types.ts +++ b/x-pack/plugins/lens/common/expressions/pie_chart/types.ts @@ -17,6 +17,7 @@ export interface SharedPieLayerState { categoryDisplay: 'default' | 'inside' | 'hide'; legendDisplay: 'default' | 'show' | 'hide'; legendPosition?: 'left' | 'right' | 'top' | 'bottom'; + showValuesInLegend?: boolean; nestedLegend?: boolean; percentDecimals?: number; legendMaxLines?: number; diff --git a/x-pack/plugins/lens/public/pie_visualization/render_function.tsx b/x-pack/plugins/lens/public/pie_visualization/render_function.tsx index 3b9fdaf094822..559a3cfc48164 100644 --- a/x-pack/plugins/lens/public/pie_visualization/render_function.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/render_function.tsx @@ -86,6 +86,7 @@ export function PieComponent( truncateLegend, hideLabels, palette, + showValuesInLegend, } = props.args; const chartTheme = chartsThemeService.useChartsTheme(); const chartBaseTheme = chartsThemeService.useChartsBaseTheme(); @@ -315,7 +316,7 @@ export function PieComponent( (legend.getShowLegendDefault?.(bucketColumns) ?? false))) } flatLegend={legend.flat} - showLegendExtra={legend.showValues} + showLegendExtra={showValuesInLegend} legendPosition={legendPosition || Position.Right} legendMaxDepth={nestedLegend ? undefined : 1 /* Color is based only on first layer */} onElementClick={props.interactive ?? true ? onElementClickHandler : undefined} diff --git a/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts b/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts index fa20eb6f20fa8..784cccd259d45 100644 --- a/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts +++ b/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts @@ -8,8 +8,9 @@ import type { Datum, LayerValue } from '@elastic/charts'; import type { Datatable, DatatableColumn } from 'src/plugins/expressions/public'; import type { LensFilterEvent } from '../types'; -import type { PieChartTypes } from '../../common/expressions/pie_chart/types'; +import type { PieChartTypes, PieLayerState } from '../../common/expressions/pie_chart/types'; import type { PaletteDefinition, PaletteOutput } from '../../../../../src/plugins/charts/public'; +import { PartitionChartsMeta } from './partition_charts_meta'; export function getSliceValue(d: Datum, metricColumn: DatatableColumn) { const value = d[metricColumn.id]; @@ -44,6 +45,11 @@ export const isPartitionShape = (shape: PieChartTypes | string) => export const isTreemapOrMosaicShape = (shape: PieChartTypes | string) => ['treemap', 'mosaic'].includes(shape); +export const shouldShowValuesInLegend = (layer: PieLayerState, shape: PieChartTypes) => + 'showValuesInLegend' in layer + ? layer.showValuesInLegend! + : Boolean(PartitionChartsMeta[shape]?.legend?.showValues); + export const extractUniqTermsMap = (dataTable: Datatable, columnId: string) => [...new Set(dataTable.rows.map((item) => item[columnId]))].reduce( (acc, item, index) => ({ diff --git a/x-pack/plugins/lens/public/pie_visualization/to_expression.ts b/x-pack/plugins/lens/public/pie_visualization/to_expression.ts index e13fbf62708ee..57270337e67a4 100644 --- a/x-pack/plugins/lens/public/pie_visualization/to_expression.ts +++ b/x-pack/plugins/lens/public/pie_visualization/to_expression.ts @@ -5,10 +5,12 @@ * 2.0. */ -import { Ast } from '@kbn/interpreter/common'; -import { PaletteRegistry } from 'src/plugins/charts/public'; -import { Operation, DatasourcePublicAPI } from '../types'; +import type { Ast } from '@kbn/interpreter/common'; +import type { PaletteRegistry } from 'src/plugins/charts/public'; +import type { Operation, DatasourcePublicAPI } from '../types'; import { DEFAULT_PERCENT_DECIMALS } from './constants'; +import { shouldShowValuesInLegend } from './render_helpers'; + import type { PieVisualizationState } from '../../common/expressions'; export function toExpression( @@ -34,6 +36,7 @@ function expressionHelper( const operations = layer.groups .map((columnId) => ({ columnId, operation: datasource.getOperationForColumnId(columnId) })) .filter((o): o is { columnId: string; operation: Operation } => !!o.operation); + if (!layer.metric || !operations.length) { return null; } @@ -55,6 +58,7 @@ function expressionHelper( categoryDisplay: [layer.categoryDisplay], legendDisplay: [layer.legendDisplay], legendPosition: [layer.legendPosition || 'right'], + showValuesInLegend: [shouldShowValuesInLegend(layer, state.shape)], percentDecimals: [ state.shape === 'waffle' ? DEFAULT_PERCENT_DECIMALS diff --git a/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx b/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx index 195a72cca9fed..21a81ab7e7f59 100644 --- a/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx @@ -6,7 +6,7 @@ */ import './toolbar.scss'; -import React from 'react'; +import React, { useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiFlexGroup, @@ -23,6 +23,7 @@ import type { PieVisualizationState, SharedPieLayerState } from '../../common/ex import { VisualizationDimensionEditorProps, VisualizationToolbarProps } from '../types'; import { ToolbarPopover, LegendSettingsPopover, useDebouncedValue } from '../shared_components'; import { PalettePicker } from '../shared_components'; +import { shouldShowValuesInLegend } from './render_helpers'; const legendOptions: Array<{ value: SharedPieLayerState['legendDisplay']; @@ -55,6 +56,67 @@ const legendOptions: Array<{ export function PieToolbar(props: VisualizationToolbarProps) { const { state, setState } = props; const layer = state.layers[0]; + + const onStateChange = useCallback( + (part: Record) => { + setState({ + ...state, + layers: [{ ...layer, ...part }], + }); + }, + [layer, state, setState] + ); + + const onCategoryDisplayChange = useCallback( + (option) => onStateChange({ categoryDisplay: option }), + [onStateChange] + ); + + const onNumberDisplayChange = useCallback( + (option) => onStateChange({ numberDisplay: option }), + [onStateChange] + ); + + const onPercentDecimalsChange = useCallback( + (option) => { + onStateChange({ percentDecimals: option }); + }, + [onStateChange] + ); + + const onLegendDisplayChange = useCallback( + (optionId) => { + onStateChange({ legendDisplay: legendOptions.find(({ id }) => id === optionId)!.value }); + }, + [onStateChange] + ); + + const onLegendPositionChange = useCallback( + (id) => onStateChange({ legendPosition: id as Position }), + [onStateChange] + ); + + const onNestedLegendChange = useCallback( + (id) => onStateChange({ nestedLegend: !layer.nestedLegend }), + [layer, onStateChange] + ); + + const onTruncateLegendChange = useCallback(() => { + const current = layer.truncateLegend ?? true; + onStateChange({ truncateLegend: !current }); + }, [layer, onStateChange]); + + const onLegendMaxLinesChange = useCallback( + (val) => onStateChange({ legendMaxLines: val }), + [onStateChange] + ); + + const onValueInLegendChange = useCallback(() => { + onStateChange({ + showValuesInLegend: !shouldShowValuesInLegend(layer, state.shape), + }); + }, [layer, state.shape, onStateChange]); + if (!layer) { return null; } @@ -87,12 +149,7 @@ export function PieToolbar(props: VisualizationToolbarProps { - setState({ - ...state, - layers: [{ ...layer, categoryDisplay: option }], - }); - }} + onChange={onCategoryDisplayChange} /> ) : null} @@ -110,12 +167,7 @@ export function PieToolbar(props: VisualizationToolbarProps { - setState({ - ...state, - layers: [{ ...layer, numberDisplay: option }], - }); - }} + onChange={onNumberDisplayChange} /> ) : null} @@ -131,59 +183,26 @@ export function PieToolbar(props: VisualizationToolbarProps { - setState({ - ...state, - layers: [{ ...layer, percentDecimals: value }], - }); - }} + setValue={onPercentDecimalsChange} /> { - setState({ - ...state, - layers: [ - { - ...layer, - legendDisplay: legendOptions.find(({ id }) => id === optionId)!.value, - }, - ], - }); - }} + onDisplayChange={onLegendDisplayChange} + valueInLegend={shouldShowValuesInLegend(layer, state.shape)} + renderValueInLegendSwitch={Boolean(PartitionChartsMeta[state.shape]?.legend?.showValues)} + onValueInLegendChange={onValueInLegendChange} position={layer.legendPosition} - onPositionChange={(id) => { - setState({ - ...state, - layers: [{ ...layer, legendPosition: id as Position }], - }); - }} + onPositionChange={onLegendPositionChange} renderNestedLegendSwitch nestedLegend={!!layer.nestedLegend} - onNestedLegendChange={() => { - setState({ - ...state, - layers: [{ ...layer, nestedLegend: !layer.nestedLegend }], - }); - }} + onNestedLegendChange={onNestedLegendChange} shouldTruncate={layer.truncateLegend ?? true} - onTruncateLegendChange={() => { - const current = layer.truncateLegend ?? true; - setState({ - ...state, - layers: [{ ...layer, truncateLegend: !current }], - }); - }} + onTruncateLegendChange={onTruncateLegendChange} maxLines={layer?.legendMaxLines} - onMaxLinesChange={(val) => { - setState({ - ...state, - layers: [{ ...layer, legendMaxLines: val }], - }); - }} + onMaxLinesChange={onLegendMaxLinesChange} /> ); From bf8fba9f84db6972bdb2f37d2b1ff4f5bdfd9e3f Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Mon, 6 Dec 2021 15:23:37 +0300 Subject: [PATCH 2/3] add tests --- .../pie_visualization/render_helpers.test.ts | 26 +++++++++++++++++++ .../pie_visualization/render_helpers.ts | 4 +-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/pie_visualization/render_helpers.test.ts b/x-pack/plugins/lens/public/pie_visualization/render_helpers.test.ts index d86500ff8a4fa..bcd9d79babbab 100644 --- a/x-pack/plugins/lens/public/pie_visualization/render_helpers.test.ts +++ b/x-pack/plugins/lens/public/pie_visualization/render_helpers.test.ts @@ -14,8 +14,10 @@ import { byDataColorPaletteMap, extractUniqTermsMap, checkTableForContainsSmallValues, + shouldShowValuesInLegend, } from './render_helpers'; import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks'; +import type { PieLayerState } from '../../common/expressions'; describe('render helpers', () => { describe('#getSliceValue', () => { @@ -374,4 +376,28 @@ describe('render helpers', () => { expect(checkTableForContainsSmallValues(datatable, columnId, 1)).toBeFalsy(); }); }); + + describe('#shouldShowValuesInLegend', () => { + it('should firstly read the state value', () => { + expect( + shouldShowValuesInLegend({ showValuesInLegend: true } as PieLayerState, 'waffle') + ).toBeTruthy(); + + expect( + shouldShowValuesInLegend({ showValuesInLegend: false } as PieLayerState, 'waffle') + ).toBeFalsy(); + }); + + it('should read value from meta in case of value in state is undefined', () => { + expect( + shouldShowValuesInLegend({ showValuesInLegend: undefined } as PieLayerState, 'waffle') + ).toBeTruthy(); + + expect(shouldShowValuesInLegend({} as PieLayerState, 'waffle')).toBeTruthy(); + + expect( + shouldShowValuesInLegend({ showValuesInLegend: undefined } as PieLayerState, 'pie') + ).toBeFalsy(); + }); + }); }); diff --git a/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts b/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts index 784cccd259d45..ef8525d2a9dbd 100644 --- a/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts +++ b/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts @@ -46,8 +46,8 @@ export const isTreemapOrMosaicShape = (shape: PieChartTypes | string) => ['treemap', 'mosaic'].includes(shape); export const shouldShowValuesInLegend = (layer: PieLayerState, shape: PieChartTypes) => - 'showValuesInLegend' in layer - ? layer.showValuesInLegend! + layer.showValuesInLegend !== undefined + ? layer.showValuesInLegend : Boolean(PartitionChartsMeta[shape]?.legend?.showValues); export const extractUniqTermsMap = (dataTable: Datatable, columnId: string) => From 8a8e5ed198791ec727d7e56d48b12831655e4300 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Tue, 7 Dec 2021 14:11:46 +0300 Subject: [PATCH 3/3] resolve comments --- .../lens/public/pie_visualization/render_helpers.ts | 11 +++++++---- .../plugins/lens/public/pie_visualization/toolbar.tsx | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts b/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts index ef8525d2a9dbd..a9685e13e1774 100644 --- a/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts +++ b/x-pack/plugins/lens/public/pie_visualization/render_helpers.ts @@ -45,10 +45,13 @@ export const isPartitionShape = (shape: PieChartTypes | string) => export const isTreemapOrMosaicShape = (shape: PieChartTypes | string) => ['treemap', 'mosaic'].includes(shape); -export const shouldShowValuesInLegend = (layer: PieLayerState, shape: PieChartTypes) => - layer.showValuesInLegend !== undefined - ? layer.showValuesInLegend - : Boolean(PartitionChartsMeta[shape]?.legend?.showValues); +export const shouldShowValuesInLegend = (layer: PieLayerState, shape: PieChartTypes) => { + if ('showValues' in PartitionChartsMeta[shape]?.legend) { + return layer.showValuesInLegend ?? PartitionChartsMeta[shape]?.legend?.showValues ?? true; + } + + return false; +}; export const extractUniqTermsMap = (dataTable: Datatable, columnId: string) => [...new Set(dataTable.rows.map((item) => item[columnId]))].reduce( diff --git a/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx b/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx index 21a81ab7e7f59..70ad4d8c07daa 100644 --- a/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx @@ -192,7 +192,9 @@ export function PieToolbar(props: VisualizationToolbarProps