react-native-snap-carousel icon indicating copy to clipboard operation
react-native-snap-carousel copied to clipboard

🔥 4.0.0 beta.x

Open bd-arc opened this issue 4 years ago • 94 comments

Release notes

Everything you want to know about the upcoming version is here

Go on and comment below, ask your questions, provide feedback, and share your insights 😉

bd-arc avatar Apr 06 '20 12:04 bd-arc

Awesome work. I am looking forward to this. One thing I noticed when testing 4.0.0-beta.2 is the ability to only move one item at a time is gone. I tested on iOS with both useExperimentalSnap on and off. This is one of the big reasons I use this library. I need the ability to only page one item at a time no matter how much momentum was used. Is this intentional?

AAAstorga avatar Apr 07 '20 17:04 AAAstorga

@AAAstorga Well, the logic is completely different and we're now leaving this to the native scroll instead of trying to compensate for it with JS hacks. That's definitely a tradeoff.

However, I suggest you take a closer look at useExperimentalSnap's description. You can get what you're after on top of the smooth native scrolling if you set both useExperimentalSnap and disableIntervalMomentum to true ;-)

bd-arc avatar Apr 07 '20 18:04 bd-arc

I am using rn 0.62.2 and got this warning

%s: Calling `getNode()` on the ref of an Animated component is no longer necessary. You can now directly use the ref instead. This method will be removed in a future release. FlatList

r0b0t3d avatar Apr 10 '20 02:04 r0b0t3d

@r0b0t3d This is the issue described in #673. Unfortunately this is is a big one.

Read this comment to understand why.

bd-arc avatar Apr 20 '20 08:04 bd-arc

I have been researching the issue of smooth scrolling and came across this exciting new update.. This carousel is the center piece of our app and we really want to have an excellent experience - well, this update brings it! I installed beta 2, removed the deprecated properties and wow - fantastic. The feel and usability is excellent - thank you to all for this great improvement.

Since our use case is pretty vanilla; simple carousel for swiping cards, we've decided to upgrade the app now and keep a watch on issues. Thanks again for all your hard work!

Update - The pagination dot doesn't update until the slide completely stops. Would be an improvement to have the dot update as soon as the destination slide is determined.

Update 2 - I see prop onScrollIndexChanged and replaced onSnapToItem with it. The carousel i'm using for test has just three cards. When swiping from card 0 to 1, 1 to 2, or 2 to 1 the slide momentum stops early and the slide really snaps into place. However, when swiping from slide 1 to 0 the momentum of slide 0 is smooth until stop (no snap in place). Pagination works as expected though ;-)

          <Carousel
            ref={c => {
              this.state._carousel = c;
            }}
            firstItem={this.state.activeSlide}
            data={visibleCards}
            renderItem={this._renderCard}
            sliderWidth={sliderWidth}
            itemWidth={itemWidth}
            inactiveSlideScale={0.9}
            inactiveSlideOpacity={1}
            activeSlideAlignment={'center'}
            onScrollIndexChanged={index =>
              this.setState({activeSlide: index, swiped: true})
            }
          />

Update 3 - I noticed that loop={true} doesn't loop. I tested with 8 cards in the carousel and the rendered set is 14 cards and stops at both ends. So 6 of the 8 cards are duplicates.

ajp8164 avatar Apr 24 '20 03:04 ajp8164

@ajp8164 Thanks a lot for the feedback! We're already using it in production as well — couldn't wait to please our users with that smooth scrolling :-)

Re: Pagination

I'll admit I haven't tried it yet. My guess is that we need to connect the relevant logic to onScrollIndexChanged for the dots to update while scrolling.

Re: onScrollIndexChanged

To be honest, I don't expect it to work properly given your example. Basically, you're updating the state in the callback, but as a result the firstItem prop is updated too and this resets the carousel. Is it working properly if you remove the firstItem prop?

Re: Loop

This is surprising! I've made sure to test the loop thoroughly and can confirm it was working on both iOS and Android. Do you mind sharing your setup?

bd-arc avatar Apr 24 '20 10:04 bd-arc

Great call on the firstItem value - removing it fixed the slide snap problem. It also solves the pagination dot rendering - it no renders while the slide is slowing down.

Regarding loop - After some experimentation I have it working. The slides in the loop are vertical stack. sliderHeight and itemHeight are set. sliderWidth and itemWidth are not. When i add itemWidth the looping works correctly. Setting sliderWidth didn't have any effect.

Your reply helped greatly - thank you very much, I appreciate you guys!

ajp8164 avatar Apr 24 '20 15:04 ajp8164

About to take the plunge by implementing beta (first time using this plugin). Just wanted to get an idea of how close we are to beta 3?

jacquesdev avatar May 21 '20 16:05 jacquesdev

Hi, i've tested version 4 beta 2, it works fine, the only problem I noticed is that the snapToItem method does not work on the first element with index 0, eg: CarouselRef.current.snapToItem(0, false, false); don't works, CarouselRef.current.snapToItem(1, false, false); works fine. I try to investigate the code to find out why.

alessandro-bottamedi avatar May 26 '20 07:05 alessandro-bottamedi

@alessandro-bottamedi Thanks for testing! It's actually been fixed with #696.

bd-arc avatar May 26 '20 07:05 bd-arc

@r0b0t3d Just implemented the solution you suggested to get rid of the getNode() warning. Thanks for that!

However I didn't test it on RN < 0.60...

Version 4.0.0-beta.3 is available to everyone who wants to give it a try!

bd-arc avatar May 26 '20 08:05 bd-arc

IMG_3403 with beta 3 i get this error when try to snapToItem (android and iOs, RN 0.61.5)

alessandro-bottamedi avatar May 26 '20 09:05 alessandro-bottamedi

@alessandro-bottamedi Is it working if you remove the following in your local node_modules folder?

If yes, then the deprecation fix is unfortunately doing more harm than good...

There are other ideas mentioned in #673, but none of them is perfect.

bd-arc avatar May 26 '20 09:05 bd-arc

@alessandro-bottamedi Is it working if you remove the following in your local node_modules folder?

If yes, then the deprecation fix is unfortunately doing more harm than good...

There are other ideas mentioned in #673, but none of them is perfect.

Yes, i confirm that everything works correctly without those lines

alessandro-bottamedi avatar May 26 '20 09:05 alessandro-bottamedi

just upgrade to beta3, works fine on my side. using rn 0.62.2

r0b0t3d avatar May 26 '20 10:05 r0b0t3d

@alessandro-bottamedi could you change above code to

       if (this._carouselRef && (
            (this._needsScrollView() && this._carouselRef.scrollTo) ||
            (!this._needsScrollView() && this._carouselRef.scrollToOffset)
        )) {
            return this._carouselRef;
        }

This code works in 0.62.2, need your confirm in 0.61.x

r0b0t3d avatar May 26 '20 15:05 r0b0t3d

@alessandro-bottamedi could you change above code to

       if (this._carouselRef && (
            (this._needsScrollView() && this._carouselRef.scrollTo) ||
            (!this._needsScrollView() && this._carouselRef.scrollToOffset)
        )) {
            return this._carouselRef;
        }

This code works in 0.62.2, need your confirm in 0.61.x

It seems that everything works correctly with this change on RN 0.61.5! :+1::+1:

alessandro-bottamedi avatar May 26 '20 15:05 alessandro-bottamedi

great! @bd-arc what do you think?

r0b0t3d avatar May 26 '20 15:05 r0b0t3d

Great idea @r0b0t3d! And thank you @alessandro-bottamedi for testing it on such a short notice :-)

Want to make a PR for this branch and one for master @r0b0t3d? (Adding a little comment to explain the reason why we had to do that if you don't mind.)

If you're pressed on time, I'll take care of it myself.

bd-arc avatar May 26 '20 15:05 bd-arc

it is ok for me. but there is another PR for this case #673. should I create another one?

r0b0t3d avatar May 26 '20 15:05 r0b0t3d

@r0b0t3d Sure, go ahead!

Once your two PRs are merged we should finally be able to say goodbye to that issue.

bd-arc avatar May 26 '20 15:05 bd-arc

Ok, version 4.0.0-beta.4 should solve the getNode() issue for everyone.

Kudos to @r0b0t3d 🙌

bd-arc avatar May 27 '20 05:05 bd-arc

Hi @bd-arc, thanks for your hard work with this library!

I am trying to test v4. Do you know if there's some away to add the types?

I've tried yarn upgrade -d @types/[email protected] but didn't work.

nelsonprsousa avatar May 27 '20 09:05 nelsonprsousa

@NelsonPRSousa Well, I'm not the one maintaining those types, so the definitions are not up-to-date with the beta.

The plan would be to migrate the whole library to TypeScript, but I haven't found the time to do so yet...

bd-arc avatar May 27 '20 09:05 bd-arc

Oh, my bad 😅

I've already tested 4.0.0-beta.4 and it is working as expected (even parallax images and pagination).

And much faster and smoothly. Awesome job! 🚀

Gonna remove @types/react-native-snap-carousel for now 👍

nelsonprsousa avatar May 27 '20 10:05 nelsonprsousa

hey everybody, thanks for the 4.x version, its awesome, just want to let you know that upgrading from 4.0.0 beta.3 to 4.0.0 beta.4 broke horizontal slider fro me, on the second cycle of sliding slide nr 4 appeared to react like it was the last slide(although there were 7 slides), so I could swipe further, only swipe back. The same issue I had with the latest 3.9 version, that's why started to test beta's

kolkol69 avatar May 28 '20 07:05 kolkol69

@kolkol69 Could you try inverting those two lines and let me know if it works properly afterwards?

Also, are you experiencing this issue on Android, iOS, or both?

bd-arc avatar May 28 '20 14:05 bd-arc

@bd-arc Just wanted to express my appreciation for you actively maintaining this library and looking at PRs/ making adjustments in a fairly timely manner... I feel like there's so many unmaintained RN libraries; it's refreshing to see one that is actually being maintained and updated 👍 Keep up the awesome work, v4 is looking nice!

bfaulk96 avatar May 28 '20 18:05 bfaulk96

@NelsonPRSousa @bfaulk96 Thanks a lot for your kind words guys, I really appreciate it!

I won't lie: it's pretty tricky to continue maintaining this library while growing my own company, managing my team, and taking on a few freelancer jobs on top of that.

Doing my best anyway because I love this project (created it in 2017 and used it in every single one of my apps since then) and am happy to give back to the Open Source community ;-)

bd-arc avatar May 28 '20 19:05 bd-arc

@bd-arc I just noticed that now I have one TypeScript error regarding the carousel ref.

This is my render code (using 4.0.0-beta.4):

<Carousel
	ref={carouselRef}
	data={pointsOfSale}
	renderItem={renderItem}
	onScrollIndexChanged={setActiveSlide}
/>
<Pagination
	dotsLength={pointsOfSale.length}
	activeDotIndex={activeSlide}
	carouselRef={carouselRef}
/>

This is how I specified carouselRef:

const carouselRef = useRef(null);

Do you have any idea how I should specified carouselRef, to get rid of the error? image

Note: This warning is only appearing in the carouselRef property from Pagination. Isn't triggering on ref prop from Carousel 🤷‍♂️

Thank you 🚀

nelsonprsousa avatar May 31 '20 21:05 nelsonprsousa