Skip to content

Conversation

@suhanmoon
Copy link
Contributor

@suhanmoon suhanmoon commented Dec 24, 2018

Fixed a bug when moving left or right when setting loop={true}

Platforms affected

all

What does this PR do?

loop bug fix

What testing has been done on this change?

Set to loop true.
Moving to the left immediately after loading, moving to the right, moving as animation.

Tested features checklist

Fixed a bug when moving left or right when setting loop={true}
@bd-arc
Copy link
Contributor

bd-arc commented Dec 27, 2018

Hi @suhanmoon,

Thanks for the PR!

To be honest, I do see how this fix is supposed to help with the loop issue. Could you please elaborate on that a little further?

@suhanmoon
Copy link
Contributor Author

This Full width, 3 items set.

  1. Left-to-Right Swipe
  2. Move to the back (Animated) after swiping.

@oliverdolgener
Copy link

oliverdolgener commented Feb 12, 2019

I can confirm this fixes the issue that causes a scroll animation to be played when loop is enabled and the user swipes to the right (to reach the last item) or to the left (to reach the first item) immediately after the component has been mounted.

@zabojad
Copy link

zabojad commented Feb 25, 2019

@bd-arc I also confirm it fixes the issue we have in loop mode with the animation being played after reaching a cloned item.

@zabojad
Copy link

zabojad commented Feb 25, 2019

In my opinion, we should even have this on line 274:

this._snapToItem(nextFirstItem, false, false, false, false);

No need to trigger the callbacks again...

@bd-arc bd-arc merged commit 853bb8d into meliorence:master May 21, 2019
@bd-arc
Copy link
Contributor

bd-arc commented May 21, 2019

Available in version 3.8.0.

@Saad9624
Copy link

comment out this line under Crousel.js
return this._carouselRef && this._carouselRef.getNode && this._carouselRef.getNode();

Thanks me later!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants