From 80db96b8230c7056cc4dee4754fdaed9567008ca Mon Sep 17 00:00:00 2001 From: Maryia Lapata Date: Wed, 4 Mar 2020 13:57:16 +0300 Subject: [PATCH] Fix TS for vis_type_vislib (#58345) * Fix TS for vislib * Fix TS * Revert table changes * Update unit test * Refactor updateAxisTitle Co-authored-by: Elastic Machine --- .eslintrc.js | 6 - .../components/common/validation_wrapper.tsx | 2 +- .../metrics_axes/category_axis_panel.tsx | 4 +- .../options/metrics_axes/index.test.tsx | 7 +- .../components/options/metrics_axes/index.tsx | 141 ++++++++++-------- .../options/metrics_axes/value_axes_panel.tsx | 2 +- .../metrics_axes/value_axis_options.tsx | 3 +- .../options/point_series/grid_panel.tsx | 2 +- 8 files changed, 87 insertions(+), 80 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 087d6276cd33f..a678243e4f07a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -76,12 +76,6 @@ module.exports = { 'react-hooks/exhaustive-deps': 'off', }, }, - { - files: ['src/legacy/core_plugins/vis_type_vislib/**/*.{js,ts,tsx}'], - rules: { - 'react-hooks/exhaustive-deps': 'off', - }, - }, { files: [ 'src/legacy/core_plugins/vis_default_editor/public/components/controls/**/*.{ts,tsx}', diff --git a/src/legacy/core_plugins/vis_type_vislib/public/components/common/validation_wrapper.tsx b/src/legacy/core_plugins/vis_type_vislib/public/components/common/validation_wrapper.tsx index 9e1d5ea5ae38f..c069d4c935669 100644 --- a/src/legacy/core_plugins/vis_type_vislib/public/components/common/validation_wrapper.tsx +++ b/src/legacy/core_plugins/vis_type_vislib/public/components/common/validation_wrapper.tsx @@ -52,7 +52,7 @@ function ValidationWrapper({ useEffect(() => { setValidity(isPanelValid); - }, [isPanelValid]); + }, [isPanelValid, setValidity]); return ; } diff --git a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/category_axis_panel.tsx b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/category_axis_panel.tsx index a19a300960abd..c1da70f5c17c2 100644 --- a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/category_axis_panel.tsx +++ b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/category_axis_panel.tsx @@ -46,7 +46,7 @@ function CategoryAxisPanel(props: CategoryAxisPanelProps) { }; setCategoryAxis(updatedAxis); }, - [setCategoryAxis] + [setCategoryAxis, axis] ); const setPosition = useCallback( @@ -89,7 +89,7 @@ function CategoryAxisPanel(props: CategoryAxisPanelProps) { setValue={setAxis} /> - {axis.show && } + {axis.show && } ); } diff --git a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx index 944ed7e20d1f7..f172a4344c940 100644 --- a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx +++ b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.test.tsx @@ -193,9 +193,10 @@ describe('MetricsAxisOptions component', () => { const updatedSeriesParams = [{ ...chart, data: { ...chart.data, label: agg.makeLabel() } }]; const updatedValues = [{ ...axis, title: { text: agg.makeLabel() } }]; - expect(setValue).toHaveBeenCalledTimes(3); - expect(setValue).toHaveBeenNthCalledWith(2, SERIES_PARAMS, updatedSeriesParams); - expect(setValue).toHaveBeenNthCalledWith(3, VALUE_AXES, updatedValues); + expect(setValue).toHaveBeenCalledTimes(5); + expect(setValue).toHaveBeenNthCalledWith(3, SERIES_PARAMS, updatedSeriesParams); + expect(setValue).toHaveBeenNthCalledWith(5, SERIES_PARAMS, updatedSeriesParams); + expect(setValue).toHaveBeenNthCalledWith(4, VALUE_AXES, updatedValues); }); it('should not set the custom title to match the value axis label when more than one agg exists for that axis', () => { diff --git a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx index cdc8996f3fdeb..32c21008c2a3a 100644 --- a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx +++ b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx @@ -89,72 +89,85 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) } ); - const updateAxisTitle = (seriesParams?: SeriesParam[]) => { - const series = seriesParams || stateParams.seriesParams; - const axes = cloneDeep(stateParams.valueAxes); - let isAxesChanged = false; - let lastValuesChanged = false; - const lastLabels = { ...lastCustomLabels }; - const lastMatchingSeriesAgg = { ...lastSeriesAgg }; - - stateParams.valueAxes.forEach((axis, axisNumber) => { - let newCustomLabel = ''; - const matchingSeries: IAggConfig[] = []; - - series.forEach((serie, seriesIndex) => { - if ((axisNumber === 0 && !serie.valueAxis) || serie.valueAxis === axis.id) { - const aggByIndex = aggs.bySchemaName('metric')[seriesIndex]; - matchingSeries.push(aggByIndex); + const updateAxisTitle = useCallback( + (seriesParams?: SeriesParam[]) => { + const series = seriesParams || stateParams.seriesParams; + let isAxesChanged = false; + let lastValuesChanged = false; + const lastLabels = { ...lastCustomLabels }; + const lastMatchingSeriesAgg = { ...lastSeriesAgg }; + + const axes = stateParams.valueAxes.map((axis, axisNumber) => { + let newCustomLabel = ''; + let updatedAxis; + const matchingSeries: IAggConfig[] = []; + + series.forEach((serie, seriesIndex) => { + if ((axisNumber === 0 && !serie.valueAxis) || serie.valueAxis === axis.id) { + const aggByIndex = aggs.bySchemaName('metric')[seriesIndex]; + matchingSeries.push(aggByIndex); + } + }); + + if (matchingSeries.length === 1) { + // if several series matches to the axis, axis title is set according to the first serie. + newCustomLabel = matchingSeries[0].makeLabel(); } - }); - - if (matchingSeries.length === 1) { - // if several series matches to the axis, axis title is set according to the first serie. - newCustomLabel = matchingSeries[0].makeLabel(); - } - if (lastCustomLabels[axis.id] !== newCustomLabel && newCustomLabel !== '') { - const lastSeriesAggType = get(lastSeriesAgg, `${matchingSeries[0].id}.type`); - const lastSeriesAggField = get(lastSeriesAgg, `${matchingSeries[0].id}.field`); - const matchingSeriesAggType = get(matchingSeries, '[0]type.name', ''); - const matchingSeriesAggField = get(matchingSeries, '[0]params.field.name', ''); + if (lastCustomLabels[axis.id] !== newCustomLabel && newCustomLabel !== '') { + const lastSeriesAggType = get(lastSeriesAgg, `${matchingSeries[0].id}.type`); + const lastSeriesAggField = get(lastSeriesAgg, `${matchingSeries[0].id}.field`); + const matchingSeriesAggType = get(matchingSeries, '[0]type.name', ''); + const matchingSeriesAggField = get(matchingSeries, '[0]params.field.name', ''); - const aggTypeIsChanged = lastSeriesAggType !== matchingSeriesAggType; - const aggFieldIsChanged = lastSeriesAggField !== matchingSeriesAggField; + const aggTypeIsChanged = lastSeriesAggType !== matchingSeriesAggType; + const aggFieldIsChanged = lastSeriesAggField !== matchingSeriesAggField; - lastMatchingSeriesAgg[matchingSeries[0].id] = { - type: matchingSeriesAggType, - field: matchingSeriesAggField, - }; - lastLabels[axis.id] = newCustomLabel; - lastValuesChanged = true; - - if ( - Object.keys(lastCustomLabels).length !== 0 && - (aggTypeIsChanged || - aggFieldIsChanged || - axis.title.text === '' || - lastCustomLabels[axis.id] === axis.title.text) - ) { - // Override axis title with new custom label - axes[axisNumber] = { - ...axis, - title: { ...axis.title, text: newCustomLabel }, + lastMatchingSeriesAgg[matchingSeries[0].id] = { + type: matchingSeriesAggType, + field: matchingSeriesAggField, }; - isAxesChanged = true; + lastLabels[axis.id] = newCustomLabel; + lastValuesChanged = true; + + if ( + Object.keys(lastCustomLabels).length !== 0 && + (aggTypeIsChanged || + aggFieldIsChanged || + axis.title.text === '' || + lastCustomLabels[axis.id] === axis.title.text) && + newCustomLabel !== axis.title.text + ) { + // Override axis title with new custom label + updatedAxis = { + ...axis, + title: { ...axis.title, text: newCustomLabel }, + }; + isAxesChanged = true; + } } - } - }); - if (isAxesChanged) { - setValue('valueAxes', axes); - } + return updatedAxis || axis; + }); - if (lastValuesChanged) { - setLastSeriesAgg(lastMatchingSeriesAgg); - setLastCustomLabels(lastLabels); - } - }; + if (isAxesChanged) { + setValue('valueAxes', axes); + } + + if (lastValuesChanged) { + setLastSeriesAgg(lastMatchingSeriesAgg); + setLastCustomLabels(lastLabels); + } + }, + [ + aggs, + lastCustomLabels, + lastSeriesAgg, + setValue, + stateParams.seriesParams, + stateParams.valueAxes, + ] + ); const onValueAxisPositionChanged = useCallback( (index: number, value: ValueAxis['position']) => { @@ -168,7 +181,7 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) }; setValue('valueAxes', valueAxes); }, - [stateParams.valueAxes, getUpdatedAxisName, setValue] + [stateParams.valueAxes, setValue] ); const onCategoryAxisPositionChanged = useCallback( @@ -226,7 +239,7 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) setValue('grid', { ...stateParams.grid, valueAxis: undefined }); } }, - [stateParams.seriesParams, stateParams.valueAxes, setValue] + [stateParams.seriesParams, stateParams.valueAxes, setValue, stateParams.grid] ); const changeValueAxis: ChangeValueAxis = useCallback( @@ -241,13 +254,13 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) updateAxisTitle(); }, - [addValueAxis, setParamByIndex] + [addValueAxis, setParamByIndex, updateAxisTitle] ); + const schemaName = vis.type.schemas.metrics[0].name; const metrics = useMemo(() => { - const schemaName = vis.type.schemas.metrics[0].name; return aggs.bySchemaName(schemaName); - }, [vis.type.schemas.metrics[0].name, aggs]); + }, [schemaName, aggs]); const firstValueAxesId = stateParams.valueAxes[0].id; @@ -278,7 +291,7 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) setValue('seriesParams', updatedSeries); updateAxisTitle(updatedSeries); - }, [metrics, firstValueAxesId]); + }, [metrics, firstValueAxesId, setValue, stateParams.seriesParams, updateAxisTitle]); const visType = useMemo(() => { const types = uniq(stateParams.seriesParams.map(({ type }) => type)); diff --git a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axes_panel.tsx b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axes_panel.tsx index b94f5ebbcce44..4aa2aee083a67 100644 --- a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axes_panel.tsx +++ b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axes_panel.tsx @@ -78,7 +78,7 @@ function ValueAxesPanel(props: ValueAxesPanelProps) { /> ), - [removeValueAxis] + [removeValueAxis, removeButtonTooltip] ); const addButtonTooltip = useMemo( diff --git a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axis_options.tsx b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axis_options.tsx index 0ebe62a70a7b1..d094a1d422385 100644 --- a/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axis_options.tsx +++ b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/value_axis_options.tsx @@ -175,7 +175,7 @@ function ValueAxisOptions(props: ValueAxisOptionsParams) { setValue={setValueAxisTitle} /> - + ) : ( @@ -204,7 +204,6 @@ function ValueAxisOptions(props: ValueAxisOptionsParams) { <>