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

Fix command key combos #9

Merged
merged 1 commit into from
Jun 21, 2016
Merged

Conversation

joefiorini
Copy link
Contributor

@joefiorini joefiorini commented Jun 15, 2016

Fixes #5

@dbrans
Copy link
Owner

dbrans commented Jun 15, 2016

@aokolish Is there actually an issue? I ask because #4 is closed.

@aokolish
Copy link
Contributor

honestly, I'm not sure at this point

@dbrans
Copy link
Owner

dbrans commented Jun 15, 2016

Ok. Closing for now. Feel free to reopen if you find this is still an issue.

@dbrans dbrans closed this Jun 15, 2016
@joefiorini
Copy link
Contributor Author

@dbrans @aokolish I reproduced it earlier today. Not very hard, just run the demo server and hit it in Safari, then try one of the key combinations mentioned in the linked issue.

@joefiorini
Copy link
Contributor Author

@dbrans I don't have a reopen button because I'm not an admin. Can you please reopen this or give me permissions to?

@joefiorini
Copy link
Contributor Author

Also, this fixes #5 not #4

@dbrans dbrans reopened this Jun 15, 2016
@dbrans
Copy link
Owner

dbrans commented Jun 15, 2016

Hey @joefiorini I'm not sure how this PR which checks for TAB and ENTER addresses #5 which talks about Command + T.

@joefiorini joefiorini force-pushed the fix-cmd-key-combos branch 2 times, most recently from f9ea379 to eaa2aa0 Compare June 21, 2016 16:01
@joefiorini
Copy link
Contributor Author

@dbrans You were correct sir, that was the wrong commit. I believe I have the right one now. Look better?

@dbrans
Copy link
Owner

dbrans commented Jun 21, 2016

Hey @joefiorini this breaks pasting, CTRL-V, doesn't it?

@joefiorini
Copy link
Contributor Author

@dbrans the problem is with the preventDefault that's happening. I moved the check into code that already sets preventDefault to false, making sure the handler always runs in its entirety. Is that better?

@dbrans dbrans merged commit d7a7cbb into dbrans:master Jun 21, 2016
@dbrans
Copy link
Owner

dbrans commented Jun 21, 2016

Thank you!

@dbrans dbrans deleted the fix-cmd-key-combos branch June 21, 2016 17:25
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

3 participants