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

Comma separated shortcuts doesn't works #3

Open
nrako opened this issue Aug 26, 2012 · 8 comments
Open

Comma separated shortcuts doesn't works #3

nrako opened this issue Aug 26, 2012 · 8 comments

Comments

@nrako
Copy link

nrako commented Aug 26, 2012

Assigning multiple shortcuts on a single line separated by comma as in keymaster.js doesn't works.

shortcuts: {
  'right, space': 'someMethod'
}

I understand you support the scope feature of keymaster.js https://github.com/madrobby/keymaster/#scopes which make it ugly to supports multiple shortcuts and a scope.

shortcuts: {
  // fulgy but could be nice to supports that for backwards compatibility
  'right, space myscope': 'notReallyNeat'
}

Maybe the above could be supported for backwards compatibility but a more clean declaration could be promoted :

shortcuts: {
  'right, space': {scope: 'myScope', method: 'someMethod'}
}

Let me know your thoughts and if you haven't time to implement it, I'll see if I have some for a fork-pull-request.

@aeosynth
Copy link

Looks like @bry4n doesn't have time.

@aeosynth
Copy link

I think it would be less funky to append the scope to the method:

shortcuts: {
  'right, space': 'someMethod myScope'
}

@nrako
Copy link
Author

nrako commented Sep 22, 2012

@aeosynth yup it's obviously a neater solution.

I almost forgot about this issue, if @bry4n brings his insights I'll still be happy to make a pull-request.

@aeosynth
Copy link

perhaps making a pull request would help @bry4n bring his insights

@bry4n
Copy link
Owner

bry4n commented Sep 24, 2012

Sorry! I have been busy. pull requests are welcome 👍 -- I can work on the fix this weekend.

how about this:

shortcuts: {
  "o, enter <myscope>": "someMethod"
}

That solution should be easier for me to update the regex pattern.
https://github.com/bry4n/backbone-shortcuts/blob/master/src/backbone.shortcuts.coffee#L33

@aeosynth
Copy link

i would find that very hard to parse - you have to check for a comma in the middle of a string

IMO my way is easier to read, since the value currently doesn't take a list

@nrako
Copy link
Author

nrako commented Sep 25, 2012

The question is to decides if having the ability to have a different scope per event is desired. Or if it would rather introduce unnecessary redundancies. Or maybe both could be supported.

IMO and in my use cases it seems that it would introduce redundancies to have the scope defined with the event.

@christiangenco
Copy link

+1

Is there a working accepted way to do this?

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

No branches or pull requests

4 participants