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

Chart renders incorrectly when using StackedAreaChart with brush and zoom #21

Open
eranshmil opened this issue Nov 20, 2020 · 6 comments
Labels
bug Something isn't working

Comments

@eranshmil
Copy link
Contributor

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Current behavior

I constructed this demo using a few examples from the stories, and I have the following issues:

  • When changing the range, the chart lines are being rendered incorrectly.
    reaviz-bug

  • The same exact chart looks different between the brush and the large chart. Is that because of the different heights?
    image

Minimal reproduction of the problem with instructions

https://stackblitz.com/edit/react-reaviz

  1. Use the chart brush to shorten the range
  2. Scroll to the right.

(The codesandbox link is not working)

Environment


Libs:
- react version: 17.0.1
- reaviz version: 9.3.14

Browser:
- [x] Chrome (desktop) version 86.0.4240.198
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: v12.18.3
- Platform:  Mac
@amcdnl
Copy link
Member

amcdnl commented Nov 20, 2020

Oh - that is fun.

This is general area where this is happening: https://github.com/reaviz/reaviz/blob/master/src/AreaChart/AreaSeries/Line.tsx#L154

IF you turn off animations does it work?

@amcdnl amcdnl added the bug Something isn't working label Nov 20, 2020
@eranshmil
Copy link
Contributor Author

eranshmil commented Nov 20, 2020

Thanks for the quick response!

I tried disabling the animation for the Series and Line, the drawing is faster but the bug still happens.
I will try to debug this method tomorrow.

@eranshmil
Copy link
Contributor Author

eranshmil commented Nov 21, 2020

So I have found out that the issue starts here:
https://github.com/reaviz/reaviz/blob/master/src/AreaChart/AreaSeries/Line.tsx#L102

I tried different approaches that fix this issue:

  1. Changing the condition in https://github.com/reaviz/reaviz/blob/master/src/AreaChart/AreaSeries/Line.tsx#L102:

    if (
      this.ghostPathRef.current &&
      (prevProps.data !== this.props.data ||
        prevProps.width !== this.props.width ||
        prevProps.xScale !== this.props.xScale ||
        prevProps.yScale !== this.props.yScale)
    ) {}
  2. Spreading the data array in https://github.com/reaviz/reaviz/blob/master/src/AreaChart/AreaSeries/AreaSeries.tsx#L182 and https://github.com/reaviz/reaviz/blob/master/src/AreaChart/AreaSeries/AreaSeries.tsx#L197:

    -data={data}
    +data={[...data]}
  3. Using the state in my component instead of the imported data and spreading the data array.

I'm no React expert but I think that if one of the solutions directly in the reaviz code is fine, it should be applied in a lot of places in the different charts, and I have no problem opening a PR.

@amcdnl
Copy link
Member

amcdnl commented Nov 23, 2020

Ya - can you open up a PR - happy to accept that.

@eranshmil
Copy link
Contributor Author

I went with the first solution.
Let me know if you want me to create a story for this scenrio.

@amcdnl
Copy link
Member

amcdnl commented Nov 30, 2020

Ya - add a story in the brush stories plz :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants