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

Many "Skipping this default shortcut because it collides with another default shortcut." warnings #12103

Closed
krassowski opened this issue Feb 23, 2022 · 4 comments · Fixed by #12122
Assignees
Labels
Milestone

Comments

@krassowski
Copy link
Member

Description

Opening JupyterLab results in dozens of warnings like:

Screenshot from 2022-02-23 18-32-57

The shortcuts it complains about are:

It could be either:

  • genuine mistake in the PRs mentioned above
  • a regression in logic as a collateral of changes added in Add settings UI #11079.

I am not sure which it is. This warning and logic was added in #9858 (CC @jasongrout).

Note: we are currently working around this check in #12091.

Reproduce

  1. Open 3.3.0rc0
  2. Open developer console
  3. See errors

Expected behavior

No warnings; all shortcuts get activated.

Context

  • Browser and version: Chrome 98
  • JupyterLab version: 3.3.0rc0 and master
@krassowski krassowski added bug status:Needs Triage Applied to new issues that need triage labels Feb 23, 2022
@krassowski krassowski added this to the 3.3.x milestone Feb 23, 2022
@fcollonval fcollonval removed the status:Needs Triage Applied to new issues that need triage label Feb 24, 2022
@fcollonval
Copy link
Member

For the settings editor shortcut, it seems appropriate to set the one on master:

   {
      "command": "settingeditor:open",
      "args": {},
      "keys": ["Accel ,"],
      "selector": "body"
    },
    {
      "command": "settingeditor:open-json",
      "args": {},
      "keys": ["Shift Accel ,"],
      "selector": "body"
    },

@fcollonval
Copy link
Member

For the run cell commands, actually when defining multiple shortcuts they need to be defined in the same object (keys is a list of shortcuts).

Pushing a PR now.

@fcollonval fcollonval self-assigned this Feb 25, 2022
@fcollonval
Copy link
Member

Ok in fact the settingeditor:open-json is in conflict with

    {
      "command": "application:activate-previous-tab-bar",
      "keys": ["Ctrl Shift ,"],
      "selector": "body"
    },

from application-extension.

@fcollonval
Copy link
Member

So I propose to drop it.

fcollonval added a commit to fcollonval/jupyterlab that referenced this issue Feb 25, 2022
fcollonval added a commit to fcollonval/jupyterlab that referenced this issue Mar 4, 2022
fcollonval added a commit that referenced this issue Mar 9, 2022
* Remove duplicated shortcuts
Fixes #12103

* Allow both Cmd + Enter and Ctrl + Enter on MacOS

* Set shortcut on OS bases for `notebook:run-cell`

* Fix for non-macOS

* Restore two entries for run cell command

* Remove shortcuts with `Cmd` on non Mac platform

* Add unit tests

* Update packages/settingregistry/src/tokens.ts

Co-authored-by: Jason Grout <jasongrout@users.noreply.github.com>

* Fix integrity and prettify

* Explicitly use mac-specific key bindings, rather than relying on new implicit behavior

Co-authored-by: Jason Grout <jasongrout@users.noreply.github.com>
Co-authored-by: Jason Grout <jason@jasongrout.org>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants