Sal.js does not work correctly if element larger than viewport

[hudri]

Sal.js does not work correctly if the animated element is larger than the viewport, because the threshold values implicitly suppose that the element is smaller than the viewport.

E.g. if you have an element with the height of three times the viewport, Sal.js will never trigger with a threshold of 0.5, because the largest overlap is 0.3333 in this case.

Possible solution:

Sal.js should not use one global threshold for all elements. Instead Sal.js should individually check each element if it is larger than the viewport, if so recalculate the threshold, else use the original threshold.

Here is an example code for this logic taken from a post on StackOverflow

function doTheThing (el) {
    el.classList.add('in-view');
}

const threshold = 0.5;

document.querySelectorAll('section').forEach(el => {
    const elHeight = el.getBoundingClientRect().height;
    var th = threshold;

    // The element is too tall to ever hit the threshold - change threshold
    if (elHeight > (window.innerHeight * threshold)) {
        th = ((window.innerHeight * threshold) / elHeight) * threshold;
    }

    new IntersectionObserver(iEls => iEls.forEach(iEl => doTheThing(iEl)), {threshold: th}).observe(el);
});

[mciastek]

Hi, thank you for rising this issue. I see that may be a problem, when animated element is larger than a viewport. Proposed solution should work, but it will impact overall performance. Imagine that you want to animate 1000 elements. In your solution you create one Intersection Observer per element, so we will get 1000 observers. I'd like to create one observer that will handle multiple elements at once. Could you provide a solution addresses the problem I mentioned?

[hudri]

Been thinking a bit in the last days, I can't think of any solution that would work with one single observer. Given that we never know the visitors viewport size, we never can savely provide a percentage based threshold.

I guess the best trade-off is to move the responsibility to the developer, who can decide the best option for the concrete usecase. Instead of making one global instance with one global oberserver and one global selector class, allow multiple instances of Sal. Each instance has one observer with one threshold. Devs should be able to create new instances by providing a custom class name/css selector string or a NodeList

[divira]

Face this issue quite often especially with gallery of images in grid. I have to add to each elements to allow them to animate var els = document.querySelectorAll('.sal>*'); for (var i = 0; i < els.length; i++) { els[i].setAttribute('data-sal', 'fade'); }

[vonba]

Hi there! Please correct if I'm misunderstanding but to me it seems like this issue makes the plugin more or less unusable on mobile devices where routinely paragraphs and images are bigger than the viewport. Are there any plans on fixing this issue? I would love to be able to keep using the plugin since it works great on desktop screens.

It's especially problematic when displaying rich text type content where I add a fade effect to the container which usually contains several paragraphs, images etc that run several times the viewport height. On mobile this usually just looks like a blank page until you scroll quite far down the page.

[Superhands89]

this really needs to be fixed. its not an edge-case issue, it will affect most users at some point.

Personally i got around this just by deleting literally all the js from sal.js and any other sal related js (but i kept the css) and just added the following js instead...

function customSalScrollAnimationHandler() { // uses sal css but all js is custom due to https://github.com/mciastek/sal/issues/62
    const theSals = document.querySelectorAll('[data-sal]');
    if (theSals.length) {
        [].forEach.call(theSals, function (e) {     
            const salPos = e.getBoundingClientRect().top;
            const percentOfWindowHeight = window.innerHeight * 0.7; // using 70% of viewport height from the top (30% from the bottom)
            if (salPos < percentOfWindowHeight) { // if my element has been scrolled in at least 30% of the viewport height from the bottom
                e.classList.add('sal-animate');
            }
        });
    }
};
window.addEventListener('DOMContentLoaded', function () {
    customSalScrollAnimationHandler();
});
window.addEventListener('scroll', function () {
    customSalScrollAnimationHandler();
});

Plus some additional css for flip up and slide up alterations...

/*sal flip up and slide up animation fix*/
[data-sal=flip-up] {
    transform: perspective(2000px) rotateX(-55deg);
    opacity: 0;
    transition: opacity 0.8s ease, transform 0.4s ease;
}
[data-sal=flip-up].sal-animate {
    opacity: 1;
}
[data-sal=slide-up] {
    transform: translateY(130px);
}

[AirTechWick]

I have this bug in my projects when using sal.js