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

Edit RISE keyboard shortcuts through default editor #367

Closed
neighthan opened this issue Apr 14, 2018 · 11 comments
Closed

Edit RISE keyboard shortcuts through default editor #367

neighthan opened this issue Apr 14, 2018 · 11 comments

Comments

@neighthan
Copy link

neighthan commented Apr 14, 2018

I just installed RISE using conda install -c damianavila82 rise. It's mostly but not all working. Two issues in particular:

  1. Keyboard shortcuts
    • There don't seem to be any of the default ones (e.g. alt-r). I can access these commands using the command palette, though, and they seem to work fine.
    • I can also set my own keyboard shortcuts for them, which will work until I reload the notebook. On reloading (that is, when the shortcuts have to be loaded from ~/.jupyter/nbconfig/notebook.json then I get error messages like
Uncaught (in promise) Error: does not know how to deal with : RISE:toggle-fragment
    at ShortcutManager.add_shortcut (keyboard.js:457)
    at ShortcutManager.add_shortcuts (keyboard.js:475)
    at keyboardmanager.js:80
  1. Toolbar icon - from this YouTube video, it seems there should be an icon on the toolbar. I don't really care about this, as I much prefer keyboard shortcuts, but I don't have that either. This error may or may not be related (I checked that I don't get any errors on loading this notebook before installing RISE, but with RISE this error appears):
TypeError: Cannot read property 'help' of undefined
    at toolbar.js:101
    at MainToolBar.ToolBar.add_buttons_group (toolbar.js:115)
    at Function.setup [as load_ipython_extension] (main.js?v=20180414142940:953)
    at utils.js:39
    at Object.execCb (require.js?v=6da8be361b9ee26c5e721e76c6d4afce:1690)
    at Module.check (require.js?v=6da8be361b9ee26c5e721e76c6d4afce:865)
    at Module.<anonymous> (require.js?v=6da8be361b9ee26c5e721e76c6d4afce:1140)
    at require.js?v=6da8be361b9ee26c5e721e76c6d4afce:131
    at require.js?v=6da8be361b9ee26c5e721e76c6d4afce:1190
    at each (require.js?v=6da8be361b9ee26c5e721e76c6d4afce:56)

RISE still seems fully functional, in the little that I've tested it, except that I have to access everything through the command palette. Any idea what's going on here?

In case it helps:

$ jupyter --version
4.3.0
$ jupyter notebook --version
5.0.0

EDIT - there do seem to be other issues as well. When I set the slide type for a cell as "Skip", it's still being shown (as part of the preceding "Slide" cell; except the very first cell in my notebook, which is a Skip with no preceding Slide and is correctly skipped).

@parmentelat
Copy link
Collaborator

parmentelat commented Apr 14, 2018 via email

@neighthan
Copy link
Author

Ah, sorry, I ran conda upgrade jupyter earlier, but I forgot that doesn't actually upgrade the notebook. After upgrading (conda upgrade notebook) to 5.4.1, I do have the icon in the toolbar now, and the default shortcuts are configured. Additionally, the error with Skip cells is gone (though that actually went away on the old notebook version too after reloading at some point).

The only problem that remains is with changing the keyboard shortcuts. I'm still getting

Uncaught (in promise) Error: does not know how to deal with : RISE:toggle-slide
    at ShortcutManager.add_shortcut (keyboard.js:457)
    at ShortcutManager.add_shortcuts (keyboard.js:475)
    at keyboardmanager.js:80

when I have

  "keys": {
    "command": {
      "bind": {
        "shift-s": "RISE:toggle-slide",
        "shift-d": "RISE:toggle-subslide",
        "shift-f": "RISE:toggle-fragment",
        "shift-n": "RISE:toggle-note",
        "shift-c": "RISE:toggle-skip",
        "shift-r": "RISE:slideshow"
      }
    }
  }

in my ~/.jupyter/nbconfig/notebook.json.

@neighthan
Copy link
Author

I saw on this page that they modify the bindings for the RISE actions in custom.js - is it required to do it this way instead of through the standard "bind" section of notebook.json?

@damianavila
Copy link
Owner

damianavila commented Apr 16, 2018

It should be possible to do it with the notebook JSON, I think... check these docs: http://rise.readthedocs.io/en/stable/customize.html#through-json (those will be the official docs soon, it is a recent merged PR). Maybe @parmentelat have some more comments about this?

@parmentelat
Copy link
Collaborator

parmentelat commented Apr 16, 2018 via email

@neighthan
Copy link
Author

Usually (?) one doesn't edit the keyboard bindings in notebook.json directly, but that's where they're saved when you change them using the GUI editor in the notebook itself. I'll switch to using the documented custom.js method instead then and maybe try the other approach again when RISE 5.3 comes out. I do think this would be nice to get working at some point, though, because using the GUI keybindings editor seems like the most natural approach; if this same error happens to others who set their bindings that way (which may not be the case, if my setup has some other issue), it would be easier if they didn't have to find this issue to see that only custom.js works. Thanks!

@parmentelat
Copy link
Collaborator

It sure would be nice; however, now that I have a better understanding of what you're doing, I just tested under 5.3 and it does not work either. So for the record:

  • open notebook classic, select Help -> Edit keyboard shortcuts

  • locate the line with toggle-note, define shortcut (need to spell out shift-n and then press the +)

  • visually check it was added: less +/'"keys"' ~/.jupyter/nbconfig/notebook.json

  • close slideshow, open again

  • open console, check for

Uncaught (in promise) Error: does not know how to deal with : RISE:toggle-note
    at ShortcutManager.add_shortcut (keyboard.js:457)
    at ShortcutManager.add_shortcuts (keyboard.js:475)
    at keyboardmanager.js:80
  • in my case when the bug is present, the new shortcut does not appear at this point in the Edit keyboard shortcuts UI, probably b/c it failed to load.

@neighthan
Copy link
Author

Ah, sorry for not making it clearer from the start how I was setting these. I focused on notebook.json to emphasize that the issue wasn't setting the shortcuts from the GUI editor (which works, initially) but reloading them. Thanks for posting a better reproduction and verifying that this persists in 5.3. As the functionality is still achievable by other means, this isn't really a blocker, but good to be aware of at least; if there isn't an easy fix, perhaps you could add to the 5.3 docs that one shouldn't set the shortcuts this way, because currently it says

The actions exposed to Jupyter are also present in Jupyter's mainstream keyboard shortcuts editor, that you can use to (un)define your custom shortcuts.

but that approach won't be fruitful.

@neighthan neighthan changed the title Faulty RISE installation Edit RISE keyboard shortcuts through default editor Apr 16, 2018
@parmentelat
Copy link
Collaborator

thanks for the feedback; I'll try to get that to work, and if I can't, as you suggest, to reword the doc to make the limitation clearer

@parmentelat
Copy link
Collaborator

Just wanted to reference this issue that I opened in the notebook repo jupyter/notebook#3549

It looks like @takluyver (thanks again) has come up with a workaround that could help us work around this issue

I will try to find the time to give all this a try shortly

@parmentelat
Copy link
Collaborator

As reported on the notebook issues board, the issue seems to have been fixed in that repo, and the change will be part of notebook-5.5, which is expected to ship shortly.

see also #371 specifically about the RISE:toggle-notes action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants