-
-
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
calculated scroll position is wrong #243
Comments
Can you not just put a 50px margin on the top of the iFrame? |
That was my first thought and I could, but the iframe isn't the only content on the page. So it creates an excessively large gap between content before the iframe and the content in the iframe. |
Are you against a scroll coords transform method? I think that would be the most flexible approach |
Not sure I full understand the issue, do you have a simple test case? |
Something like
|
I will put one together. |
Sorry it took me a Notice that the host.html:
iframe.html
|
The host page is a getting started template from boostrap. The iframe page is a getting started template from jquery mobile. I don't think this is important but it configures the css styles to demonstrate my issues. |
Forgot to mention... click the toggle button to start the timers to demonstrate the issue |
Also, I am assuming iframeResizer.js#L113 calls |
Can you please host that somewhere and I will take a quick look tomorrow. |
It is set to 3 second timeouts. So after you click toggle it will switch between the two scroll positions every 3 seconds |
I think this is a simple fix for the issue: thenewguy@58e1beb I updated the demo with that version and increased the speed at http://thenewguy.github.io/iframe-resizer/issues/243/2/host.html |
I updated the first example to use the same dimensions and timing as the second so they can be compared side by side |
Just looked at this a bit more. I think the solution is to change the code in the iFrame to the following. parentIFrame.scrollToOffset(0, -50); The scrollCallback method is their to implement animation, rather than to change the passed in values. |
The problem is also seen if you are trying to do a custom animation because the scrollCallback isn't always called. The small changeset thenewguy@58e1beb just ensures that the scrollCallback is called if it exists instead of calling window.scrollTo directly. With respect to my particular use case, it is much easier to calculate the scroll position offset on the host side. It is not very feasible for the content to be embedded into a generic host page if the content page is responsible for determining the host page layout. The -50px in my example is just a sample using one of the default bootstrap themes. But in practice every host page is different. Since the iframe is used as a generic widget to be incorporated into any host page, it isn't a simple -50px constant. Since the fix is so simple, wouldn't it be better to allow the host page to adjust for site specific style? If there is not a chance of this happening in the scrollCallback, could an offsetCallback be added to allow the host page to inject knowledge of its layout? |
@davidjbradshaw not sure if you're automatically pinged after the issue is closed. comment above ^ |
The fix isn't that simple, because if you use scrollCallback for animation, you don't what to have the scrolling animated when the position is moved due to the library moving the page during a resize event. That said it only needs to move the page when using height calculation methods that require the force down sizing code to run. Currently it runs on all methods. So fixing that is a possible solution. |
I guess I did not see the case where you would not want animation. In my particular widget I found it distracting to be animated some times but not others so I put together a css transform for the height and used the scrollCallback method to cover scrolling issues. @davidjbradshaw I feel like maybe I am missing your point... so my apologies if it went over my head. Is there already a way to handle a layout like the "getting started" bootstrap template in my example using the api in its current state (assuming that the host page layout is not known in advance and the offset adjustment is arbitrary and varies between host pages)? Since the default configuration was scrolling the host page such that the host page header overlapped the top of the iframe content, it was my understanding that this level of detail would be required from the host page integration overriding a callback option of some sort. Maybe I have overlooked something? Can this issue be re-opened? |
The issue is that some height calc methods require a double resize to detect downsizing, once down to zero and then again to find the correct new size. To stop the page scrolling during this process the page position is saved and then set back again on the new size. During this you don't want animation, because the idea is to give the impression that the page has not moved. Now that this repositioning is only need when the page is double resized, so I've restricted it to only happen in those cases, rather than all resize event types. Have a play with the version in the dev branch and let me know if that fixes things for you. |
@davidjbradshaw Issue still shows on dev branch. http://thenewguy.github.io/iframe-resizer/issues/243/3/host.html |
Just tried it on my iPad and it worked. Do a hard reload. |
Hrm... I just cleared cache and checked again in IE11 and Chrome44 against http://thenewguy.github.io/iframe-resizer/issues/243/3/host.html and they scroll over the iframe header after about 2 or 3 seconds. I don't have an ipad available. Also chrome on my android phone |
Hmm, try adding the following to the if statement that calls the scroll callback function.
|
OK now that I'm back at my Mac I've just had a bit more of a look and updated the dev channel again. The issue seems to be that when scrollCallback returns false the pagePosition var is not reset to null. So the next resize event causes the page to jump. Let me know if it does the trick. |
I've put up a new demo at http://thenewguy.github.io/iframe-resizer/issues/243/4/host.html Now it scrolls below the header every time. The Since I've set --- ps --- If you're familiar with django and/or python, I added a simple django project to thenewguy.github.io that runs the example locally with |
Seems another change I made prevented the return false working and the tests didn't pick it up. Try dev now. |
@davidjbradshaw I think that solved the issue. I will do more cross browser testing but it works on chrome and ie just fine (only two browsers on this tablet). See http://thenewguy.github.io/iframe-resizer/issues/243/5/host.html Though I am observing an odd issue that I couldn't reproduce it at first... if you open the page at 100% zoom it works as expected. If you change the zoom to 110% it alternates between scrolling to y coord of 429px and 430px. This was observed at a window width between 548px and 637px according to the chrome web developer plugin. Is the use of parseInt in getElementPosition important? Changing it to the following prevents the pixel jumping I am observing:
The modified I probably should have opened a new issue... it was late. Let me know if you would prefer for me to do so. |
Can you open a PR for it if it has fixed the issue. |
I have encountered an issue where the host page is being scrolled to the wrong offset.
I've attempted to override scrollCallback like the following:
The issue is that any time the page is manually resized via the contentframe's parentIFrame.size function the scrollCallback override function is bypassed.
I am testing embedding the iframe into a boostrapped page. The problem is that the bootstrap header overlaps the iFrame when it is scrolled to. In this case the header is 50px. So when the host page scrolls to the iFrame it scrolls too far and the header covers some of the iframe.
Is there any way to adjust the coords for all scrolling instead of only scroll methods triggered by the content frame? If not, what is the recommend approach for something like this?
The text was updated successfully, but these errors were encountered: