damianavila / RISE

RISE: "Live" Reveal.js Jupyter/IPython Slideshow Extension
Other
3.67k stars 416 forks source link

reveal.js/chalkboard API calls registered directly to jupyter shortcuts #526

Closed theck3r closed 4 years ago

theck3r commented 4 years ago

After struggling to get all the special characters working with the DOM virtual keyboard constants approach for the custom keys (as I used for PR #525 ) and then having to realize that this isn't supported in google-chrome at all - I finally had a look at registering the key bindings directly in jupyter and it was actually pretty easy.

So here is a new PR based on having all the stuff set up in jupyter directly, which makes the whole thing much more robust and intuitive I would say. It also solves certain issues of the previous approach - like key combinations can now be used and you should be able to use every key in RISE you can type into the nbextension_configurator.

I had to add one hard-wired shortcut though. Jupyter does not allow to bind anything to "," (comma). And because the API call to the function behind this key (toggleAllRiseButtons) is also not as easily accessible as the rest, I made this one hard-wired again. I think it is okay in this case, because the key cannot be configured in jupyter and hence there will be no interference.

I also experienced some strange behavior, trying to set custom keys in nbextension_configurator. I can modify one short-cut, but the second, third,.. shortcut I try to change actually modifies the first one again and the one actually selected. I guess this not a RISE issue, but something in nbextension_configurator. I will have a closer look on it and eventually raise an issue on that topic.

theck3r commented 4 years ago

I can confirm, that the issue with modifying the hotkeys in nbextensions_configurator is not a RISE issue. I send a PR with a fix to the nbextensions_configurator team.

parmentelat commented 4 years ago

for the record, this is now published in pypi.org as release 5.6.1-dev0 and it can be installed on an experimental basis with

pip install -U --pre rise
damianavila commented 4 years ago

OK, this is really what I wanted to see after reading #525. Awesome work @theck3r! Thanks again for keep pushing on this one.