search

radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.93k stars 833 forks source link

[New Primitive] `Carousel` #388

Closed chaance closed 2 months ago

chaance commented 3 years ago

See https://github.com/radix-ui/primitives/discussions/375

jjenzz commented 3 years ago

Carousel Research

Real world examples

Google cast

image

Netflix

image

Instagram

image

Squarespace

image

Usain Bolt

image

Mercedes Benz

image

Toyota

image

Lib examples

Bootstrap

https://react-bootstrap.github.io/components/carousel/

React carousel

https://brainhubeu.github.io/react-carousel/

Slick

https://kenwheeler.github.io/slick/

Flickity

https://flickity.metafizzy.co/

React Responsive Carousel

https://react-responsive-carousel.js.org/

Comprehensive features

Looking at the above, the spectrum of available features appears to be the following. I've made these checkboxes so we can agree which we would like to support.

Please feel free to and add anything not covered here to the list! Once agreed on these, I'll propose an API 👍

Questions

  1. Most carousels appear to contain images. Are we happy to say that slides are an open box for content or would we expose something like SlideImage, SlideDescription to restrict the types of content? @chaance raises an interesting point below about draggable functionality if the carousel is an open box with draggable things inside it.
chaance commented 3 years ago

Great start. Can't believe there's not a slick-carousel on the list here since that's been the one I historically have seen most indie devs reach for despite being unmaintained for years 😅

My biggest concern with drag-to-slide is how it works when composed with other draggable/slideable components composed within the carousel. Are nested carousels supported in any case? What about Slider or audio/video element controls inside of a Carousel? I'm sure we'll get into some funky edge cases here, just something I might consider early if it helps.

chaance commented 3 years ago

In the Usain Bolt example, I imagine these slide previews might overflow at certain widths, and they too would become independently draggable. A carousel inside a carousel. 🙃

image

benoitgrelard commented 3 years ago

Probably worth adding https://flickity.metafizzy.co/ to the list as well. That's arguably one of the best out there I think.

jjenzz commented 3 years ago

Can't believe there's not a slick-carousel on the list here

Your wish is my command 😛

My biggest concern with drag-to-slide is how it works when composed with other draggable/slideable components composed within the carousel

Yeah I was also wondering what sort of media we think should be allowed in these. Are we happy to say they are an open box or would we expose something like SlideImage, SlideDescription to limit the content? 🤔

chaance commented 3 years ago

Yeah I was also wondering what sort of media we think should be allowed in these. Are we happy to say they are an open box or would we expose something like SlideImage, SlideDescription to limit the content? 🤔

My thought is that it probably has to be a bit of an open box, but it's potentially problematic even with non-interactive content in ways I haven't considered fully. I'd probably want to still be able to select text with a mouse, for example. I wonder if it would be worth trying to detect the intent of a particular action -- is the user clicking or touching to pressing something, or are they intending to drag? I'd be interested to see how/if React Aria handles this when usePress and useMove are both triggered.

colmtuite commented 3 years ago

Some notes after chatting through the discovery with @jjenzz.

The feature list posted above looks solid.

Two things that require further thinking:

Some notes to clarify particular features:

Prev/Next Buttons

Thumbails Thumbnails would just be content inside the indicator button. The component would render a button by default, and the consumer just adds content inside.

Decouple events from scrolling You can set the amount of slides per pane, which are scrolled when you click a next/prev button. However, the touch/swipe/trackpad interaction should not necessarily be tied to this setting.

It works well for image gallery carousels, the kind you see when you click an image in an image grid on Twitter or FB. But there are many cases where you want the touch/swipe/trackpad interactions to function as a normal scroll area, with only the next/prev and indicator buttons sliding a whole pane.

Some research is required here to figure out exactly how best to manage this.

Option to disable swiping on pointer devices This is something to consider. Instagram and Twitter don't support swiping in their image gallery components. It's arguably a mistake, and I'd vote that it is a mistake.

Indicators We have identified three different types of indicator element.

  1. Indicator Button would render a <button> by default, and could optionally be rendered as a <div>. Sometimes they indicate the total panes count, others indicate the total slides count. This should be verified by research though, I'm now thinking they only indicate slides when they're progress bars, which would mean we may not need to support both behaviours on this component.
  2. Indicator Count would show the current slide number and the total slides count. Perhaps it should also be able to show the current pane number and the total panes count. Although perhaps it should just work with panes, as per my previous point.
  3. Indicator Progress is commonly seen on social media stories, where each slide has a timer and a progress bar. There is always only one progress bar per slide.

isScrolling state Expose the state, so consumers can animate/highlight something (like an indicator or scrollbar) while the carousel is scrolling.

Autoplay

Cursor key navigation Cursor keys should navigate slides, even if there is no focusable content inside the slides. Like an image gallery carousel.

Many carousels have focusable content in each slide. For example, focusable cards on Deliveroo, focusable cards on Netflix, tabs in a carousel on Youtube etc. So we need to ensure that, for example, cursor key navigation still works on tabs, even though each tab would be inside an element like <CarouselItem>.

Nested carousels

benoitgrelard commented 3 years ago

I really really like where this is going! ♥️

You can set the amount of slides per pane

There is something interesting related to this in Flickity which I haven't seen many libs support and we haven't mentioned here: variable width cells (ie. images of differents widths/ratios for example).

This is kinda important I would say, and does change the way things will have to be calculated probably (especially around swiping, etc).

Indicators We have identified three different types of indicator element.

Indicator Button would render a

I am a little confused by this section, would you mind posting image examples of each things you have in mind for these here? In my mind, I'm thinking of the little dots, with an active one, but this seems to mention other stuff like number of panes, number of slides, etc which confused me a bit.

colmtuite commented 3 years ago

variable width cells

I didn't think of this, so we should probably look into it a little. I see some carousel libs support it, but off the top of my head, I can't think of any valid use cases or any existing examples in the wild. It seems like it would always be a bad design.

but this seems to mention other stuff like number of panes, number of slides,

On Netflix in the screenshot posted above, the indicators reflect the number of panes/pages available, not the number of total slides.

In other carousels, like social media stories (Fleets, IG Stories, FB Stories etc.), the indicators reflect the total number of slides/items.

In both of the cases I've just mentioned, the indicators are not interactive. But often, you would want interactive indicators like the little button dots you mentioned.

benoitgrelard commented 3 years ago

variable width cells

I didn't think of this, so we should probably look into it a little. I see some carousel libs support it, but off the top of my head, I can't think of any valid use cases or any existing examples in the wild. It seems like it would always be a bad design.

Here's one of the flickity examples: https://codepen.io/desandro/pen/GgQREP I can definitely see at least this use case for image galleries as you have portrait vs. landscape.

but this seems to mention other stuff like number of panes, number of slides,

On Netflix in the screenshot posted above, the indicators reflect the number of panes/pages available, not the number of total slides.

In other carousels, like social media stories (Fleets, IG Stories, FB Stories etc.), the indicators reflect the total number of slides/items.

In both of the cases I've just mentioned, the indicators are not interactive. But often, you would want interactive indicators like the little button dots you mentioned.

Ah ok I see, I wasn't clear on whether you meant the numbers should actually appear on the page (like 3/5). I see what you're saying now, meaning how many indicators based on if they're indicating slides or panes, gotcha.

colmtuite commented 3 years ago

I can definitely see at least this use case for image galleries as you have portrait vs. landscape.

I would file this under "bad design". It's passable in this exact case (the full-width carousel), but still bad. The image should be centered horizontally and vertically inside a fixed-width container.

In a typical image gallery, where you're viewing a single image at a time (the kind on Twitter/IG/FB etc.), this would be very bad. You never want the UI (next/prev buttons etc.) around the image to shift. So the container should always be the same size, and the image is centered inside it.

So at this point, I'd still file this under "not important at all", and potentially even "we should actively discourage this".

benoitgrelard commented 3 years ago

🤔 I don't see anything shifting on this example. But anyway, just making sure we capture all the things out there.

jjenzz commented 3 years ago

This is still very much in DRAFT, but wanted to jot down progress so far in case there are any opinions on this direction so far:

Anatomy

See bullets below for detailed explanation of various bits.

<Carousel  
  as="div"                      // default `div`
  slidesPerPage={2}             // default `auto`
  loop={false}                  // default `true`?
  page={1}                      // default `undefined`
  defaultPage={1}               // default `undefined`
  onPageChange={page => {}}     // default `undefined`
  onScroll={event => {}}        // default `undefined`
  onDragScroll={event => {}}    // default `undefined`
>
  <CarouselSlide as="div" />
  <CarouselSlide as="div" />
  <CarouselSlide as="div" />
  <CarouselSlide as="div" />
  <CarouselSlide as="div" />
  <CarouselSlide as="div" />
  <CarouselSlide 
    as="div" 
    autoFocus                   // Scrolls into view
  />

  {/* non-interactive indicator */}
  <CarouselIndicator as="ol">
    <CarouselIndicatorItem as="li" />
  </CarouselIndicator>

  {/* interactive indicator */}
  <CarouselIndicator as="ol">
    <CarouselIndicatorItem as="li">
      <CarouselIndicatorButton as="a" />
    </CarouselIndicatorItem>
  </CarouselIndicator>

  <CarouselButtonPrevious as="button" />
  <CarouselButtonNext as="button" />
</Carousel>
chaance commented 3 years ago

A couple of initial thoughts, but otherwise I love the proposed API!

  • CarouselIndicatorItem and CarouselIndicatorButton - My thinking here was that the user would render one of these and we would clone it for each page and if they wanted an indicator per slide then they would do slidesPerPage={1}. However, this doesn't cover the case where users might want to put their own content inside. I need to think about this part some more (hence DRAFT). I've been thinking about this (but not hugely keen):

Didn't see this as an option in your post, but what about:

<CarouselIndicator as="ol">
  {({ pages }) => pages.map((page) => (
    <CarouselIndicatorItem as="li" key={page.id}>
      <CarouselIndicatorButton as="button" aria-label={page.title}>
        <img alt="" />
      </CarouselIndicatorButton>
    </CarouselIndicatorItem>
  ))}
</CarouselIndicator>
jjenzz commented 3 years ago

I think it might actually be kind of important to support autoplay

Yep, I think we're all agreed we need this but as Colm touched on above, we're thinking that could be the second release of this (which we could work on right away if we wanted to). There are features like autoplay that appear to be more suited to image carousels. So, we might just build a separate Slideshow component that would re-compose Carousel and include things like autoplay and arrow key navigation.

When we come to do that work, perhaps we will realise it should just be part of Carousel, which is fine. We're just omitting it now to help save time on those discussions (and the added complexity of doing it well like you say) so we can get an MVP version out there sooner.

Does autoFocus imply that as slides change automatically that the new slide receives focus as it comes into view?

Nope. I'm imagining it much like autoFocus on an input. So, when the Carousel mounts/loads, if a CarouselSlide has autoFocus it will gain focus and scrollIntoView. I added this because the consumer won't necessarily know what page a particular slide is on so they can't just do defaultPage={3}. Pages could be dependent on viewport dimensions. That's why I also added this:

onPageChange - Still fires when dragging/scrolling when the page bounds have changed.

So, if they have page={2} but also have autoFocus on a slide that isn't on page 2, it would scroll into view and call the onPageChange callback :+1:

Didn't see this as an option in your post, but what about

That is the same as my proposal I believe except I meant we would do the map under the hood for them. I think I confused the situation by rendering page in the item. That wasn't meant to be a component, it's just the page number. The focus of the proposal was the function as child thing tho like yours: 

<CarouselIndicator>
    {({ page }: { page: number }) => (
        <CarouselIndicatorItem>
            <CarouselIndicatorButton>
                Page {page}
            </CarouselIndicatorButton>
        </CarouselIndicatorItem>
    )}
</CarouselIndicator>

There is no page data object here, only what page number the indicator is trying to render an item for.

I'm just not sure if this pattern is worth it for our first release. This is the sort of thing I'd prefer to pass on personally until we understand how people are using it and it is specifically requested for something that cannot be achieved without it. I'm struggling to think of genuine use-cases that 1 and 2 don't already cover. Can anyone else?

chaance commented 3 years ago

When we come to do that work, perhaps we will realise it should just be part of Carousel, which is fine. We're just omitting it now to help save time on those discussions (and the added complexity of doing it well like you say) so we can get an MVP version out there sooner.

👍

There is no page data object here, only what page number the indicator is trying to render an item for.

Right, I think the difference is that I'm thinking of an API where we register pages in context as slides are rendered. This let's consumers map over that data if they want, or they just render the indicators one at a time, whatever they want. I think I get what you're saying about cloning them so they only have to render once, but I generally like the APIs to be as explicit as possible. But I think generally I agree that:

This is the sort of thing I'd prefer to pass on personally until we understand how people are using it and it is specifically requested for something that cannot be achieved without it.

jjenzz commented 3 years ago

This let's consumers map over that data if they want

What data are you referring to here? A page doesn't have data as far as I understand it (apart from what page number it is) and neither do slides, they just have children. So, since the indicators render one per page, we can tell it which page number it is being rendered for but I'm not sure what other data there is?

I think I get what you're saying about cloning them

Ah, no, cloning wouldn't happen here. It would only happen in example 2. With the child as function I would do something like this internally:

function CarouselIndicator({ children }) {
    const context = useCarouselContext();
    return [...Array(context.pagesCount)].map((_, index) => children({ page: index + 1 }))
}

The consumer doesn't create the pages, we do internally, they're like an invisible concept that only we know about because the number of pages created depends entirely on viewport width, number of slides & slidesPerPage etc. so there isn't really anything for them to "map" over. They don't have titles for example, slides might but slides !== pages and indicators are for pages.

Tbh, I'm actually starting to wonder if we should just do away with 2 and have 1 and 3? I like to try to avoid children as function where possible but maybe it is the best approach here. It would cover case 2 and keep things open for whatever crazy stuff people get up to.

Or, maybe we should just do 3 only 😬 It's a consistent API for all cases then 😂

chaance commented 3 years ago

The consumer doesn't create the pages, we do internally, they're like an invisible concept that only we know about because the number of pages created depends entirely on viewport width, number of slides & slidesPerPage etc. so there isn't really anything for them to "map" over.

This was my assumption, my thought is that by exposing this as data, that's what I'm imagining they could map over to render the indicators. That way they never have to think about how many indicators they need but they also have full control of composition for each indicator that is rendered.

It's very possible that I'm completely misreading the entire concept here somehow, haha. Happy to chat quickly if you want to walk me through it, because in my head what I'm thinking still kind of makes the most sense to me. 😅

jjenzz commented 3 years ago

Okay so, @chaance and I just had a quick call. We're on the same page now and have agreed to do the map thing so indicator would look something like this:

function CarouselIndicator({ children }) {
    const context = useCarouselContext();
    return children([...Array(context.pagesCount)]);
}

Even though they're just mapping over some page numbers, we decided to go with this anyway because having the map on the consumer side is more explicit and clearly shows for them that their render is rendering multiple things there.

Also, I'm going to go with this API alone and remove options 1 and 2 as this covers all cases.

Let me know if anyone has any objections 🙂

benoitgrelard commented 3 years ago

Catching up with this now, and it's awesome you got to the conclusion I was about to suggest to! ♥️ My main reason for it was that we've been trying to have all parts exposed for people to hook their own events, etc. Having us clone the indicators would kinda prevent that as users wouldn't be able to for example add an onClick to a specific indicator.

A few other minor points:

onDragScroll - Drag scroll only.

What is drag scroll?

colmtuite commented 3 years ago

Another option would be:

CarouselIndicatorList
  CarouselIndicator

This would be inline with TabList.

It seems odd to me that CarouselIndicatorButton is nested inside CarouselIndicator. I would have expected CarouselIndicator to be the actual

benoitgrelard commented 3 years ago

Another option would be:

CarouselIndicatorList
  CarouselIndicator

This would be inline with TabList.

I like that too.

It seems odd to me that CarouselIndicatorButton is nested inside CarouselIndicator. I would have expected CarouselIndicator to be the actual , no?

Isn't it so that we can do both interactive and non-interactive indicators?

jjenzz commented 3 years ago

Agree with all the naming changes. Thanks @benoitgrelard :+1:

It seems odd to me that CarouselIndicatorButton is nested inside CarouselIndicator. I would have expected CarouselIndicator to be the actual <button>, no?

@colmtuite I had originally thought that too but as Benoit has pointed out we need both non-interactive and interactive. So, CarouselIndicator would be an li and if you want an interactive one you would put a CarouselIndicatorButton inside it that would be an a under the hood and bind all the click logic for you. You would end up with something like this DOM for each:

// non-interactive
<ol>
  <li aria-current="step"><span style="visually hidden styles">1</span></li>
  <li><span style="visually hidden styles">2</span></li>
</ol>

// interactive
<ol>
     <li aria-current="step">
        <a href="#slide-id1">
            <span style="visually hidden styles">1</span>
        </a>
    </li>
    <li>
        <a href="#slide-id2">
            <span style="visually hidden styles">2</span>
        </a>
    </li>
</ol>

We would add the visually hidden number for them if they don't provide their own children for accessibility purposes. Also:

Using an anchor tag by default as I plan to get this working without JS as one of our first progressively enhanced SSR components [...].

jjenzz commented 3 years ago

What is drag scroll?

Some sliders when you drag slightly they slide the whole pane for you (scroll snap). Others, it behaves the same as dragging a scrollbar (the Google cast example does this, it just scrolls). I was differentiating these two behaviours.

For now we're thinking, just let it scroll and if people want the snap behaviour they can use CSS or control it with whichever animation libs they choose.

colmtuite commented 3 years ago

Here's a carousel where the progress bar is not attached to the indicator buttons. https://www.jpl.nasa.gov/

On Twitch front page, they have a carousel where the slides themselves act as indicators, sliding the carousel. https://www.twitch.tv/

schonert commented 3 years ago

Any news on when this will ship? 🤓 Currently using swiperjs and my hands feel dirty

colmtuite commented 3 years ago

@schonert haha. Right now we're working on Select, Toast, Menubar, and HoverMenu. We'll move back to Carousel after that, though it will likely become ScrollableList (from the Stitches website)

kharann commented 2 years ago

Hi! I saw that Select and Toast is in beta and congrats! Im curious if the Carousel next in line?

kharann commented 2 years ago

Hi again! Any update on this issue :), perhaps anything we could do to help?

its-monotype commented 2 years ago

Any updates? What can we do to help? I sincerely look forward to this primitive, so as not to connect third-party libraries, and this was already in our favorite RadixUI 🙏

noahsark769 commented 2 years ago

Hey folks, just thought I would add some info here in case it's useful for yall - we're massive fans of Radix UI and we'd love to have it power the carousels for https://replo.app - currently we're using https://splidejs.com/, but wanted to note some shortcomings we've had to work around in case they're useful input:

Hope this helps! Would love to help out any way we can here!

fnky commented 2 years ago

Would be nice to have the possibility to use ScrollTimeline with Web Animation API. Here's a pagination that I implemented that uses this API to provide smooth animation between the indicators' width and background-color based on the scroll offset of the scroll area.

https://user-images.githubusercontent.com/995050/201342289-38b52583-3fb4-464c-871e-1f2ae1e26c9f.mp4

tlouth19 commented 1 year ago

Really excited for this 🚀

ronaldruzicka commented 1 year ago

Hi, are there any updates on this primitive? Is it in development? Because it disappeared from the Radix website as coming soon.

benoitgrelard commented 1 year ago

No, it isn't in development at the moment.

FleetAdmiralJakob commented 1 year ago

Any updates here?

benoitgrelard commented 1 year ago

No, same as above.

hananint commented 1 year ago

Looking forward to this. Any updates, please?

Vintotan commented 1 year ago

I don't understand why this carousel component was not integrated into the radix-ui primitive, as it is already used on the radix-ui.com website 🤔

There is already an existing component: https://github.com/radix-ui/website/blob/main/components/marketing/Carousel.tsx

FleetAdmiralJakob commented 1 year ago

@Vintotan That's weird

jjenzz commented 1 year ago

@Vintotan tmk, the version on the website is an incomplete version of the accessible carousel we were building that supports the website's specific use-case. carousels come with some complex a11y requirements that we never got around to finishing (especially if it is to be customisable) hence why it was not added to radix yet.

CodeNinjaMatrix commented 1 year ago

@Vintotan tmk, the version on the website is an incomplete version of the accessible carousel we were building that supports the website's specific use-case. carousels come with some complex a11y requirements that we never got around to finishing (especially if it is to be customisable) hence why it was not added to radix yet.

Can we have a simple version that not so customizable first, then optimize later? Is that anything we can help?

0x6080 commented 1 year ago

@jjenzz Do you have any update for this? We would really love to see it implemented!

kungfu321 commented 1 year ago

Any update on this issue :)

jasonaravanis commented 1 year ago

Also interested in this being added to Radix!

peiris commented 1 year ago

Pretty please, give us a carousel primitive 🥺🙏🏼

0x6080 commented 1 year ago

@jjenzz Is this something that will be developed, or should we assume any progress has been paused for the rest of the year? Our team needs to know if we need to begin searching outside of the Radix UI primitives for this, or if we can hold off until this is out.

We'd love to hear an update form your end 🙏

callumflack commented 1 year ago

I don't think carousels are needed in Radix. Carousels are not a primitive. So I can understand why they don't want to do it. (Also, Radix has no responsibility to your team needing to know anything).

Carousels should be used with care, designers always seem to do the old "can we just…" dance with them, and they get complex quickly. KISS approach with a working 1st version is best.

Instead, use this simple typescript hook with CSS scroll-snap and get it to done.

FleetAdmiralJakob commented 1 year ago

And in the new Radix UI components that are prestyled?

0x6080 commented 1 year ago

I don't think carousels are needed in Radix. Carousels are not a primitive. So I can understand why they don't want to do it.

What is and is not a primitive is subjective. Plenty of the "primitives" in Radix are not direct HTML element wrappers like