Skip to content
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

Proper viewport size, do not update height in responsive mode, do not forget to rebuild on touch devices if change is less than 20% #2728

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

tmcdos
Copy link

@tmcdos tmcdos commented May 27, 2017

I would like to share a contribution. I am using the library for the landing page of a customer's web site - development copy is currently available at http://mmx.ivogelov.com
I have experienced some strange/erroneous behavior in a couple of cases:

  • fullPage.js ignores the scrollbar in its initialization - this means that the computed height for the sections is less than the full viewport height by the height of the horizontal scrollbar. To solve the issue, I am using a cross-browser compatible function to find the proper viewport height. This open-source function is viewportSize - I replaced all occurences of $window.height() with viewportSize.getHeight()
  • fullPage.js tries to apply section heights in resizeHandler() even if the responsive mode is currently active - this breaks the page layout because in this mode sections' height should be set to AUTO and not stretched to the viewport height. To solve the issue, I immediately exit from the resizeHandler() if BODY has a CSS classname of fp-responsive
  • fullPage.js forgets to rebuild the sections on touch devices if the change in height is less than 20%. To solve the issue, I start a timer - if there is no new change before the timeout, a rebuild is forced after the timeout elapses

… from https://github.com/tysonmatanich/viewportSize

2) "resizeHandler()" returns immediately when "fp-responsive" is active
3) "resizeHandler()" rebuilds sections immmediately if change in height is more than 20% and postpones it for 350ms otherwise
@@ -1,3 +1,6 @@
/*! viewportSize | Author: Tyson Matanich, 2013 | License: MIT (https://github.com/tysonmatanich/viewportSize) */
(function(n){n.viewportSize={},n.viewportSize.getHeight=function(){return t("Height")},n.viewportSize.getWidth=function(){return t("Width")};var t=function(t){var f,o=t.toLowerCase(),e=n.document,i=e.documentElement,r,u;return n["inner"+t]===undefined?f=i["client"+t]:n["inner"+t]!=i["client"+t]?(r=e.createElement("body"),r.id="vpw-test-b",r.style.cssText="overflow:scroll",u=e.createElement("div"),u.id="vpw-test-d",u.style.cssText="position:absolute;top:-1000px",u.innerHTML="<style>@media("+o+":"+i["client"+t]+"px){body#vpw-test-b div#vpw-test-d{"+o+":7px!important}}<\/style>",r.appendChild(u),i.insertBefore(r,e.head),f=u["offset"+t]==7?i["client"+t]:n["inner"+t],i.removeChild(r)):f=n["inner"+t],f}})(this);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cross-browser safe way to get the proper viewport size (does not include scrollbars) - because $window.height() returns a value which is the viewport height minus the height of the horizontal scrollbar.

@@ -2076,22 +2080,33 @@
function resizeHandler(){
//checking if it needs to get responsive
responsive();
if(document.body.classList.contains('fp-responsive')) return; // IVO GELOV
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the responsive mode is active, we should not enforce the viewport height to the sections - their height should be left to AUTO.

previousHeight = currentHeight;
if( Math.abs(currentHeight - previousHeight) > (20 * Math.max(previousHeight, currentHeight) / 100) ) reBuild(true); // IVO GELOV
else
{
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the change is less than 20%, we should not ignore the event - just postpone the rebuild for some time,

}
previousHeight = currentHeight; // IVO GELOV
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether the above statement should be inside the ELSE branch or outside of it ....

@alvarotrigo
Copy link
Owner

Thanks for that!
I'll review it later on whenever I have more time, but so far:

fullPage.js tries to apply section heights in resizeHandler() even if the responsive mode is currently active - this breaks the page layout because in this mode sections' height should be set to AUTO and not stretched to the viewport height. To solve the issue, I immediately exit from the resizeHandler() if BODY has a CSS classname of fp-responsive

fullPage.js should set the height of the sections even in responsive mode.
Use the class fp-auto-height-responsive if you want to avoid that behaviour.

fullPage.js forgets to rebuild the sections on touch devices if the change in height is less than 20%. To solve the issue, I start a timer - if there is no new change before the timeout, a rebuild is forced after the timeout elapses

That prevents fullpage.js to resize the section when the address and bottom bars get hidden. When using scrollBar:true or autoScrolling:false, this might be an issue.
Have you tested those cases?

… should not occur if RESPONSIVE mode is active
@tmcdos
Copy link
Author

tmcdos commented May 28, 2017

When using scrollBar:true or autoScrolling:false, this might be an issue.

I am using the default values for all parameters - I only define the responsiveWidth and provide handlers for the afterResize and afterResponsive events when initializing the library. I have not tested with scrollBar:true or autoScrolling:false.

Use the class fp-auto-height-responsive if you want to avoid that behaviour.

You are right - using this class solved the issue and did not require the presense of my code for early exiting the resizeHandler() in responsive mode. I will revert the changes.

There is another issue, though. If you visit http://mmx.ivogelov.com and scroll down to the Testimonials section - you will see that this particular section has wrong Y offset for the "translate3d". It is -3620px while it should be -3790px (watching it in Chrome DevTools in mobile/responsive view with 1188x905 viewport). If you scroll to the next section (Professional massage) and then scroll back up to the Testimonials - this time the Y offset will be the right one.

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

Successfully merging this pull request may close these issues.

None yet

2 participants