From 51d7dabcdfa1a33744bda70b0f042f78ea46c1b6 Mon Sep 17 00:00:00 2001 From: MV88 Date: Tue, 17 Oct 2023 16:38:42 +0200 Subject: [PATCH 1/4] Fix #9617 legends with percent for pie chart --- web/client/components/charts/WidgetChart.jsx | 10 +++++++--- .../builder/wizard/common/PieChartAdvancedOptions.jsx | 10 ++++++++++ web/client/components/widgets/view/WidgetsView.jsx | 4 ++-- web/client/translations/data.de-DE.json | 1 + web/client/translations/data.en-US.json | 1 + web/client/translations/data.es-ES.json | 1 + web/client/translations/data.fr-FR.json | 1 + web/client/translations/data.it-IT.json | 1 + 8 files changed, 24 insertions(+), 5 deletions(-) diff --git a/web/client/components/charts/WidgetChart.jsx b/web/client/components/charts/WidgetChart.jsx index 87dd81c631..18f15745f8 100644 --- a/web/client/components/charts/WidgetChart.jsx +++ b/web/client/components/charts/WidgetChart.jsx @@ -1,5 +1,5 @@ import React, { Suspense } from 'react'; -import {every, includes, isNumber, isString, union, orderBy, flatten} from 'lodash'; +import {round, every, includes, isNumber, isString, union, orderBy, flatten} from 'lodash'; import LoadingView from '../misc/LoadingView'; import { sameToneRangeColors } from '../../utils/ColorUtils'; @@ -306,6 +306,9 @@ function getData({ values: y, pull: 0.005 }; + const total = y.reduce((p, c) => { + return p + c; + }, 0); /* pie chart is classified colored */ if (classificationType !== 'default' && classificationColors.length) { const legendLabels = classifications.map((item, index) => { @@ -319,7 +322,7 @@ function getData({ }); pieChartTrace = { ...pieChartTrace, - labels: legendLabels, + labels: !yAxisOpts?.includeLegendPercent ? legendLabels : legendLabels.map((v, i) => v + " - " + round(y[i] / total, 2) + " %"), marker: {colors: classificationColors} }; return pieChartTrace; @@ -328,7 +331,7 @@ function getData({ return { ...(yDataKey && { legendgroup: yDataKey }), ...pieChartTrace, - labels: x, + labels: !yAxisOpts?.includeLegendPercent ? x : x.map((v, i) => v + " - " + round(y[i] / total * 100, 2) + "%"), ...(customColorEnabled ? { marker: {colors: x.reduce((acc) => ([...acc, autoColorOptions?.defaultCustomColor || '#0888A1']), [])} } : {}) }; @@ -541,6 +544,7 @@ export const toPlotly = (props) => { * @prop {string} [yAxisOpts.format] format for y axis value. See {@link https://d3-wiki.readthedocs.io/zh_CN/master/Formatting/} * @prop {string} [yAxisOpts.tickPrefix] the prefix on y value * @prop {string} [yAxisOpts.tickSuffix] the suffix of y value. + * @prop {boolean} [yAxisOpts.includeLegendPercent] if true, it adds the % on the label legend * @prop {string} [formula] a formula to calculate the final value * @prop {string} [yAxisLabel] the label of yAxis, to show in the legend * @prop {boolean} [cartesian] show the cartesian grid behind the chart diff --git a/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx b/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx index 74c64b9d5a..d00169517f 100644 --- a/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx +++ b/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx @@ -44,6 +44,16 @@ function PieChartAdvancedOptions({ onChange={(val) => { onChange("yAxisOpts.textinfo", val ? "none" : null); }} /> + + + + + { onChange("yAxisOpts.includeLegendPercent", val); }} + /> + + ); } diff --git a/web/client/components/widgets/view/WidgetsView.jsx b/web/client/components/widgets/view/WidgetsView.jsx index 68c76efaa0..453ed03409 100644 --- a/web/client/components/widgets/view/WidgetsView.jsx +++ b/web/client/components/widgets/view/WidgetsView.jsx @@ -33,7 +33,7 @@ export default pure(({ widgets = [], layouts, dependencies, - verticalCompact = false, + compactType = null, compactMode, useDefaultWidthProvider = true, measureBeforeMount, @@ -75,7 +75,7 @@ export default pure(({ // TODO: this prop triggers a deprecation warning // we should remove it keeping the current behavior // a user should be able to move cards everywhere without force cards on first row - verticalCompact={verticalCompact} + compactType={compactType} compactMode={compactMode} breakpoints={breakpoints} cols={cols} diff --git a/web/client/translations/data.de-DE.json b/web/client/translations/data.de-DE.json index 72d655d4c4..330d159d67 100644 --- a/web/client/translations/data.de-DE.json +++ b/web/client/translations/data.de-DE.json @@ -2161,6 +2161,7 @@ "xAxis": "X-Achse", "xAxisAngle": "Beschriftungsdrehung", "hideLabels": "Beschriftung ausblenden", + "includeLegendPercent": "Include percentages in legend", "format": "Format", "prefix": "Präfix", "suffix": "Suffix", diff --git a/web/client/translations/data.en-US.json b/web/client/translations/data.en-US.json index d7f1b70bc0..4d40a4e5e4 100644 --- a/web/client/translations/data.en-US.json +++ b/web/client/translations/data.en-US.json @@ -2124,6 +2124,7 @@ "xAxis": "X axis", "xAxisAngle": "Label rotation", "hideLabels": "Hide labels", + "includeLegendPercent": "Include percentages in legend", "format": "Format", "prefix": "Prefix", "suffix": "Suffix", diff --git a/web/client/translations/data.es-ES.json b/web/client/translations/data.es-ES.json index 344b80b0c5..07ee358d46 100644 --- a/web/client/translations/data.es-ES.json +++ b/web/client/translations/data.es-ES.json @@ -2124,6 +2124,7 @@ "xAxis": "Eje X", "xAxisAngle": "Rotación de etiquetas", "hideLabels": "Ocultar etiquetas", + "includeLegendPercent": "Include percentages in legend", "format": "Formato", "prefix": "Prefijo", "suffix": "Sufijo", diff --git a/web/client/translations/data.fr-FR.json b/web/client/translations/data.fr-FR.json index bf60b12b8c..3a7b79609e 100644 --- a/web/client/translations/data.fr-FR.json +++ b/web/client/translations/data.fr-FR.json @@ -2124,6 +2124,7 @@ "xAxis": "Axe X", "xAxisAngle": "Rotation des étiquettes", "hideLabels": "Masquer les libellés", + "includeLegendPercent": "Include percentages in legend", "format": "Format", "prefix": "Prefix", "suffix": "Suffixe", diff --git a/web/client/translations/data.it-IT.json b/web/client/translations/data.it-IT.json index 8e0412c991..ab17525d3f 100644 --- a/web/client/translations/data.it-IT.json +++ b/web/client/translations/data.it-IT.json @@ -2125,6 +2125,7 @@ "xAxis": "Asse X", "xAxisAngle": "Rotazione etichetta", "hideLabels": "Nascondi etichette", + "includeLegendPercent": "Includi percentuali nella legenda", "format": "Formato", "prefix": "Prefisso", "suffix": "Suffisso", From 69188ccef39fa75114b3eb187b7f48721a189dce Mon Sep 17 00:00:00 2001 From: MV88 Date: Tue, 17 Oct 2023 16:44:08 +0200 Subject: [PATCH 2/4] fix checked status --- .../widgets/builder/wizard/common/PieChartAdvancedOptions.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx b/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx index d00169517f..760af867aa 100644 --- a/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx +++ b/web/client/components/widgets/builder/wizard/common/PieChartAdvancedOptions.jsx @@ -49,7 +49,7 @@ function PieChartAdvancedOptions({ { onChange("yAxisOpts.includeLegendPercent", val); }} /> From b664bc95ed2759ba37842557a2fcc14b424d92fc Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Thu, 19 Oct 2023 17:17:13 +0200 Subject: [PATCH 3/4] Various minor fixes to the PR and add tests --- web/client/components/charts/WidgetChart.jsx | 48 +++++++++++++++---- .../charts/__tests__/WidgetChart-test.js | 16 ++++++- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/web/client/components/charts/WidgetChart.jsx b/web/client/components/charts/WidgetChart.jsx index 18f15745f8..4a7be487bf 100644 --- a/web/client/components/charts/WidgetChart.jsx +++ b/web/client/components/charts/WidgetChart.jsx @@ -1,5 +1,5 @@ import React, { Suspense } from 'react'; -import {round, every, includes, isNumber, isString, union, orderBy, flatten} from 'lodash'; +import {isNull, every, includes, isNumber, isString, union, orderBy, flatten} from 'lodash'; import LoadingView from '../misc/LoadingView'; import { sameToneRangeColors } from '../../utils/ColorUtils'; @@ -25,7 +25,38 @@ export const defaultColorGenerator = (total, colorOptions) => { const { base, range, ...opts } = colorOptions; return (sameToneRangeColors(base, range, total + 1, opts) || [0]).slice(1); }; - +// +/** + * Returns the labels for the pie chart, adds % to the labels, for legend, if the prop `includeLegendPercent` is true + * @param {string|number[]} keys the values of the chart ["California", "Ohio", ...] + * @param {number[]} y array of values to be used to calculate the percentage of the label + * @param {*} opts.includeLegendPercent if true, it adds the % on the label legend + * @returns {string[]} the labels for the pie chart + */ +export const renderLabels = (keys = [], y = [], {includeLegendPercent} = {}) => { + if (!includeLegendPercent) { + return keys; + } + const total = y.reduce((p, c) => { + if (isNumber(c) && !isNaN(p)) { + return p + c; + } + return p; + }, 0); + if (includeLegendPercent && isNumber(total) && total !== 0) { + return keys.map((v, i) => { + if (!isNull(y[i])) { // avoid implicit conversion of null to 0 + const percent = (y[i] / total * 100).toPrecision(3); // use precision to be consistent with formatting of plotlyJS (3 digits) + if (!isNaN(percent)) { + return v + " - " + percent + "%"; + } + } + return v; + }); + } + // prevent cases when division by zero + return keys; +}; /** * Checks the parameters and return the color classification type for the chart. Classification type can be: * - 'value' for classifications based on exact value. Used if the type of `classificationAttr` is "string". @@ -294,10 +325,11 @@ function getData({ if (formula) { y = preProcessValues(formula, y); } - + // if includeLegendPercent is true, the percent is already included in the label + const percentHover = !yAxisOpts?.includeLegendPercent ? '%{percent}' : ''; let pieChartTrace = { name: yAxisLabel || yDataKey, - hovertemplate: `%{label}
${yDataKey}
${yAxisOpts?.tickPrefix ?? ""}%{value${yAxisOpts?.format ? `:${yAxisOpts?.format}` : ''}}${yAxisOpts?.tickSuffix ?? ""}
%{percent}`, + hovertemplate: `%{label}
${yDataKey}
${yAxisOpts?.tickPrefix ?? ""}%{value${yAxisOpts?.format ? `:${yAxisOpts?.format}` : ''}}${yAxisOpts?.tickSuffix ?? ""}
${percentHover}`, type, textinfo: yAxisOpts?.textinfo, // hide labels with textinfo = "none", in this case we have to omit texttemplate which would win over this. @@ -306,9 +338,7 @@ function getData({ values: y, pull: 0.005 }; - const total = y.reduce((p, c) => { - return p + c; - }, 0); + /* pie chart is classified colored */ if (classificationType !== 'default' && classificationColors.length) { const legendLabels = classifications.map((item, index) => { @@ -322,7 +352,7 @@ function getData({ }); pieChartTrace = { ...pieChartTrace, - labels: !yAxisOpts?.includeLegendPercent ? legendLabels : legendLabels.map((v, i) => v + " - " + round(y[i] / total, 2) + " %"), + labels: renderLabels(legendLabels, y, yAxisOpts), marker: {colors: classificationColors} }; return pieChartTrace; @@ -331,7 +361,7 @@ function getData({ return { ...(yDataKey && { legendgroup: yDataKey }), ...pieChartTrace, - labels: !yAxisOpts?.includeLegendPercent ? x : x.map((v, i) => v + " - " + round(y[i] / total * 100, 2) + "%"), + labels: renderLabels(x, y, yAxisOpts), ...(customColorEnabled ? { marker: {colors: x.reduce((acc) => ([...acc, autoColorOptions?.defaultCustomColor || '#0888A1']), [])} } : {}) }; diff --git a/web/client/components/charts/__tests__/WidgetChart-test.js b/web/client/components/charts/__tests__/WidgetChart-test.js index 1029f317ff..ad004b91aa 100644 --- a/web/client/components/charts/__tests__/WidgetChart-test.js +++ b/web/client/components/charts/__tests__/WidgetChart-test.js @@ -27,7 +27,7 @@ import { DATASET_WITH_DATES, SPLIT_DATASET_4 } from './sample_data'; -import WidgetChart, { toPlotly, defaultColorGenerator, COLOR_DEFAULTS } from '../WidgetChart'; +import WidgetChart, { toPlotly, defaultColorGenerator, COLOR_DEFAULTS, renderLabels } from '../WidgetChart'; describe('WidgetChart', () => { beforeEach((done) => { @@ -902,4 +902,18 @@ describe('Widget Chart: data conversions ', () => { expect(layout.xaxis.tickangle).toEqual('auto'); }); }); + it('renderLabels', () => { + const tests = [ + [["a", "b", "c"], [3, 5, 2], ['a - 30.0%', 'b - 50.0%', 'c - 20.0%']], + [["a", "b", "c"], [3, 7], ['a - 30.0%', 'b - 70.0%', 'c']], // handle undefined values + [["a", "b", "c"], [3, 7, null], ['a - 30.0%', 'b - 70.0%', 'c']], // handle null values + [["a", "b", "c"], [0, 0, 0], ["a", "b", "c"]], // handle 0 sum + [["a", "b", "c"], [0, 0, 1], ['a - 0.00%', 'b - 0.00%', 'c - 100%']], // check 100% rendering + [["a", "b", "c"], [1, 1, 1], ['a - 33.3%', 'b - 33.3%', 'c - 33.3%']] // check 3 digits rendering + ]; + tests.forEach(([keys, values, expectedPercent]) => { + expect(renderLabels(keys, values)).toEqual(keys); // remains the same + expect(renderLabels(keys, values, {includeLegendPercent: true})).toEqual(expectedPercent); + }); + }); }); From 5bd1c079b585ceeab3e1c156da631c38118bae14 Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Thu, 19 Oct 2023 17:21:01 +0200 Subject: [PATCH 4/4] Update web/client/components/charts/WidgetChart.jsx --- web/client/components/charts/WidgetChart.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/client/components/charts/WidgetChart.jsx b/web/client/components/charts/WidgetChart.jsx index 4a7be487bf..0f81891b9d 100644 --- a/web/client/components/charts/WidgetChart.jsx +++ b/web/client/components/charts/WidgetChart.jsx @@ -30,7 +30,7 @@ export const defaultColorGenerator = (total, colorOptions) => { * Returns the labels for the pie chart, adds % to the labels, for legend, if the prop `includeLegendPercent` is true * @param {string|number[]} keys the values of the chart ["California", "Ohio", ...] * @param {number[]} y array of values to be used to calculate the percentage of the label - * @param {*} opts.includeLegendPercent if true, it adds the % on the label legend + * @param {boolean} opts.includeLegendPercent if true, it adds the % on the label legend * @returns {string[]} the labels for the pie chart */ export const renderLabels = (keys = [], y = [], {includeLegendPercent} = {}) => {