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: add new stacking strategies #17086

Merged
merged 3 commits into from
May 28, 2022

Conversation

villebro
Copy link
Member

@villebro villebro commented May 23, 2022

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

This PR adds a new property stackStrategy to the SeriesStackOptionMixin which defaults to samesign (current behavior). Associated PR on the docs repo: apache/echarts-doc#255

Fixed issues

Details

Before: What was the problem?

Currently stacking is only applied if the cumulative sum has the same sign as the value to be stacked. This makes it impossible to mix positive and negative values in the same series, something typically needed when creating confidence bands:

image

After: How is it fixed in this PR?

Here we introduce the following new stacking options:

  • all: stack all values, irrespective of the sign of the stackable value:

image

  • positive: only stack positive values:

image

  • negative: only stack negative values:

image

We also add the strategy samesign which corresponds to the current behavior which we default to to avoid introducing a breaking change. However, I propose defaulting to all in the next major version, as I assume this is the behavior users will expect to see.

Misc

  • The API has been changed (apache/echarts-doc#xxx).
  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

New test cases added to the area-stack.html test.

Others

Merging options

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

Other information

@echarts-bot
Copy link

echarts-bot bot commented May 23, 2022

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.

@villebro
Copy link
Member Author

Docs for the proposed feature: apache/echarts-doc#255

@villebro
Copy link
Member Author

@pissang let me know if you feel this should be implemented some other way so I can change the PR accordingly

test/area-stack.html Outdated Show resolved Hide resolved
@pissang
Copy link
Contributor

pissang commented May 27, 2022

@villebro Thanks for the wonderful work! All looks good to me except a tiny code style issue.

@susiwen8
Copy link
Contributor

This could fix a lot of issues. I have added in PR description

@villebro villebro merged commit 1a8002b into apache:master May 28, 2022
@echarts-bot
Copy link

echarts-bot bot commented May 28, 2022

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

@villebro villebro deleted the villebro/negative-stack branch May 28, 2022 05:34
@Ovilia Ovilia added the PR: awaiting doc Document changes is required for this PR. label Jun 8, 2022
@Ovilia Ovilia added this to the 5.3.3 milestone Jun 10, 2022
Ovilia added a commit to apache/echarts-doc that referenced this pull request Jun 13, 2022
@Ovilia Ovilia added PR: doc ready and removed PR: awaiting doc Document changes is required for this PR. labels Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment