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

feat(pictorialBar): support clip for pictorialBar series #19197

Merged
merged 3 commits into from Nov 30, 2023

Conversation

Ovilia
Copy link
Contributor

@Ovilia Ovilia commented Oct 12, 2023

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Support clip for pictorialBar series.

Fixed issues

#19173

Details

Before: What was the problem?

Pictorial bars may overflow the grid area.

After: How does it behave after the fixing?

User can set series.clip: true to enable clipping.

Document Info

One of the following should be checked.

Misc

ZRender Changes

  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

N.A.

Others

Merging options

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

Other information

@echarts-bot echarts-bot bot added PR: author is committer PR: awaiting doc Document changes is required for this PR. PR: awaiting review labels Oct 12, 2023
@Ovilia Ovilia added this to the 5.5.0 milestone Oct 12, 2023
@echarts-bot
Copy link

echarts-bot bot commented Oct 12, 2023

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.

Document changes are required in this PR. Please also make a PR to apache/echarts-doc for document changes and update the issue id in the PR description. When the doc PR is merged, the maintainers will remove the PR: awaiting doc label.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 12, 2023

The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-19197@71799e9

Ovilia added a commit to apache/echarts-doc that referenced this pull request Oct 12, 2023
@echarts-bot echarts-bot bot added PR: doc ready and removed PR: awaiting doc Document changes is required for this PR. labels Oct 12, 2023
@linghaoSu
Copy link
Contributor

image

It seems that when the label is placed outside the bar, it will be clipped. Is this expected?

label cliped live demo

@Ovilia
Copy link
Contributor Author

Ovilia commented Oct 25, 2023

@linghaoSu Thanks for correcting. I think this is not ideal. I will try to fix this.

@Ovilia
Copy link
Contributor Author

Ovilia commented Oct 25, 2023

@linghaoSu It seems that if the clipPath is added on the bar.__pictorialBundle, there would be an extra transform on the clipPath and makes the whole shape invisible. Do you have any suggestion?

Copy link
Member

@plainheart plainheart left a comment

Choose a reason for hiding this comment

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

How about preventing the label from being clipped by setting the label ignoreClip to be true?

@Ovilia
Copy link
Contributor Author

Ovilia commented Nov 8, 2023

How about preventing the label from being clipped by setting the label ignoreClip to be true?

Thanks for the hint! It's a good idea.

@linghaoSu @plainheart Please help review again. Thanks

Copy link
Contributor

@linghaoSu linghaoSu left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@linghaoSu linghaoSu merged commit 7731729 into master Nov 30, 2023
2 checks passed
Copy link

echarts-bot bot commented Nov 30, 2023

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

Successfully merging this pull request may close these issues.

None yet

3 participants