airfranceklm / ng-lazy-image

Angular directive for loading responsive image when container (which is preventing reflow) is in viewport.
http://afklm.github.io/ng-lazy-image/
MIT License
351 stars 64 forks source link

Image not loaded when using ng-repeat with filter #28

Closed GilHelfgott closed 8 years ago

GilHelfgott commented 8 years ago

I am witnessing an issue that prevents me from using lazy-image on a scrollable container with query text that filters a list:

<div class="dropdown-list" afkl-image-container>
        <div class="spacer"></div>
        <div ng-repeat="item in valuesList | filter : { query : query.text }">
            <div class="dropdown-option">
                <div class="dropdown-option-img-wrapper">
                    <div afkl-lazy-image="{{item[thumbnail]}}" class="afkl-img-ratio-1-1"></div>
                </div>
                <span class="img-label">
                    {{item.text}}
                </span>
            </div>
        </div>
    </div>

The result that I am seeing, is that after entering a search query, the list shortens but the images (that are now in viewport) are not loaded until I manually perform a scroll.

Thank you for any input.

NirHemed commented 8 years ago

I've seen this happen as well... no solution at the moment.

+1

SquadraCorse commented 8 years ago

Can you add your sample to the sample repo? The current list shows now: http://afklm.github.io/ng-lazy-image/sample-repeat.html

awalker commented 8 years ago

I can reproduce the issue (Mac/Chrome) with the repo's sample (http://afklm.github.io/ng-lazy-image/sample-repeat.html) by adding an option to display all as the default.

<option value>All</option>

Default to "all", all items display, do not scroll. Select filter 1, then filter 2. On one or the other filter will have some of the images near the bottom (at least on my screen, depending on your setup you may have to change the sizes or number generate a bit) will not load until you scroll.

It appears that if the span element that holds the image is added to the dom but not displayed (viewed), then the image is never loaded. The test as written passes because all of the spans are never added at the beginning. I don't know if that makes sense, but I'm going to try to fix it.

awalker commented 8 years ago

Well, it is a ugly hack, but here is what I have working for my app...

1) Inject $rootScope into the afklLazyImage directive.

2) Update the code around $on('$destroy' to read...

// Remove all events when destroy takes place
scope.$on('$destroy', function () {
  $rootScope.$broadcast('resize');
  return _eventsOff();
});

scope.$on('resize', _onResize);

This lets any child of root scope know that a lazy image was removed and maybe it should check its position.

SquadraCorse commented 8 years ago

Thanks, so we can do like this:

scope.$on('$destroy', function () {
    _checkIfNewImage();
    _onViewChange();
    return _eventsOff();
});
SquadraCorse commented 8 years ago

I see, on scope destroy i remove the image if it is there (in our case only ui-view destoyed the view). Then 300ms later the plugin looks if it has new images... So i need the timer here as well. Thanks Awalker!

awalker commented 8 years ago

Not sure if we are talking about the same thing here. Consider a page with a blue image and green image. The green image is far enough down the page that it is not show (and not loaded). Filtering the destroys the blue image. Invoking _onViewChange on the destroyed blue image does not solve the problem, with or without a timer. The problem is the green image's wrapper is now on screen from DOM manipulation without a resize or scroll event being fired. We need some way to notify the green image wrapper to load the green image.

Short video showing the issue

DOM MutationObserver exists, but I'm not sure how well supported it is. Broadcasting from the $rootScope works, but feels like a hack. I'm wondering if I could tap into ngRepeat animation support to achieve this goal. ngRepeat has support for a "move" animation event and that seems like a good place to look but it could require a dependency on ngAnimation.

SquadraCorse commented 8 years ago

Thanks Awalker. First i wanted to explain that our coding conventions disallowed us to misuse rootScope but since most of the concepts will not be there in 2.0 i decided for now it fits the goal which is using it inside a ng-repeat. Thanks a lot for your clear explanation!

SquadraCorse commented 8 years ago

It still doesn't solve other cases where you toggle other elements since it still doesn't see the change, but at least they can now use a trigger :)