floating-ui / react-popper

🍿⚛Official React library to use Popper, the positioning library
https://popper.js.org/react-popper/
MIT License
2.5k stars 226 forks source link

Popper not positioning properly when using React.createElement #446

Closed sebpowell closed 2 years ago

sebpowell commented 2 years ago

In my app, I have a generic list component to which I pass a component prop which will determine the appearance and contents of each list item.

Each list item has an options dropdown. To avoid polluting the DOM with many dropdowns, I have created a single dropdown to which I pass the reference for the clicked element, which then determines it's positioning.

I have used this pattern successfully before, without any issues – the only difference this time is that I'm using this 'component' prop I mentioned. This gets passed in to the parent component, which then renders it using createElement.

Example 2 in the below demo shows the basic idea. You'll see that the reference elements are set properly in both cases – but in example 2, the popup moves to the top of the viewport, suggesting the actual position hasn't been picked up properly.

I then tried using https://popper.js.org/docs/v2/virtual-elements/ – and had some success, although I then ran into issues when resizing the viewport.

So my question is:

1.) Has anyone had any success implementing something like this?

2.) If it is possible, what am I doing wrong?

Any tips gratefully accepted!

Reproduction demo

https://codesandbox.io/s/react-popper-v2-x-issue-template-forked-x6jsij?file=/src/index.js

import React, { useState, createElement } from "react";
import { render } from "react-dom";
import { usePopper } from "react-popper";

const ComponentWithRenderProp = ({ component, ...rest }) => {
  return createElement(component, { ...rest });
};

const popperStyle = {
  width: 20,
  height: 20,
  backgroundColor: "purple"
};

const Repro = () => {
  const [reference, setReference] = useState(null);
  const [popper, setPopper] = useState(null);
  const { styles, attributes } = usePopper(reference, popper, {
    placement: "left",
    modifiers: []
  });

  return (
    <>
      <div
        ref={setPopper}
        style={{ ...popperStyle, ...styles.popper }}
        {...attributes.popper}
      />

      {/* Example 1: This works without any issues */}
      <div
        onClick={(e) => {
          setReference(e.currentTarget);
        }}
      >
        Example 1
      </div>

      {/* Example 2: This doesn't work – the reference is set properly, but Popper doesn't seem to register it. */}
      <ComponentWithRenderProp
        component={(props) => {
          return (
            <div
              onClick={(e) => {
                e.persist();
                console.log(e.currentTarget);
                setReference(e.currentTarget);
              }}
            >
              Example 2
            </div>
          );
        }}
      />
    </>
  );
};

render(<Repro />, document.getElementById("root"));
atomiks commented 2 years ago

I think the DOM node is getting recreated on every render, so reference is not pointing to the actual DOM node on the next render.

If you log this out in render, you'll see it becomes null because the one stored in state is unmounted:

console.log(reference?.parentNode)

One solution is to ensure it doesn't get recreated every render:

  const jsx = useCallback(
    (props) => (
      <div
        onClick={(e) => {
          e.persist();
          console.log(e.currentTarget);
          setReference(e.currentTarget);
        }}
      >
        Example 2
      </div>
    ),
    []
  );

      <ComponentWithRenderProp component={jsx} />
sebpowell commented 2 years ago

Amazing @atomiks!!

That solved my problem – spent ages trying to work out, thank you very much. I've updated the example so anyone else facing this issue can see what the solution was:

https://codesandbox.io/s/react-popper-v2-x-issue-template-forked-x6jsij?file=/src/index.js