floating-ui / popper.js.org

🍿🦉Website and Documentation for Popper and friends
https://popper.js.org
MIT License
74 stars 392 forks source link

[tutorial] Wrong popper position when initialized with display:none #17

Closed Vovan-VE closed 3 years ago

Vovan-VE commented 3 years ago

CodeSandbox demo

https://codesandbox.io/s/react-popper-v2x-issue-template-forked-59dv1

Steps to reproduce the problem

Follow tutorial: https://popper.js.org/docs/v2/tutorial/#functionality

  1. Both reference and popper are inside of container with position: relative (this is a part of page layout). Without position: relative in ant parents an arrow will be positioned wrong.
  2. Popper initialized being display: none, similar to tutorial.
  3. User trigger an event, so a popper became display: block, similar to tutorial.

Tutorial lead you right to the bug which wouldn't be fixed (#413).

What is the expected behavior?

A popper positioned correctly. Tutorial helps you. Tutorial does explain or mention a problem with display: none positioning, and doesn't advice well-known buggy things without well-known solutions to workaround them.

What went wrong?

A popper position is wrong at first render. However if you resize viewport or scroll it, a popper will take correct position. You are trying to research what did you do wrong. Tutorial mess you.

Any other comments?

popperjs/popper-core#413 was found a time ago after a bug hitted. I suppose, I should to initialize a popper only after it becomes display: block and destroy when it becomes display: none. In theory it should to work.

@popperjs/core: 2.6.0

Vovan-VE commented 3 years ago

I suppose, I should to initialize a popper only after it becomes display: block

Yes, with react-popper hook it now works correct if I render popper conditionally instead of playing with display:

<>
  <div ...>reference</div>
  {visible && (
    <div ...>popper</div>
  )}
</>

So, please, fix tutorial.

welkinwong commented 3 years ago

same😢

FezVrasta commented 3 years ago

I'm not sure I understand your issue, the tutorial code creates a Popper instance after the tooltip is shown, and destroys it after it's hidden.

Where do you see it's trying to initialize Popper before?

Vovan-VE commented 3 years ago

Tutorial advices to use CSS rules to toggle display between none <=> block. By default it's none. If I'm wrong here?

However tutorial itself implements it in a different way: https://github.com/popperjs/website/blob/master/src/components/Tutorial.js#L139 But a reader doesn't suspect anything. I'm one on such reader. I did follow tutorial literally, and - ta-da - https://github.com/popperjs/popper-core/issues/413 (wontfix) reproduced in action with help of tutorial.

FezVrasta commented 3 years ago

I see what you mean, I just updated the tutorial with a cleaner approach. Please let me know how it looks now.