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.

Sign up for GitHub

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

Remove empty labels from being candidates for overlap checking #19982

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+8 −1
Open

Remove empty labels from being candidates for overlap checking #19982

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5172395
Remove empty labels from being candidates for overlap checking
JesseSDevaney May 29, 2024
a5aa28a
Fix linting errors
JesseSDevaney Jun 19, 2024
b525645
update conditional to check the bounding rect of the label and not th…
JesseSDevaney Jun 28, 2024
205b2b1
fix added white space
JesseSDevaney Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/label/labelLayoutHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,13 @@ export function hideOverlap(labelList: LabelLayoutInfo[]) {
const transform = labelItem.transform;
const label = labelItem.label;
const labelLine = labelItem.labelLine;

// if the label has no text, remove it as a candidate for overlap checking
if (label.style?.text === '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

After discussing with other members, we tend to believe it's better to make the change here instead of in ZRender because it may be controversial to define whether it's overlapping for an object with empty height/width inside other elements. So we still need to do the change here. I'll close the ZRender PR.

I think the overall logic is great, only that it may make greater sense if we could check the width and height of the text here instead of checking text === ''. And we try to use ?. in ECharts to reduce the compiled size, so label.style && lable.style.text is a better way to check like this.

Thanks for your contribution!

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the update.

I updated the conditional to check the width and height of the bounding rectangle instead of checking the label's text itself.

Let me know if the updated conditional is what you were expecting.

Thank you for your help, Ovilia!

hideEl(label);
labelLine && hideEl(labelLine);
continue;
}
globalRect.copy(labelItem.rect);
// Add a threshold because layout may be aligned precisely.
globalRect.width -= 0.1;
Expand Down Expand Up @@ -362,4 +369,4 @@ export function hideOverlap(labelList: LabelLayoutInfo[]) {
displayedLabels.push(labelItem);
}
}
}
}
Loading