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

[RFC] Shortcuts support for iOS devices #749

Merged
merged 1 commit into from May 14, 2018

Conversation

aadsm
Copy link
Contributor

@aadsm aadsm commented Apr 22, 2018

This is a proof of concept for some of the ideas I suggested on #507.
tl;dr: iOS doesn’t trigger key events just by pressing alt, shift, etc.

The solution I’m proposing to fix this comes with pros/cons:

Pros:

  • The ability to have shortcuts on iOS 😎.

Cons:

  • Can only use Alt and Shift as key modifiers.
  • Not able anymore to use the keys generated by one of the shortcuts (e.g.: Alt+Shift+P prints ∏ on my keyboard) [I have another workaround in mind to solve this though].

@CompuIves
Copy link
Member

I love this! Great to give iPad users the ability to use keybindings too. Is this ready to merge in?

@aadsm
Copy link
Contributor Author

aadsm commented Apr 25, 2018 via email

@aadsm
Copy link
Contributor Author

aadsm commented May 14, 2018

@CompuIves, I’ve updated the PR with better code to implement secondary bindings. I’ve also “fixed“ an assumption that existed in the code about the keyCode. It was doing String.fromCharCode(keyCode) but the keyCode is not a charCode 😛 (you gotta love the key event api). Anyway, check the comment I wrote there as I explain a bit better what’s going on.

I haven’t had the time yet to look into the monoco/vscode implementation like we discussed, but this PR is good enough that will give some type of support to iOS users, specially iPad with keyboard. So I’ll support merging it and then solve the rest of the issues later.
For the record, what is missing right now is handling shortcuts that are handled by CodeMirror (e.g.: Alt-K), and also secondary bindings.

@CompuIves
Copy link
Member

I love the clear implementation and the comments. Thank you so much for the work and effort in this PR! I'll merge this in now, everything looks good to me!

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.

None yet

2 participants