search

floating-ui / react-popper

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

The legacy class components are not working correctly in v2.x #387

Closed harryfino closed 4 years ago

harryfino commented 4 years ago

The legacy class components are not working correctly when updating from v1.3.7 to v2.x. I'm using the referenceElement property in the <Popper> component and something isn't working correctly.

Reproduction demo

This link shows a working example of Popper as a tooltip when you hover over the word "editing". https://codesandbox.io/s/legacy-popper-usage-m5v83?file=/src/App.js

When you change to react-popper to v2.x in the dependencies on the left, this example starts failing on mouseover with the error Cannot read property 'x' of undefined.

Steps to reproduce the problem

  1. Hover over the word "editing" and observe the correct behavior using v1.3.7 of react-popper Screen Shot 2020-09-22 at 1 58 21 PM

  2. Switch dependencies to v.2.2.3 of react-popper. Screen Shot 2020-09-22 at 1 53 24 PM

  3. Hover over the word "editing" and observe the error message. Screen Shot 2020-09-22 at 2 01 04 PM

What is the expected behavior?

The expected behavior is that this would show the Popper in the correct position without raising an error.

Packages versions

harryfino commented 4 years ago

It looks like this error is caused by top-middle not being supported in v2. I switched to top instead and it worked correctly. I'm closing the issue, but keeping this around in case someone else runs into this problem.

FezVrasta commented 4 years ago

top-middle was never supported, the placements have always been top-start, top, top-end

harryfino commented 4 years ago

Maybe it was a typo that I made a long time ago, but I was never punished for it in v1 and it placed the popper in the correct position. The error handling is v2 did not tell me this was invalid and sent me on a goose chase for hours. Maybe something like this would go a long way?

Popper.propTypes = {
     placement: PropTypes.oneOf([
          'top-start',
          'top',
          'top-end',
          ...
     ])
};

In my components, this helps prevents me from passing in the wrong text.

Anyway, cheers and thank you for a great library!

FezVrasta commented 4 years ago

We ship Flow and TS type defs that should warn you about this, PropTypes are basically deprecated at this point.