fix(sunburst): redistribute angles constrained by minAngle. close #20602 - #21711
Open
Nas01010101 wants to merge 1 commit into
Open
fix(sunburst): redistribute angles constrained by minAngle. close #20602#21711Nas01010101 wants to merge 1 commit into
Nas01010101 wants to merge 1 commit into
Conversation
|
Thanks for your contribution! Please DO NOT commit the files in dist, i18n, and ssr/client/dist folders in a non-release pull request. These folders are for release use only. To reviewers: If this PR is going to be described in the changelog in the future release, please make sure this PR has one of the following labels: This message is shown because the PR description doesn't contain the document related template. |
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.
Brief Information
This pull request is in the type of:
What does this PR do?
Makes
sunburst.minAngleshrink the sibling sectors that are large enough, so that a group of children still fits exactly inside its parent.Fixed issues
Details
Before: What was the problem?
sunburstLayoutenlarged any sector smaller thanminAngleup tominAngle, but never took the extra angle back from the other sectors. The code that was supposed to do so was left commented out:So the children of a node could add up to more than the node itself. In the demo of #20602, hovering
cshows it overlappingd. With four children of100 / 1 / 1 / 1andminAngle: 30, a parent spanning 360 degrees gets children spanning 439.5 degrees.After: How does it behave after the fixing?
getChildrenAngles()now lays out a whole sibling group at once, in the same two passespieLayoutalready uses:minAngle, subtract what they took from the angle left over, and total the values of the ones left untouched;minAnglecannot be satisfied at all, split the parent evenly.Because a sunburst is nested, this runs per sibling group with the parent's own angle as the budget, rather than once over a flat list as in a pie. Uncommenting the two lines above would not have been enough: they were reaching for a single series-wide
restAngle, which would let a shortfall in one subtree be paid for by an unrelated subtree elsewhere in the ring.Using the parent's assigned angle as the budget also fixes the second-order case: when a node is itself enlarged to
minAngle, its children are redistributed into the enlarged span instead of leaving a gap in the outer ring. With the group's natural sum as the budget instead, a node with a natural span of 21.3 degrees enlarged to 30 degrees still had children totalling only 21.3.minAngledefaults to0, and withminAngle: 0nothing is ever clamped, so the redistribution pass is skipped entirely and existing charts lay out bit-for-bit as before.Document Info
sunburst.minAngleis currently not documented (onlysunburst.label.minAngleis), so nothing needs updating, though it may be worth documenting separately now that it behaves likepie.minAngle.Others
Merging options
Other information
Added
test/ut/spec/series/sunburst.test.tswith five cases: the overflow itself, the nested/enlarged-parent case, the zero-sum case (20 zero-value nodes withminAngle: 30spanned 600 degrees before), that small sectors are still enlarged, and thatminAngle: 0is unchanged.Verified on Linux x86_64, node 22:
Without
TZset,spec/util/time.test.tsfails on a locale-dependent assertion unrelated to this change.Possible conflict: open PRs #21687 and #21598 each also create
test/ut/spec/series/sunburst.test.ts, so whichever lands second will need a trivial merge.