ui
ui copied to clipboard
feat(carousel): navigation dots
As I was using Carousel
component, I needed to implement Dots Pagination in my project. They are pretty common in many Carousel/Slider libraries, such as slick, Keen-Slider, Swiper and etc.
This pull request adds a CarouselDots
component to improve the current Carousel
.
@huri3l is attempting to deploy a commit to the shadcn-pro Team on Vercel.
A member of the Team first needs to authorize it.
Seems pretty solid and useful. I would love to see this merged.
please approve this
Please approve this
bruh, my big head homie is a total genius, you just gotta approve it. please.
Pagination with dots is a highly beneficial feature, especially when dealing with a substantial number of rendered cards. This implementation significantly enhances the user experience, allowing users to navigate directly to a specific card without the need to scroll through them one by one.
Great improvement!
That problem is being solved but I got another issue: While we have even numbers of item it breaks on last slides. When we are already at last slide it shows wrong in dots as well as in arrows.
Solution for previous issue:
const { api } = useCarousel()
// const isCurrent = (index: number) => index === api?.selectedScrollSnap()
const length = api?.scrollSnapList().length ?? 1
const [current, setCurrent] = React.useState(0)
React.useEffect(() => {
if (!api) return
setCurrent(api.selectedScrollSnap())
api.on("select", () => setCurrent(api.selectedScrollSnap()))
}, [api])
current === index ? "bg-muted-foreground" : "bg-muted-foreground/40"
New issues found:
https://github.com/shadcn-ui/ui/assets/118199354/7eb11649-d2a4-458e-876b-3f3a2cf4e285
I intend to continue the issue until it is merged and available for everyone.
Thanks for the feedback, I'll do my best to fix the issues you found.
I loved the suggestion for dotsSize
and dotsGap
. Already working on that.
About the issue that the carousel has an extra step, I have opened an issue into embla-carousel repository.
In the moment, I am not sure if the Carousel component from shadcn/ui
is using it wrong, or if it is actually a bug existent on Embla Carousel.
As far as I have debugged, in the specific case of 6 contents and 3 visible per slide, embla-carousel
is generating one extra point in the snapList that is really close to the last one.
And if you look closely, you can see the carousel actually moving in the last step.
So this solution will be handled through the issue linked above. If it is something that embla-carousel
should fix, there is nothing else to be done. If there is something that needs to be changed in shadcn/ui
, I will open another pull request fixing it in the future.
The goal is to see if embla-carousel
can detect that the 3rd point is really close to the 4th point (in this example) to merge them into one. I suppose this is happening due to the fact that flex-basis
is the one who rules how many content will be visible in each slide, and some values of the flex-basis
are float, leading to this bug.
@soham2k06 the gap
and size
props were added. Considering that David from embla-carousel
is analysing the issue you have mentioned before, can we proceed with this Pull Request?
@soham2k06 a bug fix for this has already been released in v8.0.0-rc20
👍🏻.
@soham2k06 but we still need to bump the packages in this PR and someone also has to approve it I think:
- #2286
Of course, if everything else has been solved in that pull request.
Hey @huri3l,
After working on my own implementation of dot pagination for the Carousel component, I came across your PR and found some great ideas that complemented my approach. I've since refined my version to include a streamlined pagination logic within the useCarousel hook and flexible dot placement for different orientations.
Feel free to check it out https://github.com/shadcn-ui/ui/pull/3463 Perhaps there's an opportunity to combine our efforts.
Hey @huri3l,
After working on my own implementation of dot pagination for the Carousel component, I came across your PR and found some great ideas that complemented my approach. I've since refined my version to include a streamlined pagination logic within the useCarousel hook and flexible dot placement for different orientations.
Feel free to check it out #3463 Perhaps there's an opportunity to combine our efforts.
Hi! I will check it out right now.