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

Collection-repeat image recycling while loading #1742

Closed
udfalkso opened this issue Jul 7, 2014 · 36 comments
Closed

Collection-repeat image recycling while loading #1742

udfalkso opened this issue Jul 7, 2014 · 36 comments

Comments

@udfalkso
Copy link

udfalkso commented Jul 7, 2014

I've noticed that while scrolling, if the image to be displayed has not yet downloaded, the user will be shown the last image that existed in that recycled node's "spot" until the new one loads.

It's misleading behavior and can be a bit confusing for the user. More preferable would be for later items to somehow be reset to initial conditions so the user sees an empty container while the image loads.

I've set up this codepen which reproduces the problem:
http://codepen.io/udfalkso/pen/aojet5

You'll have to throttle your internet connection somehow to consistently see the issue in action. Either on a mobile device without wifi or with SpeedLimit.prefPane. Scroll down quickly and you'll see that the images from up-top are repeated for some time until the new images load on top of them.

I hope this was clear. Any solutions? Thanks!

@ajoslin
Copy link
Contributor

ajoslin commented Jul 7, 2014

Your codepen isn't working, but I've seen this problem too.

One solution is to use data-uris and prefetch your images.

If we want to actually reset the img, the only way I know of is to remove the element and replace it with a clone. This is possible, but it may hurt performance to remove and replace an element on scroll.

But to try, we can create a directive that will make a new image element every time the src changes.

Try this:

<img resetting-img ng-src="{{something}}">
myApp.directive('resettingImg', function() {
  return {
    restrict: 'A',
    link: function(scope, element, attr) {

      var currentElement = element;
      attr.$observe('src', function(src) {
        var newImg = element.clone(true);
        newImg.attr('src', src);
        currentElement.replaceWith(newImg);
        currentElement = newImg;
      });

    }
  };
});

@ajoslin ajoslin self-assigned this Jul 7, 2014
@udfalkso
Copy link
Author

Sorry, the codepen url is: http://codepen.io/udfalkso/pen/aojet

@udfalkso
Copy link
Author

Thanks, your directive solution seems to work well and doesn't hurt scrolling performance (on my iphone 5 at least, haven't tried android). That solves the issue for my personal needs.

Maybe collection-repeat could have a "reset-images='true'" option built in, for others that run into this issue?

@cgspicer
Copy link

Confirmed working here, as well.

@udfalkso
Copy link
Author

Be careful. I just found a case where the original image element had some additional attributes set on it (like ng-style="") and those got wiped out by the new directive. I'll try write an improved version that handles this tomorrow.

@ajoslin
Copy link
Contributor

ajoslin commented Jul 11, 2014

You could also try doing:

attr.$observe('src', function(src) {
  img.attr('src', '');
  //on next frame, set src again
  ionic.requestAnimationFrame(function() {
    img.attr('src', src);
  });
});

I don't know if it would work, but it's worth trying the simpler way.

@udfalkso
Copy link
Author

Unfortunately that trick doesn't seem to work @ajoslin

@ajoslin
Copy link
Contributor

ajoslin commented Aug 6, 2014

So for now, the best way to fix this is to always use data-uris for your images.

We'll have to find a way to really make the img tags work, though...

@ajoslin ajoslin added this to the 1.0.0-beta12 milestone Aug 6, 2014
@ajoslin ajoslin added the ready label Aug 6, 2014
@adamdbradley adamdbradley modified the milestones: 1.0.0-beta12, 1.0.0-rc0 Sep 12, 2014
@dhcar
Copy link

dhcar commented Oct 22, 2014

+1 with iOS 7 and beta 13

edit - ajoslin's technique didn't work for me so I went and modified it slightly

.directive('resettingImg', function() {
  return {
    restrict: 'A',
    link: function(scope, element, attr) {

      var currentElement = element;
      var applyNewSrc = function(src) {
        var newImg = element.clone(true);
        newImg.attr('src', src);
        currentElement.replaceWith(newImg);
        currentElement = newImg;
      };
      attr.$observe('src', applyNewSrc);
      attr.$observe('ngSrc', applyNewSrc);
    }
  };
})

lanceli added a commit to lanceli/cnodejs-ionic that referenced this issue Dec 1, 2014
@asterism612
Copy link

I have same issue nightly version in IOS

resettingImg does not work

@jfitz0
Copy link

jfitz0 commented Jan 24, 2015

Also have this same issue.
Would be a really nice addition to this feature to be able to clear the iamge src or set a loader image when the element gets cloned and inserted into the dom.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 9, 2015

I'm going to try to implement something similar to @dhcar's solution.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 9, 2015

So I found a less expensive way that works: every time we change an item's data, we refresh its images by simply setting them all to display: none then back.

We should definitely not always do this. How does an attribute called force-refresh-images="true" sound?

@jfitz0
Copy link

jfitz0 commented Feb 10, 2015

An attribute called force-refresh-images="true" sounds good!

@dhcar
Copy link

dhcar commented Feb 10, 2015

force-refresh-images="true" has my blessing!

@ajoslin ajoslin removed the ready label Feb 10, 2015
@ajoslin ajoslin reopened this Feb 10, 2015
@ajoslin
Copy link
Contributor

ajoslin commented Feb 10, 2015

Could you guys test this out? There are now two new attributes: collection-buffer-size and collection-refresh-images.

Read the nightly docs for an explanation: http://ionicframework.com/docs/nightly/api/directive/collectionRepeat

@ajoslin
Copy link
Contributor

ajoslin commented Feb 10, 2015

Additionally, doing this revealed an opportunity for some huge optimizations to collection-repeat! Those are coming soon.

@asterism612
Copy link

nice~!
work fine.
thank you for ajoslin :)

i expect optimizing~!

@adamdbradley adamdbradley modified the milestones: 1.0.0-rc4, 1.0.0-rc3 Apr 13, 2015
@perrygovier perrygovier modified the milestones: 1.1, 1.0.0-rc5 Apr 22, 2015
@JerryBels
Copy link

Hey guys, don't know if it's implemented in RC5, but it doesn't seems to work for me.

<ion-list>
    <ion-item class="photoListItem" collection-repeat="media in mediasData | filter:filterNew | filter:filterAlbum | orderBy:'date_creation' : true track by date_creation" item-width="imageDimensions + 5" item-height="imageDimensions + 5" force-refresh-images="true">
        <img ng-src="{{media.$mediaUrl}}" ui-sref="app.photo({mediaId : media.$id})">
    </ion-item>
</ion-list>

Any idea why ?

[EDIT] @ajoslin I can't see why Github would unassign you by me posting a comment :/ but this just popped up : "ajoslin was unassigned by JerryBels 19 seconds ago"

@JerryBels
Copy link

Bump

1 similar comment
@JerryBels
Copy link

Bump

@perrygovier
Copy link
Contributor

@JerryBels, are you seeing this still a problem in 1.0?

@perrygovier perrygovier added the needs: reply the issue needs a response from the user label May 20, 2015
@JerryBels
Copy link

@perrygovier yes, still occurs with 1.0.0. I recorded it in video for you to see :

https://www.youtube.com/watch?v=0ZaeD2nlKoA

No comments about the photos of our office :p

The CDN delivering the photos is very fast so you see the bug very slightly but it's even more pronounced when running on a bad 3G connection.

@JerryBels
Copy link

@perrygovier anything else I can do to help track down the issue ?

@RobbinHabermehl
Copy link

I can confirm the existence of this problem in v1.0.0.

Unfortunately that's not all as I've encountered a lot of difficulties combining collectionRepeat with my custom caching directive. In short, this directive downloads an image to the file system and then sets the path to this image as src attribute. The problem is that the first item gets copied by collectionRepeat before the linking phase, i.e. the caching, completes. Therefore the src attribute never gets set, leaving the first image blank.

There's no easy way to resolve this due to the isolated nature of collectionRepeat. For now I've created a workaround by watching the first item of a collection with a directive, once it changes it applies the src manually:

angular.module('app').directive('collectionRepeatFirstImageFix', function($compile, $timeout) {
  return {
    link: function(scope, iElement) {
      var renderImage = function() {
        var firstImage = iElement[0].querySelector('[collection-repeat] img');
        if (!firstImage || !window.cordova) {
          return;
        }
        // Put logic here
        firstImage.onerror = function() {
          firstImage.src = '';
          $timeout(renderImage, 100);
        };
        firstImage.src = 'file:///path/to/image.jpg';
      };
      scope.$watch(function() {
        return scope.collection && scope.collection[0] ? scope.collection[0]._id : null;
      }, function() {
        $timeout(renderImage);
      });
    },
    scope: {
      collection: '=collectionRepeatFirstImageFix'
    }
  };
});

HTML:

<ion-list collection-repeat-first-image-fix="images">
  <ion-item
    collection-repeat="image in images"
    force-refresh-images="true">
    <img cache-src="{{ image.url }}">
  </ion-item>
</ion-list>

EDIT: Created issue #4042 for this problem.

@Ionitron
Copy link
Collaborator

Greetings @udfalkso!

My sensors indicate a reply was requested, but we have not received one yet, I am using my robot powers to close this issue. However, if you are still experiencing this issue, please feel free to reopen it by creating a new issue, and include any examples or necessary information.

Thank you for allowing me to assist you.

@Ionitron Ionitron removed this from the 1.1 milestone Jun 20, 2015
@Ionitron Ionitron added ionitron:closed and removed needs: reply the issue needs a response from the user labels Jun 20, 2015
@JerryBels
Copy link

@Ionitron @perrygovier as you can see with the last comments, the issue is still here... Do I need to open a new issue and reference this one in it or can you reopen this ?

@Yuripetusko
Copy link

force-refresh-images doesn't work for me I can still see duplicated images

@valiantvu
Copy link

I am also still seeing duplicated images with force-refresh-images. Did anyone else figure out a workaround?

@mhartington
Copy link
Contributor

Please provide a codepen exmaple in a new issue.

@marfago
Copy link

marfago commented Oct 31, 2015

Same issue for me.

@yianpuodiaotu
Copy link

Tks all, @ajoslin's method resolved my problem.

@miparnisari
Copy link

miparnisari commented Jun 8, 2016

I'm having a similar issue in which the first time I open the view the images are not shown. If I scroll down and then up again the images are shown. What could be the issue? I posted my general problem here: http://stackoverflow.com/questions/37707413/ionics-collection-repeat-directive-works-erratically-when-a-custom-directive-is

<div class="list" ng-controller="ctrl">
            <div class="item item-avatar" collection-repeat="contact in contacts | orderBy: title" ng-click="selectContact(contact)">
              <ion-item>
                <custom-avatar from="contact"></custom-avatar>
                <h2>{{contact.title}}</h2>
                <p>{{contact.subtitle}}</p>
              </ion-item>
            </div>
          </div>

@jdnichollsc
Copy link

jdnichollsc commented May 8, 2017

Same issue for here with images from a template using ng-include into the collection-repeat, check the issue guys: https://www.screencast.com/t/00PrU98MThAW

Thanks in advance, Nicholls

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 3, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests