From 41fccfd801e0927fe037d9bd1ec7ce731cde510f Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Mon, 24 Feb 2020 17:19:08 +0300 Subject: [PATCH 1/5] Fix TS for vislib --- .eslintrc.js | 12 -- .../components/common/validation_wrapper.tsx | 2 +- .../metrics_axes/category_axis_panel.tsx | 4 +- .../components/options/metrics_axes/index.tsx | 138 ++++++++++-------- .../options/metrics_axes/value_axes_panel.tsx | 2 +- .../metrics_axes/value_axis_options.tsx | 3 +- .../options/point_series/grid_panel.tsx | 2 +- 7 files changed, 80 insertions(+), 83 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 9b00135df5bac..a678243e4f07a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -76,18 +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_type_table/**/*.{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.tsx b/src/legacy/core_plugins/vis_type_vislib/public/components/options/metrics_axes/index.tsx index cdc8996f3fdeb..40c43225db131 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,82 @@ 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; + 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); + } + }); + + 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) + ) { + // Override axis title with new custom label + axes[axisNumber] = { + ...axis, + title: { ...axis.title, text: newCustomLabel }, + }; + isAxesChanged = true; + } } - } - }); + }); - if (isAxesChanged) { - setValue('valueAxes', axes); - } + if (isAxesChanged) { + setValue('valueAxes', axes); + } - if (lastValuesChanged) { - setLastSeriesAgg(lastMatchingSeriesAgg); - setLastCustomLabels(lastLabels); - } - }; + if (lastValuesChanged) { + setLastSeriesAgg(lastMatchingSeriesAgg); + setLastCustomLabels(lastLabels); + } + }, + [ + aggs, + lastCustomLabels, + lastSeriesAgg, + setValue, + stateParams.seriesParams, + stateParams.valueAxes, + ] + ); const onValueAxisPositionChanged = useCallback( (index: number, value: ValueAxis['position']) => { @@ -168,7 +178,7 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) }; setValue('valueAxes', valueAxes); }, - [stateParams.valueAxes, getUpdatedAxisName, setValue] + [stateParams.valueAxes, setValue] ); const onCategoryAxisPositionChanged = useCallback( @@ -226,7 +236,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 +251,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 +288,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) { <> From 16a3570065b1b204976cfe582deff9fe1b2b913b Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Mon, 24 Feb 2020 17:51:36 +0300 Subject: [PATCH 2/5] Fix TS --- .../vis_type_table/public/components/table_vis_options.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx b/src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx index 5729618b6ae07..290e29035c12b 100644 --- a/src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx +++ b/src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx @@ -47,14 +47,14 @@ function TableOptions({ .filter(col => get(col.aggConfig.type.getFormat(col.aggConfig), 'type.id') === 'number') .map(({ name }) => ({ value: name, text: name })), ], - [aggs, stateParams.percentageCol, stateParams.dimensions] + [aggs] ); const isPerPageValid = stateParams.perPage === '' || stateParams.perPage > 0; useEffect(() => { setValidity(isPerPageValid); - }, [isPerPageValid]); + }, [isPerPageValid, setValidity]); useEffect(() => { if ( @@ -64,7 +64,7 @@ function TableOptions({ ) { setValue('percentageCol', percentageColumns[0].value); } - }, [percentageColumns, stateParams.percentageCol]); + }, [percentageColumns, stateParams.percentageCol, setValue]); return ( From bbd3e16f98d8135e2de59c6c86cb8329a3ab23f7 Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Tue, 25 Feb 2020 16:17:27 +0300 Subject: [PATCH 3/5] Revert table changes --- .eslintrc.js | 6 ++++++ .../vis_type_table/public/components/table_vis_options.tsx | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index a678243e4f07a..12e5c68ac6c17 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -76,6 +76,12 @@ module.exports = { 'react-hooks/exhaustive-deps': 'off', }, }, + { + files: ['src/legacy/core_plugins/vis_type_table/**/*.{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_table/public/components/table_vis_options.tsx b/src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx index 290e29035c12b..5729618b6ae07 100644 --- a/src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx +++ b/src/legacy/core_plugins/vis_type_table/public/components/table_vis_options.tsx @@ -47,14 +47,14 @@ function TableOptions({ .filter(col => get(col.aggConfig.type.getFormat(col.aggConfig), 'type.id') === 'number') .map(({ name }) => ({ value: name, text: name })), ], - [aggs] + [aggs, stateParams.percentageCol, stateParams.dimensions] ); const isPerPageValid = stateParams.perPage === '' || stateParams.perPage > 0; useEffect(() => { setValidity(isPerPageValid); - }, [isPerPageValid, setValidity]); + }, [isPerPageValid]); useEffect(() => { if ( @@ -64,7 +64,7 @@ function TableOptions({ ) { setValue('percentageCol', percentageColumns[0].value); } - }, [percentageColumns, stateParams.percentageCol, setValue]); + }, [percentageColumns, stateParams.percentageCol]); return ( From f98b0566bd28bcfcafbcdedb7cd40252ce54ef17 Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Tue, 25 Feb 2020 17:47:05 +0300 Subject: [PATCH 4/5] Update unit test --- .../public/components/options/metrics_axes/index.test.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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', () => { From 45a7a215835f749e51e43e00d33812c013e4f9bd Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Tue, 3 Mar 2020 16:40:18 +0300 Subject: [PATCH 5/5] Refactor updateAxisTitle --- .../public/components/options/metrics_axes/index.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) 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 40c43225db131..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 @@ -92,14 +92,14 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) const updateAxisTitle = useCallback( (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) => { + const axes = stateParams.valueAxes.map((axis, axisNumber) => { let newCustomLabel = ''; + let updatedAxis; const matchingSeries: IAggConfig[] = []; series.forEach((serie, seriesIndex) => { @@ -135,16 +135,19 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps) (aggTypeIsChanged || aggFieldIsChanged || axis.title.text === '' || - lastCustomLabels[axis.id] === axis.title.text) + lastCustomLabels[axis.id] === axis.title.text) && + newCustomLabel !== axis.title.text ) { // Override axis title with new custom label - axes[axisNumber] = { + updatedAxis = { ...axis, title: { ...axis.title, text: newCustomLabel }, }; isAxesChanged = true; } } + + return updatedAxis || axis; }); if (isAxesChanged) {