locomotivemtl / locomotive-scroll

🛤 Detection of elements in viewport & smooth scrolling with parallax.
https://locomotivemtl.github.io/locomotive-scroll
MIT License
7.86k stars 1.12k forks source link

Some elements are missing 'is-inview' active class on initial visible viewport #36

Closed regenrek closed 5 years ago

regenrek commented 5 years ago

Hi,

I stumpled upon on a strange problem. Some elements are missing the 'is-inview' class even if they in the current viewport. This problem happens only on initial viewport you see if the page has loaded. Only if I do a slightly scroll then they appear. This looks very messy if you have initial animations. Is there some workaround or something I should consider?

Here is a codesandbox where you can see that some elements get fadein only after you have scrolled: https://codesandbox.io/embed/locomotive-scroll-nuxt-init-anim-rcok0

Take this markup for example which gets loaded in the initial first visible viewport

  <section data-scroll-section>
      <div>
        <div class="a-fadein" data-scroll>
          <Logo :width="350"/>
        </div>
        <h1 class="title">
          NUXT
          <span class="green">JS</span>
        </h1>
        <h2 data-scroll class="subtitle a-fadein">Starter for CodeSandBox</h2>
        <!-- NOT visible on page load only on scroll... ! -->
        <div>
          <div data-scroll class="a-fadein">
            <p>TEST 1</p>
          </div>
          <div data-scroll data-scroll-speed="1" class="a-fadein">
            <h2>TEST 2</h2>
            <!-- NOT visible on page load only on scroll... ! -->
          </div>
          <div data-scroll data-scroll-speed="1" class="a-fadein">
            <h2>TEST 3</h2>
          </div>
        </div>
      </div>
    </section>

EDIT: This problem happens also in the official example here. But only on mobile phones for example here:

regenrek commented 5 years ago

The combination of this.els.splice(i,1); and this.els.forEach((el, i) => seems to produce side effects in Core.setInview(). Since the array gets manipulated while looping through it...

I did some fast workaround tweaks to the class to make it work but since I'm not familiar with the code I don't know if any side effects happen. Maybe the filter isn't even necessary but maybe for performance purpose dunno...

Here is the update:

    detectElements(hasCallEventSet) {
        const scrollTop = this.instance.scroll.y;
        const scrollBottom = scrollTop + this.windowHeight;

        this.els.forEach((el, i) => {  
            if (!el.inView || hasCallEventSet) {
                if ((scrollBottom >= el.top) && (scrollTop < el.bottom)) {
                    this.setInView(el, i);
                }
            }

++            if(el !== null) {
                if (el.inView) {
                    if ((scrollBottom < el.top) || (scrollTop > el.bottom)) {
                        this.setOutOfView(el, i);
                    }
                }
++            }
        });

++        this.els = this.els.filter(function(current, i) {
++            return current !== null;
++        });

        console.info(this.els);

        this.hasScrollTicking = false;
    }

    setInView(current, i) {
        this.els[i].inView = true;
        current.el.classList.add(current.class);

        if (current.call && this.hasCallEventSet) {
            this.dispatchCall(current, 'enter');

            if (!current.repeat) {
                this.els[i].call = false
            }
        }

        if (!current.repeat && !current.speed && !current.sticky) {
            if (!current.call || current.call && this.hasCallEventSet) {
++                this.els[i] = null;
--                this.els.splice(i,1);
            }
        }
    }
Jerek0 commented 5 years ago

Thank's for the heads up! Implemented your solution in 6857ace 👍