-
-
Notifications
You must be signed in to change notification settings - Fork 983
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
Incorrect initial height. 'init' event stacking + called excessively. Page bogged down. tolerance ignored #1309
Comments
I can have more of a look tomorrow, but the issue is going to be the footer at the bottom of the page causing a resize loop. The workaround is to pick another element on the page to use to measure the content size and then add a |
@davidjbradshaw . I have updated the pen to include an |
That is very strange. It's getting late here in the U.K., I will dig into this more tomorrow. |
Thank you. For the record, we were not getting this same issue with the v4. Instead we were able to replicate #1156 on v4 and thought upgrading would fix the issue, which it did but now we are getting this new issue. |
One last thing, could you install the latest beta version in this example and set the log option to true. The beta builds have a lot more logging details in them. |
@davidjbradshaw Done. Both child frame and parent pen have been updated to |
Another thing to note, the child frame is using react + react css transition replace library https://www.npmjs.com/package/react-css-transition-replace to animate height changes between navigation states. This could be something we are doing that could be effecting the height calculations. |
Thanks, that will make finding the source of the loop easier. |
Having slept on this and taken a look, I think issue is that you are calling the child script every time the React render runs and I accidentally removed the check from this in the latest version. Can you please try Going forward you should move the import for the child script to outside of the React Render. I expect the footer might still cause issues, but if so, that is a different problem we can look at. |
I had a look at the source code of react-css-transition-replace and I think it will cause issues, but these can be worked around using the Custom page size options. |
@davidjbradshaw I was thinking it was somehow related to react rendering as well, so on the child frame I made sure to include it via CDN in the main html index just so make sure it was never touched by JS. So I don't think that's the problem, react never initializes it. |
@davidjbradshaw I am using the custom frame size option on the 'main' tag as per your suggestion and the problem persists. You can see in the child frame on the pen I'm using the 'data-iframe-size'. The 'main' tag does contain the element subject to react-css-transition-replace but we've hadn't had this issue with v4. |
@Gamerweazel, can you please give the latest beta a try, so we can rule it out one way or the other. |
I placed your iframe in my test page and I get the following logged out. The child script gets initialised twice on this page. This is either down to you navigating from one page to another or it being loaded twice. The other odd thing is that it thinks the iframe has been hidden a few times, due to the content height being set to zero. Not a major issue, as when this happens it returns the existing iframe height, so nothing happens. But it did strike me a little strange. Are you completely deleting all the content on the page a few times? I will need a login, if you would like me to see what happens on the next page.
|
@davidjbradshaw You can cycle back and forth using the "Forgot Password" button. Going back and forth causes the slowdown. |
Found another bug and have just published |
@davidjbradshaw I bumped the version on the pen and child frame to |
My bad, can you please try Beta.3. I published from the branch and forgot to fold in the latest update from Master. |
@davidjbradshaw It seems like the stacking issue is still present after bumping to latest beta. I updated the pen and the child frame to |
OK bugger, looking at the logs a bit more, and tracking things back, I'm wondering if the following function is the issue. https://github.com/davidjbradshaw/iframe-resizer/blob/master/packages/common/utils.js#L3-L11 Think I need to sleep on this some more, I can pick this up again on Thursday. Hopefully this two bugs down and only one more to go. |
@davidjbradshaw Are there local development instructions we can follow and see if we can encounter a solution and post back about it here? |
If you fork and download the repo, the build process is simple enough. npm run build:dev You can also do npm run test:all If you go into the The dev built versions of the child script will be in the To run the examples, your best option is to run a http server in the project route. I use 'http-server' The npm modules can be created with |
What I think is happening, is that when the page updates, it triggers a MutaionObserver, which then looks for page element that are positioned to be outside the document flow and then adds resizeObsevers to them. When this happens it is triggering the second part of the init script, that by default has to wait for this to happen during page setup. That call is wrapped with a I'm afraid the call stack is going to bounce through a number of events, so might not be that easy to follow. The file to look at is |
@davidjbradshaw This commit here seemed to fix our issue: https://github.com/davidjbradshaw/iframe-resizer/compare/master...aptivada:iframe-resizer:master?diff=unified&w=# I did notice another instance of |
Your fix makes sense to me, although I’m not exactly sure why moving the Would you like to submit this as a PR and I will test it out in the morning. |
@davidjbradshaw Done #1314. It's because it's called from more places than what was wrapped in |
@Gamerweazel In the process of trying to create simple examples of each bug, I have found and fixed another two issues! I've not directly used your PR, but it did give the direction of the fix, the onChange event is now much simpler, and just calls for the content size to be rechecked, rather than the Anyway if this all works for you. I will release it as the final version. Thanks for all the help on this one. |
@davidjbradshaw No problem! I have updated my pen and can confirm that |
Release v5.2.5 |
Describe the bug
We have purchased a business license but are using GPLv3 in the codepen for testing.
The initial height is incorrect and doesn't get the right height.
The 'init' event keeps stacking and getting called multiple times. Even when I have tolerance set, it is ignored.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
The iframe should get a correct initial height.
The iframe-resizer should not be stacking and bogging down the page.
Screenshots
If applicable, add screenshots to help explain your problem.
Screen.Recording.2024-08-19.at.1.30.16.PM.mov
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: