fix: Iframe case for tooltip user component#4467
Conversation
5b42dde to
f80513c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4467 +/- ##
=======================================
Coverage 97.40% 97.40%
=======================================
Files 933 934 +1
Lines 29486 29501 +15
Branches 10698 10701 +3
=======================================
+ Hits 28720 28735 +15
Misses 718 718
Partials 48 48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * Resolves the portal container to the ownerDocument.body of the referenced element. | ||
| * Ensures portaled content renders in the correct document context (e.g. inside iframes). | ||
| */ | ||
| export function usePortalContainer( |
There was a problem hiding this comment.
Why not let the Portal from component-toolkit use this by default? Is it not desired in general to append portal elements to the owner document instead of the child document if using an iframe?
This fix does not include feature prompt and popover, which also use Portal without arguments. Can they not be affected too? Fixing this as default behavior would address them out of the box.
There was a problem hiding this comment.
yes, I noticed the feature prompt and the popover components. Wanted to minimize the PR size and add those as a follow up to make it easy to review.
There was a problem hiding this comment.
Why not let the Portal from component-toolkit use this by default?
Interesting, I don't see any argument against that. I think that would fix every single usecase no?
There was a problem hiding this comment.
But still defaulting to ownerDocument in the toolkit like here solves only half the problem. We still have to attach scroll listener to the right document, instead of glbal window.
c0ee6e8 to
6e59666
Compare
Fix Iframe tooltip and Portal usage bugs.
Description
Related links, issue AWSUI-61932
How has this been tested?
Created Test Pages and did manual test and added Integ tests too.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.