-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
Update detectElementResize to support rendering into iframes and child windows #900
Conversation
Can you point me at an example of the type of thing you're trying to do that this change enables? A Codesandbox demo would be great! |
…rget * Create detectElementResize CSS in the window containing the rendered AutoSizer. * Create the resize trigger element using the document containing the rendered AutoSizer.
0827bde
to
7b89645
Compare
Sure, I created a Codesandbox here: https://codesandbox.io/s/nro7yzrqv0. Two popup windows are opened -
I also updated the PR to remove some changes to AutoSizer that turned out to be unnecessary. |
Both pop-up windows appear to resize the same for me. (Although neither shows the background color correctly. I have to inspect the HTML manually and watch it being updated.) |
Oops, turns out some of those changes are needed for Chrome, but not for Firefox. Working on adding them back now. |
Pushed a new commit and updated the Codesandbox. It now works properly in both Chrome and Firefox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for updating the CodeSandbox. Sorry it has taken me so long to circle back here. I've been busy. 😄
I think this looks okay, although I haven't messed around with portals in popup windows before so I don't have any direct experience.
I've tested your change locally and verified that it doesn't seem to cause any regressions. So let's roll with it. 👍
@bvaughn Is it possible to cover it with puppeteer tests? |
I dunno. Maybe? |
@ahutchings Would you like to take care about test for this case? |
Sure, I will take a look at creating a test case for this. Looks like I can use the WindowScroller end-to-end test as an example. |
Nice! Thanks! |
Without these changes, the size of elements rendered in remote windows is incorrectly detected as height 0/width 0.
I'm not sure how to go about testing for this change - any suggestions are welcome.