From b664bc95ed2759ba37842557a2fcc14b424d92fc Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Thu, 19 Oct 2023 17:17:13 +0200 Subject: [PATCH] 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); + }); + }); });