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(axisLabel): fix axisLabel rotating with grid.containLabel #12259 #12556

Merged
merged 4 commits into from Aug 4, 2020
Merged

fix(axisLabel): fix axisLabel rotating with grid.containLabel #12259 #12556

merged 4 commits into from Aug 4, 2020

Conversation

quillblue
Copy link
Contributor

@quillblue quillblue commented May 3, 2020

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Update function of calculating updated height & width for rotated axis labels
Fix issue #12259

Fixed issues

Close #12259

Details

In previous rotateTextRect() in axisHelper.js, take cos value of negative rotate will got negative result, which causes calculated width/height of rotated axis label is smaller than actual size. In grid, it will cause axis labels be cut.

After: How is it fixed in this PR?

As physically we need cos/sin of rotate angle to be positive (or 0), add Math.abs here to solve this issue,

Usage

Are there any API changes?

  • The API has been changed.

Related test cases or examples to use the new APIs

NA.

Others

Merging options

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

Other information

@echarts-bot
Copy link

echarts-bot bot commented May 3, 2020

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.

Copy link
Contributor

@susiwen8 susiwen8 left a comment

Choose a reason for hiding this comment

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

Please don't submit any dist file

@quillblue
Copy link
Contributor Author

@susiwen8 Thanks for kind reminder. Revert changes in dist back to previous version in new commit. Accordingly please squash to one during merging.

@pissang pissang added this to the 4.9.0 milestone May 3, 2020
@susiwen8
Copy link
Contributor

susiwen8 commented May 3, 2020

@quillblue Could you add a test case for this PR?

@quillblue
Copy link
Contributor Author

@susiwen8 Sure. In latest commit I added a test case using same option as original issue using
and below are before&after comparision
Before:
Before

After:
After

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.

Source code changes look good to me.
I think the test case could be included in the current test case axisLabel.html.

@Ovilia Ovilia changed the title Fix #12259 fix(axisLabel): fix axisLabel rotating with grid.containLabel #12259 May 6, 2020
@quillblue
Copy link
Contributor Author

@Ovilia combined to axisLabel.html case 2 per your idea. The existing case has animation to show the rotate of labels, which has greater idea than my demo.
Though rotate: 330 is same as rotate: -30, I set xAxis rotate from -180 to 180 (instead of 0-360) as users are more used to use negative value to represent clockwise rotation.

@pissang pissang merged commit 38bd9eb into apache:master Aug 4, 2020
@echarts-bot
Copy link

echarts-bot bot commented Aug 4, 2020

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

在grid.containLabel模式下xAxis的rotate算错了
4 participants