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

Remove focus on esc in console #6822

Merged
merged 1 commit into from Jul 29, 2019
Merged

Conversation

ciyer
Copy link

@ciyer ciyer commented Jul 13, 2019

References

Code changes

Remove focus from the console input on 'Esc' (27).

User-facing changes

The 'Esc' in the input cell of the console removes focus and switches to command mode.

Backwards-incompatible changes

None.

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@ciyer
Copy link
Author

ciyer commented Jul 13, 2019

I would like a little help with this PR. I have been able to remove focus from the input (though this requires a small hack), but the keyboard shortcuts (e.g., 0,0) are not correctly handled. Does anyone know what needs to be done for this?

@ian-r-rose
Copy link
Member

@ciyer You are right, this is a stubborn one! I have had a bit of a hard time as well wresting control from CodeMirror here. I'll play around a bit more this weekend to see if I can figure out what is going on...

@ciyer
Copy link
Author

ciyer commented Jul 20, 2019

@ian-r-rose Appreciate it, thanks!

@ian-r-rose
Copy link
Member

ian-r-rose commented Jul 20, 2019

Hi @ciyer, okay, I think I have figured it out -- in addition to calling event.preventDefault(), you also will need to call event.stopPropagation(). Otherwise, something else in the DOM structure can catch the event and do something with it (in this case, it seems that something is re-focusing the content, I'm not sure what). Once the event is stopped, it should be enough to call this.node.focus(), and you shouldn't need to mess around with codemirror internals or any setTimeouts.

@ciyer
Copy link
Author

ciyer commented Jul 20, 2019

Nice work! That makes sense. I’m on vacation and don’t have a computer with me ATM, but I can fix the code next week, when I’m back. Or you can submit your fix, and I will close my PR - whichever you prefer.

@ian-r-rose
Copy link
Member

Happy to wait for you to finish it off, @ciyer

@ciyer ciyer force-pushed the 3484-console-esc branch 2 times, most recently from 1bb0fd9 to bc6d7bc Compare July 29, 2019 14:07
@ciyer ciyer changed the title wip: work on #3484 -- removing focus on esc. Remove focus on esc in console Jul 29, 2019
@ciyer
Copy link
Author

ciyer commented Jul 29, 2019

@ian-r-rose Thanks to your help, I've been able to make fix for this issue!

@ian-r-rose
Copy link
Member

Looks good, thanks @ciyer!

@ian-r-rose ian-r-rose added this to the 1.1 milestone Jul 29, 2019
@ian-r-rose ian-r-rose merged commit 75166c6 into jupyterlab:master Jul 29, 2019
@ciyer ciyer deleted the 3484-console-esc branch July 30, 2019 08:30
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement pkg:console status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants