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(pie): label layout and text wrapping #16034

Merged
merged 23 commits into from
Dec 21, 2021
Merged

fix(pie): label layout and text wrapping #16034

merged 23 commits into from
Dec 21, 2021

Conversation

Ovilia
Copy link
Contributor

@Ovilia Ovilia commented Nov 9, 2021

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Fix pie label layout in multiple cases. Here are some of the typical cases:

A. Plain text cannot control wrapping and may be displayed outside of the canvas

(Left: before; Right: after)

image

B. The distances between labelLine and labels are not correct due to the width calculation

image

C. The background area of rich text is not correct

image

D. Line width of rich text is not correct

image

E. When the text width is explicitly set, rich text area breaks the width constraint.

image

Most of the problems relates to text bounding rect calculation and this PR also fixes related logic with pie charts.

A full list of changes are listed in the "View Visual Test Result" part of this PR.

Fixed issues

#16023

Misc

Related test cases or examples to use the new APIs

test/pie-label-alignTo-adjust.html

Others

Merging options

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

Other information

  • Run visual test of pie charts.

@echarts-bot
Copy link

echarts-bot bot commented Nov 9, 2021

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.

The pull request is marked to be PR: author is committer because you are a committer of this project.

This PR depends on ZRender changes. Please update the ZRender dependency to the latest nightly version including this change, which takes place everyday at 8:00 UTC (16:00 Beijing Time).
You can use npm i zrender@npm:zrender-nightly to update package.json.
If you have any question about this, please leave a comment and we will give you extra help on this.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Nov 26, 2021
@Ovilia Ovilia changed the title Fix pie label position with rich text fix(pie): label layout and text wrapping Nov 26, 2021
@Ovilia
Copy link
Contributor Author

Ovilia commented Nov 26, 2021

TODO:

  • Explain the diff of visual test result if necessary.
  • Code style cleaning.

@Ovilia Ovilia added this to the 5.3 milestone Nov 26, 2021
@Ovilia Ovilia linked an issue Nov 26, 2021 that may be closed by this pull request
@pull-request-size pull-request-size bot added size/M and removed size/L labels Nov 29, 2021
@pull-request-size pull-request-size bot added size/L and removed size/M labels Dec 3, 2021
@Ovilia Ovilia marked this pull request as ready for review December 7, 2021 11:21
src/chart/pie/labelLayout.ts Outdated Show resolved Hide resolved
src/chart/pie/labelLayout.ts Outdated Show resolved Hide resolved
src/chart/pie/labelLayout.ts Outdated Show resolved Hide resolved
src/chart/pie/labelLayout.ts Outdated Show resolved Hide resolved
src/chart/pie/labelLayout.ts Outdated Show resolved Hide resolved
src/chart/pie/labelLayout.ts Outdated Show resolved Hide resolved
@Ovilia
Copy link
Contributor Author

Ovilia commented Dec 8, 2021

@pissang Thanks for your review. I've made a new commit to fix them and no new diff in this commit. Please review again when you have time.

pissang
pissang previously approved these changes Dec 21, 2021
@pissang pissang merged commit 933c06f into master Dec 21, 2021
@echarts-bot
Copy link

echarts-bot bot commented Dec 21, 2021

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

@pissang pissang deleted the fix-pie-label branch December 21, 2021 04:22
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.

Pie chart rich labels overflow
2 participants