Skip to content
This repository has been archived by the owner on Jan 24, 2018. It is now read-only.

Add forceHeight: 'content' option for mobile #633

Closed

Conversation

Klowner
Copy link

@Klowner Klowner commented Nov 15, 2014

I kept having issues on mobile browsers in situations where I had keyframes that didn't 'end' until after the end of the content. When using forceHeight, the height is expanded to allow all keyframes to be
reachable by extending the page content beyond where it would normally end. This small change adds a 'content' option for forceHeight which measures the elements within the skrollr-body and adjusts the maximum scroll area to be within the boundaries of the page content, regardless of the final keyframe(s).

I'm not sure if this is the proper way to address this issue, but it seems like other people have been having similar issues with extraneous content at the end of their pages.

Thanks!

I kept having issues on mobile browsers in situations where I had
keyframes that didn't 'end' until after the end of the content. When
using forceHeight, the height is expanded to allow all keyframes to be
reachable by extending the page content beyond where it would normally
end. This small change adds a 'content' option for forceHeight which
measures the elements within the skrollr-body and adjusts the maximum
scroll area to be within the boundaries of the page content, regardless
of the final keyframe(s).
@Klowner Klowner force-pushed the forceheight-content-for-mobile branch from 692e451 to b850faa Compare November 15, 2014 01:35
@Prinzhorn
Copy link
Owner

Thank you. The idea was that skrollr-body expands to the height of its content which allows us to measure the height. But many people add a height other than auto to it, for various reasons, breaking this assumption.

I like simple solutions and yours put me in the right direction.

In this line

var skrollrBodyHeight = (_skrollrBody && _skrollrBody.offsetHeight || 0);
replace _skrollrBody.offsetHeight with _skrollrBody.scrollHeight. It's a simple change but should solve this issue. It'd be great if you could test this and report back.

@Klowner
Copy link
Author

Klowner commented Nov 15, 2014

Happy to help! Thanks for the quick response.

Replacing _skrollrBody.offsetHeight with _skrollrBody.scrollHeight does result in the desired height (aside from not taking my skrollr-body's top margin into account, although I can probably adjust my page design accordingly, or add + _.skrollrBody.offsetTop ... that's probably a troublesome path to follow).

If I also replace _maxKeyFrame = Math.max(_maxKeyFrame, _getDocumentHeight()); with _maxKeyFrame = _getDocumentHeight(); then I get the desired result, but I'm assuming that may not be the proper solution for other cases. Without this change, the _maxKeyFrame is greater than _getDocumentHeight() so I still end up with extra space after my content.

@Prinzhorn
Copy link
Owner

If I also replace _maxKeyFrame = Math.max(_maxKeyFrame, _getDocumentHeight()); with _maxKeyFrame = _getDocumentHeight(); then I get the desired result, but I'm assuming that may not be the proper solution for other cases. Without this change, the _maxKeyFrame is greater than _getDocumentHeight() so I still end up with extra space after my content.

Isn't that what forceHeight: false is for?

@Klowner
Copy link
Author

Klowner commented Nov 15, 2014

Oh hey, good point, that was silly of me. I set forceHeight: false and everything works perfectly for me with your proposed changes to _getDocumentHeight()

@Prinzhorn Prinzhorn closed this in 1ce6350 Nov 15, 2014
@Prinzhorn
Copy link
Owner

Released as 0.6.28. I should really call it 1.0.0 any time soon...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants