This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 6a83b6fd87 fix(pie chart): Total now positioned correctly with all
Legend positions, and respects theming (#34435)
6a83b6fd87 is described below
commit 6a83b6fd8782758666655191f9c962692b3096fc
Author: Evan Rusackas <[email protected]>
AuthorDate: Fri Aug 1 12:00:23 2025 -0700
fix(pie chart): Total now positioned correctly with all Legend positions,
and respects theming (#34435)
---
.../plugin-chart-echarts/src/Pie/transformProps.ts | 29 ++--
.../test/Pie/transformProps.test.ts | 151 +++++++++++++++++++++
2 files changed, 166 insertions(+), 14 deletions(-)
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts
index c0ec8f9242..e3980a02b3 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts
@@ -95,27 +95,27 @@ function getTotalValuePadding({
top: donut ? 'middle' : '0',
left: 'center',
};
- const LEGEND_HEIGHT = 15;
- const LEGEND_WIDTH = 215;
if (chartPadding.top) {
padding.top = donut
- ? `${50 + ((chartPadding.top - LEGEND_HEIGHT) / height / 2) * 100}%`
- : `${((chartPadding.top + LEGEND_HEIGHT) / height) * 100}%`;
+ ? `${50 + (chartPadding.top / height / 2) * 100}%`
+ : `${(chartPadding.top / height) * 100}%`;
}
if (chartPadding.bottom) {
padding.top = donut
- ? `${50 - ((chartPadding.bottom + LEGEND_HEIGHT) / height / 2) * 100}%`
+ ? `${50 - (chartPadding.bottom / height / 2) * 100}%`
: '0';
}
if (chartPadding.left) {
- padding.left = `${
- 50 + ((chartPadding.left - LEGEND_WIDTH) / width / 2) * 100
- }%`;
+ // When legend is on the left, shift text right to center it in the
available space
+ const leftPaddingPercent = (chartPadding.left / width) * 100;
+ const adjustedLeftPercent = 50 + leftPaddingPercent * 0.25;
+ padding.left = `${adjustedLeftPercent}%`;
}
if (chartPadding.right) {
- padding.left = `${
- 50 - ((chartPadding.right + LEGEND_WIDTH) / width / 2) * 100
- }%`;
+ // When legend is on the right, shift text left to center it in the
available space
+ const rightPaddingPercent = (chartPadding.right / width) * 100;
+ const adjustedLeftPercent = 50 - rightPaddingPercent * 0.75;
+ padding.left = `${adjustedLeftPercent}%`;
}
return padding;
}
@@ -220,7 +220,7 @@ export default function transformProps(
name: otherName,
value: otherSum,
itemStyle: {
- color: theme.colors.grayscale.dark1,
+ color: theme.colorText,
opacity:
filterState.selectedValues &&
!filterState.selectedValues.includes(otherName)
@@ -368,7 +368,7 @@ export default function transformProps(
const defaultLabel = {
formatter,
show: showLabels,
- color: theme.colors.grayscale.dark2,
+ color: theme.colorText,
};
const chartPadding = getChartPadding(
@@ -403,7 +403,7 @@ export default function transformProps(
label: {
show: true,
fontWeight: 'bold',
- backgroundColor: theme.colors.grayscale.light5,
+ backgroundColor: theme.colorBgContainer,
},
},
data: transformedData,
@@ -445,6 +445,7 @@ export default function transformProps(
text: t('Total: %s', numberFormatter(totalValue)),
fontSize: 16,
fontWeight: 'bold',
+ fill: theme.colorText,
},
z: 10,
}
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts
b/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts
index 36fefc1f49..087f1f4929 100644
---
a/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts
+++
b/superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts
@@ -221,6 +221,157 @@ describe('Pie label string template', () => {
});
});
+describe('Total value positioning with legends', () => {
+ const getChartPropsWithLegend = (
+ showTotal = true,
+ showLegend = true,
+ legendOrientation = 'right',
+ donut = true,
+ ): EchartsPieChartProps => {
+ const formData: SqlaFormData = {
+ colorScheme: 'bnbColors',
+ datasource: '3__table',
+ granularity_sqla: 'ds',
+ metric: 'sum__num',
+ groupby: ['category'],
+ viz_type: 'pie',
+ show_total: showTotal,
+ show_legend: showLegend,
+ legend_orientation: legendOrientation,
+ donut,
+ };
+
+ return new ChartProps({
+ formData,
+ width: 800,
+ height: 600,
+ queriesData: [
+ {
+ data: [
+ { category: 'A', sum__num: 10, sum__num__contribution: 0.4 },
+ { category: 'B', sum__num: 15, sum__num__contribution: 0.6 },
+ ],
+ },
+ ],
+ theme: supersetTheme,
+ }) as EchartsPieChartProps;
+ };
+
+ it('should center total text when legend is on the right', () => {
+ const props = getChartPropsWithLegend(true, true, 'right', true);
+ const transformed = transformProps(props);
+
+ expect(transformed.echartOptions.graphic).toEqual(
+ expect.objectContaining({
+ type: 'text',
+ left: expect.stringMatching(/^\d+(\.\d+)?%$/),
+ top: 'middle',
+ style: expect.objectContaining({
+ text: expect.stringContaining('Total:'),
+ }),
+ }),
+ );
+
+ // The left position should be less than 50% (shifted left)
+ const leftValue = parseFloat(
+ (transformed.echartOptions.graphic as any).left.replace('%', ''),
+ );
+ expect(leftValue).toBeLessThan(50);
+ expect(leftValue).toBeGreaterThan(30); // Should be reasonable positioning
+ });
+
+ it('should center total text when legend is on the left', () => {
+ const props = getChartPropsWithLegend(true, true, 'left', true);
+ const transformed = transformProps(props);
+
+ expect(transformed.echartOptions.graphic).toEqual(
+ expect.objectContaining({
+ type: 'text',
+ left: expect.stringMatching(/^\d+(\.\d+)?%$/),
+ top: 'middle',
+ }),
+ );
+
+ // The left position should be greater than 50% (shifted right)
+ const leftValue = parseFloat(
+ (transformed.echartOptions.graphic as any).left.replace('%', ''),
+ );
+ expect(leftValue).toBeGreaterThan(50);
+ expect(leftValue).toBeLessThan(70); // Should be reasonable positioning
+ });
+
+ it('should center total text when legend is on top', () => {
+ const props = getChartPropsWithLegend(true, true, 'top', true);
+ const transformed = transformProps(props);
+
+ expect(transformed.echartOptions.graphic).toEqual(
+ expect.objectContaining({
+ type: 'text',
+ left: 'center',
+ top: expect.stringMatching(/^\d+(\.\d+)?%$/),
+ }),
+ );
+
+ // The top position should be adjusted for top legend
+ const topValue = parseFloat(
+ (transformed.echartOptions.graphic as any).top.replace('%', ''),
+ );
+ expect(topValue).toBeGreaterThan(50); // Shifted down for top legend
+ });
+
+ it('should center total text when legend is on bottom', () => {
+ const props = getChartPropsWithLegend(true, true, 'bottom', true);
+ const transformed = transformProps(props);
+
+ expect(transformed.echartOptions.graphic).toEqual(
+ expect.objectContaining({
+ type: 'text',
+ left: 'center',
+ top: expect.stringMatching(/^\d+(\.\d+)?%$/),
+ }),
+ );
+
+ // The top position should be adjusted for bottom legend
+ const topValue = parseFloat(
+ (transformed.echartOptions.graphic as any).top.replace('%', ''),
+ );
+ expect(topValue).toBeLessThan(50); // Shifted up for bottom legend
+ });
+
+ it('should use default positioning when no legend is shown', () => {
+ const props = getChartPropsWithLegend(true, false, 'right', true);
+ const transformed = transformProps(props);
+
+ expect(transformed.echartOptions.graphic).toEqual(
+ expect.objectContaining({
+ type: 'text',
+ left: 'center',
+ top: 'middle',
+ }),
+ );
+ });
+
+ it('should handle regular pie chart (non-donut) positioning', () => {
+ const props = getChartPropsWithLegend(true, true, 'right', false);
+ const transformed = transformProps(props);
+
+ expect(transformed.echartOptions.graphic).toEqual(
+ expect.objectContaining({
+ type: 'text',
+ top: '0', // Non-donut charts use '0' as default top position
+ left: expect.stringMatching(/^\d+(\.\d+)?%$/), // Should still adjust
left for right legend
+ }),
+ );
+ });
+
+ it('should not show total graphic when showTotal is false', () => {
+ const props = getChartPropsWithLegend(false, true, 'right', true);
+ const transformed = transformProps(props);
+
+ expect(transformed.echartOptions.graphic).toBeNull();
+ });
+});
+
describe('Other category', () => {
const defaultFormData: SqlaFormData = {
colorScheme: 'bnbColors',
