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

Can't add keyboard shortcuts to actions of extension #2954

Closed
shingo78 opened this issue Oct 19, 2017 · 3 comments · Fixed by #3561
Closed

Can't add keyboard shortcuts to actions of extension #2954

shingo78 opened this issue Oct 19, 2017 · 3 comments · Fixed by #3561
Assignees
Milestone

Comments

@shingo78
Copy link

We are developing https://github.com/NII-cloud-operation/Jupyter-multi_outputs

I can't add a keyboard shortcut to actions of this extension using Edit keyboard shortcuts.
The added keyboard shortcuts are enabled before reloading the notebook, but disabled with the following error after reloading.

Uncaught (in promise) Error: does not know how to deal with : MultiOutputs:pin_output
    at ShortcutManager.add_shortcut (keyboard.js:457)
    at ShortcutManager.add_shortcuts (keyboard.js:475)
    at keyboardmanager.js:80
    at <anonymous>

Reproduction procedure:

  1. Select the Edit keyboard Shortcuts menu item
  2. Add a shortcut to the action of multi_outputs extension, click OK button
  3. Reload the notebook

This MultiOutputs:pin_output action is registered by the multi_outputs extension.
https://github.com/NII-cloud-operation/Jupyter-multi_outputs/blob/master/lc_multi_outputs/nbextension/main.js#L604-L608

Could you give me some advice?

@takluyver
Copy link
Member

I guess there's a race condition, that it's trying to add the shortcut before the extension has loaded and defined the action. It's a tricky one; it should be possible to bind a shortcut to an action name that hasn't been registered yet, but it's also nice to be able to catch incorrect action names.

Maybe add_shortcut could have an extra parameter to allow it to bind to actions which aren't registered yet. @Carreau @gnestor ?

@Carreau
Copy link
Member

Carreau commented Oct 19, 2017

Maybe add_shortcut could have an extra parameter to allow it to bind to actions which aren't registered yet. @Carreau @gnestor ?

I though it was already the case by default; I'll have to re-check.
Though I think that might be because of Edit shortcuts, I believe Edit is more complicated (as it may interfere with Codemirror); so was not completely implemented.

@Carreau Carreau self-assigned this Oct 19, 2017
@gnestor
Copy link
Contributor

gnestor commented Nov 3, 2017

The issue is that the actions and keyboard_manager objects are created before extensions are loaded. This is for good reason because otherwise the notebook could take a long time to load.

The simple solution is to move all the actions/keyboard manager creation stuff into the .then after extensions are loaded. Unfortunately, almost all of the main objects depend on actions or keyboard manager, including the notebook, so that wouldn't buy us much.

Another approach woulds be to break out the adding of config shortcuts (vs. default shortcuts) to a new method on keyboard manager that could be called after extensions are loaded.

takluyver added a commit to takluyver/notebook that referenced this issue Apr 24, 2018
When users bind custom shortcuts to actions coming from extensions, the
shortcuts can be loaded before the extensions, so we need to allow
defining a shortcut for an action that doesn't exist yet.

Closes jupytergh-3549
Closes jupytergh-2954
@takluyver takluyver added this to the 5.5 milestone Apr 24, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants