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

chore(stack): remove stack only same sign #15211

Closed
wants to merge 2 commits into from
Closed

chore(stack): remove stack only same sign #15211

wants to merge 2 commits into from

Conversation

susiwen8
Copy link
Contributor

@susiwen8 susiwen8 commented Jun 22, 2021

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

According @100pah comment in 9317, ECharts only stack those value with same sign. I can't see why this rule was made, but seems this has cause a lot of question for areaStyle in stacked chart, also stack on different sign is reaonable for statistic. So I tried to lift this limitation, and done some experiment under line and bar, looks good for me.

Fixed issues

Close #9317 #9141 #13747 #15083 #14365

Details

Before: What was the problem?

Screen Shot 2021-06-23 at 12 17 46 AM

Screen Shot 2021-06-23 at 12 17 54 AM

Screen Shot 2021-06-23 at 12 18 00 AM

Screen Shot 2021-06-23 at 12 18 09 AM

After: How is it fixed in this PR?

Screen Shot 2021-06-23 at 12 16 06 AM

Screen Shot 2021-06-23 at 12 16 17 AM

Screen Shot 2021-06-23 at 12 16 24 AM

Screen Shot 2021-06-23 at 12 18 44 AM

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

NA.

Others

Merging options

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

Other information

@echarts-bot
Copy link

echarts-bot bot commented Jun 22, 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.

@susiwen8 susiwen8 requested review from 100pah and pissang June 22, 2021 16:23
@pissang pissang added the discussion-required Further discussion is required before we decide if this issue should be fixed. label Jun 23, 2021
@pissang
Copy link
Contributor

pissang commented Jun 23, 2021

@susiwen8 It's an ancient rule that exists since echarts 1.0.

Check the case https://echarts.apache.org/examples/zh/editor.html?c=bar-negative. It will separate the stack of positive and negative values, usually more useful on the bar chart.

But seems it brings more issues than the benefits on the area chart.

@susiwen8
Copy link
Contributor Author

susiwen8 commented Jun 23, 2021

@pissang It won't be the problem for stack bar. Maybe somewere has "accidently" fix this. No, still a issue for bar, in this case '收入' are not right.
Screen Shot 2021-06-23 at 10 51 59 PM

@susiwen8 susiwen8 marked this pull request as draft June 24, 2021 01:15
@susiwen8
Copy link
Contributor Author

susiwen8 commented Jun 24, 2021

@pissang I stand correct, bar chart stack with negative still ok, It has seperated positive and negative value, and final result are correct.
Screen Shot 2021-06-24 at 11 06 57 PM
Screen Shot 2021-06-24 at 11 09 15 PM

I have found some remaining issues.

  1. value axis, the example above I have to set max to axis so the charts can display completly
  2. To make negative stack work in bar, the first serie item mush have a negative value. Otherwise negative bar would disappear.

Beside issues listed above, looks like 'stack value with different sign' works on bar.

Test case for bar stack has added, you can check the option there.

@pissang
Copy link
Contributor

pissang commented Jun 25, 2021

@susiwen8 I'm not sure if it can work as before because this logic is completely removed.

@susiwen8 susiwen8 closed this Aug 23, 2021
@susiwen8 susiwen8 deleted the stack branch May 23, 2022 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion-required Further discussion is required before we decide if this issue should be fixed. PR: author is committer size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stacked line chart with negative values
2 participants