Fix #226

[FluorescentHallucinogen]

The index of the last element in the array is one less than the length of the array.

[FluorescentHallucinogen]

@YIZHUANG @abhinavdalal PTAL.

[abhinavdalal]

@FluorescentHallucinogen good catch!

could we also add a test case for this exact scenario where if the currentSlide = lastSlide and we remove that last item from carousel it will reset it to newLastSlide (i.e. oldLastSlide - 1). Request that we add test cases along with PRs; as we plan to move this old code base to functional components with hooks some day; and having a good test cases will make the refactor much easier.

[FluorescentHallucinogen]

@abhinavdalal

I've never written tests before. :-[ I have looked into all the existing tests, but unfortunately didn't understand how to write this test.

Could you please write a test for this specific case yourself? And I'll write more tests for other cases (removing an element from the beginning, removing an element from the middle, removing a few elements, adding elements, etc.) based on your test.

test('Removing the last element', () => {
  const children = [1, 2, 3, 4, 5];
  const state = { currentSlide: 4, totalItems: 4 };
  const props = { infinite: false };

  // What should I call here?
  children.pop();
  const newState = something(children, state, props); // Should I pass both the old and the new children?

  expect(newState).toEqual({ currentSlide: 3, totalItems: 3 });
})

[xenry]

@YIZHUANG @abhinavdalal are you planning on merging this fix? We stumbled upon the same issue and was about to submit a pr when I saw this one.

[abhinavdalal]

Only @YIZHUANG will need to do it..