react-multi-carousel icon indicating copy to clipboard operation
react-multi-carousel copied to clipboard

Bad index when infinite={true}

Open niteshsrivats opened this issue 4 years ago • 6 comments

DESCRIPTION

For developers looking to obtain the current active index or current active index range, there are a couple of ways mentioned such as using a ref or afterChange or beforeChange which passes a zero-based index.

However, when infinite={true} is used, the number of rendered nodes is significantly more than the number passed and the current index doesn't seem to follow any obvious pattern:

  • When the number of items displayed is 3, the start index is 6 for 9 items
  • When the number of items displayed is 2, the start index is 4 for 9 items

Steps To Reproduce

Steps to reproduce the behavior:

  1. Go to github.com/niteshsrivats/react-multi-carousel-bug
  2. Clone the repository, run npm install and npm start
  3. Go to http://localhost:8000 and open the console. Use the slider buttons to change slides
  4. The indexes shown on the cards do not match the ones printed in the console

Expected behavior

There should be no additional nodes and the current index should not have any offset or at the very least a variable offset

Hosted Application

https://modest-meitner-323011.netlify.app/

Other Information

Desktop

  • OS: [Windows version 20H2 (OS Build 19042.450) running wsl2 Ubuntu 18.04]
  • Browser [Google Chrome - Version 84.0.4147.125 (Official Build)]
  • React Multi Carousel Version [2.5.5]

Note [Edit] I tried going through clones.ts If someone could provide an explanation for why the children are being cloned and why the indexes are being changed, i would really appreciate it in case this is the expected behaviour.

niteshsrivats avatar Aug 14 '20 12:08 niteshsrivats

+1

zeing avatar Aug 17 '20 09:08 zeing

hi i know the documentation doesnt mention this but there is a helper function getOriginalCounterPart that is exposed.. you will need to pass it a few params but afaik it will return the index without infinite mode.. https://github.com/YIZHUANG/react-multi-carousel/blob/18d3687b56de4aa48e64d9b4315e52e60e2a6f3b/src/utils/clones.ts#L10

/* getOriginalCounterPart gets the index of the original children. For example, we have an array [clones, originalChildren, clones]; Before making the clones, an item's index is 0, but after the clone, the index is different it could be 4, because we added clones to the array after "componentDidMount". And this function gets the "index" of the item after the clones. */ const originalIndex = getOriginalCounterPart( index, { slidesToShow, currentSlide } childrenArr );

abhinavdalal avatar Aug 17 '20 11:08 abhinavdalal

Thank you for your response @abhinavdalal I think this solution will suffice for my current use case.

However, I did run a lighthouse test and i got a suggestion that said Avoid excessive DOM size when infinite={true}. Is there a way we can achieve the same results without actually cloning the children.

niteshsrivats avatar Aug 20 '20 10:08 niteshsrivats

@niteshsrivats i understand what your saying.. this is a known issue #74 .. we have evaluated to implement a react-window like solution.. this is a an open source project and contributions are welcome! as this requires significant effort.. i don't expect to able to work on this in the near future.. but do hope to get there eventually..

abhinavdalal avatar Aug 20 '20 11:08 abhinavdalal

Can someone explain a bit what the arguments to getOriginalCounterPart are? @niteshsrivats were you able to get this to work as desired?

SamJacobs avatar Mar 22 '21 21:03 SamJacobs

const sliderIndex = getOriginalCounterPart(slider.state.currentSlide, slider.state, React.Children.toArray(slider.props.children));

There's some more discussion about this here.

OscarBarrett avatar Mar 30 '21 06:03 OscarBarrett