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

Setting the viewport when minimized causes node/edge misalignment #2276

Closed
joeyballentine opened this issue Jul 11, 2022 · 10 comments
Closed
Assignees

Comments

@joeyballentine
Copy link
Contributor

Describe the Bug

Initial discord conversation for context: https://discord.com/channels/771389069270712320/859774873500778517/995439004969353246

If the viewport gets set when the window is minimized, edges and nodes become misaligned. I have a useEffect that sets nodes, edges, and viewport to cached values when refreshing, and if I refresh, minimize, wait a bit, then open the window again, I can consistently get this bug. It does seem to be misaligned by the amount the viewport changed while minimized as well.

Your Example Website or App

The current main version (not release version) of chaiNNer has this problem

Steps to Reproduce the Bug or Issue

  1. Have code to set nodes, edges, and viewport on start/mount
  2. Refresh page
  3. Minimize page immediately after refresh
  4. Wait a bit for refresh to finish
  5. Open the window back up

Expected behavior

I would expect this to not happen

Screenshots or Videos

unknown-12

Platform

  • OS: windows
  • Browser: chrome

Additional context

No response

@joeyballentine joeyballentine changed the title Updating the viewport when minimized causes node/edge misalignment Setting the viewport when minimized causes node/edge misalignment Jul 11, 2022
@moklick
Copy link
Member

moklick commented Jul 11, 2022

Thanks for the report! Could you create a codesandbox for this issue? I saw this while developing, but not in a production app.

@joeyballentine
Copy link
Contributor Author

I'll try to recreate it in a codesandbox.

@joeyballentine
Copy link
Contributor Author

https://nfj1y4.csb.app/

Refresh, minimize, wait, and open again. It'll be misaligned.

image

@joeyballentine
Copy link
Contributor Author

joeyballentine commented Jul 11, 2022

I actually just discovered something interesting when messing around in that codesandbox (I think now my messing around is saved in there if you wanna check). I replaced

    setViewport({
      x: 200,
      y: 200,
      zoom: 0.5,
    });

with

    setViewport({
      x: 200,
      y: 200
    });
    zoomTo(0.5);

and now it causes the issue no matter what, AND locks the viewport up every time (which is another issue I've run into randomly)

@joeyballentine
Copy link
Contributor Author

joeyballentine commented Jul 11, 2022

I believe the misalignment is caused by state update batching from react 18. In that CSB I replaced the root rendering with the legacy version that does not batch state updates, and it was no longer misaligned (the viewport did lock up still though, at least until I put everything back in the setViewport and got rid of zoomTo).

If you wanna see that, I left the legacy rendering code in there commented out so you can easily switch back to it if you wanna try that out

@moklick moklick added this to the 10.3.9 milestone Jul 12, 2022
@moklick moklick self-assigned this Jul 12, 2022
@moklick
Copy link
Member

moklick commented Jul 12, 2022

Hey @joeyballentine

could you check if react-flow-renderer@next (10.3.9-next.0) works for you?

@joeyballentine
Copy link
Contributor Author

Yes I will test as soon as I can, which won't be until much later today.

@joeyballentine
Copy link
Contributor Author

@moklick it does appear to be fixed now! The viewport locking up issue I mentioned (and replicated by doing that zoomTo) still happens though. But the edges thing is definitely fixed. Thanks!

@moklick
Copy link
Member

moklick commented Jul 13, 2022

Ok, that's good. I haven't looked into the zoomTo bug. Will do later!

@moklick
Copy link
Member

moklick commented Jul 14, 2022

Thanks! Fixed in v10.3.9.

  1. We no longer rely on the zoom level for calculating the handle positions.
  2. setViewport doesn't break anymore when you pass a part of a viewport.

@moklick moklick closed this as completed Jul 14, 2022
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

No branches or pull requests

2 participants