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.

Sign up for GitHub

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

Jump to bottom

fix(FEC-9281): ignore shortcut handling if focused on editable node #400

Merged
merged 6 commits into from
Nov 28, 2019
Merged

fix(FEC-9281): ignore shortcut handling if focused on editable node #400

merged 6 commits into from
Nov 28, 2019

Conversation

esakal
Copy link

@esakal esakal commented Aug 6, 2019 •
edited by OrenMe

Description of the Changes

ignore shortcut handling if focused on editable node

Solves FEC-9281

@esakal esakal requested a review from OrenMe August 6, 2019 10:22
@esakal esakal self-assigned this Aug 6, 2019
@OrenMe OrenMe changed the title FEC-9281 ignore shortcut handling if focused on editable node fix(FEC-9281): ignore shortcut handling if focused on editable node Aug 15, 2019
OrenMe
OrenMe requested changes Aug 19, 2019
View reviewed changes
Copy link
Contributor

@OrenMe OrenMe left a comment

Choose a reason for hiding this comment

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

please check FLOW failing

@OrenMe
Copy link
Contributor

OrenMe commented Aug 25, 2019

I would like to refactor the keyboard shortcuts implementation and possibly make this as a HoC provider style that will allow each component to register it's own shortcut in the component did mount.
In this case I'm still debating if the responsibility to prevent propagation will be of the component or the provider.
Can this be done locally in the meanwhile until we refactor the support?

@esakal esakal marked this pull request as ready for review September 18, 2019 13:16
@esakal
Copy link
Author

esakal commented Sep 18, 2019

@OrenMe I fixed the flow typecheck issue. It appears that flow declaration is partial for PReact keyboard event so checking for the type solve it. Please review and if possible merge to next version.

@esakal
Copy link
Author

esakal commented Oct 14, 2019

@OrenMe what about this PR? when are you planning to merge it? Note that this is essential support for us and the suggested code change in this PR is minimal and will not affect any design you would decide upon later

OrenMe
OrenMe requested changes Nov 4, 2019
View reviewed changes
Copy link
Contributor

@OrenMe OrenMe left a comment

Choose a reason for hiding this comment

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

@esakal please fix merge conflict. Thanks.

@esakal
Copy link
Author

esakal commented Nov 4, 2019

@OrenMe done

@OrenMe OrenMe merged commit 23bc7b9 into master Nov 28, 2019
@OrenMe OrenMe deleted the FEC-9281-contrib branch November 28, 2019 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OrenMe OrenMe OrenMe approved these changes

Assignees

@esakal esakal

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@esakal @OrenMe @eransakal