-
Notifications
You must be signed in to change notification settings - Fork 649
fix: PieLabel 标注线遮挡饼图 #2147
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?
fix: PieLabel 标注线遮挡饼图 #2147
Conversation
Summary of ChangesHello @huaxiabuluo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where annotation lines in pie charts could obscure the chart itself. The core fix involves refining the label positioning logic by sorting labels vertically before rendering, which significantly reduces overlaps. Additionally, a new test case has been introduced to validate the behavior of spider-type pie labels when handling data slices with zero values, thereby enhancing the overall stability and correctness of the component's rendering. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
这个 PR 修复了 PieLabel 标注线可能会遮挡饼图的问题。通过在调整标签位置之前按 y 坐标对标签进行排序,可以防止标签重叠。同时,还增加了一个新的测试用例,覆盖了值为 0 的数据点场景,这很可能就是触发原始 bug 的原因。这个修复是合理的,但在 adjustPosition 函数中有一个小改进可以避免修改输入数组,我已对此提出了评论。
| let delta; | ||
|
|
||
| half.forEach((label) => { | ||
| const sortedHalf = half.sort((a, b) => a.y - b.y); |
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.
Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: |
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.
效果上是不是应该在左上边,因为是最后一个数据,第一个数据是0的话应该在右边(第一象限)
| let delta; | ||
|
|
||
| half.forEach((label) => { | ||
| const sortedHalf = half.sort((a, b) => a.y - b.y); |
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.
这样可以绕过去,但感觉应该改锚点角度 锚点坐标那边?两种布局应该都有这个问题
40eec53 to
b551db7
Compare
| const inflectionPoint = getEndPoint(center, anchorAngle, radius + inflectionOffset); | ||
| // 锚点方向 | ||
| const side = anchorPoint.x < center.x ? 'left' : 'right'; | ||
| const side = anchorPoint.x <= center.x ? 'left' : 'right'; |
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.
这样改第一个数据也会变到左边。但标签应该是有顺序的,从右边(第一象限)绕一圈到左边第四象限
No description provided.