Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #9617 legends with percent for pie chart #9618

Merged
merged 4 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading