dohooo / react-native-reanimated-carousel

🎠 React Native swiper/carousel component, fully implemented using reanimated v2, support to iOS/Android/Web. (Swiper/Carousel)
https://react-native-reanimated-carousel.vercel.app
MIT License
2.87k stars 330 forks source link

onProgressChanged and onSnapToItem takes too long to give back an index #284

Open RGDEV2022 opened 2 years ago

RGDEV2022 commented 2 years ago

Describe the bug onProgressChanged and onSnapToItem takes too long to give back an index If I want to set a scrollAnimationDuration of 1000 for example, and use onSnapToItem to set the index of the dot in a pagination component, it takes 1sec to update.

Expected behavior Gives the next index immediately on swipe. Not wait until it has snapped and settled.

Versions (please complete the following information):

dohooo commented 2 years ago

This question is about timing, and I'll update the new index after the animation is done rather than begin, but onProgressChange always be synced. Why It took a long time? Maybe I can add new Props to get the next index information before the animation. Be like onBeforeSnapToItem. But there's no way to calculate exactly for onBeforeSnapToItem callback until the animation stops completely. How do you calculate the final index? e.g. If you scroll for half of the screen width, the index will doesn't change. It will scroll back.

RGDEV2022 commented 2 years ago

@dohooo ahh I see the thinking behind the decision. I'm still confused how the old carousel library did this - the snap-carousel. The index seems to change a lot smoother in that one. Here it almost feels like it takes about 200-300ms before actually changing the index even after snapping.

dohooo commented 2 years ago

@dohooo ahh I see the thinking behind the decision. I'm still confused how the old carousel library did this - the snap-carousel. The index seems to change a lot smoother in that one. Here it almost feels like it takes about 200-300ms before actually changing the index even after snapping.

oh, I know what you mean. The timing function causes this.

withTiming withSpring

image

When you think the animation is done and the index should be changing, but actually, the animation isn’t done because the timing function is causing that. You should read the documentation of reanimated. 🥸

dohooo commented 2 years ago

Maybe this new prop isn’t necessary, and you’ll solve the problem when you figure out the correct usage of reanimated. 🚀

dohooo commented 2 years ago

Re. #267. I'll make a simple pagination component for basic usage. OHHHHH, Sry! I just understood what you meant. You want a simple way to control the pagination state rather than delicate animation. Let me think it over.

dohooo commented 2 years ago

It will be delayed If you want to use the react state to control the pagination status. There haven’t any way to do that currently. I’ll solve it.

mehthabux commented 1 year ago

any luck ?

DomVinyard commented 1 year ago

But there's no way to calculate exactly for onBeforeSnapToItem callback until the animation stops completely. How do you calculate the final index? e.g. If you scroll for half of the screen width, the index will doesn't change. It will scroll back.

In this case you would call onBeforeSnapToItem again with the previous index

ajp8164 commented 1 year ago

onProgressChange shows that the index does change when progress reaches 50% (call getCurrentIndex inside onProgressChange). Setting state in onProgressChange seems to work well on device - not so good on sim.

Edit: I originally thought this wasn't working well until I tried on device rather than sim.

              onProgressChange={(
                _offsetProgress: number,
                absoluteProgress: number,
              ) => {
                if (
                  carouselRef.current &&
                  (absoluteProgress > 0.5 ||
                    carouselRef.current?.getCurrentIndex() === 0)
                ) {
                  setActiveSlide(carouselRef.current.getCurrentIndex());
                }
              }}
MatthewPattell commented 1 year ago

Faced the same problem. If you scroll very quickly, onSnapToItem is not called at all. Temporary solution (updated):

const carouselRef = useRef<ICarouselInstance>();
const isNewSwap = useRef(false);

onScrollBegin={() => {
  isNewSwap.current = true;
}}
onProgressChange={(_, absoluteProgress) => {
  const progress = absoluteProgress - carouselRef.current.getCurrentIndex();

  if (carouselRef.current && Math.abs(progress) >= 0.45 && isNewSwap.current) {
    isNewSwap.current = false;

    setTimeout(() => {
      setWallpaperIndex(carouselRef.current.getCurrentIndex());
    }, 50);
  }
}}

setWallpaperIndex - custom function which change state

mehthabux commented 1 year ago

I tried the above solutions , its works fine , but have issue when we are using carouselRef.current?.next(). sometime it skips slides and show white screen. I made few changes in the above and what worked for me is: const onProgressChange = useCallback( (_, progress) => { progressValue.value = progress; const currentIndex = carouselRef.current?.getCurrentIndex(); if (currentIndex > 0 && currentIndex <= 0.05 && activeIndex !== 0) { setActiveIndex(0); } else if ( currentIndex > 0.05 && currentIndex <= 1.05 && activeIndex !== 1 ) { setActiveIndex(1); } else if ( currentIndex > 1.05 && currentIndex <= 2 && activeIndex !== 2 ) { setActiveIndex(2); } }, [activeIndex], );

the above one only works for 3 slide data

douglas-esportudo commented 1 year ago

onProgressChange shows that the index does change when progress reaches 50% (call getCurrentIndex inside onProgressChange). Setting state in onProgressChange seems to work well on device - not so good on sim.

Edit: I originally thought this wasn't working well until I tried on device rather than sim.

              onProgressChange={(
                _offsetProgress: number,
                absoluteProgress: number,
              ) => {
                if (
                  carouselRef.current &&
                  (absoluteProgress > 0.5 ||
                    carouselRef.current?.getCurrentIndex() === 0)
                ) {
                  setActiveSlide(carouselRef.current.getCurrentIndex());
                }
              }}

works fine for me, thank you!

JoeDareZone commented 1 year ago

Slightly cleaner version of the above fix

onProgressChange={(_offsetProgress: number, absoluteProgress: number) => {
  const currentIndex = carouselRef.current?.getCurrentIndex() || 0;

  if (absoluteProgress > 0.5 || currentIndex === 0) {
    setTutorialImageIndex(currentIndex);
  }
}}
seungyoubkim commented 10 months ago

Slightly cleaner version of the above fix

onProgressChange={(_offsetProgress: number, absoluteProgress: number) => {
  const currentIndex = carouselRef.current?.getCurrentIndex() || 0;

  if (absoluteProgress > 0.5 || currentIndex === 0) {
    setTutorialImageIndex(currentIndex);
  }
}}

The getCurrentIndex function strangely returns 0, 1, 2, and 3 when the length of the data is 2. I expect it to return either 0 or 1. This issue does not occur when the length of the data is more than 3. Is this an issue that only I am encountering?

It seems to work well when I use the following code:

onProgressChange={(_, absoluteProgress) => {
  const currentIndex = Math.round(absoluteProgress) % data.length;
  setTutorialImageIndex(currentIndex);
}}
bozha-step commented 8 months ago

Slightly cleaner version of the above fix

onProgressChange={(_offsetProgress: number, absoluteProgress: number) => {
  const currentIndex = carouselRef.current?.getCurrentIndex() || 0;

  if (absoluteProgress > 0.5 || currentIndex === 0) {
    setTutorialImageIndex(currentIndex);
  }
}}

The getCurrentIndex function strangely returns 0, 1, 2, and 3 when the length of the data is 2. I expect it to return either 0 or 1. This issue does not occur when the length of the data is more than 3. Is this an issue that only I am encountering?

It seems to work well when I use the following code:

onProgressChange={(_, absoluteProgress) => {
  const currentIndex = Math.round(absoluteProgress) % data.length;
  setTutorialImageIndex(currentIndex);
}}

I have met the same issue, the posted solution works!

bozha-step commented 8 months ago

I found setState will fire multiple times as it's not sync. Solution is to add a local ref to check when index is updated, this is useful if we have some complex onIndexUpdate handler in onProgressChange:

    const localIndexRef = useRef(0)

    <Carousel
         ...
          onProgressChange={(_, absoluteProgress: number) => {
              if (data.length === 0) {
                  return
              }

              // Calculate the current index. Using Math.round to get the nearest index.
              // Using mod to prevent index overflow when the length of data changes.
              const newIndex = Math.round(absoluteProgress) % data.length

              // Check if the new index is different from the current ref index.
              // If so, update the ref and invoke the onIndexChange callback.
              // This prevents multiple calls to onIndexChange when the index hasn't actually changed.
              if (localIndexRef.current !== newIndex) {
                  localIndexRef.current = newIndex
                  if (onIndexChange) {
                      onIndexChange(newIndex)
                  }
              }
          }}
  />
dohooo commented 6 months ago

If you're looking for a pagination, please see this example for more details: https://reanimated-carousel.dev/usage