angular-carousel icon indicating copy to clipboard operation
angular-carousel copied to clipboard

Feat (attribute) : Infinite Loop Support

Open dcjohnston opened this issue 9 years ago • 42 comments

I've implemented infinite looping via the addition of an attribute 'rn-carousel-loop' to the element on which rn-carousel directive is placed. I've included tests, and I've noted in the README that looping is disabled if buffering is turned on.

dcjohnston avatar Oct 29 '14 11:10 dcjohnston

The last Jasmine test is failing, but I wrote a quick $timeout function that adds a slide in the demo and the code behaves as expected, removing the old virtual slides and replacing them with the new head and tail of the repeatCollection. I'm not sure why it's failing in the test, any ideas? EDIT: As far as I can tell, in the tests, the query for the DOM virtual slides is returning nothing, which explains why the new copies are just added on top of the old copies as opposed to replacing them. Again, this is not happening in the actual demo, and I think the error is specific to the testing environment.

dcjohnston avatar Oct 29 '14 15:10 dcjohnston

thanks for the great feature. About the implementation, what do you think about simply swapping the collection when the index change and let angular update the view ? Buffering is based on this principle;

revolunet avatar Nov 06 '14 20:11 revolunet

Tested the new Feature and it's working great with ngRepeat. Fails if I use a prerendered HTML ul li with following error:

