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

When multiple charts with Brush are synced, Brush doesn't get updated #3027

Open
1 task done
hanbyul-here opened this issue Oct 19, 2022 · 12 comments
Open
1 task done
Labels
enhancement Enhancement to a current API feature request Issues that are feature requests

Comments

@hanbyul-here
Copy link

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

Reproduction link

Edit on CodeSandbox

Steps to reproduce

Interact with any Brush. All the charts will react to the change of Brush, but other Brush components don't update.

What is expected?

Brush components get synced (update their handle positions accordingly).

What is actually happening?

Brush components not responding.

Environment Info
Recharts v2.1.6
React v18.0.2
System Mac OS
Browser Chrome

I found a similar issue from 2 years ago: #1796
This issue is supposed to be fixed, and I still see the problem. I wonder if I am missing anything?
(It seems recent releases are missing from the dropdown, but I did try with the latest release.)

@natBizitza
Copy link

I use one brush for various sync charts. In case that is not a requirement to have a brush for each chart, it works. Though, it's a workaround and does not close this issue.

@ckifer ckifer added enhancement Enhancement to a current API feature request Issues that are feature requests labels Dec 29, 2022
@ckifer
Copy link
Member

ckifer commented Dec 29, 2022

Hi @hanbyul-here @natBizitza

I think the intention is @natBizitza 's suggestion which is to have one brush to control all synced charts.

I will label this as a feature request for prioritization

@snelsi
Copy link

snelsi commented Feb 7, 2024

@ckifer
Bumping this ticket. Just encountered the same issue.

Brushes from synced charts do not get automatically updated when you drag one of them.

To make matters worse, the Brush component completely ignores the startIndex / endIndex parameters if you pass them. Thus, it's impossible to override this default behavior with your custom controlled logic. 😕

I feel like it's more of a bug than a feature request, to be honest.

@ckifer
Copy link
Member

ckifer commented Feb 7, 2024

Not respecting start and end index is a bug (though one I thought was fixed), but supporting a synced brush is a feature request. Feel free to create a new issue for the index problem

@ckifer
Copy link
Member

ckifer commented Feb 7, 2024

This PR should have fixed the index issue.. #4034. Released in 2.10.4

@snelsi
Copy link

snelsi commented Feb 7, 2024

@ckifer
Here's a sandbox with a reproduction 👇
https://codesandbox.io/p/devbox/recharts-brush-issue-c56k82

image

@yogasanas
Copy link

I am facing a similar issue. I tried to create an empty Composed Graph with just a brush child component. While this somewhat solves the problem. I started getting a lot of cant read properties of null.

From what I could understand this was because tooltip was being synced across the charts and the chart with only brush has no tooltip/graphical components to fetch tooltip information from.

I was also not able to hide the tooltip to make the null issue go away. This is quite frustrating because currently there is no way to sync multiple graphs cleanly that use brushes. My layout doesn't allow me to just put brush on a random chart.

@ckifer
Copy link
Member

ckifer commented Feb 14, 2024

Please see my responses here #4163 (comment)

You get errors with an empty chart because Brush relies on state from other components (XAxis) in order to work correctly. We can make error messaging better there.

Hopefully, this becomes an easier issue to resolve in 3.x with the refactoring we're doing, but right now it doesn't seem possible without making the UX worse than it currently is (see video in the other thread)

@yogasanas
Copy link

yogasanas commented Feb 14, 2024

@ckifer A hacky fix but I fixed it by adding a hidden <Line> to the chart that has the brush. This was able to solve it for me, but its quite hacky.

<ComposedChart
        width={1000}
        height={50}
        data={data}
        margin={{
          top: 0,
          right: 100,
          bottom: 20,
          left: 100,
        }}
        {...(syncId && { syncId })}
        className="col-start-1 row-start-1"
      >
        <Line
          type="monotone"
          dataKey="Value"
          stroke="#ff7300"
          className="hidden"
        />

        <Brush
          dataKey="Date"
          height={30}
          stroke="#8884d8"
          startIndex={dataLength - 20}
          travellerWidth={5}
        />
      </ComposedChart>

@ckifer
Copy link
Member

ckifer commented Feb 14, 2024

Decent workaround for now for sure! I know exactly what the work is to allow this the correct way and luckily it will get done indirectly as a result of other ongoing refactoring

@yogasanas
Copy link

Are there any updates for this? Being unable to setState inside the onchange controller on a brush is blocking some features we would like to ship.

@ckifer
Copy link
Member

ckifer commented Mar 26, 2024

@yogasanas see #4163 (comment) and the video below that. Refactoring is ongoing but we haven't gotten here yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to a current API feature request Issues that are feature requests
Projects
None yet
Development

No branches or pull requests

5 participants