Fix the carousel option
#47
Conversation
Sync latest changes
This changes make sure that the className doesn’t contain any trailing spaces.
* Detect the correct direction in the `goTo` function to set the
correct classes.
* To detect a forwards direction we need to check if the `index` is
higher than the `currentItemIndex` **or** if the `index` is `0` an the
`currentItemIndex` is the index of the last item (go from the last item
to the first).
* To detect a backwards direction we need to check if the `index`
is lower than the `currentItemIndex` **or** if the `index` is the index
of the last item and the `currentItemIndex` is `0` (go from the first
item to the last).
* Fix the initial button state when the `carousel` option is disabled
|
Thanks @jackmu95 will take a proper look at this later on! 👏 |
|
No problem :) I forgot to create a separate branch for my fist pull request (#46) so this is also included in this PR. I hope it's OK... But while testing anything was fine 👍 |
|
Yes it does. And you can close #46. |
|
This will need some tests written to make sure it's all working properly. For example (in pseudo code) Similar tests for backwards direction If you are happy writing the tests, I can then double check everything is working and merge this PR. Otherwise, I will write tests later |
|
I will write the tests and update the PR 👍 |
|
I added a test for the button state on initialization when the To test the button state I made some changes to the |
| this.currentItemIndex = 0; | ||
| addClass(this.allItemsArray[this.currentItemIndex], this.options.currentItemClass); | ||
| } | ||
|
|
||
| // Update the button state on initialization when the `carousel` option isn't enabled | ||
| if (!this.carousel) { | ||
| this.updateButtonStates(); |
There was a problem hiding this comment.
What's the use case of doing this?
There was a problem hiding this comment.
When the carousel option is disabled, the previous button gets disabled when you are on the first item and the next button gets disabled when you are on the last item. When the wallop slider is initialized and the start item is the first or the last one, the depending button isn't disabled (see http://codepen.io/anon/pen/zvrWpp).
| this.currentItemIndex = 0; | ||
| addClass(this.allItemsArray[this.currentItemIndex], this.options.currentItemClass); | ||
| } | ||
|
|
||
| // Update button states to make sure the correct state is set on initialization | ||
| this.updateButtonStates(); |
There was a problem hiding this comment.
This is needed to set the correct button state on initialization when the carousel option is disabled. This was not the case before (see http://codepen.io/anon/pen/zvrWpp). To get the correct state for the previous button, you need to click next and the previous.
goTofunction to set thecorrect classes.
indexishigher than the
currentItemIndexor if theindexis0an thecurrentItemIndexis the index of the last item (go from the last itemto the first).
indexis lower than the
currentItemIndexor if theindexis the indexof the last item and the
currentItemIndexis0(go from the firstitem to the last).
carouseloption is disabled.