ReferenceError: copy is not defined
    at http://0.0.0.0:9000/bower_components/angular-carousel/dist/angular-carousel.js:326:58
    at nodeLinkFn (http://0.0.0.0:9000/bower_components/angular/angular.js:6711:13)
    at compositeLinkFn (http://0.0.0.0:9000/bower_components/angular/angular.js:6105:13)
    at compositeLinkFn (http://0.0.0.0:9000/bower_components/angular/angular.js:6108:13)
    at compositeLinkFn (http://0.0.0.0:9000/bower_components/angular/angular.js:6108:13)
    at compositeLinkFn (http://0.0.0.0:9000/bower_components/angular/angular.js:6108:13)
    at publicLinkFn (http://0.0.0.0:9000/bower_components/angular/angular.js:6001:30)
    at http://0.0.0.0:9000/bower_components/angular/angular.js:1449:27
    at Scope.$eval (http://0.0.0.0:9000/bower_components/angular/angular.js:12702:28)
    at Scope.$apply (http://0.0.0.0:9000/bower_components/angular/angular.js:12800:23) <ul rn-carousel="" rn-carousel-index="0" rn-carousel-loop="" class="carousel ng-scope" id="carousel1"> angular.js:10072(anonymous function) angular.js:10072(anonymous function) angular.js:7364nodeLinkFn angular.js:6714compositeLinkFn angular.js:6105compositeLinkFn angular.js:6108compositeLinkFn angular.js:6108compositeLinkFn angular.js:6108publicLinkFn angular.js:6001(anonymous function) angular.js:1449Scope.$eval angular.js:12702Scope.$apply angular.js:12800(anonymous function) angular.js:1447invoke angular.js:3966doBootstrap angular.js:1445bootstrap angular.js:1459angularInit angular.js:1368(anonymous function) angular.js:22025fire jquery.js:3073self.fireWith jquery.js:3185jQuery.extend.ready jquery.js:3391completed jquery.js:3407

It's reproducable using following HTML:

<ul rn-carousel rn-carousel-index="0" rn-carousel-loop class="carousel">
      <li><img src="images/slider/0103.jpg"/></li>
      <li><img src="images/slider/0203.jpg"/></li>
      <li><img src="images/slider/0303.jpg"/></li>
</ul>

Update:

Figured out variable copy was defined in a different scope.

BernhardBehrendt avatar Nov 09 '14 00:11 BernhardBehrendt

Well that was a bit sloppy on my part, sorry about that. I meant to be referencing firstCopy but used copy accidentally. Anyways just pushed the fix, everything should be good now.

dcjohnston avatar Nov 09 '14 19:11 dcjohnston

Hey @dcjohnston Thanks for that fast fix. You're awesome. It's working now but still a bit buggy. If I slide "from last to first" or "first to last" the image appears in the moment when slide transition is completed. As long as I'm sliding (touch) or by a clicking on the carousel controls there's noting. Here's the behavior different to my latest test when I used the carousel with ngRepeat.

I also found out (I have Headlines and a paragraph too in each <li>) that the bug does only affect the image. My contentboxes are slided 100% correctly. Problem exists even the contents are removed so that I've an image carousel only.

BernhardBehrendt avatar Nov 09 '14 20:11 BernhardBehrendt

Sorry @BernhardBezdek I'm not sure I follow. To reproduce, I use non-ngrepeated

  • 's with elements inside?

    As a sidenote I just added a lock check to $scope.previousSlide, which was not there although the lock check was present in $scope.nextSlide (it was causing jittery animations if you tapped quickly on the back control).

  • dcjohnston avatar Nov 10 '14 07:11 dcjohnston

    Hey @dcjohnston "To reproduce, I use non-ngrepeated

  • 's with elements inside" -> Exactly. But in the moment when you click to get from last image in that carousel to the first one you will see an empty container sliding in which seem to be replaced by the correct (first) one after transition is complete.

    Yuo can use these HTML to reproduce:

    <ul rn-carousel rn-carousel-index="0" rn-carousel-loop class="carousel">
          <li><img src="images/slider/0103.jpg"/></li>
          <li><img src="images/slider/0203.jpg"/></li>
          <li><img src="images/slider/0303.jpg"/></li>
    </ul>
    
  • BernhardBehrendt avatar Nov 10 '14 13:11 BernhardBehrendt

    Sorry I can't reproduce. <ul rn-carousel rn-carousel-index="0" rn-carousel-loop rn-carousel-controls class="carousel1"> <li><img src="http://www.maslafineart.com/images/galleries/sketchbook/2)%20PenGestureDrawingSiejiOzawa.jpg" ></li> <li><img src="http://www.maslafineart.com/images/galleries/sketchbook/3)%20Sketch,%20face%20gesture72dpi%20copy.jpg"/></li> <li><img alt="" src="http://www.maslafineart.com/images/galleries/sketchbook/5)%20Sketch,%20thumbnail%20demo%20-%20negative%20space,%20Boca72dpi%20copy.jpg"/></li> </ul> This is working fine on my end. At least with the controls, I didnt try the dragging. Is the bug you're experiencing only occuring when you are dragging, or does it also happen with the carousel controls? Keep me updated I want to figure this out ASAP.

    dcjohnston avatar Nov 10 '14 16:11 dcjohnston

    Hey @dcjohnston I'm very sorry that was my fault.... Instead of

    <img src="/path/to/image">
    

    I wrote

    <!-- used ng-src -->
    <img ng-src="/path/to/image">
    

    So I can confirm your code is working if a developer is using it the right way ;-) Thank you very much.

    BernhardBehrendt avatar Nov 11 '14 08:11 BernhardBehrendt

    anyone know when this is going to be merged ? I've just implemented the carousel, and im in a bit of a need of this feature :).. Great work!

    Ngschumacher avatar Nov 12 '14 14:11 Ngschumacher

    @Ngschumacher if you're using bower in your project you can create your own fork or use the fork from @dcjohnston s repository in your bower.json file.

        {
          "name": "myApp",
          "version": "0.0.1",
          "dependencies": {
               "angular-carousel": "https://github.com/dcjohnston/angular-carousel.git#master"
           }
    

    BernhardBehrendt avatar Nov 12 '14 15:11 BernhardBehrendt

    I dont like dat the #Id tag of < ul rn-carousel > is changed by your script. All css I applied through a css #id is then nulliefied.

    wietseva avatar Nov 18 '14 09:11 wietseva

    +1 would love to get this merged into master. currently using dcjohnston's fork live in production

    ryanio avatar Dec 03 '14 07:12 ryanio

    Hey @dcjohnston I run into a new problem (don't know if it happened when I did a bower update) Inside an infinite loop a see by sliding from last to first and from first to last slide no image anymore (know I had a similar problem in the past).

    You can see an example here http://goo.gl/QeKO22 when you slide trough the entire carousel. Any idea what's the reason for that problem?

    BernhardBehrendt avatar Dec 16 '14 15:12 BernhardBehrendt

    Good to see my work exploding in production :p. Do me a favor, and check the demo file in my repo and let me know if it's working for you (particularly the infinite loop demos). Everything there seems to be working. I'm looking at the site (nice work btw) and for some reason it looks like the virtual slides aren't visible, but all of the sliding logic is working as it should. I'm tempted to say it's an issue with the CSS used on the site, but I'll keep looking around.

    dcjohnston avatar Dec 16 '14 18:12 dcjohnston

    @dcjohnston Issue finally found. I'm using a lazyload mechanism for images and if there's no background image until the carousel initializes last slide is empty. Solved the issue by retriggering the lazyload.

    BernhardBehrendt avatar Dec 21 '14 23:12 BernhardBehrendt

    @revolunet What about applying the pull request?

    Using it in many sections in production for a while now without problems. http://cobi.bike/

    @dcjohnston made a great job!

    BernhardBehrendt avatar Feb 01 '15 16:02 BernhardBehrendt

    This would be awesome!

    rryter avatar Apr 01 '15 09:04 rryter

    +1

    bypotatoes avatar May 21 '15 08:05 bypotatoes

    how i can use infinite loop in hexagon animation? which distro i must use ? there is a callback afterchange?

    valix85 avatar Jul 15 '15 16:07 valix85

    Is this likely to be merged soon? Really sort-after feature

    jacquipickup avatar Oct 23 '15 10:10 jacquipickup

    can you please resolve it ;)

    karlvonbonin avatar Feb 15 '16 11:02 karlvonbonin

    Hello,

    thanks for your slider. Is it possible to run the slider continuesly and smooth ? Like here : (see below References - the company logos) http://www.paywithatweet.com/index?locale=en

    karlvonbonin avatar Feb 19 '16 11:02 karlvonbonin

    Thanks for this version! Saved me a day of work! :)

    MichMich avatar Feb 25 '16 09:02 MichMich

    Hello guys, is there any reason for not merging this branch. I will test it as it is exactly what I'm looking for.

    Regards, Julien

    jup31 avatar May 02 '16 09:05 jup31

    Hi, there's no legimate reason to not merge this except i can't maintain this repo anymore.

    Is there anyone willing to become collaborator here, rebase this feature and test that it doesnt break anything?

    Thanks

    revolunet avatar May 02 '16 09:05 revolunet

    Ok I understand, I'm just starting to work now on open source world, so I'm not fully aware of how to manage this kind of project and the time needed to do it.

    jup31 avatar May 02 '16 09:05 jup31

    Ok this is quite simple; The bare minimum is to merge the most important pull requests, after have checked that it doesnt make any breaking changes. (ex: from the demo page)

    Depending on your time, you can also triage the issues/PR

    Once merged, you should publish a new version by respecting semantic versionning.

    I'll then publish to bower and npm.

    Available for any questions !

    revolunet avatar May 02 '16 09:05 revolunet

    But is it possible to try a merge in my own fork ?

    jup31 avatar May 02 '16 09:05 jup31

    can you please rebase your code on the last master branch ?

    revolunet avatar May 02 '16 09:05 revolunet

    I have done a test it work base on the fork, now I will fork your master and create a new branch to compare with this fork.

    jup31 avatar May 02 '16 10:05 jup31

    example rebase from your fork folder :

    git remote add upstream https://github.com/revolunet/angular-carousel.git
    git fetch
    git rebase upstream/master
    

    revolunet avatar May 02 '16 10:05 revolunet

    Bonjour Julien,

    Je viens de voir que tu étais français, ce sera plus simple pour les échanges car je suis un peu novice sur github et git. Donc voilà ce que j'ai fait, j'ai créé un fork de ton origin/master que j'ai ensuite chargé en local, puis j'ai fait un merge avec la branche de dcjohnson incluant la directive de loop et en faisant ce qui me semblait logique pour les conflits. J'ai ensuite réalisé un grunt build pour générer la version minifiée et fait un push sur origin/master de mon fork. Avant cela j'avais installé ma version mergée sur mon application cordova pour vérifier que cela fonctionner mais tu t'en doute je n'ai du coup tester que le infinite loop. Peux tu me donner plus de précisions sur ce que tu attends en terme de test et sur la manière dont je dois les réaliser, j'ai vu qu'il y avait du karma mais je ne connais pas encore ?

    Désolé pour le délai j'étais moi aussi assez occupé. Cordialement, Julien PALAS 0676765603

    2016-05-02 12:08 GMT+02:00 Julien Bouquillon [email protected]:

    example rebase from your fork folder :

    git remote add upstream https://github.com/revolunet/angular-carousel.git git fetch git rebase upstream/master

    — You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/revolunet/angular-carousel/pull/237#issuecomment-216186401

    Julien PALAS

    jup31 avatar May 12 '16 20:05 jup31

    Sorry guys a french message just to be sure that I'm fully aware on how to test my merge on this topic.

    Bonjour,

    Je viens de voir que tu étais français, ce sera plus simple pour les échanges car je suis un peu novice sur github et git. Donc voilà ce que j'ai fait, j'ai créé un fork de ton origin/master que j'ai ensuite chargé en local, puis j'ai fait un merge avec la branche de dcjohnson incluant la directive de loop et en faisant ce qui me semblait logique pour les conflits. J'ai ensuite réalisé un grunt build pour générer la version minifiée et fait un push sur origin/master de mon fork. Avant cela j'avais installé ma version mergée sur mon application cordova pour vérifier que cela fonctionner mais tu t'en doute je n'ai du coup tester que le infinite loop. Peux tu me donner plus de précisions sur ce que tu attends en terme de test et sur la manière dont je dois les réaliser, j'ai vu qu'il y avait du karma mais je ne connais pas encore ?

    Désolé pour le délai j'étais moi aussi assez occupé. Cordialement, Julien P

    jup31 avatar May 12 '16 20:05 jup31

    Hey Julien, can you contact me on [email protected] so we can have a real frenchy chat ? thanks

    revolunet avatar May 12 '16 20:05 revolunet

    Hi, i've remerged all the work done in this fork : https://github.com/dcjohnston/angular-carousel

    Its currently in this branch : https://github.com/revolunet/angular-carousel/commits/merge-loop

    Would be great if some of you could test before any release

    thanks to @dcjohnston and @jup31and so sorry for the long delay on this PR but i dont use this project anymore so if some of you want to maintain the project, you're very welcome.

    revolunet avatar May 13 '16 01:05 revolunet

    Hi guys, I have some strange issue with the branch. I have 2 controls:

    %ul.slider.images{"rn-carousel"=>"", "rn-carousel-controls-allow-loop"=>"", "rn-carousel-loop"=>"", "rn-carousel-controls"=>"", "rn-carousel-index"=>"carouselIndex"}
        %li{"ng-repeat"=>"image in sliderImages"}
             .layer
                %img{"ng-src"=>"{{image.unchanged}}"}
    
    

    One control in ul odd out one

    sunchess avatar May 23 '16 20:05 sunchess

    I found the problem. There are two similar features. https://github.com/revolunet/angular-carousel/blob/merge-loop/dist/angular-carousel.js#L377-L384 and https://github.com/revolunet/angular-carousel/blob/merge-loop/dist/angular-carousel.js#L550-L559

    sunchess avatar May 23 '16 20:05 sunchess

    And one more strange effect: when you try fast swipe 2 or more slides on edge a slide become empty. This effect appears when it has 3 slides in carousel

    sunchess avatar May 23 '16 21:05 sunchess

    Hi,

    I saw that stuff and it instantly filled me with energy and hope, just take a look here http://spykikida.17again.com/xyxeus

    Typos courtesy of my iPhone, [email protected]

    Ngschumacher avatar Jul 10 '16 22:07 Ngschumacher

    Hey,

    Have you heard about that really wowsome stuff? You won't regret, believe me, read more here http://spymisasho.politicalresumes.net/xywjms

    My best to you, [email protected]

    Ngschumacher avatar Jul 10 '16 22:07 Ngschumacher

    Last link sent by Ngschumacher seems infected (JS:Downloader-DEH Trojan), maybe a bot.

    jup31 avatar Jul 11 '16 08:07 jup31