Skip to content

feat: clone marquee slides for infinite effect #21

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.

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kalon-robson
Copy link

The feature previously snapped to the start when reaching the end, but now the last selected slides are duplicated to create an infinite and seamless effect.

This could be a breaking change for some like me, who have patched this by duplicating the slides manually prior to passing them to the SliderTrack. Due to this, you may want to change this to a toggled feature in the SliderProvider.

@kalon-robson
Copy link
Author

Also noticed a bug (potential unexpected feature)

If you only provide 1 slide but set slidesToShow to 10. It will duplicate the single slide and marquee rather than the previous result which would show 1 and not move.

@kalon-robson kalon-robson changed the title feat: clone marquee slides for infinate effect feat: clone marquee slides for infinite effect Aug 4, 2023
@jacobsfletch
Copy link
Member

jacobsfletch commented Aug 4, 2023

Hey @kalon-robson!!! This is amazing. I have noticed the same effect before and always wished to the find time to fix it, so THANK YOU. I'll be sure to dive deeper into this as soon as I can.

but now the last selected slides are duplicated to create an infinite and seamless effect.

Can you explain this a little but further? What do you mean by "last selected slides" and also can you describe your method of "duplicating"?

@kalon-robson
Copy link
Author

kalon-robson commented Aug 5, 2023

@jacobsfletch I can't even remember why I wrote that. I think I meant to say "the first selection of slides are duplicated to the end". What I'm doing is running a useEffect in SliderTrack that checks to see if marquee is enabled. If so, it will then clone a selection of slides from the start, depending on the number of slidesToShow. Once cloned, it appends them to the track.

If I have 5 slides and slidesToShow is 2 it will take the first two slides and duplicate them onto the end (making the number of slides 7). Then, when it scrolls to the end the view looks identical ready to snap back to the start.

          if (i !== 0) {
            selectedSlide = selectedSlide.nextSibling as HTMLElement;
          }

          const clone = selectedSlide.cloneNode(true) as HTMLElement; <-- this is what I'm using to clone the slide

@kalon-robson
Copy link
Author

Hi @jacobsfletch, do you need me to make any changes/improvements to get this merged?

Kind Regards,
Kalon Robson

Copy link
Member

@jacobsfletch jacobsfletch left a comment

Choose a reason for hiding this comment

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

Hey @kalon-robson,

I'm failing to understand how this would achieve infinite scroll. Unless I'm misunderstanding something here, how you currently have it written duplicates all slides once over. This would mean that it would still reach them end of the marquee, but would just take twice as long to get there.

Instead, don't we need to to run an effect every time a slide enters the slide container? This way there is an off-frame clone of every visible slide that is continually changing, effectively achieving an infinite set of slides. Then once the slide exits the frame, its clone should be removed.

Thoughts?

@@ -218,6 +218,7 @@ const SliderProvider: React.FC<SliderProviderProps> = (props) => {
pauseOnHover,
alignLastSlide,
isDragging,
marquee: Boolean(marquee),
Copy link
Member

Choose a reason for hiding this comment

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

marquee is already a boolean and so it doesn't need to be cast here.

}
}
}
}, [sliderTrackRef, marquee, scrollSnap, slidesToShow]);
Copy link
Member

Choose a reason for hiding this comment

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

sliderTrackRef and scrollSnap are unnecessary dependencies.

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.

2 participants