Skip to content

Perform current slide calculation based on child elements #1334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
indykoning wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Perform current slide calculation based on child elements #1334

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
748da97
Perform current slide calculation based on child elements
indykoning Jul 20, 2026
47571c9
Apply fixes from Prettier
indykoning Jul 20, 2026
40f72ed
Use total children since these also affect scroll width/height
indykoning Jul 20, 2026
74fd31d
Apply fixes from Prettier
indykoning Jul 20, 2026
26dd07d
Fix loop getting no slides
indykoning Jul 23, 2026
39ebd5a
Merge branch 'bugfix/current-calculation' of github.com:rapidez/core …
indykoning Jul 23, 2026
a18226c
Apply fixes from Prettier
indykoning Jul 23, 2026
8bd864f
Fix bestguess for loop sliders
indykoning Jul 27, 2026
edbad95
Apply fixes from Prettier
indykoning Jul 27, 2026
0f4f2f6
Implement useScroll to determine scroll positions
indykoning Jul 27, 2026
cffc5d2
Apply fixes from Prettier
indykoning Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions resources/js/components/Elements/Slider.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export default {
this.navigate(next)
},
navigate(index, behavior = 'smooth') {
index = this.loop ? index + this.slides.length : index
index = Math.min(Math.max(this.loop ? index + this.slides.length : index, 0), this.container?.children.length - 1)

this.vertical
? this.slider.scrollTo({ top: this.container?.children[index]?.offsetTop, behavior: behavior })
Expand Down Expand Up @@ -202,14 +202,38 @@ export default {
if (!this.mounted) {
return 0
}
return Math.round(this.position / this.childSpan) % this.slides.length
// First make a calculated guess, improving performance by not having to loop through all slides
const bestGuess = Math.round(this.position / this.childSpan) % this.slides.length

@indykoning indykoning Jul 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This bestGuess is correct on regularly sized sliders.
Image
Depending on the size differences in other childs it may only have to check one additional element.
Image

const getSlideByGuess = (slide) => {
const bestGuessChild = this.container.children[slide]
if (!bestGuessChild) {
return slide
}
const bestGuessSpan = this.vertical ? bestGuessChild.offsetHeight : bestGuessChild.offsetWidth
const bestGuessOffset = this.vertical ? bestGuessChild.offsetTop : bestGuessChild.offsetLeft

// Past halfway to the slide it will snap
if (this.position + bestGuessSpan / 2 < bestGuessOffset) {
return getSlideByGuess(slide - 1)
}

if (this.position >= bestGuessOffset + bestGuessSpan) {
return getSlideByGuess(slide + 1)
}
Comment thread
Jade-GG marked this conversation as resolved.

return slide
}

return getSlideByGuess(bestGuess)
},
slidesVisible() {
if (!this.mounted) {
return 0
}

return Math.round(this.sliderSpan / this.childSpan)
return Math.round(
(this.sliderSpan / (this.vertical ? this.container.scrollHeight : this.container.scrollWidth)) * this.container.children.length,
)
Comment on lines +300 to +303

@indykoning indykoning Jul 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The reason we're not calculating from the currentSlide to the last visible slide is because this could cause the visibleSlides to jump up and down depending on your position in the slider.

While visibleSlides is also used in slidesTotal, causing the total to change

@Jade-GG Jade-GG Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this will still cause incorrect calculations for the slidesTotal now. A correct calculation here would take into account first how many items there are, and then only subtract how many items are visible when the slider is all the way at the end.

Is this value still relevant? Because on most shops we already hide the "progress bar" under sliders anyway.

@indykoning indykoning Jul 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Where do you see this calculation being incorrect?
Say we have a slider of 7 items where 4 are visible.
The slider is 1252px wide and elements are 313px wide. This would turn into:
1252 / 2191 * 7 = 4
We're taking the visible size, dividing it by the total scroll width (each element including margins, paddings and gaps), and multiplying it by the amount of items in the slide.
Another way you could write it is:

let averageSlideWidth = 2191 / 7; // 313
let visibleSlides = 1252 / 313; // 4

The only change in this calculation is that instead of using the width of the first child, we're using the average width of all childs.
I see slidesVisible being used and shown: In slidesTotal, in the "progress bars", the numbers and next and previous buttons where we want the slider to slide pages instead of single slides

@Jade-GG Jade-GG Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm imagining a hypothetical situation where you have 100 wide slides (say, 100px wide each) followed by a bunch extremely tiny slides (say, 1px) at the end, where only two of the wide slides are visible (so the slider span itself is 200px).

slidesVisible would still be 2 even if you have up to 25 of the tiny slides at the end. With the current calculation of the slidesTotal, you'd have 24 extra useless "slides" in your progress bar (as all of the tiny slides would all be visible after slide number 100). Obviously this is an absurd example, but more realistically I could see there being 1 too few or 1 too many slides in real world situations.

},
slidesTotal() {
if (!this.mounted) {
Expand Down
Loading