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

[Proposal] Carousel Component #409

Open
voronianski opened this issue Jun 11, 2019 · 12 comments

Comments

Projects
None yet
5 participants
@voronianski
Copy link
Member

commented Jun 11, 2019

Recently we've implemented a Carousel component on the marketing website.

It was made with re-usability in mind, so that in the future it can be ported to Circuit UI and other teams can benefit from reusing it as well.

Visual

carousel

You can also play with it on website's storybook page or see it in action as part of Testimonials component on careers page for example.

Context

Currently it's used only inside Testimonials component on the website but it will be also featured in a new fancy sliding Hero components as well.

Here's an example how it can be used on Dashboard:

carousel on dashboard

Stateful vs. Composed versions

There's a stateful version of component which just receives a bunch of props and handles all the animation and state management under the hood:

import { Carousel } from '@sumup/circuit-ui';

const slidesData = [
  {  image: { src, alt } }, 
  {  image: { src, alt } }
  //...
];

<Carousel
  slides={slidesData}
  aspectRatio={1.8}
  slideDuration={5000}
  animationDuration={600}
  cycle
  swipe
  autoPlay
  pauseOnHover
/>

But carousel can also be used to compose completely different slider using the provided components (though you will need to manage state by yourself):

import { Carousel } from '@sumup/circuit-ui';

const { 
  Container, 
  Slides, 
  Slide, 
  SlideImage, 
  Controls, 
  Status,
  PrevButton,
  NextButton
} = Carousel

const slides = [
  {  image: { src, alt } }, 
  {  image: { src, alt } }
  //...
];

const CustomCarousel = () => {
  const total = slides.length;
  const [step, setStep] = useState(0);
  const goBack = () => setStep(step === 0 ? total - 1 : step - 1);
  const goForward = () => setStep(step === total - 1 ? 0 : step + 1);

  return (
    <Container>
      <Slides>
        {slides.map(({ image }, index) => (
          <Slide
            key={index}
            index={index}
            step={step}
          >
            <SlideImage
              src={image.src}
              alt={image.alt}
            />
          </Slide>
        ))}
      </Slides>
      <Controls>
        <PrevButton onClick={goBack} />
        <Status step={step} total={total}  />
        <NextButton onClick={goForward} />
      </Controls>
    </Container>
  );
};

...which will end up in something like -

composed carousel

Progressive enhancement

Carousel component scales up and down according to the provided image aspect ratio:

responsive carousel

@fernandofleury

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

Great idea! I'm just a little bit concerned about the developer providing aspect ratio to the component

@voronianski

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

Great idea! I'm just a little bit concerned about the developer providing aspect ratio to the component

There's a default value so it can be omitted.

@connor-baer

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

@fernandofleury Can you elaborate on your concern, please? Do you have an alternative solution in mind?

AFAIK, you don't even need to use the <SlideImage /> component — you can put anything you like inside the <Slide />.

@voronianski

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

@fernandofleury Can you elaborate on your concern, please? Do you have an alternative solution in mind?

AFAIK, you don't even need to use the <SlideImage /> component — you can put anything you like inside the <Slide />.

true

@cristianoliveira

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

WOW. That's awesome.
giphy

One question:

Where does the step come from in this code below?

....
  <Controls>
    <PrevButton onClick={goBack} />
    <Status step={step} total={total}  />
    <NextButton onClick={goForward} />
  </Controls>
@voronianski

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

One question:

Where does the step come from in this code below?

....
  <Controls>
    <PrevButton onClick={goBack} />
    <Status step={step} total={total}  />
    <NextButton onClick={goForward} />
  </Controls>

sorry, it was not complete example, here it is -

const CustomCarousel = () => {
  const total = slides.length;
  const [step, setStep] = useState(0);
  const goBack = () => setStep(step === 0 ? total - 1 : step - 1);
  const goForward = () => setStep(step === total - 1 ? 0 : step + 1);

  return (
    <Container>
      <Slides>
        {slides.map(({ image }, index) => (
          <Slide
            key={index}
            index={index}
            step={step}
          >
            <SlideImage
              src={image.src}
              alt={image.alt}
            />
          </Slide>
        ))}
      </Slides>
      <Controls>
        <PrevButton onClick={goBack} />
        <Status step={step} total={total}  />
        <NextButton onClick={goForward} />
      </Controls>
    </Container>
  );
};
@fernandofleury

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

@connor-baer I'm missing the point of the aspect ratio. is it to apply the padding styles to the container?

@voronianski

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

@connor-baer I'm missing the point of the aspect ratio. is it to apply the padding styles to the container?

yes, it assures that the image is rendered in specified proportion so we don't need to specify the exact width/height of the image.

@fernandofleury

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

@voronianski That's what I thought. (I mean, I know why we use the aspect ratio to define how the image should be displayed)

I'm just thinking out loud how often we would use it. But since this has a defaultValue we should be fine. As long as we document it properly this shouldn't be an issue.

I guess I'm a bit used to those "anything sliders", you know? Where the content isn't even necessarily images

@fernandofleury

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

I was also gonna propose (sorry if I'm missing it in the proposal) to be able to hide controls as well. We might want to use it as a page Hero for instance

@connor-baer

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

All subcomponents are fully composable, so it's up to you to pick the ones you want to use. If you don't want any controls, then just don't use them :)

That being said, controls are crucial for accessibility and arguably for UX. They allow people who are affected by motion sickness to pause the slider and give everyone a sense of control over the action.

We have a design for a page hero (with controls) for the marketing website: https://www.figma.com/file/JoL3G6Ma8Sq3WdLlcyCth2tJ/Marketing-Site?node-id=19%3A289

@voronianski

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2019

Just a small update on this. Today during the hack day I started the process of porting the Carousel from the website to Circuit UI.

The first step is to get rid of dub-step dependency. This npm module handles all state and swipe functionality in the current stateful version of Carousel component but unfortunately it is not maintained anymore and has few issues as well.

The idea here is to create improved and simplified version of it as StepController component inside Circuit UI and use it instead. Example of how it can look like:

import { StepController } from '@sumup/circuit-ui';

function CustomCarousel() {
  return (
    <StepController
      cycle
      autoPlay
      stepDuration={1500}
      animationDuration={300}
    >
      {({
        step,
        paused,
        getStepProps,
        getNextControlProps,
        getPreviousControlProps,
        getPauseControlProps,
        getPlayControlProps
      }) => (
        {/* render here any carousel, slider, gallery components... */}
      )}
    </StepController>
  );
}

The second step will be to literally move all Carousel components from the website into Circuit UI and adjust them a little bit to match the guidelines.

Hopefully in 1 or 2 more hack days there will be a ready PR for you to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.