search

techniq / mui-downshift

Thin layer over paypal's downshift to use Material UI visual components
https://techniq.github.io/mui-downshift/
MIT License
92 stars 24 forks source link

breaking change on downshift 2.0.20 #46

Closed sibelius closed 6 years ago

sibelius commented 6 years ago

https://github.com/paypal/downshift/releases/tag/v2.0.20

the validation of getMenuProps is causing troubles, like this:

Uncaught Error: downshift: The ref prop "ref" from getMenuProps was not applied correctly on your menu element

I think the problem is that we don't always use getMenuProps, like here

https://github.com/techniq/mui-downshift/blob/master/src/Menu.js#L190

An idea to fix this is to use suppressRefError prop

techniq commented 6 years ago

@sibelius I've been looking at this since you've reported it. The root cause appears to be the use of a Portal within Popper (we used to use react-travel's Portal but then switched to Material-UI's impl, and then removed the direct usage when switching to Material-UI's Popper instead of react-popper.

If I add disablePortal prop to Popper this fixes the downshift error, but breaks usage with Dialogs.

I then tried to add a Portal reference explicitly back, but with unsatisfactory results. Lastly, I added the { suppressRefError: true } as the second argument to getMenuProps and this resolved the issue, although I question if there isn't an underlying issue here being exposed (maybe the cause of #43)

techniq commented 6 years ago

Released version 1.0.4 with the fix.

sibelius commented 6 years ago

Awesome work, tks

techniq commented 6 years ago

@sibelius welcome