From 2b39b451fb40cc2517bd215e32013f07c6a6ee8e Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 1 Jun 2021 19:12:11 +0200 Subject: [PATCH 01/17] :sparkles: New summary row feature for datatable --- .../components/dimension_editor.tsx | 78 ++++++++++++++- .../components/table_basic.tsx | 21 ++++ .../datatable_visualization/expression.tsx | 9 ++ .../public/datatable_visualization/summary.ts | 96 +++++++++++++++++++ .../datatable_visualization/visualization.tsx | 11 +++ 5 files changed, 213 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/lens/public/datatable_visualization/summary.ts diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index 76c47a9c743c5..84d4694ba3b74 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiFormRow, @@ -16,10 +16,12 @@ import { EuiFlexItem, EuiFlexGroup, EuiButtonEmpty, + EuiSuperSelect, + EuiFieldText, } from '@elastic/eui'; import { PaletteRegistry } from 'src/plugins/charts/public'; import { VisualizationDimensionEditorProps } from '../../types'; -import { DatatableVisualizationState } from '../visualization'; +import { ColumnState, DatatableVisualizationState } from '../visualization'; import { getOriginalId } from '../transpose_helpers'; import { CustomizablePalette, @@ -27,14 +29,17 @@ import { defaultPaletteParams, FIXED_PROGRESSION, getStopsForFixedMode, + useDebouncedValue, } from '../../shared_components/'; import { PalettePanelContainer } from './palette_panel_container'; import { findMinMaxByColumnId } from './shared_utils'; import './dimension_editor.scss'; +import { getDefaultSummaryLabel, getSummaryRowOptions } from '../summary'; const idPrefix = htmlIdGenerator()(); type ColumnType = DatatableVisualizationState['columns'][number]; +type SummaryRowType = Extract; function updateColumnWith( state: DatatableVisualizationState, @@ -58,6 +63,23 @@ export function TableDimensionEditor( const { state, setState, frame, accessor } = props; const column = state.columns.find(({ columnId }) => accessor === columnId); const [isPaletteOpen, setIsPaletteOpen] = useState(false); + const onSummaryLabelChangeToDebounce = useCallback( + (newSummaryLabel: string | undefined) => { + setState({ + ...state, + columns: updateColumnWith(state, accessor, { summaryLabel: newSummaryLabel }), + }); + }, + [accessor, setState, state] + ); + const { + inputValue: summaryLabel, + handleInputChange: onSummaryLabelChange, + initialValue, + } = useDebouncedValue({ + onChange: onSummaryLabelChangeToDebounce, + value: column?.summaryLabel, + }); if (!column) return null; if (column.isTransposed) return null; @@ -69,9 +91,18 @@ export function TableDimensionEditor( currentData?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor) ?.meta.type === 'number'; + // check for array values + const hasFieldNumericValues = + isNumericField && + currentData?.rows.every(({ [accessor]: value }) => typeof value === 'number' || value == null); + const currentAlignment = column?.alignment || (isNumericField ? 'right' : 'left'); const currentColorMode = column?.colorMode || 'none'; const hasDynamicColoring = currentColorMode !== 'none'; + const summaryRowOptions = getSummaryRowOptions(); + // when switching from one operation to another, make sure to keep the configuration consistent + const summaryRowType = column?.summaryRow || 'none'; + const fallbackSummaryLabel = getDefaultSummaryLabel(summaryRowType); const visibleColumnsCount = state.columns.filter((c) => !c.hidden).length; @@ -175,6 +206,49 @@ export function TableDimensionEditor( /> )} + {hasFieldNumericValues && ( + <> + + + data-test-subj="lnsDatatable_summaryrow_function" + valueOfSelected={summaryRowType} + options={summaryRowOptions} + compressed + onChange={(newValue) => { + setState({ + ...state, + columns: updateColumnWith(state, accessor, { summaryRow: newValue }), + }); + }} + /> + + {summaryRowType !== 'none' && ( + + { + onSummaryLabelChange(e.target.value); + }} + placeholder={initialValue || fallbackSummaryLabel} + /> + + )} + + )} {isNumericField && ( <> { [onEditAction, sortBy, sortDirection] ); + const renderSummaryRow = useMemo(() => { + const columnsWithSummary = columnConfig.columns.filter(({ summaryRow }) => summaryRow); + + if (columnsWithSummary.length) { + const summaryLookup = Object.fromEntries( + columnsWithSummary.map(({ summaryRowValue, summaryLabel, columnId }) => [ + columnId, + `${summaryLabel}: ${summaryRowValue}`, + ]) + ); + return ({ columnId }: { columnId: string }) => { + const currentAlignment = alignments && alignments[columnId]; + const alignmentClassName = `lnsTableCell--${currentAlignment}`; + return summaryLookup[columnId] != null ? ( +
{summaryLookup[columnId]}
+ ) : null; + }; + } + }, [columnConfig.columns, alignments]); + if (isEmpty) { return ; } @@ -323,6 +343,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { sorting={sorting} onColumnResize={onColumnResize} toolbarVisibility={false} + renderFooterCellValue={renderSummaryRow} /> diff --git a/x-pack/plugins/lens/public/datatable_visualization/expression.tsx b/x-pack/plugins/lens/public/datatable_visualization/expression.tsx index 2d5f4aea98856..72aaf6b4855e9 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/expression.tsx @@ -28,10 +28,12 @@ import { ColumnState } from './visualization'; import type { FormatFactory, ILensInterpreterRenderHandlers, LensMultiTable } from '../types'; import type { DatatableRender } from './components/types'; import { transposeTable } from './transpose_helpers'; +import { computeSummaryRowForColumn } from './summary'; export type ColumnConfigArg = Omit & { type: 'lens_datatable_column'; palette?: PaletteOutput; + summaryRowValue?: unknown; }; export interface Args { @@ -116,6 +118,11 @@ export const getDatatable = ({ return memo; }, {}); + const columnsWithSummary = args.columns.filter((c) => c.summaryRow); + for (const column of columnsWithSummary) { + column.summaryRowValue = computeSummaryRowForColumn(column, firstTable, formatters); + } + if (sortBy && columnsReverseLookup[sortBy] && sortDirection !== 'none') { // Sort on raw values for these types, while use the formatted value for the rest const sortingCriteria = getSortingCriteria( @@ -173,6 +180,8 @@ export const datatableColumn: ExpressionFunctionDefinition< types: ['palette'], help: '', }, + summaryRow: { types: ['string'], help: '' }, + summaryLabel: { types: ['string'], help: '' }, }, fn: function fn(input: unknown, args: ColumnState) { return { diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.ts new file mode 100644 index 0000000000000..6f83c586a694e --- /dev/null +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.ts @@ -0,0 +1,96 @@ +/* + * 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 { i18n } from '@kbn/i18n'; +import { FieldFormat } from 'src/plugins/data/public'; +import { Datatable } from 'src/plugins/expressions/public'; +import { ColumnConfigArg } from './datatable_visualization'; +import { getOriginalId } from './transpose_helpers'; + +type SummaryRowType = Extract; + +export function getDefaultSummaryLabel(type: SummaryRowType) { + return getSummaryRowOptions().find(({ value }) => type === value)!.inputDisplay!; +} + +export function getSummaryRowOptions(): Array<{ value: SummaryRowType; inputDisplay: string }> { + return [ + { + value: 'none', + inputDisplay: i18n.translate('xpack.lens.table.summaryRow.none', { + defaultMessage: 'None', + }), + }, + { + value: 'count', + inputDisplay: i18n.translate('xpack.lens.table.summaryRow.count', { + defaultMessage: 'Count', + }), + }, + { + value: 'sum', + inputDisplay: i18n.translate('xpack.lens.table.summaryRow.sum', { + defaultMessage: 'Sum', + }), + }, + { + value: 'avg', + inputDisplay: i18n.translate('xpack.lens.table.summaryRow.average', { + defaultMessage: 'Average', + }), + }, + { + value: 'min', + inputDisplay: i18n.translate('xpack.lens.table.summaryRow.minimum', { + defaultMessage: 'Minimum', + }), + }, + { + value: 'max', + inputDisplay: i18n.translate('xpack.lens.table.summaryRow.maximum', { + defaultMessage: 'Maximum', + }), + }, + ]; +} + +export function computeSummaryRowForColumn( + columnArgs: ColumnConfigArg, + table: Datatable, + formatters: Record +) { + const summaryValue = computeFinalValue(columnArgs.summaryRow, columnArgs.columnId, table.rows); + // ignore the coluymn formatter for the count case + if (columnArgs.summaryRow === 'count') { + return summaryValue; + } + return formatters[getOriginalId(columnArgs.columnId)].convert(summaryValue); +} + +function computeFinalValue( + type: ColumnConfigArg['summaryRow'], + columnId: string, + rows: Datatable['rows'] +) { + const validRows = rows.filter((v) => v[columnId] != null); + const count = validRows.length; + const sum = validRows.reduce((partialSum, v) => partialSum + (v[columnId] as number), 0); + switch (type) { + case 'sum': + return sum; + case 'count': + return count; + case 'avg': + return sum / count; + case 'min': + return Math.min(...validRows.map((v) => v[columnId])); + case 'max': + return Math.max(...validRows.map((v) => v[columnId])); + default: + throw Error('No summary function found'); + } +} diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index efde4160019e7..4a4c62b2169ff 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -23,6 +23,7 @@ import { TableDimensionEditor } from './components/dimension_editor'; import { CUSTOM_PALETTE } from '../shared_components/coloring/constants'; import { CustomPaletteParams } from '../shared_components/coloring/types'; import { getStopsForFixedMode } from '../shared_components'; +import { getDefaultSummaryLabel } from './summary'; export interface ColumnState { columnId: string; @@ -38,6 +39,8 @@ export interface ColumnState { alignment?: 'left' | 'right' | 'center'; palette?: PaletteOutput; colorMode?: 'none' | 'cell' | 'text'; + summaryRow?: 'none' | 'sum' | 'avg' | 'count' | 'min' | 'max'; + summaryLabel?: string; } export interface SortingState { @@ -376,6 +379,14 @@ export const getDatatableVisualization = ({ alignment: typeof column.alignment === 'undefined' ? [] : [column.alignment], colorMode: [column.colorMode ?? 'none'], palette: [paletteService.get(CUSTOM_PALETTE).toExpression(paletteParams)], + summaryRow: + column.summaryRow == null || column.summaryRow === 'none' + ? [] + : [column.summaryRow], + summaryLabel: + column.summaryRow == null || column.summaryRow === 'none' + ? [] + : [column.summaryLabel || getDefaultSummaryLabel(column.summaryRow)], }, }, ], From 0ec6e9c2444da6444067472b2eb95b4fbae00f74 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 3 Jun 2021 14:19:00 +0200 Subject: [PATCH 02/17] :sparkles: Allow empty strings behind flag + tests --- .../shared_components/debounced_value.test.ts | 55 +++++++++++++++++++ .../shared_components/debounced_value.ts | 23 +++++--- 2 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/lens/public/shared_components/debounced_value.test.ts diff --git a/x-pack/plugins/lens/public/shared_components/debounced_value.test.ts b/x-pack/plugins/lens/public/shared_components/debounced_value.test.ts new file mode 100644 index 0000000000000..7aa93fcad95e9 --- /dev/null +++ b/x-pack/plugins/lens/public/shared_components/debounced_value.test.ts @@ -0,0 +1,55 @@ +/* + * 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 { act, renderHook } from '@testing-library/react-hooks'; +import { useDebouncedValue } from './debounced_value'; + +jest.mock('lodash', () => { + const original = jest.requireActual('lodash'); + + return { + ...original, + debounce: (fn: unknown) => fn, + }; +}); + +describe('useDebouncedValue', () => { + it('should update upstream value changes', () => { + const onChangeMock = jest.fn(); + const { result } = renderHook(() => useDebouncedValue({ value: 'a', onChange: onChangeMock })); + + act(() => { + result.current.handleInputChange('b'); + }); + + expect(onChangeMock).toHaveBeenCalledWith('b'); + }); + + it('should fallback to initial value with empty string (by default)', () => { + const onChangeMock = jest.fn(); + const { result } = renderHook(() => useDebouncedValue({ value: 'a', onChange: onChangeMock })); + + act(() => { + result.current.handleInputChange(''); + }); + + expect(onChangeMock).toHaveBeenCalledWith('a'); + }); + + it('should allow empty input to be updated', () => { + const onChangeMock = jest.fn(); + const { result } = renderHook(() => + useDebouncedValue({ value: 'a', onChange: onChangeMock }, { allowEmptyString: true }) + ); + + act(() => { + result.current.handleInputChange(''); + }); + + expect(onChangeMock).toHaveBeenCalledWith(''); + }); +}); diff --git a/x-pack/plugins/lens/public/shared_components/debounced_value.ts b/x-pack/plugins/lens/public/shared_components/debounced_value.ts index 1f8ba0fa765b2..5525f6b16b316 100644 --- a/x-pack/plugins/lens/public/shared_components/debounced_value.ts +++ b/x-pack/plugins/lens/public/shared_components/debounced_value.ts @@ -13,15 +13,19 @@ import { debounce } from 'lodash'; * are in flight because the user is currently modifying the value. */ -export const useDebouncedValue = ({ - onChange, - value, -}: { - onChange: (val: T) => void; - value: T; -}) => { +export const useDebouncedValue = ( + { + onChange, + value, + }: { + onChange: (val: T) => void; + value: T; + }, + { allowEmptyString }: { allowEmptyString?: boolean } = {} +) => { const [inputValue, setInputValue] = useState(value); const unflushedChanges = useRef(false); + const shouldUpdateWithEmptyString = Boolean(allowEmptyString); // Save the initial value const initialValue = useRef(value); @@ -45,7 +49,10 @@ export const useDebouncedValue = ({ const handleInputChange = (val: T) => { setInputValue(val); - onChangeDebounced(val || initialValue.current); + const valueToUpload = shouldUpdateWithEmptyString + ? val ?? initialValue.current + : val || initialValue.current; + onChangeDebounced(valueToUpload); }; return { inputValue, handleInputChange, initialValue: initialValue.current }; From e684272d89441c824b1e75c7abebbad032cc94d1 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 3 Jun 2021 14:22:54 +0200 Subject: [PATCH 03/17] :bug: Address the transition problem + refactor --- .../components/dimension_editor.tsx | 53 +++++++++---------- .../components/table_basic.tsx | 13 +++-- .../public/datatable_visualization/summary.ts | 19 ++++++- .../public/datatable_visualization/utils.ts | 22 ++++++++ .../datatable_visualization/visualization.tsx | 14 +++-- 5 files changed, 80 insertions(+), 41 deletions(-) create mode 100644 x-pack/plugins/lens/public/datatable_visualization/utils.ts diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index 84d4694ba3b74..7ed4e5a5a9b4c 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -34,7 +34,8 @@ import { import { PalettePanelContainer } from './palette_panel_container'; import { findMinMaxByColumnId } from './shared_utils'; import './dimension_editor.scss'; -import { getDefaultSummaryLabel, getSummaryRowOptions } from '../summary'; +import { getFinalSummaryConfiguration, getSummaryRowOptions } from '../summary'; +import { isNumericField } from '../utils'; const idPrefix = htmlIdGenerator()(); @@ -72,14 +73,15 @@ export function TableDimensionEditor( }, [accessor, setState, state] ); - const { - inputValue: summaryLabel, - handleInputChange: onSummaryLabelChange, - initialValue, - } = useDebouncedValue({ - onChange: onSummaryLabelChangeToDebounce, - value: column?.summaryLabel, - }); + const { inputValue: summaryLabel, handleInputChange: onSummaryLabelChange } = useDebouncedValue< + string | undefined + >( + { + onChange: onSummaryLabelChangeToDebounce, + value: column?.summaryLabel, + }, + { allowEmptyString: true } // empty string is a valid label for this feature + ); if (!column) return null; if (column.isTransposed) return null; @@ -87,22 +89,16 @@ export function TableDimensionEditor( const currentData = frame.activeData?.[state.layerId]; // either read config state or use same logic as chart itself - const isNumericField = - currentData?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor) - ?.meta.type === 'number'; - - // check for array values - const hasFieldNumericValues = - isNumericField && - currentData?.rows.every(({ [accessor]: value }) => typeof value === 'number' || value == null); - - const currentAlignment = column?.alignment || (isNumericField ? 'right' : 'left'); + const { isNumeric, hasAllNumericValues } = isNumericField(currentData, accessor); + const currentAlignment = column?.alignment || (isNumeric ? 'right' : 'left'); const currentColorMode = column?.colorMode || 'none'; const hasDynamicColoring = currentColorMode !== 'none'; - const summaryRowOptions = getSummaryRowOptions(); // when switching from one operation to another, make sure to keep the configuration consistent - const summaryRowType = column?.summaryRow || 'none'; - const fallbackSummaryLabel = getDefaultSummaryLabel(summaryRowType); + const { summaryRow, summaryLabel: fallbackSummaryLabel } = getFinalSummaryConfiguration( + accessor, + column, + currentData + ); const visibleColumnsCount = state.columns.filter((c) => !c.hidden).length; @@ -206,7 +202,7 @@ export function TableDimensionEditor( /> )} - {hasFieldNumericValues && ( + {hasAllNumericValues && ( <> data-test-subj="lnsDatatable_summaryrow_function" - valueOfSelected={summaryRowType} - options={summaryRowOptions} + valueOfSelected={summaryRow} + options={getSummaryRowOptions()} compressed onChange={(newValue) => { setState({ @@ -228,7 +224,7 @@ export function TableDimensionEditor( }} /> - {summaryRowType !== 'none' && ( + {summaryRow !== 'none' && ( { onSummaryLabelChange(e.target.value); }} - placeholder={initialValue || fallbackSummaryLabel} /> )} )} - {isNumericField && ( + {isNumeric && ( <> ({}); @@ -287,13 +288,19 @@ export const DatatableComponent = (props: DatatableRenderProps) => { ); const renderSummaryRow = useMemo(() => { - const columnsWithSummary = columnConfig.columns.filter(({ summaryRow }) => summaryRow); + const columnsWithSummary = columnConfig.columns + .map((config) => ({ + columnId: config.columnId, + summaryRowValue: config.summaryRowValue, + ...getFinalSummaryConfiguration(config.columnId, config, firstTable), + })) + .filter(({ summaryRow }) => summaryRow !== 'none'); if (columnsWithSummary.length) { const summaryLookup = Object.fromEntries( columnsWithSummary.map(({ summaryRowValue, summaryLabel, columnId }) => [ columnId, - `${summaryLabel}: ${summaryRowValue}`, + summaryLabel === '' ? `${summaryRowValue}` : `${summaryLabel}: ${summaryRowValue}`, ]) ); return ({ columnId }: { columnId: string }) => { @@ -304,7 +311,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { ) : null; }; } - }, [columnConfig.columns, alignments]); + }, [columnConfig.columns, alignments, firstTable]); if (isEmpty) { return ; diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.ts index 6f83c586a694e..1c32233423d12 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/summary.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.ts @@ -10,9 +10,26 @@ import { FieldFormat } from 'src/plugins/data/public'; import { Datatable } from 'src/plugins/expressions/public'; import { ColumnConfigArg } from './datatable_visualization'; import { getOriginalId } from './transpose_helpers'; +import { isNumericField } from './utils'; type SummaryRowType = Extract; +export function getFinalSummaryConfiguration( + columnId: string, + columnArgs: Pick | undefined, + table: Datatable | undefined +) { + const { hasAllNumericValues } = isNumericField(table, columnId); + + const summaryRow = hasAllNumericValues ? columnArgs?.summaryRow || 'none' : 'none'; + const summaryLabel = columnArgs?.summaryLabel ?? getDefaultSummaryLabel(summaryRow); + + return { + summaryRow, + summaryLabel, + }; +} + export function getDefaultSummaryLabel(type: SummaryRowType) { return getSummaryRowOptions().find(({ value }) => type === value)!.inputDisplay!; } @@ -28,7 +45,7 @@ export function getSummaryRowOptions(): Array<{ value: SummaryRowType; inputDisp { value: 'count', inputDisplay: i18n.translate('xpack.lens.table.summaryRow.count', { - defaultMessage: 'Count', + defaultMessage: 'Value count', }), }, { diff --git a/x-pack/plugins/lens/public/datatable_visualization/utils.ts b/x-pack/plugins/lens/public/datatable_visualization/utils.ts new file mode 100644 index 0000000000000..5395458a72c56 --- /dev/null +++ b/x-pack/plugins/lens/public/datatable_visualization/utils.ts @@ -0,0 +1,22 @@ +/* + * 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 { Datatable } from 'src/plugins/expressions/public'; +import { getOriginalId } from './transpose_helpers'; + +export function isNumericField(currentData: Datatable | undefined, accessor: string) { + const isNumeric = + currentData?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor) + ?.meta.type === 'number'; + + // check for array values + const hasFieldNumericValues = + isNumeric && + currentData?.rows.every(({ [accessor]: value }) => typeof value === 'number' || value == null); + + return { isNumeric, hasAllNumericValues: hasFieldNumericValues }; +} diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index 4a4c62b2169ff..06bdc1c3cba5a 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -361,6 +361,8 @@ export const getDatatableVisualization = ({ reverse: false, // managed at UI level }; + const HasNoSummaryRow = column.summaryRow == null || column.summaryRow === 'none'; + return { type: 'expression', chain: [ @@ -379,14 +381,10 @@ export const getDatatableVisualization = ({ alignment: typeof column.alignment === 'undefined' ? [] : [column.alignment], colorMode: [column.colorMode ?? 'none'], palette: [paletteService.get(CUSTOM_PALETTE).toExpression(paletteParams)], - summaryRow: - column.summaryRow == null || column.summaryRow === 'none' - ? [] - : [column.summaryRow], - summaryLabel: - column.summaryRow == null || column.summaryRow === 'none' - ? [] - : [column.summaryLabel || getDefaultSummaryLabel(column.summaryRow)], + summaryRow: HasNoSummaryRow ? [] : [column.summaryRow!], + summaryLabel: HasNoSummaryRow + ? [] + : [column.summaryLabel ?? getDefaultSummaryLabel(column.summaryRow!)], }, }, ], From 0e14bd49ce18a7eaf82ac66f718b0751f47cd773 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 3 Jun 2021 17:14:25 +0200 Subject: [PATCH 04/17] :white_check_mark: Add some unit tests --- .../components/dimension_editor.test.tsx | 42 ++++++- .../components/table_basic.test.tsx | 68 +++++++++++ .../components/table_basic.tsx | 7 +- .../datatable_visualization/summary.test.ts | 106 ++++++++++++++++++ 4 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/lens/public/datatable_visualization/summary.test.ts diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx index 88948e9a7615b..220e4e1fde3f8 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { EuiButtonGroup } from '@elastic/eui'; +import { EuiButtonGroup, EuiFieldText, EuiSuperSelect } from '@elastic/eui'; import { FramePublicAPI, VisualizationDimensionEditorProps } from '../../types'; import { DatatableVisualizationState } from '../visualization'; import { createMockDatasource, createMockFramePublicAPI } from '../../editor_frame_service/mocks'; @@ -212,4 +212,44 @@ describe('data table dimension editor', () => { expect(instance.find(PalettePanelContainer).exists()).toBe(true); }); + + it('should show the summary field for non numeric columns', () => { + const instance = mountWithIntl(); + expect(instance.find('[data-test-subj="lnsDatatable_summaryrow_function"]').exists()).toBe( + false + ); + expect(instance.find('[data-test-subj="lnsDatatable_summaryrow_label"]').exists()).toBe(false); + }); + + it('should set the summary row function default to "none"', () => { + frame.activeData!.first.columns[0].meta.type = 'number'; + const instance = mountWithIntl(); + expect( + instance + .find('[data-test-subj="lnsDatatable_summaryrow_function"]') + .find(EuiSuperSelect) + .prop('valueOfSelected') + ).toEqual('none'); + + expect(instance.find('[data-test-subj="lnsDatatable_summaryrow_label"]').exists()).toBe(false); + }); + + 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'; + const instance = mountWithIntl(); + expect( + instance + .find('[data-test-subj="lnsDatatable_summaryrow_function"]') + .find(EuiSuperSelect) + .prop('valueOfSelected') + ).toEqual('sum'); + + expect( + instance + .find('[data-test-subj="lnsDatatable_summaryrow_label"]') + .find(EuiFieldText) + .prop('value') + ).toBe('Sum'); + }); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx index 509969c2b71ec..051d7fb0742b5 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx @@ -539,4 +539,72 @@ describe('DatatableComponent', () => { c: { min: 3, max: 3 }, }); }); + + test('it does render a summary footer if at least one column has it configured', () => { + const { data, args } = sampleArgs(); + + const wrapper = mountWithIntl( + ({ convert: (x) => x } as IFieldFormat)} + dispatchEvent={onDispatchEvent} + getType={jest.fn()} + renderMode="display" + paletteService={chartPluginMock.createPaletteRegistry()} + uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient} + /> + ); + expect(wrapper.find('[data-test-subj="lnsDataTable-footer-a"]').exists()).toEqual(false); + expect(wrapper.find('[data-test-subj="lnsDataTable-footer-c"]').first().text()).toEqual( + 'Sum: 3' + ); + }); + + test('it does render a summary footer with just the raw value for empty label', () => { + const { data, args } = sampleArgs(); + + const wrapper = mountWithIntl( + ({ convert: (x) => x } as IFieldFormat)} + dispatchEvent={onDispatchEvent} + getType={jest.fn()} + renderMode="display" + paletteService={chartPluginMock.createPaletteRegistry()} + uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient} + /> + ); + + expect(wrapper.find('[data-test-subj="lnsDataTable-footer-c"]').first().text()).toEqual('3'); + }); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx index 5c1cfeea0e971..6f53aa4f068d4 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx @@ -307,7 +307,12 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const currentAlignment = alignments && alignments[columnId]; const alignmentClassName = `lnsTableCell--${currentAlignment}`; return summaryLookup[columnId] != null ? ( -
{summaryLookup[columnId]}
+
+ {summaryLookup[columnId]} +
) : null; }; } diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.test.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.test.ts new file mode 100644 index 0000000000000..a11d4c9501288 --- /dev/null +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.test.ts @@ -0,0 +1,106 @@ +/* + * 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 { IFieldFormat } from 'src/plugins/data/public'; +import { Datatable } from 'src/plugins/expressions'; +import { computeSummaryRowForColumn, getFinalSummaryConfiguration } from './summary'; + +describe('Summary row helpers', () => { + const mockNumericTable: Datatable = { + type: 'datatable', + columns: [{ id: 'myColumn', name: 'My Column', meta: { type: 'number' } }], + rows: [{ myColumn: 45 }], + }; + + const mockNumericTableWithArray: Datatable = { + type: 'datatable', + columns: [{ id: 'myColumn', name: 'My Column', meta: { type: 'number' } }], + rows: [{ myColumn: [45, 90] }], + }; + + const mockNonNumericTable: Datatable = { + type: 'datatable', + columns: [{ id: 'myColumn', name: 'My Column', meta: { type: 'string' } }], + rows: [{ myColumn: 'myString' }], + }; + + describe('getFinalSummaryConfiguration', () => { + it('should return the base configuration for an unconfigured column', () => { + expect(getFinalSummaryConfiguration('myColumn', {}, mockNumericTable)).toEqual({ + summaryRow: 'none', + summaryLabel: 'None', + }); + }); + + it('should return the right configuration for a partially configured column', () => { + expect( + getFinalSummaryConfiguration('myColumn', { summaryRow: 'sum' }, mockNumericTable) + ).toEqual({ + summaryRow: 'sum', + summaryLabel: 'Sum', + }); + }); + + it('should return the base configuration for a transitioned invalid column', () => { + expect( + getFinalSummaryConfiguration('myColumn', { summaryRow: 'sum' }, mockNumericTableWithArray) + ).toEqual({ + summaryRow: 'none', + summaryLabel: 'None', + }); + }); + + it('should return the base configuration for a non numeric column', () => { + expect( + getFinalSummaryConfiguration('myColumn', { summaryRow: 'sum' }, mockNonNumericTable) + ).toEqual({ + summaryRow: 'none', + summaryLabel: 'None', + }); + }); + }); + + describe('computeSummaryRowForColumn', () => { + for (const op of ['avg', 'sum', 'min', 'max'] as const) { + it(`should return formatted value for a ${op} summary function`, () => { + expect( + computeSummaryRowForColumn( + { summaryRow: op, columnId: 'myColumn', type: 'lens_datatable_column' }, + mockNumericTable, + { + myColumn: { convert: (x: number) => x.toFixed(2) } as IFieldFormat, + } + ) + ).toBe('45.00'); + }); + } + + it('should ignore the column formatter, rather return the raw value for count operation', () => { + expect( + computeSummaryRowForColumn( + { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, + mockNumericTable, + { + myColumn: { convert: (x: number) => x.toFixed(2) } as IFieldFormat, + } + ) + ).toBe(1); + }); + + it('should only count non-null/empty values', () => { + expect( + computeSummaryRowForColumn( + { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, + { ...mockNumericTable, rows: [...mockNumericTable.rows, { myColumn: null }] }, + { + myColumn: { convert: (x: number) => x.toFixed(2) } as IFieldFormat, + } + ) + ).toBe(1); + }); + }); +}); From 79e715b4103af5c0a61698415809a065d6b618f3 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 3 Jun 2021 17:34:07 +0200 Subject: [PATCH 05/17] :white_check_mark: Add first functional tests --- x-pack/test/functional/apps/lens/table.ts | 10 ++++++++++ x-pack/test/functional/page_objects/lens_page.ts | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/x-pack/test/functional/apps/lens/table.ts b/x-pack/test/functional/apps/lens/table.ts index f048bf47991f2..9833c6d5e8b61 100644 --- a/x-pack/test/functional/apps/lens/table.ts +++ b/x-pack/test/functional/apps/lens/table.ts @@ -144,5 +144,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // should also set text color when in cell mode expect(styleObj.color).to.be('rgb(0, 0, 0)'); }); + + it('should allow to show a summary table for metric columns', async () => { + await PageObjects.lens.openDimensionEditor('lnsDatatable_metrics > lns-dimensionTrigger'); + await PageObjects.lens.setTableSummaryRowFunction('sum'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.lens.assertExactText( + '[data-test-subj="lnsDataTable-footer-c"]', + 'Sum: 100' + ); + }); }); } diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index b16944cd73060..df7f098b43c1e 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -725,6 +725,21 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont return buttonEl.click(); }, + async setTableSummaryRowFunction( + summaryFunction: 'none' | 'sum' | 'avg' | 'count' | 'min' | 'max' + ) { + const target = await testSubjects.find('lnsDatatable_summaryrow_function'); + await comboBox.openOptionsList(target); + await comboBox.setElement(target, summaryFunction); + }, + + async setTableSummaryRowLabel(newLabel: string) { + await testSubjects.setValue('lnsDatatable_summaryrow_label', newLabel, { + clearWithKeyboard: true, + typeCharByChar: true, + }); + }, + async setTableDynamicColoring(coloringType: 'none' | 'cell' | 'text') { await testSubjects.click('lnsDatatable_dynamicColoring_groups_' + coloringType); }, From cea3233ac9d7992a387a2d4bcf99ad3154479457 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 3 Jun 2021 18:03:13 +0200 Subject: [PATCH 06/17] :ok_hand: first feedback addressed --- .../lens/public/datatable_visualization/visualization.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index 06bdc1c3cba5a..e48cb1b28c084 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -361,7 +361,7 @@ export const getDatatableVisualization = ({ reverse: false, // managed at UI level }; - const HasNoSummaryRow = column.summaryRow == null || column.summaryRow === 'none'; + const hasNoSummaryRow = column.summaryRow == null || column.summaryRow === 'none'; return { type: 'expression', @@ -381,8 +381,8 @@ export const getDatatableVisualization = ({ alignment: typeof column.alignment === 'undefined' ? [] : [column.alignment], colorMode: [column.colorMode ?? 'none'], palette: [paletteService.get(CUSTOM_PALETTE).toExpression(paletteParams)], - summaryRow: HasNoSummaryRow ? [] : [column.summaryRow!], - summaryLabel: HasNoSummaryRow + summaryRow: hasNoSummaryRow ? [] : [column.summaryRow!], + summaryLabel: hasNoSummaryRow ? [] : [column.summaryLabel ?? getDefaultSummaryLabel(column.summaryRow!)], }, From 48e85c36381faae8fd6d798572891cab8f446f68 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 3 Jun 2021 19:22:31 +0200 Subject: [PATCH 07/17] :sparkles: Make it handle numeric array values --- .../components/dimension_editor.tsx | 4 +-- .../datatable_visualization/summary.test.ts | 16 +++++++-- .../public/datatable_visualization/summary.ts | 15 ++++---- .../public/datatable_visualization/utils.ts | 33 +++++++++++++++--- .../datatable_visualization/visualization.tsx | 34 ++++++++++++++++++- 5 files changed, 86 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index 7ed4e5a5a9b4c..2f9c4db4cf0d6 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -89,7 +89,7 @@ export function TableDimensionEditor( const currentData = frame.activeData?.[state.layerId]; // either read config state or use same logic as chart itself - const { isNumeric, hasAllNumericValues } = isNumericField(currentData, accessor); + const { isNumeric, hasNumericValues } = isNumericField(currentData, accessor); const currentAlignment = column?.alignment || (isNumeric ? 'right' : 'left'); const currentColorMode = column?.colorMode || 'none'; const hasDynamicColoring = currentColorMode !== 'none'; @@ -202,7 +202,7 @@ export function TableDimensionEditor( /> )} - {hasAllNumericValues && ( + {hasNumericValues && ( <> { expect( getFinalSummaryConfiguration('myColumn', { summaryRow: 'sum' }, mockNumericTableWithArray) ).toEqual({ - summaryRow: 'none', - summaryLabel: 'None', + summaryRow: 'sum', + summaryLabel: 'Sum', }); }); @@ -102,5 +102,17 @@ describe('Summary row helpers', () => { ) ).toBe(1); }); + + it('should count numeric arrays as valid and distinct values', () => { + expect( + computeSummaryRowForColumn( + { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, + mockNumericTableWithArray, + { + myColumn: { convert: (x) => x } as IFieldFormat, + } + ) + ).toBe(2); + }); }); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.ts index 1c32233423d12..02eab91fa957d 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/summary.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.ts @@ -19,9 +19,9 @@ export function getFinalSummaryConfiguration( columnArgs: Pick | undefined, table: Datatable | undefined ) { - const { hasAllNumericValues } = isNumericField(table, columnId); + const { hasNumericValues } = isNumericField(table, columnId); - const summaryRow = hasAllNumericValues ? columnArgs?.summaryRow || 'none' : 'none'; + const summaryRow = hasNumericValues ? columnArgs?.summaryRow || 'none' : 'none'; const summaryLabel = columnArgs?.summaryLabel ?? getDefaultSummaryLabel(summaryRow); return { @@ -93,9 +93,12 @@ function computeFinalValue( columnId: string, rows: Datatable['rows'] ) { - const validRows = rows.filter((v) => v[columnId] != null); + // flatten the row structure, to easier handle numeric arrays + const validRows = rows.filter((v) => v[columnId] != null).flatMap((v) => v[columnId]); const count = validRows.length; - const sum = validRows.reduce((partialSum, v) => partialSum + (v[columnId] as number), 0); + const sum = validRows.reduce((partialSum: number, value: number) => { + return partialSum + value; + }, 0); switch (type) { case 'sum': return sum; @@ -104,9 +107,9 @@ function computeFinalValue( case 'avg': return sum / count; case 'min': - return Math.min(...validRows.map((v) => v[columnId])); + return Math.min(...validRows); case 'max': - return Math.max(...validRows.map((v) => v[columnId])); + return Math.max(...validRows); default: throw Error('No summary function found'); } diff --git a/x-pack/plugins/lens/public/datatable_visualization/utils.ts b/x-pack/plugins/lens/public/datatable_visualization/utils.ts index 5395458a72c56..dbc9cfbf2a565 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/utils.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/utils.ts @@ -8,15 +8,38 @@ import { Datatable } from 'src/plugins/expressions/public'; import { getOriginalId } from './transpose_helpers'; +function isValidNumber(value: unknown): boolean { + return typeof value === 'number' || value == null; +} + +/** + * It checks the configuration and content of the current datatable + * It returns an object with the following content: + * * isNumeric: whether the field is numeric + * * hasNumberValues: whether the table rows contain all values of type number (empty values are ignored) + * * hasNumericValues: whether the table rows contain numeric compatible values (arrays with numbers are considered valid here, empty values are ignored) + * @param currentData + * @param accessor + * @returns An object containing stats metadata retrieved from both configuration and current data + */ export function isNumericField(currentData: Datatable | undefined, accessor: string) { const isNumeric = currentData?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor) ?.meta.type === 'number'; - // check for array values - const hasFieldNumericValues = - isNumeric && - currentData?.rows.every(({ [accessor]: value }) => typeof value === 'number' || value == null); + let hasFieldOnlyNumberValues = isNumeric; + let hasFieldNumericValues = isNumeric; + for (const row of currentData?.rows || []) { + const value = row[accessor]; + hasFieldOnlyNumberValues = hasFieldOnlyNumberValues && isValidNumber(value); + hasFieldNumericValues = + (hasFieldNumericValues && hasFieldOnlyNumberValues) || + (Array.isArray(value) && value.every(isValidNumber)); + } - return { isNumeric, hasAllNumericValues: hasFieldNumericValues }; + return { + isNumeric, + hasNumberValues: hasFieldOnlyNumberValues, + hasNumericValues: hasFieldNumericValues, + }; } diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index e48cb1b28c084..39fe7e8877d6a 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { render } from 'react-dom'; import { Ast } from '@kbn/interpreter/common'; -import { I18nProvider } from '@kbn/i18n/react'; +import { FormattedMessage, I18nProvider } from '@kbn/i18n/react'; import { i18n } from '@kbn/i18n'; import { DatatableColumn } from 'src/plugins/expressions/public'; import { PaletteOutput, PaletteRegistry } from 'src/plugins/charts/public'; @@ -445,6 +445,38 @@ export const getDatatableVisualization = ({ return state; } }, + getWarningMessages(state, frame) { + if (state?.columns.length === 0 || !frame.activeData) { + return; + } + + const metricColumnsWithArrayValues = []; + + for (const { columnId, summaryRow } of state.columns) { + const rows = frame.activeData[state.layerId] && frame.activeData[state.layerId].rows; + if (!rows || summaryRow == null || summaryRow === 'none') { + continue; + } + const columnToLabel = frame.datasourceLayers[state.layerId].getOperationForColumnId(columnId) + ?.label; + + const hasArrayValues = rows.some((row) => Array.isArray(row[columnId])); + if (hasArrayValues) { + metricColumnsWithArrayValues.push(columnToLabel || columnId); + } + } + return metricColumnsWithArrayValues.map((label) => ( + {label}, + }} + /> + )); + }, }); function getDataSourceAndSortedColumns( From 743606a1e4e0fc8d291f89ac70f2d6abfe87e573 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 3 Jun 2021 19:25:13 +0200 Subject: [PATCH 08/17] :memo: Improved message --- .../lens/public/datatable_visualization/visualization.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index 39fe7e8877d6a..48f9aec76e009 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -468,9 +468,8 @@ export const getDatatableVisualization = ({ return metricColumnsWithArrayValues.map((label) => ( {label}, }} From 9417ccccf6d6c21d8e8f67b63a9452fa6f4bd143 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 4 Jun 2021 11:52:34 +0200 Subject: [PATCH 09/17] :white_check_mark: Fix functional test --- .../components/table_basic.tsx | 6 ++++-- .../lens/public/datatable_visualization/summary.ts | 12 +++++++++++- x-pack/test/functional/apps/lens/table.ts | 6 +++--- x-pack/test/functional/page_objects/lens_page.ts | 9 ++++++--- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx index 6f53aa4f068d4..552d056e4a17b 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx @@ -306,17 +306,19 @@ export const DatatableComponent = (props: DatatableRenderProps) => { return ({ columnId }: { columnId: string }) => { const currentAlignment = alignments && alignments[columnId]; const alignmentClassName = `lnsTableCell--${currentAlignment}`; + const columnName = + columns.find(({ id }) => id === columnId)?.displayAsText?.replace(/ /g, '-') || columnId; return summaryLookup[columnId] != null ? (
{summaryLookup[columnId]}
) : null; }; } - }, [columnConfig.columns, alignments, firstTable]); + }, [columnConfig.columns, alignments, firstTable, columns]); if (isEmpty) { return ; diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.ts index 02eab91fa957d..bb58c105210a1 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/summary.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.ts @@ -34,43 +34,53 @@ export function getDefaultSummaryLabel(type: SummaryRowType) { return getSummaryRowOptions().find(({ value }) => type === value)!.inputDisplay!; } -export function getSummaryRowOptions(): Array<{ value: SummaryRowType; inputDisplay: string }> { +export function getSummaryRowOptions(): Array<{ + value: SummaryRowType; + inputDisplay: string; + 'data-test-subj': string; +}> { return [ { value: 'none', inputDisplay: i18n.translate('xpack.lens.table.summaryRow.none', { defaultMessage: 'None', }), + 'data-test-subj': 'lns-datatable-summary-none', }, { value: 'count', inputDisplay: i18n.translate('xpack.lens.table.summaryRow.count', { defaultMessage: 'Value count', }), + 'data-test-subj': 'lns-datatable-summary-count', }, { value: 'sum', inputDisplay: i18n.translate('xpack.lens.table.summaryRow.sum', { defaultMessage: 'Sum', }), + 'data-test-subj': 'lns-datatable-summary-sum', }, { value: 'avg', inputDisplay: i18n.translate('xpack.lens.table.summaryRow.average', { defaultMessage: 'Average', }), + 'data-test-subj': 'lns-datatable-summary-avg', }, { value: 'min', inputDisplay: i18n.translate('xpack.lens.table.summaryRow.minimum', { defaultMessage: 'Minimum', }), + 'data-test-subj': 'lns-datatable-summary-min', }, { value: 'max', inputDisplay: i18n.translate('xpack.lens.table.summaryRow.maximum', { defaultMessage: 'Maximum', }), + 'data-test-subj': 'lns-datatable-summary-max', }, ]; } diff --git a/x-pack/test/functional/apps/lens/table.ts b/x-pack/test/functional/apps/lens/table.ts index 9833c6d5e8b61..f499c5bf0cfe8 100644 --- a/x-pack/test/functional/apps/lens/table.ts +++ b/x-pack/test/functional/apps/lens/table.ts @@ -143,15 +143,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(styleObj['background-color']).to.be('rgb(168, 191, 218)'); // should also set text color when in cell mode expect(styleObj.color).to.be('rgb(0, 0, 0)'); + await PageObjects.lens.closeTablePalettePanel(); }); it('should allow to show a summary table for metric columns', async () => { - await PageObjects.lens.openDimensionEditor('lnsDatatable_metrics > lns-dimensionTrigger'); await PageObjects.lens.setTableSummaryRowFunction('sum'); await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.lens.assertExactText( - '[data-test-subj="lnsDataTable-footer-c"]', - 'Sum: 100' + '[data-test-subj="lnsDataTable-footer-169.228.188.120-›-Average-of-bytes"]', + 'Sum: 18,994' ); }); }); diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index ca58583cbfb6a..86895b5192c66 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -747,9 +747,8 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont async setTableSummaryRowFunction( summaryFunction: 'none' | 'sum' | 'avg' | 'count' | 'min' | 'max' ) { - const target = await testSubjects.find('lnsDatatable_summaryrow_function'); - await comboBox.openOptionsList(target); - await comboBox.setElement(target, summaryFunction); + await testSubjects.click('lnsDatatable_summaryrow_function'); + await testSubjects.click('lns-datatable-summary-' + summaryFunction); }, async setTableSummaryRowLabel(newLabel: string) { @@ -767,6 +766,10 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont await testSubjects.click('lnsDatatable_dynamicColoring_trigger'); }, + async closeTablePalettePanel() { + await testSubjects.click('lns-indexPattern-PalettePanelContainerBack'); + }, + // different picker from the next one async changePaletteTo(paletteName: string) { await testSubjects.click('lnsDatatable_dynamicColoring_palette_picker'); From 678fd95aeb554f7718eb93660d4cd898bffb3741 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 4 Jun 2021 17:51:09 +0200 Subject: [PATCH 10/17] :fire: Remove warning message for last value --- .../datatable_visualization/visualization.tsx | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index 48f9aec76e009..c2b6708da57f8 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -445,37 +445,6 @@ export const getDatatableVisualization = ({ return state; } }, - getWarningMessages(state, frame) { - if (state?.columns.length === 0 || !frame.activeData) { - return; - } - - const metricColumnsWithArrayValues = []; - - for (const { columnId, summaryRow } of state.columns) { - const rows = frame.activeData[state.layerId] && frame.activeData[state.layerId].rows; - if (!rows || summaryRow == null || summaryRow === 'none') { - continue; - } - const columnToLabel = frame.datasourceLayers[state.layerId].getOperationForColumnId(columnId) - ?.label; - - const hasArrayValues = rows.some((row) => Array.isArray(row[columnId])); - if (hasArrayValues) { - metricColumnsWithArrayValues.push(columnToLabel || columnId); - } - } - return metricColumnsWithArrayValues.map((label) => ( - {label}, - }} - /> - )); - }, }); function getDataSourceAndSortedColumns( From 842add2896a569bfcee5412158ee3dafdd64eaaf Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 4 Jun 2021 18:11:20 +0200 Subject: [PATCH 11/17] :rotating_light: Remove unused import --- .../lens/public/datatable_visualization/visualization.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index c2b6708da57f8..e48cb1b28c084 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { render } from 'react-dom'; import { Ast } from '@kbn/interpreter/common'; -import { FormattedMessage, I18nProvider } from '@kbn/i18n/react'; +import { I18nProvider } from '@kbn/i18n/react'; import { i18n } from '@kbn/i18n'; import { DatatableColumn } from 'src/plugins/expressions/public'; import { PaletteOutput, PaletteRegistry } from 'src/plugins/charts/public'; From bd069e4d2ca478dfdb2598f16f635911a5511dad Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 7 Jun 2021 15:34:30 +0200 Subject: [PATCH 12/17] :bug: Fix a bug with last value --- .../public/datatable_visualization/utils.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/utils.ts b/x-pack/plugins/lens/public/datatable_visualization/utils.ts index dbc9cfbf2a565..64e6ba5d30f02 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/utils.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/utils.ts @@ -27,14 +27,25 @@ export function isNumericField(currentData: Datatable | undefined, accessor: str currentData?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor) ?.meta.type === 'number'; + // if the field type is not numeric, do not proceed any further + if (!isNumeric) { + return { + isNumeric, + hasNumberValues: false, + hasNumericValues: false, + }; + } + let hasFieldOnlyNumberValues = isNumeric; let hasFieldNumericValues = isNumeric; for (const row of currentData?.rows || []) { const value = row[accessor]; - hasFieldOnlyNumberValues = hasFieldOnlyNumberValues && isValidNumber(value); - hasFieldNumericValues = - (hasFieldNumericValues && hasFieldOnlyNumberValues) || - (Array.isArray(value) && value.every(isValidNumber)); + if (!hasFieldOnlyNumberValues) { + hasFieldOnlyNumberValues = hasFieldOnlyNumberValues && isValidNumber(value); + hasFieldNumericValues = + hasFieldNumericValues && + (hasFieldOnlyNumberValues || (Array.isArray(value) && value.every(isValidNumber))); + } } return { From 9a0e2a1d29a2989963c02e28f10b2fb53d3fe125 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 7 Jun 2021 18:34:14 +0200 Subject: [PATCH 13/17] :ok_hand: Integrated feedback --- .../components/dimension_editor.tsx | 4 +- .../public/datatable_visualization/summary.ts | 4 +- .../public/datatable_visualization/utils.ts | 43 +++---------------- 3 files changed, 11 insertions(+), 40 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index 2f9c4db4cf0d6..2c26d2dc6adb4 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -89,7 +89,7 @@ export function TableDimensionEditor( const currentData = frame.activeData?.[state.layerId]; // either read config state or use same logic as chart itself - const { isNumeric, hasNumericValues } = isNumericField(currentData, accessor); + const isNumeric = isNumericField(currentData, accessor); const currentAlignment = column?.alignment || (isNumeric ? 'right' : 'left'); const currentColorMode = column?.colorMode || 'none'; const hasDynamicColoring = currentColorMode !== 'none'; @@ -202,7 +202,7 @@ export function TableDimensionEditor( /> )} - {hasNumericValues && ( + {isNumeric && ( <> | undefined, table: Datatable | undefined ) { - const { hasNumericValues } = isNumericField(table, columnId); + const isNumeric = isNumericField(table, columnId); - const summaryRow = hasNumericValues ? columnArgs?.summaryRow || 'none' : 'none'; + const summaryRow = isNumeric ? columnArgs?.summaryRow || 'none' : 'none'; const summaryLabel = columnArgs?.summaryLabel ?? getDefaultSummaryLabel(summaryRow); return { diff --git a/x-pack/plugins/lens/public/datatable_visualization/utils.ts b/x-pack/plugins/lens/public/datatable_visualization/utils.ts index 64e6ba5d30f02..64fdee233e830 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/utils.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/utils.ts @@ -12,45 +12,16 @@ function isValidNumber(value: unknown): boolean { return typeof value === 'number' || value == null; } -/** - * It checks the configuration and content of the current datatable - * It returns an object with the following content: - * * isNumeric: whether the field is numeric - * * hasNumberValues: whether the table rows contain all values of type number (empty values are ignored) - * * hasNumericValues: whether the table rows contain numeric compatible values (arrays with numbers are considered valid here, empty values are ignored) - * @param currentData - * @param accessor - * @returns An object containing stats metadata retrieved from both configuration and current data - */ export function isNumericField(currentData: Datatable | undefined, accessor: string) { const isNumeric = currentData?.columns.find((col) => col.id === accessor || getOriginalId(col.id) === accessor) ?.meta.type === 'number'; - // if the field type is not numeric, do not proceed any further - if (!isNumeric) { - return { - isNumeric, - hasNumberValues: false, - hasNumericValues: false, - }; - } - - let hasFieldOnlyNumberValues = isNumeric; - let hasFieldNumericValues = isNumeric; - for (const row of currentData?.rows || []) { - const value = row[accessor]; - if (!hasFieldOnlyNumberValues) { - hasFieldOnlyNumberValues = hasFieldOnlyNumberValues && isValidNumber(value); - hasFieldNumericValues = - hasFieldNumericValues && - (hasFieldOnlyNumberValues || (Array.isArray(value) && value.every(isValidNumber))); - } - } - - return { - isNumeric, - hasNumberValues: hasFieldOnlyNumberValues, - hasNumericValues: hasFieldNumericValues, - }; + return ( + isNumeric && + currentData?.rows.every((row) => { + const val = row[accessor]; + return isValidNumber(val) || (Array.isArray(val) && val.every(isValidNumber)); + }) + ); } From 36b9745b5738e8f29eef2670ec03b9550e54ac04 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 7 Jun 2021 19:10:41 +0200 Subject: [PATCH 14/17] :lipstick: Migrated to combobox --- .../components/dimension_editor.tsx | 24 ++++++++++++++----- .../public/datatable_visualization/summary.ts | 16 ++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index 2c26d2dc6adb4..f09af0f18088b 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -16,8 +16,8 @@ import { EuiFlexItem, EuiFlexGroup, EuiButtonEmpty, - EuiSuperSelect, EuiFieldText, + EuiComboBox, } from '@elastic/eui'; import { PaletteRegistry } from 'src/plugins/charts/public'; import { VisualizationDimensionEditorProps } from '../../types'; @@ -211,12 +211,24 @@ export function TableDimensionEditor( })} display="columnCompressed" > - - data-test-subj="lnsDatatable_summaryrow_function" - valueOfSelected={summaryRow} - options={getSummaryRowOptions()} + { + isClearable={false} + data-test-subj="indexPattern-dimension-field" + placeholder={i18n.translate('xpack.lens.indexPattern.fieldPlaceholder', { + defaultMessage: 'Field', + })} + options={getSummaryRowOptions()} + selectedOptions={[ + { + label: fallbackSummaryLabel, + value: summaryRow, + }, + ]} + singleSelection={{ asPlainText: true }} + onChange={(choices) => { + const newValue = choices[0].value as SummaryRowType; setState({ ...state, columns: updateColumnWith(state, accessor, { summaryRow: newValue }), diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.ts index 94bcc3a2532e0..80955d6c0657f 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/summary.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.ts @@ -31,53 +31,53 @@ export function getFinalSummaryConfiguration( } export function getDefaultSummaryLabel(type: SummaryRowType) { - return getSummaryRowOptions().find(({ value }) => type === value)!.inputDisplay!; + return getSummaryRowOptions().find(({ value }) => type === value)!.label!; } export function getSummaryRowOptions(): Array<{ value: SummaryRowType; - inputDisplay: string; + label: string; 'data-test-subj': string; }> { return [ { value: 'none', - inputDisplay: i18n.translate('xpack.lens.table.summaryRow.none', { + label: i18n.translate('xpack.lens.table.summaryRow.none', { defaultMessage: 'None', }), 'data-test-subj': 'lns-datatable-summary-none', }, { value: 'count', - inputDisplay: i18n.translate('xpack.lens.table.summaryRow.count', { + label: i18n.translate('xpack.lens.table.summaryRow.count', { defaultMessage: 'Value count', }), 'data-test-subj': 'lns-datatable-summary-count', }, { value: 'sum', - inputDisplay: i18n.translate('xpack.lens.table.summaryRow.sum', { + label: i18n.translate('xpack.lens.table.summaryRow.sum', { defaultMessage: 'Sum', }), 'data-test-subj': 'lns-datatable-summary-sum', }, { value: 'avg', - inputDisplay: i18n.translate('xpack.lens.table.summaryRow.average', { + label: i18n.translate('xpack.lens.table.summaryRow.average', { defaultMessage: 'Average', }), 'data-test-subj': 'lns-datatable-summary-avg', }, { value: 'min', - inputDisplay: i18n.translate('xpack.lens.table.summaryRow.minimum', { + label: i18n.translate('xpack.lens.table.summaryRow.minimum', { defaultMessage: 'Minimum', }), 'data-test-subj': 'lns-datatable-summary-min', }, { value: 'max', - inputDisplay: i18n.translate('xpack.lens.table.summaryRow.maximum', { + label: i18n.translate('xpack.lens.table.summaryRow.maximum', { defaultMessage: 'Maximum', }), 'data-test-subj': 'lns-datatable-summary-max', From 8ebeede37194edd14b37c6e1da6a92691646f4a7 Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 8 Jun 2021 10:27:05 +0200 Subject: [PATCH 15/17] :white_check_mark: Fix unit tests + restore right data-test-id --- .../components/dimension_editor.test.tsx | 14 +++++++------- .../components/dimension_editor.tsx | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx index 220e4e1fde3f8..ede8b026248f0 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { EuiButtonGroup, EuiFieldText, EuiSuperSelect } from '@elastic/eui'; +import { EuiButtonGroup, EuiComboBox, EuiFieldText, EuiSuperSelect } from '@elastic/eui'; import { FramePublicAPI, VisualizationDimensionEditorProps } from '../../types'; import { DatatableVisualizationState } from '../visualization'; import { createMockDatasource, createMockFramePublicAPI } from '../../editor_frame_service/mocks'; @@ -227,9 +227,9 @@ describe('data table dimension editor', () => { expect( instance .find('[data-test-subj="lnsDatatable_summaryrow_function"]') - .find(EuiSuperSelect) - .prop('valueOfSelected') - ).toEqual('none'); + .find(EuiComboBox) + .prop('selectedOptions') + ).toEqual([{ value: 'none', label: 'None' }]); expect(instance.find('[data-test-subj="lnsDatatable_summaryrow_label"]').exists()).toBe(false); }); @@ -241,9 +241,9 @@ describe('data table dimension editor', () => { expect( instance .find('[data-test-subj="lnsDatatable_summaryrow_function"]') - .find(EuiSuperSelect) - .prop('valueOfSelected') - ).toEqual('sum'); + .find(EuiComboBox) + .prop('selectedOptions') + ).toEqual([{ value: 'sum', label: 'Sum' }]); expect( instance diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index f09af0f18088b..2142583a13c49 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -215,7 +215,7 @@ export function TableDimensionEditor( fullWidth compressed isClearable={false} - data-test-subj="indexPattern-dimension-field" + data-test-subj="lnsDatatable_summaryrow_function" placeholder={i18n.translate('xpack.lens.indexPattern.fieldPlaceholder', { defaultMessage: 'Field', })} From cf47e1d7dc62fab14727099f79e38f0e9d2ace7a Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 8 Jun 2021 11:05:10 +0200 Subject: [PATCH 16/17] :label: Fix type issue --- .../components/dimension_editor.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx index ede8b026248f0..02d6db8f0e56f 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { EuiButtonGroup, EuiComboBox, EuiFieldText, EuiSuperSelect } from '@elastic/eui'; +import { EuiButtonGroup, EuiComboBox, EuiFieldText } from '@elastic/eui'; import { FramePublicAPI, VisualizationDimensionEditorProps } from '../../types'; import { DatatableVisualizationState } from '../visualization'; import { createMockDatasource, createMockFramePublicAPI } from '../../editor_frame_service/mocks'; From 77a4017c02b2a36513dca1c05ccd005a8de7624d Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 8 Jun 2021 15:05:29 +0200 Subject: [PATCH 17/17] :ok_hand: Address all issues reported --- .../components/dimension_editor.test.tsx | 20 +++++++++++ .../components/dimension_editor.tsx | 8 +++-- .../components/table_basic.test.tsx | 34 +++++++++++++++++++ .../components/table_basic.tsx | 1 + .../datatable_visualization/expression.tsx | 7 +++- .../datatable_visualization/summary.test.ts | 23 ++++++++----- .../public/datatable_visualization/summary.ts | 5 +-- 7 files changed, 85 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx index 02d6db8f0e56f..49003af28f3f1 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.test.tsx @@ -252,4 +252,24 @@ describe('data table dimension editor', () => { .prop('value') ).toBe('Sum'); }); + + 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'; + const instance = mountWithIntl(); + expect( + instance + .find('[data-test-subj="lnsDatatable_summaryrow_function"]') + .find(EuiComboBox) + .prop('selectedOptions') + ).toEqual([{ value: 'sum', label: 'Sum' }]); + + expect( + instance + .find('[data-test-subj="lnsDatatable_summaryrow_label"]') + .find(EuiFieldText) + .prop('value') + ).toBe('MySum'); + }); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index 2142583a13c49..6c39a04ae1504 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -34,7 +34,11 @@ import { import { PalettePanelContainer } from './palette_panel_container'; import { findMinMaxByColumnId } from './shared_utils'; import './dimension_editor.scss'; -import { getFinalSummaryConfiguration, getSummaryRowOptions } from '../summary'; +import { + getDefaultSummaryLabel, + getFinalSummaryConfiguration, + getSummaryRowOptions, +} from '../summary'; import { isNumericField } from '../utils'; const idPrefix = htmlIdGenerator()(); @@ -222,7 +226,7 @@ export function TableDimensionEditor( options={getSummaryRowOptions()} selectedOptions={[ { - label: fallbackSummaryLabel, + label: getDefaultSummaryLabel(summaryRow), value: summaryRow, }, ]} diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx index 051d7fb0742b5..0ec6f832cd642 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.test.tsx @@ -607,4 +607,38 @@ describe('DatatableComponent', () => { expect(wrapper.find('[data-test-subj="lnsDataTable-footer-c"]').first().text()).toEqual('3'); }); + + test('it does not render the summary row if the only column with summary is hidden', () => { + const { data, args } = sampleArgs(); + + const wrapper = mountWithIntl( + ({ convert: (x) => x } as IFieldFormat)} + dispatchEvent={onDispatchEvent} + getType={jest.fn()} + renderMode="display" + paletteService={chartPluginMock.createPaletteRegistry()} + uiSettings={({ get: jest.fn() } as unknown) as IUiSettingsClient} + /> + ); + + expect(wrapper.find('[data-test-subj="lnsDataTable-footer-c"]').exists()).toBe(false); + }); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx index 552d056e4a17b..cd990149fdaf5 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx @@ -289,6 +289,7 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const renderSummaryRow = useMemo(() => { const columnsWithSummary = columnConfig.columns + .filter((col) => !!col.columnId && !col.hidden) .map((config) => ({ columnId: config.columnId, summaryRowValue: config.summaryRowValue, diff --git a/x-pack/plugins/lens/public/datatable_visualization/expression.tsx b/x-pack/plugins/lens/public/datatable_visualization/expression.tsx index 72aaf6b4855e9..79a541b0288ab 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/expression.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/expression.tsx @@ -120,7 +120,12 @@ export const getDatatable = ({ const columnsWithSummary = args.columns.filter((c) => c.summaryRow); for (const column of columnsWithSummary) { - column.summaryRowValue = computeSummaryRowForColumn(column, firstTable, formatters); + column.summaryRowValue = computeSummaryRowForColumn( + column, + firstTable, + formatters, + formatFactory({ id: 'number' }) + ); } if (sortBy && columnsReverseLookup[sortBy] && sortDirection !== 'none') { diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.test.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.test.ts index 2905ee7ed8bf1..f92c83fbbfdc8 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/summary.test.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.test.ts @@ -28,6 +28,9 @@ describe('Summary row helpers', () => { rows: [{ myColumn: 'myString' }], }; + const defaultFormatter = { convert: (x) => x } as IFieldFormat; + const customNumericFormatter = { convert: (x: number) => x.toFixed(2) } as IFieldFormat; + describe('getFinalSummaryConfiguration', () => { it('should return the base configuration for an unconfigured column', () => { expect(getFinalSummaryConfiguration('myColumn', {}, mockNumericTable)).toEqual({ @@ -72,8 +75,9 @@ describe('Summary row helpers', () => { { summaryRow: op, columnId: 'myColumn', type: 'lens_datatable_column' }, mockNumericTable, { - myColumn: { convert: (x: number) => x.toFixed(2) } as IFieldFormat, - } + myColumn: customNumericFormatter, + }, + defaultFormatter ) ).toBe('45.00'); }); @@ -85,8 +89,9 @@ describe('Summary row helpers', () => { { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, mockNumericTable, { - myColumn: { convert: (x: number) => x.toFixed(2) } as IFieldFormat, - } + myColumn: customNumericFormatter, + }, + defaultFormatter ) ).toBe(1); }); @@ -97,8 +102,9 @@ describe('Summary row helpers', () => { { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, { ...mockNumericTable, rows: [...mockNumericTable.rows, { myColumn: null }] }, { - myColumn: { convert: (x: number) => x.toFixed(2) } as IFieldFormat, - } + myColumn: customNumericFormatter, + }, + defaultFormatter ) ).toBe(1); }); @@ -109,8 +115,9 @@ describe('Summary row helpers', () => { { summaryRow: 'count', columnId: 'myColumn', type: 'lens_datatable_column' }, mockNumericTableWithArray, { - myColumn: { convert: (x) => x } as IFieldFormat, - } + myColumn: defaultFormatter, + }, + defaultFormatter ) ).toBe(2); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/summary.ts b/x-pack/plugins/lens/public/datatable_visualization/summary.ts index 80955d6c0657f..6c267445aab76 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/summary.ts +++ b/x-pack/plugins/lens/public/datatable_visualization/summary.ts @@ -88,12 +88,13 @@ export function getSummaryRowOptions(): Array<{ export function computeSummaryRowForColumn( columnArgs: ColumnConfigArg, table: Datatable, - formatters: Record + formatters: Record, + defaultFormatter: FieldFormat ) { const summaryValue = computeFinalValue(columnArgs.summaryRow, columnArgs.columnId, table.rows); // ignore the coluymn formatter for the count case if (columnArgs.summaryRow === 'count') { - return summaryValue; + return defaultFormatter.convert(summaryValue); } return formatters[getOriginalId(columnArgs.columnId)].convert(summaryValue); }