-
Notifications
You must be signed in to change notification settings - Fork 108
Improved behavior of the run shortcut key (ctrl+return (ctrl+enter)) on Apple devices #187
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
Conversation
app/try_ruby.rb
Outdated
| #If hold down the control and the Enter key goes down, run | ||
| $document.on :keydown, '#editor' do |e| | ||
| if e.key == "Enter" && e.ctrl? | ||
| if e.key == "Enter" && ((e.ctrl? && !@navigator.user_agent&.match?(/\b(iPad|iPhone|iPod)\b/)) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kinda hard to read... maybe we could add helper functions like:
def ios?
@navigator.user_agent&.match?(/\b(iPad|iPhone|iPod)\b/)
end
def macos?
@navigator.user_agent&.match?(/\bMac\b/)
endThen the modified code could be just:
$document.on :keydown, '#editor' do |e|
if e.key == "Enter" && ((!ios? && e.ctrl?) || ((ios? || macos?) && e.meta?))
e.prevent
do_run
end
endThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the Helper class to make my code more reusable
app/try_ruby.rb
Outdated
| #If hold down the control and the Enter key goes down, run | ||
| $document.on :keydown, '#editor' do |e| | ||
| if e.key == "Enter" && e.ctrl? | ||
| if (e.key == "Enter" && (e.ctrl? && !Helper.ios?)) || ((Helper.macos? || Helper.ios?) && e.meta?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right-hand side of the || expression is missing check for Enter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing this out. I've fixed the issue, please check it out.
|
Looks good to me :D Let's merge |
At the time I posted #183, I didn't have a Mac or other Apple device, so I couldn't validate the issue with Apple devices. I have now purchased a Mac and verified the issue that occurs on Apple devices, and this PR improves the run shortcut to work well on Apple devices. The detailed improvements are as follows: