Proposed solution for issue #16

[manifestinteractive]

Below the fold images are sometimes being loaded.

After some digging I found that elements acting as the container MUST be visible when lazy image initializes. If the container has an animation on it and starts off hidden, it will not work and all images will load in container. This initial hidden state seems to screw up the calculations as the container element is not technically visible at the time lazy image is executed. This edge case causes the remaining to be a negative version of height. Simply checking if remaining and height cancel each other out seemed to fix the edge case.

Demo of updated code working with originally hidden containers ( which previously did not work ):

http://peter.build/doing/app/#/app/events/now/

[manifestinteractive]

In regards to the Unit Tests, these were not passing for me with the initial code base. I tried two versions of node ( v0.10 & v0.12 ) with the actual master branch and the grunt unit fails. This PR only changes one line and uses two existing variables with a basic math operator.

I did look at the unit tests and it seemed the only way to get them to pass was to add { 'nolazy': true } in the afkl-lazy-image-options. Seems without it the HTML never rendered and there are three tests that failed because the HTML rendered as an empty string.

[SquadraCorse]

The test fails because there are conditions not met :) But thanks for the insights into your problem. Your solution now gives errors for items where both are equal (logo left top, or image inside scrollable container). I think the option to check if image container is visible or not is better. Only jqlite has no .is(:visible). So i will check for innerWidth, that can only be set if parent is blocklevel (breaking change, since normally i also could put it in span) AND has a width (ergo: is visible).

var _elementOffsetWidth = function () {
    if (element.offset) {
        return element.offset().width;
    }
    return element[0].getBoundingClientRect().width;
};

then

var _onViewChange = function () {
    // Config vars
    var remaining, shouldLoad, windowBottom;

    var height = _containerInnerHeight();
    var scroll = _containerScrollTop();
    var visible = _elementOffsetWidth() === 0 ? false : true; // element must be block level, check for visiblity is then possible

    var elOffset = $container[0] === $window ? _elementOffset() : _elementOffsetContainer();
    windowBottom = $container[0] === $window ? height + scroll : height;

    remaining = elOffset - windowBottom;

    // Is our top of our image container in bottom of our viewport?
    //console.log($container[0].className, _elementOffset(), _elementPosition(), height, scroll, remaining, elOffset);
    shouldLoad = remaining <= offset;

    // Append image first time when it comes into our view, after that only resizing can have influence
    if (shouldLoad && !loaded && visible) {

        _placeImage();

    }

};

and for these three tests i will add

it('We only have one image', function() {
    var el = angular.element('<div afkl-lazy-image="img/foo.png 480w, img/foo.png 480w"></div>');
    $compile(el)(scope);
    scope.$digest();

    window.setTimeout(function() {
        expect(el.html()).toBe('<img alt="" class="afkl-lazy-image" src="img/foo.png">');
    }, 300);
});

See if this also works for you then i will release new major.

[SquadraCorse]

See if this dev version checked in now is solving your case as well? Thanks for investigating into this issue!

[manifestinteractive]

@SquadraCorse Unfortunately, no, this fix does not work and has the same issue as the original. All images load. You can see the issue again on that same link. Your code changes are found in this file:

http://peter.build/doing/app/assets/js/angular.js

If you search for var _onViewChange you'll see your changes made.

I know you are WAY more familiar with your own project that I am, but maybe explaining why I updated what I did would be helpful. Even if it breaks other experiences, you might be able to figure out root case.

It seems like any time the remaining was the negative version of height this issue shows itself. I am not 100% sure if getting the width of the element will make a difference as my quick checks said it was visible, event though I knew for a fact it was not. I am not sure if makes a difference, but the animation I have is not a toggle of show/ hidden. There is a 500ms animation that loads an element. This animated element, FYI, is not the afkl-image-container. There is a div.transition element for each page that shows up as a child within afkl-image-container which is what eventually contains the afkl-lazy-image.

I am OK just using my forked version of the project, as I might just be a weird edge case that no one else has. But my solution of checking if remaining and height cancel each other out seems correct for my usage. Especially since height should never be zero, and remaining, as far as I could tell, was never negative. So I could not think of a situation where height and remaining could ever add up to zero unless this weird edge case happened. But, this is your baby, so I know you'll know like 10 reasons off the top of your head.

[manifestinteractive]

@SquadraCorse Also, I would be curious if you tested ng-show rather than ng-if on http://afklm.github.io/ng-lazy-image/sample-hide-show.html.

As you know, ng-if does not actually render the element into the DOM. This is not the same as ng-show which does render the element into the DOM, but adds a hidden class. ng-show is more realistic to what I was having issues with. It makes sense why ng-if still works as it seems like it would be running afkl-lazy-image on an element that is both visible and in the DOM.

[SquadraCorse]

For now, forking is maybe the best way. I did see you start with

.transition {
    display: none;
    height: 100%;
}

and then do your transititon. If i have time today i will make a different test (you are right about ng-if), i also saw it gave a width so my visible assumption doesn't work so i will remove it. I will also look at you change again and see why my own usecases didn't work anymore. Is transition in visibilty an option for you :) ?

[manifestinteractive]

So now I am totally lost as to the cause. I duplicated the circumstances I had using your existing demo.

I put it up on plnkr for live editing: http://plnkr.co/edit/oKBvtdfLHzoYIK2qEpja?p=preview

However, I was never able to get it able to break the way mine is without that fix. So I will just stick the the fork I made and carry on before I get stuck down the rabbit hole.