Skip to content

Commit

Permalink
Barchart: Fix percent stacking regression (#79903)
Browse files Browse the repository at this point in the history
Co-authored-by: Leon Sorokin <leeoniya@gmail.com>
  • Loading branch information
2 people authored and zserge committed Jan 19, 2024
1 parent 95559de commit 278912b
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 7 deletions.
2 changes: 1 addition & 1 deletion public/app/plugins/panel/barchart/BarChartPanel.tsx
Expand Up @@ -213,7 +213,7 @@ export const BarChartPanel = ({ data, options, fieldConfig, width, height, timeZ
}
}

return <PlotLegend data={info.viz} config={config} maxHeight="35%" maxWidth="60%" {...options.legend} />;
return <PlotLegend data={[info.legend]} config={config} maxHeight="35%" maxWidth="60%" {...options.legend} />;
};

const rawValue = (seriesIdx: number, valueIdx: number) => {
Expand Down
6 changes: 6 additions & 0 deletions public/app/plugins/panel/barchart/types.ts
Expand Up @@ -10,6 +10,12 @@ export interface BarChartDisplayValues {
*/
viz: [DataFrame];

/**
* The fields we can display, first field is X axis.
* Contains same data as viz, but without config modifications (e.g: unit override)
*/
legend: DataFrame;

/** Potentialy color by a field value */
colorByField?: Field;
}
Expand Down
17 changes: 15 additions & 2 deletions public/app/plugins/panel/barchart/utils.test.ts
Expand Up @@ -227,6 +227,19 @@ describe('BarChart utils', () => {
null,
]
`);

const displayLegendValuesAsc = assertIsDefined('legend' in result ? result : null).legend;
const legendField = displayLegendValuesAsc.fields[1];

expect(legendField.values).toMatchInlineSnapshot(`
[
-10,
null,
10,
null,
null,
]
`);
});

it('should remove unit from legend values when stacking is percent', () => {
Expand All @@ -242,11 +255,11 @@ describe('BarChart utils', () => {
const resultAsc = prepareBarChartDisplayValues([frame], createTheme(), {
stacking: StackingMode.Percent,
} as Options);
const displayLegendValuesAsc = assertIsDefined('viz' in resultAsc ? resultAsc : null).viz[0];
const displayLegendValuesAsc = assertIsDefined('legend' in resultAsc ? resultAsc : null).legend;

expect(displayLegendValuesAsc.fields[0].config.unit).toBeUndefined();
expect(displayLegendValuesAsc.fields[1].config.unit).toBeUndefined();
expect(displayLegendValuesAsc.fields[2].config.unit).toBeUndefined();
expect(displayLegendValuesAsc.fields[3].config.unit).toBeUndefined();
});
});
});
15 changes: 11 additions & 4 deletions public/app/plugins/panel/barchart/utils.ts
@@ -1,3 +1,4 @@
import { cloneDeep } from 'lodash';
import uPlot, { Padding } from 'uplot';

import {
Expand Down Expand Up @@ -491,9 +492,10 @@ export function prepareBarChartDisplayValues(
}
}

// If stacking is percent, we need to correct the fields unit and display
// If stacking is percent, we need to correct the legend fields unit and display
let legendFields: Field[] = cloneDeep(fields);
if (options.stacking === StackingMode.Percent) {
fields.map((field) => {
legendFields.map((field) => {
const alignedFrameField = frame.fields.find(
(f) => getFieldDisplayName(f, frame) === getFieldDisplayName(f, frame)
);
Expand All @@ -503,18 +505,23 @@ export function prepareBarChartDisplayValues(
});
}

// String field is first
// String field is first, make sure fields / legend fields indexes match
fields.unshift(firstField);
legendFields.unshift(firstField);

return {
aligned: frame,
colorByField,
viz: [
{
length: firstField.values.length,
fields: fields, // ideally: fields.filter((f) => !Boolean(f.config.custom?.hideFrom?.viz)),
length: firstField.values.length,
},
],
legend: {
fields: legendFields,
length: firstField.values.length,
},
};
}

Expand Down

0 comments on commit 278912b

Please sign in to comment.