Skip to content

Commit

Permalink
Fix #9617 legends with percent for pie chart (#9618)
Browse files Browse the repository at this point in the history
* Fix #9617 legends with percent for pie chart
  • Loading branch information
MV88 authored Oct 19, 2023
1 parent 57088f5 commit e50eecd
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 9 deletions.
46 changes: 40 additions & 6 deletions web/client/components/charts/WidgetChart.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { Suspense } from 'react';
import {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';
Expand All @@ -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 {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} = {}) => {
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".
Expand Down Expand Up @@ -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}<br>${yDataKey}<br>${yAxisOpts?.tickPrefix ?? ""}%{value${yAxisOpts?.format ? `:${yAxisOpts?.format}` : ''}}${yAxisOpts?.tickSuffix ?? ""}<br>%{percent}<extra></extra>`,
hovertemplate: `%{label}<br>${yDataKey}<br>${yAxisOpts?.tickPrefix ?? ""}%{value${yAxisOpts?.format ? `:${yAxisOpts?.format}` : ''}}${yAxisOpts?.tickSuffix ?? ""}<br>${percentHover}<extra></extra>`,
type,
textinfo: yAxisOpts?.textinfo,
// hide labels with textinfo = "none", in this case we have to omit texttemplate which would win over this.
Expand All @@ -306,6 +338,7 @@ function getData({
values: y,
pull: 0.005
};

/* pie chart is classified colored */
if (classificationType !== 'default' && classificationColors.length) {
const legendLabels = classifications.map((item, index) => {
Expand All @@ -319,7 +352,7 @@ function getData({
});
pieChartTrace = {
...pieChartTrace,
labels: legendLabels,
labels: renderLabels(legendLabels, y, yAxisOpts),
marker: {colors: classificationColors}
};
return pieChartTrace;
Expand All @@ -328,7 +361,7 @@ function getData({
return {
...(yDataKey && { legendgroup: yDataKey }),
...pieChartTrace,
labels: x,
labels: renderLabels(x, y, yAxisOpts),
...(customColorEnabled ? { marker: {colors: x.reduce((acc) => ([...acc, autoColorOptions?.defaultCustomColor || '#0888A1']), [])} } : {})
};

Expand Down Expand Up @@ -541,6 +574,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
Expand Down
16 changes: 15 additions & 1 deletion web/client/components/charts/__tests__/WidgetChart-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ function PieChartAdvancedOptions({
onChange={(val) => { onChange("yAxisOpts.textinfo", val ? "none" : null); }}
/>
</Col>
<Col componentClass={ControlLabel} sm={6}>
<Message msgId="widgets.advanced.includeLegendPercent" />
</Col>
<Col sm={6}>
<SwitchButton
checked={data?.yAxisOpts?.includeLegendPercent ?? false}
onChange={(val) => { onChange("yAxisOpts.includeLegendPercent", val); }}
/>
</Col>

</FormGroup>
</SwitchPanel>);
}
Expand Down
4 changes: 2 additions & 2 deletions web/client/components/widgets/view/WidgetsView.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default pure(({
widgets = [],
layouts,
dependencies,
verticalCompact = false,
compactType = null,
compactMode,
useDefaultWidthProvider = true,
measureBeforeMount,
Expand Down Expand Up @@ -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}
Expand Down
1 change: 1 addition & 0 deletions web/client/translations/data.de-DE.json
Original file line number Diff line number Diff line change
Expand Up @@ -2161,6 +2161,7 @@
"xAxis": "X-Achse",
"xAxisAngle": "Beschriftungsdrehung",
"hideLabels": "Beschriftung ausblenden",
"includeLegendPercent": "Include percentages in legend",
"format": "Format",
"prefix": "Präfix",
"suffix": "Suffix",
Expand Down
1 change: 1 addition & 0 deletions web/client/translations/data.en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -2124,6 +2124,7 @@
"xAxis": "X axis",
"xAxisAngle": "Label rotation",
"hideLabels": "Hide labels",
"includeLegendPercent": "Include percentages in legend",
"format": "Format",
"prefix": "Prefix",
"suffix": "Suffix",
Expand Down
1 change: 1 addition & 0 deletions web/client/translations/data.es-ES.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions web/client/translations/data.fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions web/client/translations/data.it-IT.json
Original file line number Diff line number Diff line change
Expand Up @@ -2125,6 +2125,7 @@
"xAxis": "Asse X",
"xAxisAngle": "Rotazione etichetta",
"hideLabels": "Nascondi etichette",
"includeLegendPercent": "Includi percentuali nella legenda",
"format": "Formato",
"prefix": "Prefisso",
"suffix": "Suffisso",
Expand Down

0 comments on commit e50eecd

Please sign in to comment.