Skip to content
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

fix(sunburst) set the position of label in center when angle is 2π. c… #16425

Merged
merged 6 commits into from
Feb 18, 2022

Conversation

FrankChencc
Copy link
Contributor

@FrankChencc FrankChencc commented Jan 25, 2022

…lose #16296

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Fix a bug that the Sunburst center text cannot be centered vertically.

Fixed issues

#16296

Details

Before: What was the problem?

when the sector's angle is 2π, and inner radius is 0(which means at the first level). the position of label cannot be the center of the circle, which lead to the text of label cannot be centered vertically in Sunburst.

image

After: How is it fixed in this PR?

set the position of label in center when angle is 2π and inner radius is 0 so that the text of label can be centered vertically in Sunburst.

image

Misc

  • The API has been changed (apache/echarts-doc#xxx).
  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

N.A.

Others

Merging options

  • Please squash the commits into a single one when merging.

Other information

@echarts-bot
Copy link

echarts-bot bot commented Jan 25, 2022

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

Ovilia
Ovilia previously requested changes Jan 25, 2022
Copy link
Contributor

@Ovilia Ovilia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placing labels with position: 'center' at the center of the sunburst may not be universal. Consider the case when the second level sectors having the angle of 2π, if its label is put at the center of the sunburst, there may be another sector at the first level also being a circle and having label. I think maybe it's safe to put the label at center only when the sector's inner radius is 0, or provide a new value for labels at the center like 'inner'.

@@ -219,7 +219,13 @@ class SunburstPiece extends graphic.Sector {
}
else {
if (!textAlign || textAlign === 'center') {
r = (layout.r + layout.r0) / 2;
// set the position of label in center when angle is 2π & r0 is 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be more clear to be commented

// Put label in the center if it's a circle.

Current comment is just an plain explaination on the code.

@pissang pissang merged commit 6a42c39 into apache:master Feb 18, 2022
@echarts-bot
Copy link

echarts-bot bot commented Feb 18, 2022

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants