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 f282799458ae..9cb6e96da935 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Pie/transformProps.ts @@ -55,15 +55,29 @@ import { getDefaultTooltip } from '../utils/tooltip'; import { Refs } from '../types'; import { getContributionLabel } from './utils'; -const percentFormatter = getNumberFormatter(NumberFormats.PERCENT_2_POINT); +const defaultPercentFormatter = getNumberFormatter( + NumberFormats.PERCENT_2_POINT, +); + +/** + * Percentage labels only make sense under a percentage D3 format. Any other + * format (SMART_NUMBER, currency, integer) would render the underlying + * fraction — 0.42 rather than 42% — so those keep the default formatter. + */ +const resolvePercentFormatter = (numberFormat?: string) => + numberFormat?.includes('%') + ? getNumberFormatter(numberFormat) + : defaultPercentFormatter; export function parseParams({ params, numberFormatter, + percentFormatter = defaultPercentFormatter, sanitizeName = false, }: { params: Pick; numberFormatter: ValueFormatter; + percentFormatter?: ValueFormatter; sanitizeName?: boolean; }): string[] { const { name: rawName = '', value, percent } = params; @@ -296,6 +310,7 @@ export default function transformProps( currencyCodeColumn, detectedCurrency, ); + const percentFormatter = resolvePercentFormatter(numberFormat); let data = rawData; const otherRows: DataRecord[] = []; @@ -446,6 +461,7 @@ export default function transformProps( const [name, formattedValue, formattedPercent] = parseParams({ params, numberFormatter, + percentFormatter, }); switch (labelType) { case EchartsPieLabelType.Key: @@ -566,6 +582,7 @@ export default function transformProps( const [name, formattedValue, formattedPercent] = parseParams({ params, numberFormatter, + percentFormatter, sanitizeName: true, }); if (params?.data?.isOther) { 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 3d18c755237c..cff5c402b792 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 @@ -211,6 +211,16 @@ describe('formatPieLabel', () => { }), ).toEqual(['<NULL>', '1.23k', '12.34%']); }); + + test('should use the supplied percent formatter', () => { + expect( + parseParams({ + params: { name: 'My Label', value: 1234, percent: 12.34 }, + numberFormatter: getNumberFormatter(), + percentFormatter: getNumberFormatter(',.1%'), + }), + ).toEqual(['My Label', '1.23k', '12.3%']); + }); }); describe('Pie label string template', () => { @@ -320,6 +330,30 @@ describe('Pie label string template', () => { }), ).toEqual('Tablet:123456\n55.5'); }); + + test('should format percentages with a percentage number format', () => { + expect( + format({ label_type: 'key_percent', number_format: ',.1%' }), + ).toEqual('Tablet: 55.5%'); + }); + + test('should leave percentages at the default for non-percentage formats', () => { + // A value format such as ',d' would render the underlying fraction as `1`, + // so percentages keep the default two-decimal formatter. + expect(format({ label_type: 'key_percent', number_format: ',d' })).toEqual( + 'Tablet: 55.50%', + ); + }); + + test('should apply the percentage format to template labels', () => { + expect( + format({ + label_type: 'template', + label_template: '{name}:{value}\n{percent}', + number_format: ',.1%', + }), + ).toEqual('Tablet:12,345,600.0%\n55.5%'); + }); }); describe('Total value positioning with legends', () => {