Skip to content

Showing initial slide in infinite loop when going backwards (previous)#47

Merged
zautumnz merged 2 commits intojane:masterfrom
nedimb86:hotfix/inifinte-loop-show-initial-slide
Oct 14, 2017
Merged

Showing initial slide in infinite loop when going backwards (previous)#47
zautumnz merged 2 commits intojane:masterfrom
nedimb86:hotfix/inifinte-loop-show-initial-slide

Conversation

@nedimb86
Copy link
Copy Markdown
Contributor

Change Details

When going to previous slide while in infinite loop initial slide was skipped due to minor logic inconsistencies.
I have replaced this.getPartiallyObscuredSlides() with Math.max(activeIndex - 1, firstIndex) because this function is behaving strange and causes slider to show same slide as previous over and over. This is an issue on your demo where it breaks as slide nr 12.

Lines 233 and 238 are actually the same. They won't set slide index to last slide until initial slide is showing (until it is active)

Change Type

  • Feature
  • Chore
  • [x ] Bug Fix

Change Level

  • major
  • minor
  • [x ] patch

Checklist

  • Added tests / Task did not decrease code coverage
  • If UI change, tested across browsers

Other Context (if appropriate)

Screenshots (if appropriate)

Comment thread src/whirligig.js Outdated
if (!slideBy) {
const [prevSlide] = this.getPartiallyObscuredSlides()
const prevInfinteSlide = (prevSlide === firstIndex) ? childCount - 1 : prevSlide
const [prevSlide] = Math.max(activeIndex - 1, firstIndex)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the destructuring here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. I have removed it.

@zautumnz
Copy link
Copy Markdown
Contributor

Thanks for the contribution! There's just one change needed on line 232, then I'll merge it in.

@zautumnz
Copy link
Copy Markdown
Contributor

Thanks!

@zautumnz zautumnz merged commit 8cf9b8a into jane:master Oct 14, 2017
@zautumnz
Copy link
Copy Markdown
Contributor

Released in 6.1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants