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

Adds support for multiple hotkey combinations to callers. #40

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kylearch
Copy link

I wanted the ability to reuse a method with multiple hotkeys. Given that space is its own hotkey, a whitespace character was the best separator for multiple combinations.

Copy link
Collaborator

@zcuric zcuric left a comment

Choose a reason for hiding this comment

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

First of all thanks for the PR. I'm not 100% sold on the functionality, but I don't mind it. Your thoughts on this @Dafrok @vladimyr?

Can you add tests, lint and update README.md with this new functionality?

Object.keys(combinations).forEach(combination => {
const { keyup, keydown } = combinations[combination]
Object.keys(indices).forEach(index => {
const {keyup, keydown} = indices[index]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep it consistent with the codebase style. Bring back the spaces.

@kylearch
Copy link
Author

Thanks for the feedback! I added what was requested.

I'm open to another way to separate multiples, but couldn't think of a less intrusive or more intuitive way to accomplish it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants