feat(carousel): navigation dots

[huri3l]

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.

[vercel]

@huri3l is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

[jotelles]

Seems pretty solid and useful. I would love to see this merged.

[lavnicki]

please approve this

[Chrystianpaim]

Please approve this

[elcharrua]

bruh, my big head homie is a total genius, you just gotta approve it. please.

[Dunkode]

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!

[soham2k06]

Approve your efforts.
Functionality works fine with 5 or less items but when it comes to having more than 5 items, then it breaks (API is giving incorrect value, you did good).

Also, setting primary background color is not a good idea as it is mostly changed as per user preference, instead you should try using muted-foreground like:
isCurrent(index) ? "bg-muted-foreground" : "bg-muted-foreground/40"

If you continue the issue, I have a suggestion for you to add dotsSize and dotsGap for better customisation.

Screen.Recording.2024-01-19.at.8.33.21.PM.mov

[soham2k06]

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:

Screen.Recording.2024-01-19.at.9.17.56.PM.mov

[huri3l]

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.

[huri3l]

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.

[huri3l]

@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]

Great job for adding gap and size prop in dots to add better customisation.
I hope you quickly get solution for snap value in carousel.
Good luck.

[davidjerleke]

@soham2k06 a bug fix for this has already been released in v8.0.0-rc20 👍🏻.

[davidjerleke]

@soham2k06 but we still need to bump the packages in this PR and someone also has to approve it I think:

• chore(carousel): update embla-carousel package #2286

Of course, if everything else has been solved in that pull request.

[Jacksonmills]

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.

[huri3l]

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.