Skip to content

Conversation

acontreras89
Copy link

This is done by checking the current highlighted series.

This is done by checking the current highlighted series
@mirabilos mirabilos merged commit d92cd5a into danvk:master Dec 9, 2022
@acontreras89
Copy link
Author

Hey @mirabilos, thanks for merging our PR! 🎉

After some time using our own version of the library (which included this change), we spotted a little bug 🐛 eventually point would be undefined, causing the following error:

plot-zoom-out-of-sync-error

Not sure why this was happening,* we fixed it by simply adding a guard in commit redradix@1a74f4d.

* It is possible that when the user is moving the mouse, we try to calculate the next legend position before the highlight series has been updated. In this case, a point for the highlighted series may not be available 🤷‍♂️

@mirabilos
Copy link
Collaborator

Hi @acontreras89, interesting. Why not simply default to points[0] then? Is there any negative effect to expect from returning early, or will the code just be called again once points are available again?

@acontreras89
Copy link
Author

@mirabilos falling back to points[0] would most likely be a better idea.

Our use case is very sensitive in terms of performance, so we opted for the guard assuming this was an unnecessary legend computation (following on my previous comment.)

But honestly, I think we simply did not put that much effort into figuring out what was really happening. I just wanted to let you know this can happen now that the PR (finally) got merged 🙃

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

Successfully merging this pull request may close these issues.

2 participants