-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(Popup): fix positioning in scrollable container #4446
Conversation
💖 Thanks for opening this pull request! 💖 Here is a list of things that will help get it across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4446 +/- ##
=======================================
Coverage 99.51% 99.51%
=======================================
Files 186 186
Lines 3511 3512 +1
=======================================
+ Hits 3494 3495 +1
Misses 17 17 ☔ View full report in Codecov by Sentry. |
@CoryDuncan really appreciate your help! @layershifter what's necessary to get this merge to version |
@CoryDuncan thanks ❤️ I will check some PRs/issues this week and trigger a release for next beta in v3. |
@layershifter would it be possible to release this as a patch in v2? The project I'm working on is using version |
@layershifter I'm in the same position as Cory. Is it possible to release this as a patch in v2? |
@layershifter would it be possible to release this as a patch in v2 itself as we would need this feature to be fixed for our requirement |
Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com> (cherry picked from commit 076f818)
@CoryDuncan @c3-juanca @adithya1310 as an exception I released the fix in |
Really appreciate it, @layershifter 🙌 |
Fixes #4444
This PR addresses a regression for the
Popup
component where aPopup
inside a non-document scrollable container would not update its position on scroll.Context:
Popper.JS
attempts to find scroll ancestors based on the reference element here: https://github.com/floating-ui/floating-ui/blob/92195083958c9496988ed4526b82b75b7256b5a4/src/createPopper.js#L80-L83.The problem is that the reference element provided via
Popup
fails theisElement()
check and the fallback, acontextElement
property does not exist.This PR updates
ReferenceProxy
to add thecontextElement
property.Original fix: #3607