Skip to content
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

feat: carousel component using embla carousel #527

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Waishnav
Copy link

No description provided.

Copy link

netlify bot commented Nov 10, 2024

👷 Deploy request for kobalte pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit d64dd4d

@Waishnav Waishnav changed the title feat: tried implementing it using embla carousel feat: carousel component using embla carousel Nov 10, 2024
@MengLinMaker
Copy link
Contributor

@Waishnav Might want to check if using embla-carousel dependency is allowed.

From what I understand, UI primitive libraries like this one will generally avoid adding dependencies.

@Waishnav
Copy link
Author

@Waishnav Might want to check if using embla-carousel dependency is allowed.

From what I understand, UI primitive libraries like this one will generally avoid adding dependencies.

Hmm make sense
@jer3m01, I'd love to hear your thoughts and any suggestions on how we might proceed with implementing this. Thank you both for your insights!

@jer3m01
Copy link
Member

jer3m01 commented Nov 13, 2024

Thanks for the draft!

I think this could be a good component, rewriting an entire carousel is out of scope for now.
Using embla (specifically https://www.embla-carousel.com/get-started/solid/) should be the way forward.

The API should ideally be fully wrapped in Kobalte with props on the Carousel.Root for all embla options. (The end user should not access embla themselves).
Only the interactivity should be left to embla, so that the entire layout of the component is generated during SSR.

Looking forward to this, good luck!

@Waishnav Waishnav marked this pull request as ready for review November 16, 2024 19:12
@Waishnav
Copy link
Author

Hey @jer3m01,

I've implemented all the suggestions from your previous message.

The Carousel component now uses Embla under the hood and exposes it through the options and plugins props, allowing users to enjoy the flexibility and power of Embla while retaining full control.

With Kobalte's WAI-ARIA compliance and unstyled component philosophy, users can now compose their own custom carousel components as needed.

That said, I’m still relatively new to writing composable UI components in this pattern. I’d greatly appreciate it if you could review it thoroughly and share any feedback or suggestions for improvement. Looking forward to iterating further based on your input!

@Waishnav
Copy link
Author

one more thing, should we add the keyboard interaction support just like we have for Tabs component?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants