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

Issue 623: Keyboard controls will only work when nuka is in focus. #639

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

mariano-formidable
Copy link
Member

@mariano-formidable mariano-formidable commented Jan 15, 2020

Description

This PR updates our keyboard controls so they'll only work if enableKeyboardControls is true and the carousel is in focus.

Note:
Technically, this introduces a breaking change because before the keyboard controls worked even if the carousel wasn't in focus (although, not sure how many people were actually taking advantage of that or appreciated that fact). If we want, we can add in an additional prop that'll let the keyboard controls work regardless of carousel focus.

Fixes #623

Type of Change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Manually using the demo, and fixed existing tests as needed so they pass.

Screenshots

In the animation I click on the top carousel and the navigate with keyboard, and then click on bottom one and animate with the keyboard again:
focus-controls

@@ -60,6 +61,8 @@ export default class ScrollTransition3D extends React.Component {
}`}
style={this.getSlideStyles(index, positionValue)}
key={index}
onClick={handleSelfFocus}
tabIndex={-1}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting to -1 lets us programmatically set the focus without having to set any specific tab index.

@mariano-formidable mariano-formidable changed the title [WIP] Issue 623: Keyboard controls will only work when nuka is in focus. Issue 623: Keyboard controls will only work when nuka is in focus. Jan 15, 2020
@YouKnowEno
Copy link

I have a use case in which I would like to enable keyboard controls even if the carousel is not in focus. Is it possible to modify enableKeyboardControls to enable this use case, if so desired? Thank you!

@jkrehm
Copy link
Contributor

jkrehm commented Jun 3, 2020

Ditto, I'm also trying to figure out a work-around. I wish this had been an opt-in change, i.e. consuming application tracks the focus and toggles keyboard controls on/off. I think it should have been called out as a breaking change with a major bump.

@TomGeshury
Copy link

TomGeshury commented Jun 17, 2020

Hello to get the old functionality, a workaround I found was exposing the nuka carousel ref in the component its being called in and in ComponentDidMount using the function that focuses the ref.

So it looks like this this:

componentDidMount() {
// the stateful logic in NukaCarousel that handles focusing the carousel
this.carouselRef.handleFocus()
}
<Carousel
          ref={(carouselRef) => {
                this.carouselRef = carouselRef;
              }}
/>

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.

Carousel keyboard controls trigger on all carousels
5 participants