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

Keybindings: Add Keyboard Shortcuts to nteract web #4101

Closed
4 of 6 tasks
stormpython opened this issue Jan 23, 2019 · 10 comments
Closed
4 of 6 tasks

Keybindings: Add Keyboard Shortcuts to nteract web #4101

stormpython opened this issue Jan 23, 2019 · 10 comments
Assignees
Labels
keybindings new-contributor-friendly outdated workflow: issue should stay closed stale workflow: stale issue

Comments

@stormpython
Copy link
Member

stormpython commented Jan 23, 2019

Is your feature request related to a problem? Please describe.
In order to bring nteract up to par with Jupyter Classic, we need to add keyboard shortcuts. I am closing issue #1870 in favor of this one.

Describe the solution you'd like
This will be a parent issue that has several child issues. This will require setting up a process for adding keyboard shortcuts, which can be hard-coded initially with the end goal being to have a keyboard shortcut config file that is eventually configurable in the UI. Once a system is in place, each class of keyboard shortcuts can be broken out and tackled in their own separate issues.

To Do List:

Describe alternatives you've considered
Tried using blueprintjs Hotkey component. I ran into an issue with the HotkeysTarget decorator component. The first error I was getting was not being able to use the decorator because it was expecting to call the wrapped component with new.

`Uncaught TypeError Class constructor App cannot be invoked without 'new'`

This still seems to be an issue, but I've been looking at this issue to help solve it. (Workaround did not work)

I also had an issue getting the event listeners to fire for keydown events. Kyle removed hot module loading to see if it had anything to do with the build, but I am still experiencing issues with getting the event listeners to fire. Turns out, there is an open issue for Hotkeys not working on anything besides a DOM element.

@rgbkrk
Copy link
Member

rgbkrk commented Jan 23, 2019

Turns out, there is an open issue for Hotkeys not working on anything besides a DOM element.

Oh that makes sense. We'd definitely need a wrapper div. What did you think about the other react-hotkeys?

@rgbkrk
Copy link
Member

rgbkrk commented Jan 23, 2019

I just noticed that react-hotkeys (only one dash) uses mousetrap underneath, which also allows key sequences. That was the one @theengineear originally was recommending we start with back in the day.

@stormpython
Copy link
Member Author

Ok, let me take another look.

@stormpython
Copy link
Member Author

@rgbkrk you should take a look at this issue. He mentions wanting to refactor react-hotkeys to remove mousetrap because it is no longer being maintained. Do you know if that is true?

@rgbkrk
Copy link
Member

rgbkrk commented Jan 24, 2019

Omg wow what a good find. Reading now.

@rgbkrk
Copy link
Member

rgbkrk commented Jan 24, 2019

Couldn't help myself and made a PR to the project. It looks like the maintainer felt burnt out then came back to the project. greena13/react-hotkeys#139

If you like the project, we could always offer to host it in the nteract org and maintain it if you like the way it's written. Mousetrap does make new releases it looks like. Perhaps there are outstanding issues folks want fixed?

@stormpython
Copy link
Member Author

@rgbkrk Yes, on further review, it seems to be the case. Let me dig around more in the code and get back to you.

@stormpython
Copy link
Member Author

Ok, so the path forward now is to use the React HotKeys which uses mousetrap.js under the hood. This library was chosen because it wraps mousetrapjs and it allows for key combinations, sequences, etc. The only slight drawback may be that the library may not be maintained. See this issue.

@stale
Copy link

stale bot commented Dec 1, 2019

This issue hasn't had any activity on it in the last 90 days. Unfortunately we don't get around to dealing with every issue that is opened. Instead of leaving issues open we're seeking to be transparent by closing issues that aren't being prioritized. If no other activity happens on this issue in one week, it will be closed.
It's more than likely that just by me posting about this, the maintainers will take a closer look at these long forgotten issues to help evaluate what to do next.
If you would like to see this issue get prioritized over others, there are multiple avenues 🗓:

  • Ask how you can help with this issue 👩🏿‍💻👨🏻‍💻
  • Help solve other issues the team is currently working on 👨🏾‍💻👩🏼‍💻
  • Donate to nteract so we can support developers to work on these features and bugs more regularly 💰🕐

Thank you!

@captainsafia
Copy link
Member

Closing in favor of #4781.

@lock lock bot added the outdated workflow: issue should stay closed label Mar 21, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
keybindings new-contributor-friendly outdated workflow: issue should stay closed stale workflow: stale issue
Projects
None yet
Development

No branches or pull requests

3 participants