Skip to content

added key handler for control-s to notebook, seems to work pretty well #1334

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

Merged
merged 3 commits into from
Jan 29, 2012
Merged

added key handler for control-s to notebook, seems to work pretty well #1334

merged 3 commits into from
Jan 29, 2012

Conversation

fawce
Copy link
Contributor

@fawce fawce commented Jan 27, 2012

Hi,

There was a thread on the dev list about key bindings and hooking ctrl-save to trigger a new save (http://mail.scipy.org/pipermail/ipython-dev/2012-January/008674.html).

What makes this hard is that the browser will only trigger keyboard events for the element that currently has focus. To see what this means, open a google docs spreadsheet. Hit ctrl-s, and you'll see a quick status in the header that your doc is being saved. Now, click on a menu and leave the menu dropped down - ctrl-s now will ask you to (completely pointlessly) save the html of the page. In google's spreadsheet this oddity is almost never an issue, because you almost never notice this discrepancy as you're almost always keyboard focused on the sheet.

In the updated UI, notebook's layout has the same convenient property as google docs, where focus is almost always in the notebook element or a child. The only way I was able to end up without focus in the notebook was to click the menu. As long as a menu is dropped down, the ctrl-s triggers the browser's dialog.

thanks,
fawce

p.s.
This PR is from a branch based on a pull from ellisonbg/master.


// Save (CTRL+S) or (AppleKey+S)
//metaKey = applekey on mac
if ((event.ctrlKey || event.metaKey) && event.keyCode==83) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, this will catch either Ctrl-S or Meta-S (which is Alt-S?) on any platform, which isn't ideal. I don't do much Javascript - is there a neat way to only catch the shortcut relevant to the platform?

@takluyver
Copy link
Member

Thanks for tackling this, I think it will be good for usability.

@fperez
Copy link
Member

fperez commented Jan 29, 2012

Thanks @fawce!

@takluyver, I tested this on Firefox and Chrome (linux), with an Apple keyboard, and I see no problems. Alt (meta) and the Apple key are not captured by this, only Control-S works. And I assume that @fawce is on OSX, so for him this makes Cmd-S (Apple key) work as well.

It is indeed a big usability win, so I'm merging it now. There's always the chance we can get a bug report from some user on some combination of platform/browser, but the only way to find out is to merge and put it out for public use.

fperez added a commit that referenced this pull request Jan 29, 2012
Make Control-S (or Cmd-S on Apple) save the actual notebook instead of bringing up the useless 'save as html' dialog.  This doesn't disable the C-m-s keybinding, simply adds the far more familiar and common C-s.
@fperez fperez merged commit 93bb469 into ipython:master Jan 29, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Make Control-S (or Cmd-S on Apple) save the actual notebook instead of bringing up the useless 'save as html' dialog.  This doesn't disable the C-m-s keybinding, simply adds the far more familiar and common C-s.
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.

3 participants