Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(getKeyDownHandler): Pass props to client's "onKeyDown" handler#595

Merged
sophieH29 merged 5 commits intomasterfrom
fix/getKeyDownHandlers-pass-props
Dec 11, 2018
Merged

fix(getKeyDownHandler): Pass props to client's "onKeyDown" handler#595
sophieH29 merged 5 commits intomasterfrom
fix/getKeyDownHandlers-pass-props

Conversation

@sophieH29
Copy link
Contributor

No description provided.

@@ -42,7 +42,7 @@ const getKeyDownHandlers = (
eventHandler && eventHandler(event)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to add the props to the eventHandler as well? We can define then the actions in the following manner:

  protected actionHandlers: AccessibilityActionHandlers = {
    performClick: (event, props) => this.handleClick(event, props),
  }

Not sure, if the previous definition with the event is necessary:

  protected actionHandlers: AccessibilityActionHandlers = {
    performClick: event => this.handleClick(event),
  }

because the event is send in the getKeyDownHandler, I don't see why we cannot send the props too. Let me know if this works and make sense (it makes sense in my opinion, as we would have unified way of defining the handlers).

Copy link
Contributor

Choose a reason for hiding this comment

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

Disregard my comment, I was mixing it with other problems. The changes for this PR are sufficient 👍

Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Please take a look into my comment. It would be great if we can add this too.

@sophieH29 sophieH29 merged commit d31efd5 into master Dec 11, 2018
@sophieH29 sophieH29 deleted the fix/getKeyDownHandlers-pass-props branch December 11, 2018 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🧰 fix Introduces fix for broken behavior. 🚀 ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants