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(pie): hide labelLine emphasis when inside #14702
Conversation
Thanks for your contribution! The pull request is marked to be |
LGTM, @pissang could you double check? |
src/chart/pie/PieView.ts
Outdated
@@ -193,7 +193,7 @@ class PiePiece extends graphic.Sector { | |||
|
|||
const labelPosition = seriesModel.get(['label', 'position']); | |||
if (labelPosition !== 'outside' && labelPosition !== 'outer') { | |||
sector.getTextGuideLine()?.hide(); | |||
sector.removeTextGuideLine(); | |||
return; | |||
} |
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.
Seems the guideline can't be restored if the position is changed from 'inside' to 'outside' again. Perhaps we need to recreate the guideline here instead of in
echarts/src/chart/pie/PieView.ts
Line 49 in 2c88dec
this.setTextGuideLine(polyline); |
if (..) { ... }
else {
let polyline = this.getTextGuideLine();
if (!polyline) {
polyline = new graphic.Polyline();
this.setTextGuideLine(polyline);
}
// Default use item visual color
setLabelLineStyle(this, getLabelLineStatesModels(itemModel), {
stroke: visualColor,
opacity: retrieve3(labelLineModel.get(['lineStyle', 'opacity']), visualOpacity, 1)
});
}
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.
Thanks for the review @pissang ! This change makes sense, and I did the proposed changes. I also added yet another flip-flop (inside/outside) to the test to make sure it can change back yet one more time, just in case.
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.
Congratulations! Your PR has been merged. Thanks for your contribution! 👍 |
Brief Information
This pull request is in the type of:
What does this PR do?
Currently the label line will stay visible on emphasis when changing from outside to inside. This PR fixes this problem and adds a test to show that emphasis works even after changing back and forth.
Details
Before: What was the problem?
After: How is it fixed in this PR?
Merging options
Other information