Vue-carousel: Bug: scrollPerPage doesn't work in the 0.6.7 version

Created on 26 Feb 2018  路  13Comments  路  Source: SSENSE/vue-carousel

Hello!

scrollPerPage doesn't work in the 0.6.7 version, I tried to set "true" and "false", but it always works like "true".

bug help wanted need repro

Most helpful comment

@quinnlangille just to make sure we are on the same page :)

Carousel with 4 slides, perPage = 2 and navigationEnabled = true
When scrollPerPage = true:

  • Navigation next button will move the page right and show the next 2 slides. (hide slide 0 and 1 and show slide 2 and 3)
  • Mouse dragging will move the page right and show the next 2 slides.
  • Pagination number of dots are as page number and move to the selected page.
  • NavigateTo val will be the page number to goto.

When scrollPerPage = false:

  • Navigation next button will move to the next slides. (hide slide 0 and show slide 1 and 2)
  • Mouse dragging will move the slide by slide.
  • Pagination number of dots are as slide number and move to the selected slide.
  • NavigateTo val will be the slide number to goto.

If that is the case, then I only need to fix the pagination in my code and add some tests. The rest is ready.

All 13 comments

I also got the same issue with the latest version.

Hey guys, I'll take a look into the bug this morning. Which version of vue are you on? Can I get some more info about the bug - a code snippet from your project or a JSfiddle with the bug happening would be helpful. A copy of your package.json would also help debug, if you could paste that here.

Also, have you guys tried using dash case for the props? Depending on your vue version this might be necessary, so you would use as scroll-per-page rather than scrollPerPage. Also, in the mean time, you could add per-page: 1 as another prop, which would give you the same functionality as scroll-per-page: false. Let me know if that helps!

Hey @quinnlangille, I've made a dummy repo you can use to check. I would've made a JSFiddle but the CDN link is still using 0.6.4.

I set up a quick project with vue-cli and really only edited the App.vue file (other than the main.js file, just to import the package itself), so you should be able to see what's going on from there.
I also did a quick yarn build and put it up on my domain for now, for ease of viewing (link).

What I've noticed is that if you either set it to false or don't set anything, you can still use your mouse to traverse through the pages one-by-one by dragging the panels. If you set it to true, it behaves as it should in both that regard and the default behavior. But, as mentioned in the original comment, they all scroll through the entire page, regardless of the setting.

Normally I'd accept the workaround, but in my current project, I need to display 3 per page.

Hope this helps. 馃槃

Hi,
Is this bug going to be fix for v. 0.7.0?
If not, how can I help to make it to this version?

Hey @kulaone, there is a PR that's currently open which deals with some duplicate functionality that I think might be causing this. I'm going to review and merge that hopefully at the start of this week, and if it fixes it will release all changes as part of v0.7.0 right away. If not, I agree that this fix needs to be included in v0.7.0, so I'll post back here with some possible solutions

Hey guys, just confirming that the new changes that I was hoping would fix do not address this. Can confirm it's an issue, and will remove the need for repro from the tags. If anyone wants to take a closer look it would be greatly appreciated! v0.6.10 coming out later today - once we resolve this issue I'll be comfortable rolling to v0.7.0!

@quinnlangille I need some clarification. When we say scrollPerPage, do we mean that it's only affect scrolling and dragging event or it's including navigation and pagination?
I'm asking because I start writing code that will allow navigating only by slide and not by page. I was wondering should I use scrollPerPage as my prop or should I create a new one called navigatePerSlide.

As far as I can see, it's just that scrollPerPage is always on and always moves as if it's to slide the whole page (i.e - if there are 4 items displayed per page, it moves 4 at a time. If there are 2 items, it moves 2 a time). If scroll per page is false, it should move by single items, and if scrollPerPage has a custom set value with perPageCustom, it should move based on that.

Thanks for looking into it @kulaone , would be a huge help 馃榾

@quinnlangille just to make sure we are on the same page :)

Carousel with 4 slides, perPage = 2 and navigationEnabled = true
When scrollPerPage = true:

  • Navigation next button will move the page right and show the next 2 slides. (hide slide 0 and 1 and show slide 2 and 3)
  • Mouse dragging will move the page right and show the next 2 slides.
  • Pagination number of dots are as page number and move to the selected page.
  • NavigateTo val will be the page number to goto.

When scrollPerPage = false:

  • Navigation next button will move to the next slides. (hide slide 0 and show slide 1 and 2)
  • Mouse dragging will move the slide by slide.
  • Pagination number of dots are as slide number and move to the selected slide.
  • NavigateTo val will be the slide number to goto.

If that is the case, then I only need to fix the pagination in my code and add some tests. The rest is ready.

Just merged in @kulaone 's fix! Everything is working fine on my end 馃帀

I'll leave this open for a few days just to see if anyone still has unresolved bugs. Thanks to everyone for the bug reporting, glad to see this was resolved :~)

No word from anyone so assuming this is good to go, will close this! Feel free to open any other issues if anything comes up :~)

Sorry for the late response, but I just now got around to testing this out and everything seems to be working as expected now! Thank you @kulaone @quinnlangille. 馃槃

The only thing I've noticed is that the docs on the actual site still says the default is false when the PR updated it to be true. That being said, I've noticed that inconsistency with multiple options before. Of course, that's a separate issue/PR for a separate time, but just thought I'd note that. 馃憤

Thanks for pointing it out @Danktuary, I've actually got a PR updating the docs open currently that I"m working on. Feel free to comment on it if you have any functionality you feel isn't properly documented :~)

Was this page helpful?
0 / 5 - 0 ratings