search

SSENSE / vue-carousel

A flexible, responsive, touch-friendly carousel for Vue.js
https://ssense.github.io/vue-carousel/
MIT License
1.72k stars 505 forks source link

Refactor carousel/slide width calculation logic #274

Open quinnlangille opened 6 years ago

quinnlangille commented 6 years ago

As indicated by numerous bugs and contributions to our scrolling logic, it's clear that something is amiss here. The logic is really complicated, and it's very likely it could be simplified into something much more elegant. This is a big issue and will affect most of the carousel in one way or another. If anyone wants to take it on, feel free to start a discussion here or message me privately by email if you need any guidance! :octocat:

jackrugile commented 4 years ago

Any progress on this issue? Still getting pixel rounding errors that cause edges of slides to be visible. This problem is accentuated the more slides you have.

For example, the browser will calculate a percentage width container at 677.3px (for example), but Vue Carousel sets the flex-basis to 677px (a rounded number). This error adds up, the more slides you add. The slides still have a calculated width of 677.3px, so things get out of sync.

Line 633 of Carousel.vue calculates the width with clientWidth, which rounds the value. Perhaps using getBoundingClientRect().width would fix the issues, since that returns values with decimals.

https://github.com/SSENSE/vue-carousel/blob/master/src/Carousel.vue#L633

Tested locally and this is working well for me! Would love to hear any thoughts on why clientWidth was used though, because there may be an edge case I'm missing.