Semantic-Org / Semantic-UI-React

The official Semantic-UI-React integration
https://react.semantic-ui.com
MIT License
13.21k stars 4.05k forks source link

fix(Popup): fix positioning in scrollable container #4446

Closed CoryDuncan closed 9 months ago

CoryDuncan commented 10 months ago

Fixes https://github.com/Semantic-Org/Semantic-UI-React/issues/4444

This PR addresses a regression for the Popup component where a Popup 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 the isElement() check and the fallback, a contextElement property does not exist.

This PR updates ReferenceProxy to add the contextElement property.

Original fix: https://github.com/Semantic-Org/Semantic-UI-React/pull/3607

welcome[bot] commented 10 months ago

💖 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[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (888b2f7) 99.51% compared to head (33bc3b7) 99.51%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #4446 +/- ## ======================================= Coverage 99.51% 99.51% ======================================= Files 186 186 Lines 3511 3512 +1 ======================================= + Hits 3494 3495 +1 Misses 17 17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

c3-juanca commented 10 months ago

@CoryDuncan really appreciate your help!

@layershifter what's necessary to get this merge to version 2.1.4?

layershifter commented 10 months ago

@CoryDuncan thanks ❤️ I will check some PRs/issues this week and trigger a release for next beta in v3.

CoryDuncan commented 10 months ago

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 2.1.4 and doesn't have any near term plans to migrate to 3 (at least while it's in beta).

c3-juanca commented 10 months ago

@layershifter I'm in the same position as Cory. Is it possible to release this as a patch in v2?

adithya1310 commented 10 months ago

@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

welcome[bot] commented 9 months ago

Congrats on merging your first pull request! 🎉🎉🎉

robot victory dance

layershifter commented 9 months ago

@CoryDuncan @c3-juanca @adithya1310 as an exception I released the fix in semantic-ui-react@2.1.5.

c3-juanca commented 9 months ago

Really appreciate it, @layershifter 🙌

adithya1310 commented 9 months ago

[heart] Adithya Ganapathy reacted to your message:


From: Oleksandr Fediashov @.> Sent: Sunday, December 10, 2023 1:51:44 PM To: Semantic-Org/Semantic-UI-React @.> Cc: Adithya Ganapathy @.>; Mention @.> Subject: Re: [Semantic-Org/Semantic-UI-React] fix(Popup): fix positioning in scrollable container (PR #4446)

@CoryDuncanhttps://urldefense.com/v3/__https://github.com/CoryDuncan__;!!NgNcsg68QA!nkDTvpHrIsLsAvphh7QGM5SftZzrQA4F9TEXKdFHTHkPFLyENwnA1WmdMrG4wur0iV21yDZA-225W2R2YxejmsvQMxqQegM$ @c3-juancahttps://urldefense.com/v3/__https://github.com/c3-juanca__;!!NgNcsg68QA!nkDTvpHrIsLsAvphh7QGM5SftZzrQA4F9TEXKdFHTHkPFLyENwnA1WmdMrG4wur0iV21yDZA-225W2R2YxejmsvQM9ilMkE$ @adithya1310https://urldefense.com/v3/__https://github.com/adithya1310__;!!NgNcsg68QA!nkDTvpHrIsLsAvphh7QGM5SftZzrQA4F9TEXKdFHTHkPFLyENwnA1WmdMrG4wur0iV21yDZA-225W2R2YxejmsvQ0c0q_Cw$ as an exception I released the fix in @.***

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/Semantic-Org/Semantic-UI-React/pull/4446*issuecomment-1848971363__;Iw!!NgNcsg68QA!nkDTvpHrIsLsAvphh7QGM5SftZzrQA4F9TEXKdFHTHkPFLyENwnA1WmdMrG4wur0iV21yDZA-225W2R2YxejmsvQyGxJ8-g$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AMD5LDMPSK7HD54GCOFLQMLYIW47BAVCNFSM6AAAAABAAJKGQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYHE3TCMZWGM__;!!NgNcsg68QA!nkDTvpHrIsLsAvphh7QGM5SftZzrQA4F9TEXKdFHTHkPFLyENwnA1WmdMrG4wur0iV21yDZA-225W2R2YxejmsvQ58Ghg1Q$. You are receiving this because you were mentioned.Message ID: @.***>