[BUG] Keyboard Controls + Multiple Carousels Advances All Carousels

[willdavidow]

Describe the bug With multiple carousels on the page keyboard controls move all carousels instead of just the current carousel.

To Reproduce Steps to reproduce the behavior:

1. Create a page with two or more carousels.
2. Enable keyboard controls.
3. Click an arrow in any carousel to advance a slide.
4. Use left/right arrows to advance carousel slides
5. Observe all carousels advance slides

Expected behavior Expect that only one carousel's slides advance and the others stay put.

Desktop (please complete the following information):

• OS: MacOS
• Browser All

Reproduction https://codesandbox.io/s/dazzling-raman-ikm6j?file=/src/App.js

[jacob-ebey]

Is this planning on being fixed?

[abhinavdalal]

@willdavidow @jacob-ebey the only way IMO is to use a const [keyBoardControls, setKeyBoardControls] = useState([false, false]); and pass manage this state externally on hover.

https://codesandbox.io/s/ecstatic-silence-thclx?file=/src/App.js

i dont know what is the expected behaviour here actually. i assumed you want it to use keyboard controls on hover only.

honestly, not really clear why this is a bug in this component. IMO you are talking about functionality that is outside of this component.

you may need to write your own logic to say which carousel is current.

[willdavidow]

@abhinavdalal I'm not sure I agree.

I've used other carousels that handle the behavior as I expected in the initial bug report: once a user has focused a carousel by clicking its controls, or tabbing to it in the keyboard-only use case, the carousel should/would infer that instance as the current carousel and retain its focus. Hovering and clicking the carousel in this particular case feels like a paradigm conflict since someone using keyboard controls likely isn't using the mouse to interact with the carousel, so hover can't be assumed to be in play for setting any sort of current state.

Also interesting: in my codesandbox link, you can click anywhere in the rendered example portion of the codesandbox (to grab focus), then use tab key to advance focus to the prev/next buttons within each individual carousel; from there, spacebar will activate that button. Maybe it's a bad assumption, but in that scenario I would expect the carousel with its prev/next button focused would be the current carousel and the left/right arrows would advance only that carousel's slides. This is not the case and all carousels still advance when pressing the left/right arrows.

[hoop71]

@willdavidow @jacob-ebey the only way IMO is to use a const [keyBoardControls, setKeyBoardControls] = useState([false, false]); and pass manage this state externally on hover.

https://codesandbox.io/s/ecstatic-silence-thclx?file=/src/App.js

i dont know what is the expected behaviour here actually. i assumed you want it to use keyboard controls on hover only.

honestly, not really clear why this is a bug in this component. IMO you are talking about functionality that is outside of this component.

you may need to write your own logic to say which carousel is current.

Seems to work for me by doing the following

const [activeKeyBoardControl, setActiveKeyBoardControl] = useState(false);

<Wrapper
      onFocus={() => setActiveKeyBoardControl(true)}
      onMouseEnter={() => setActiveKeyBoardControl(true)}
      onBlur={() => setActiveKeyBoardControl(false)}
      onMouseLeave={() => setActiveKeyBoardControl(false)}
>
  <Carousel
     keyBoardControl={activeKeyBoardControl}
     {...props}
     >
     {children}
   </Carousel>
</Wrapper>

[willdavidow]

@hoop71 This seems like it will work and is an option we explored (before our product owner decided we would just disable keyboard controls), but ultimately I'd really love to see the carousel manage multiple instances on the page properly, out of the box, rather than having to bake in my own "active carousel" management.