SSENSE/vue-carousel

Bug: Issue when perPage not defined, but breakpointSlidesPerPage is

Open

#39 opened on Mar 13, 2017

View on GitHub
 (3 comments) (0 reactions) (0 assignees)JavaScript (1,705 stars) (513 forks)batch import
bughelp wantedneed repro

Description

Hey there,

I encountered a bug and was only able to create a workaround as I am too inexperienced to fix it. It's about the following code in Carousel.vue: breakpointSlidesPerPage() { if (!this.perPageCustom) { return this.perPage } const breakpointArray = this.perPageCustom const width = this.browserWidth const breakpoints = breakpointArray.sort((a, b) => ((a[0] > b[0]) ? -1 : 1)) // Reduce the breakpoints to entries where the width is in range // The breakpoint arrays are formatted as [widthToMatch, numberOfSlides] const matches = breakpoints.filter(breakpoint => width >= breakpoint[0]) // If there is a match, the result should return only // the slide count from the first matching breakpoint const match = matches[0] && matches[0][1] return match || this.perPage },

What's wrong is, that, if I define a carousel like such: <carousel class="ProjectSlider" :perPageCustom="[[480, 1],[768, 2]]" :navigationEnabled="true">

For devices with width > 480, 1 slide will be shown, for width > 768, 2 slides will be shown and for < 480, match will be undefined (I debugged that) and if perPage is not defined, the view will be buggy. I just set perPage to 1 to circumvent that bug, but this behaviour is not the behaviour described in the documentation.

Contributor guide

Tech stack
javascriptvue
Domain
frontend
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Vue.jsJavaScript
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
70
Research direction
The bug is in Carousel.vue in the breakpointSlidesPerPage method. When perPage is not defined but perPageCustom is set, the method returns undefined for breakpoints smaller than the smallest custom breakpoint. The fix should either set a default value for perPage or modify the method to handle undefined perPage gracefully. Review the documentation to ensure the expected behavior is consistent with the fix.