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

Brush component ignores passed startIndex / endIndex params in Charts with syncId #4163

Open
1 task done
snelsi opened this issue Feb 7, 2024 · 4 comments
Open
1 task done
Labels
bug General bug label

Comments

@snelsi
Copy link

snelsi commented Feb 7, 2024

  • I have searched the issues of this repository and believe that this is not a duplicate.

Reproduction link

Edit on CodeSandbox

Steps to reproduce

Top and bottom charts are synced.
Drag the top (blue) brush to change its range:

image

New startIndex / endIndex params are saved in the state and passed to the bottom (green) brush component.
Even though new startIndex / endIndex params are passed, nothing happens.
The previous range is still displayed on the bottom (green) brush.

image

What is expected?

All brushes in synced charts need to be updated when one of them changes.

Or make sure that Brush component respects passed startIndex / endIndex params and updates when startIndex or endIndex changes

What is actually happening?

startIndex / endIndex values are ignored. Previous range is displayed in the second brush. Brushes are out of sync.

Environment Info
Recharts v2.11
React 18.2.0
System Windows 11
Browser Chrome 121

Related:
#3027

@ckifer
Copy link
Member

ckifer commented Feb 7, 2024

Thanks for this, will look when I get a chance

@ckifer ckifer added the bug General bug label label Feb 7, 2024
@ckifer
Copy link
Member

ckifer commented Feb 7, 2024

Okay so my recent change to fix this fixed a partial problem but not all of it.

Edit recharts-brush-issue (forked)

Here if you update the indices via inputs, everything updates correctly. However if you update state via the onChange function, the chart updates but the Brush does not. Suspect its a simple fix somewhere in the same place as #4034

Feel free to submit a PR, otherwise I'll take this when I can

@ckifer ckifer added good first issue Issues with this label are great for first time contributors and removed good first issue Issues with this label are great for first time contributors labels Feb 7, 2024
@ckifer
Copy link
Member

ckifer commented Feb 8, 2024

this is unfortunately more daunting than meets the eye, not a good first issue. You can semi-fix the problem by always incrementing updateId here https://github.com/recharts/recharts/pull/4034/files#diff-aa1b4fbed632e1bdab786e0521346d3a1a2386c633f22f5778d4bcbc47581a65R1292 but that adds more issues than it solves.

@ckifer
Copy link
Member

ckifer commented Feb 8, 2024

I could "fix" this, but it breaks draggability of the Brush handles because of too many re-renders

synced-brush-issue-recharts.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug General bug label
Projects
None yet
Development

No branches or pull requests

2 participants