Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Single frame positioning glitch in concurrent mode #349

Closed
jamiewinder opened this issue Apr 13, 2020 · 7 comments · Fixed by #440
Closed

Single frame positioning glitch in concurrent mode #349

jamiewinder opened this issue Apr 13, 2020 · 7 comments · Fixed by #440
Labels

Comments

@jamiewinder
Copy link
Contributor

jamiewinder commented Apr 13, 2020

I think this is specific to React's experimental concurrent mode, though it occurs with both createRoot and createBlockingRoot.

When used in either, the popper can briefly appear in the top-left of the viewport before shifting into the correct position. If the popper instance had calculated a position already, then it'll briefly appear in the old position before moving to the new one (though that isn't demonstrated in this demo to keep it simple).

This only occurs for a frame, and is often easy to miss. It appears to affect both the hook and the old render prop. If you change React back to the non-concurrent mode (e.g. ReactDOM.render) then the problem appears to go away.

I've made a demo to demonstrate this.

Reproduction demo

https://codesandbox.io/s/react-popper-glitch-lcvdm?file=/src/index.js

Steps to reproduce the problem

  1. Click the 'Toggle' button
  2. (Hopefully!) Observe that the 'Popper!' element appears in the top-left of the viewport before shifting to the correct position to the right of the 'Toggle' button.
  3. Click the 'Reset' button to repeat (this forces the popper instance to reinitialise).
@FezVrasta
Copy link
Member

Thanks for the report!

I think we could set a visibility: hidden to the popper element until we get it positioned, it should fix the issue.

@jamiewinder
Copy link
Contributor Author

Thanks.

I was thinking along those lines, though there is the other detail of the issue (which I'm having trouble creating an effective demo for), where the popper appears at the old position before updating to the new one on some occasions. It basically looks like the popper tries to show before it's ready when the reference is (re-)positioned rather than just on the first update.

I'll keep trying to create a demo for this, or failing that a screencap of it on my project.

@FezVrasta FezVrasta added the bug label May 13, 2020
@circlingthesun
Copy link

@jamiewinder @FezVrasta if you make react do a bit more work it's easy to reproduce. See https://codesandbox.io/s/react-popper-glitch-forked-vnt5z

I've worked around this in my code via:

<div
  style={{
    ...styles.popper,
    visibility: !styles.popper.transform ? 'hidden': 'visible',
  }}
>
...
</div>

@therealgilles
Copy link

therealgilles commented Apr 6, 2022

This same issue has been reported in semantic-ui-react, see here, which uses react-popper. Adding a link here for reference. I suggested a possible fix in usePopper.js, but not sure if it is the right fix.

@atomiks
Copy link
Collaborator

atomiks commented Apr 11, 2022

@therealgilles This also happens in https://floating-ui.com/docs/react-dom which doesn't set top/left initially, the user sets top: y ?? ''

From debugging, it appears to be due to the positioning state update occurring asynchronously (in a microtask). Popper does this for the first update, while Floating UI's whole API is async to support async platform measurement methods like in React Native.

The visibility trick therefore might be the only solution, however, I noticed it doesn't handle cases like this (6x throttling):

Screen.Recording.2022-04-11.at.3.17.18.pm.mov

I think this is because the x and y values "live" after the first computation, but need to be unset again every time the floating element is closed.

@atomiks
Copy link
Collaborator

atomiks commented Apr 11, 2022

It looks like we need to use the new flushSync() API

@atomiks
Copy link
Collaborator

atomiks commented Apr 11, 2022

Apparently flushSync() has existed for >3 years now (was there when hooks got introduced), genuinely had no idea 😬

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

Successfully merging a pull request may close this issue.

5 participants