-
Notifications
You must be signed in to change notification settings - Fork 19.8k
feat(sunburst): align root node label to center #21306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat(sunburst): align root node label to center #21306
Conversation
Thanks for your contribution! |
@Ovilia Can you review the PR |
The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-21306@c611cba |
c611cba
to
2362925
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the test case you provided, if you print out console.log(angle, layout.r0, layout.r0 === 0, angle === 2 * Math.PI);
, you should see SunburstPiece.ts:236 6.283185307179587 0 true false
, which means the problem is caused by the precision of Math.PI * 2. So this can be simply fixed as
if (layout.r0 === 0 && isRadianAroundZero(angle - 2 * Math.PI))
And please don't use forced pushes because we need to reserve the commit history. Thanks!
@Ovilia Thank you for the feedback. I guess it works perfectly. can you take a look now |
Brief Information
This pull request is in the type of:
What does this PR do?
The PR aligns the root node label in sunburst chart at the center.
Fixed issues
This is enhancement request
Details
Before: What was the problem?
The root node label would appear at the bottom half of the inner circle in non expanded mode. there is no way that we could align it to center.
After: How does it behave after the fixing?
Document Info
One of the following should be checked.
Misc
ZRender Changes
Related test cases or examples to use the new APIs
N.A.
Others
Merging options
Other information