fix(pie): apply percentage number formats to pie chart labels - #42913
Closed
Kr1dhay wants to merge 2 commits into
Closed
fix(pie): apply percentage number formats to pie chart labels#42913Kr1dhay wants to merge 2 commits into
Kr1dhay wants to merge 2 commits into
Conversation
The percent half of a pie chart label was pinned to ',.2%' by a hardcoded
module constant, so choosing a Number format had no effect on it.
Percentage labels now honour the Number format control when the selected
format is a percentage D3 format (',.1%', '.0%', ...). Any other format
(SMART_NUMBER, currency, integer) keeps the previous two-decimal default,
since applying a value format to a fraction would render 0.42 rather than
42%. Existing charts are therefore unaffected.
Fixes apache#42834
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Fixes #42834.
On a pie chart, setting Labels → Percentage and then choosing a Number format had no effect on the percentage label. The percent formatter was a hardcoded module constant:
number_formatonly ever reachednumberFormatter, so the percent half of every label was pinned to,.2%.This implements approach #2 from the issue thread, which the reporter asked for: the
number_formatcontrol now drives percentage labels when the selected format is a percentage D3 format (,.1%,.0%, …). Any other format —SMART_NUMBER, currency,,d— keeps the existing two-decimal default.The type check matters for backwards compatibility: applying a value format such as
,dto a fraction would render12.34%as0. Gating on the format type means no existing chart changes output; only a user who deliberately picks a%format sees a difference.Detection uses
.includes('%'), the idiom already used for this purpose insharedControls.tsx(y_axis_format,x_axis_number_format).The change covers every percent-bearing surface, since they all share one formatter: the four percent label types (
percent,key_percent,key_value_percent,value_percent), thetemplatelabel type's{percent}, tooltips, and the "Other" bucket rows.One consequence worth calling out: because a single control drives both halves of the label, picking a
%format also formats the value half as a percentage. That is inherent to reusingnumber_formatrather than adding a second control — happy to switch to a dedicatedpercentage_formatcontrol if maintainers prefer that shape.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Automated —
superset-frontend/plugins/plugin-chart-echarts/test/Pie/transformProps.test.ts:cd superset-frontend npx jest plugins/plugin-chart-echarts/test/Pie/transformProps.test.tsFour cases added:
parseParamshonours an injected percent formatter; a%number format reaches the label; a non-%format leaves the percentage at,.2%(the backwards-compatibility guard); and the template label type picks up the format. The pre-existingshould be formatted using the number formattertest already asserts,dleaves percentages at55.50%and still passes unchanged.Manual:
,.1%→ percentages render with one decimal (12.3%).SMART_NUMBERor,d→ percentages return to12.34%, exactly as before this change.ADDITIONAL INFORMATION