Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(polar): allow setting angleAxis.endAngle #19099

Merged
merged 7 commits into from
Sep 27, 2023

Conversation

yassilah
Copy link
Contributor

@yassilah yassilah commented Sep 9, 2023

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Allow setting an endAngle alongside startAngle on a polar coordinate system.

Fixed issues

Details

Before: What was the problem?

Capture d’écran 2023-09-10 à 09 01 54

After: How does it behave after the fixing?

Capture d’écran 2023-09-10 à 09 01 24

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
Copy link

echarts-bot bot commented Sep 9, 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.

@yassilah yassilah changed the title Feat polar end angle feat: allow setting polar endAngle Sep 9, 2023
@yassilah yassilah changed the title feat: allow setting polar endAngle feat(polar): allow setting endAngle Sep 9, 2023
@pull-request-size pull-request-size bot added size/S and removed size/XS labels Sep 9, 2023
@yassilah yassilah changed the title feat(polar): allow setting endAngle feat(polar): allow setting angleAxis.endAngle Sep 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2023

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

@yassilah
Copy link
Contributor Author

Link to a working example here

if (radiusExtent[r0Id] === 0) {
shape = new graphic.Circle({
shape = new graphic.Arc({
Copy link
Contributor

Choose a reason for hiding this comment

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

I run the visual tests (npm run test:visual) and find there are some subtle diffs with polarLine.html, which is a case when startAngle is not the default value. I would suggest using graphic.Circle when Math.abs(angleExtent[1] - angleExtent[0]) === 360 (or is there any other situations?) and use graphic.Arc otherwise.

lineColors = lineColors instanceof Array ? lineColors : [lineColors];

const splitLines: graphic.Circle[][] = [];

for (let i = 0; i < ticksCoords.length; i++) {
const colorIndex = (lineCount++) % lineColors.length;
splitLines[colorIndex] = splitLines[colorIndex] || [];
splitLines[colorIndex].push(new graphic.Circle({
splitLines[colorIndex].push(new graphic.Arc({
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar as above.

@Ovilia
Copy link
Contributor

Ovilia commented Sep 26, 2023

image

The yellow areas in the right image shows the slight differences before and after this change.

@yassilah
Copy link
Contributor Author

Your suggestion indeed fixes the regression :)

Capture d’écran 2023-09-26 à 13 06 52

@Ovilia Ovilia merged commit d08664f into apache:master Sep 27, 2023
@echarts-bot
Copy link

echarts-bot bot commented Sep 27, 2023

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

@Ovilia
Copy link
Contributor

Ovilia commented Sep 27, 2023

@yassilah Thanks for this useful PR!

@Ovilia Ovilia added this to the 5.5.0 milestone Sep 27, 2023
@yassilah yassilah deleted the feat-polar-endAngle branch September 27, 2023 08:24
@harshsharma-11
Copy link

hey @Ovilia i want to join slack channel of apache so how can i

@echarts-bot echarts-bot bot added PR: awaiting doc Document changes is required for this PR. and removed PR: doc unchanged labels Feb 1, 2024
Copy link

echarts-bot bot commented Feb 1, 2024

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants