Shared events not updating correct children/ancestors in complex component trees

[jljorgenson18]

Bugs and Questions

Checklist

• [x] I have read through the FAQ and Guides before asking a question

• [x] I am using the latest version of Victory

• [x] I've searched open issues to make sure I'm not opening a duplicate issue

The Problem

My project has functionality where when you hover over labels inside of a legend in a stacked or grouped line chart, the corresponding line is supposed to be emphasized while the the other lines are de-emphasized. I attempted to get this working with shared events but noticed that the last line would never update. After many hours of debugging, I was able to pinpoint the problem here

https://github.com/FormidableLabs/victory/blob/main/packages/victory-shared-events/src/victory-shared-events.js#L244

What seems to be happening is that for complex components trees, getNewChildren in VictorySharedEvents has a disconnect between childNames and childComponents. childNames ends up not including every single victory component and once the recursive step is executed for alterChildren, the last index for VictoryLine is now pointing to a childName of "parent", thus this condition fails

if (
          childNames[index] !== "parent" &&
          child.type &&
          isFunction(child.type.getBaseProps)
        ) 

when it shouldn't.

I was trying to come up with a PR to address this but I'm still trying to wrap my head around the Victory internals. Basically though, looking up childNames by index and using the keys from baseProps to get child names does not seem to be sufficient for shared events.

Reproduction

I was able to mostly reproduce the problem, though my project is seeing slightly different behavior in that the last item never works. For this repro, it briefly flickers and then stops working on the next re-render. https://codesandbox.io/s/jolly-lalande-f20i8?file=/index.js

[becca-bailey]

Hi @jljorgenson18, thanks for explaining this bug! A few issues have come up lately around VictorySharedEvents, and it's on our roadmap to give the Victory event system a pretty major overhaul so that these issues are easier to debug. As you have probably noticed, there's a lot of magic happening around how Victory handles events and passes event handling props down to its children. I'll look into this a little more to see if there's a temporary fix, but if not we should be able to address it in a larger-scale update.

[jljorgenson18]

@becca-bailey Thank you for getting back to me. Is there a timeline on when a refactor might happen? A temporary fix would be great if possible but the project is on hold for a few months anyway. I can try to take another look as well and try to wrap my head around the internals.

[chriszrc]

@jljorgenson18 I removed the <VictoryStack> and added real data, and now it seems to be working?

https://codesandbox.io/s/focused-water-bk6mjt?file=/index.js

[jljorgenson18]

@chriszrc I'm not using Victory anymore so I don't think I can help further.

[chriszrc]

@jljorgenson18 did you decide to use another react charting framework? Just curious

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. If there is no activity in the next 7 days, the issue will be closed.

[github-actions[bot]]

This issue was closed because it has been inactive for 7 days since being marked as stale. Please open a new issue if you believe you are encountering a related problem.