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

[Discussion] Roadmap to Version 4 #574

Closed
33 of 37 tasks
JohannesKlauss opened this issue Jul 23, 2021 · 26 comments
Closed
33 of 37 tasks

[Discussion] Roadmap to Version 4 #574

JohannesKlauss opened this issue Jul 23, 2021 · 26 comments
Labels
Needs more info Further information is requested v4 Issue regarding v4

Comments

@JohannesKlauss
Copy link
Owner

JohannesKlauss commented Jul 23, 2021

👋 Announcing Version 4

I am currently overhauling the docs and while writing new and extensive documentation and tutorials on the hook, I realized a lot of shortcomings that are based on the fact that I never planned this to be a popular hook.

This is why I'd like to write out a roadmap to Version 4 that should address all those issues including its own basis on registering hotkeys instead of relying on an external package.

Version 4 will target modern browsers (otherwise writing and maintaining a hotkeys handling basis would be too time consuming), so there is a chance that some users will be stuck on version 3.
To minimize the pain I'd like to hear from you what some of your pain points are when using the hook (what you think is confusing, what doesn't work as expected, what could be improved, etc), what you would like to see as new features and in which context (browser support, etc) you are using the package. Thank you all in advance 😊

🚧 Current Problems

  • Handling of the alt modifier is complicated
  • There is no support for scopes Support for hotkey-js scope is missing #290
  • The API is cluttered with inconsistencies (filter, enable seem counterintuitive)
  • The + key is not working when using a different splitKey splitKey is not working. #544
  • You cannot use key codes
  • Doesn't work with React Portals Use with Portal / outside DOM ? #405
  • enableOnTags is not very intuitive if you don't use TypeScript since you need to know that you have to type out the tag names in capital letters
  • enableOnTags should at least be case-insensitive.
  • The meaning of the return of filter is currently inversed. Returning false prevents the callback from getting executed, true bypasses the filter.
  • Exposing hotkeys.isPressed as a hook is unnecessary.
  • isHotkeyPressed should be able to take a combination of keys.
  • onKeyDown and onKeyUp doesn't trigger correctly. If the key stays down, it triggers multiple times, should only trigger once

🎯 Goals for Version 4

  • All current features should be preserved
  • Drop hotkeys.js package
  • Only target modern browsers
  • Support scoping
  • Support Portals
  • Clean up API
  • Add a function which returns all currently pressed keys
  • Add a function which returns all currently bound hotkeys
  • Bind events to ref element otherwise to document
  • Set up CI/CD pipeline for publishing

🏗️ Track of V4 API changes

  • Merge filter into enabled
  • Rename splitKey to combinationKey
  • splitKey refers to the actual splitting of multiple hotkeys
  • Make the options object and dependency array interchangable
  • Allow the first parameter to be an array.
  • Add scopes option to options object
  • Add preventDefault to the options object
  • Replace any[] as dependency type with DependecyList
  • cmd/command becomes meta
  • enableOnTags now accepts a boolean (if set to true, all input tags are enabled)
  • enableOnTags becomes enableOnFormTags
  • isHotkeyPressed takes an array or string as first argument
  • isHotkeyPressed takes a splitChar as an optional second argument (defaults to ,)

🐛 Bugs

  • Nested Scoping doesn't work
  • Arrow keys (down, left, etc.) aren't mapped to ArrowDown, ArrowLeft, etc.
@JohannesKlauss JohannesKlauss added Needs more info Further information is requested v4 Issue regarding v4 labels Jul 23, 2021
@JohannesKlauss JohannesKlauss pinned this issue Jul 23, 2021
@phamquocduy
Copy link

Thank you for your work!

@piotrpawlik
Copy link

Is there any progress on those planned?

@JohannesKlauss
Copy link
Owner Author

@piotrpawlik Currently I am overhauling the docs. After that work on version 4 begins. I have a very limited time to work on this, so it might take a while to get there.

@sonicolasj
Copy link

Hello! Great work done here!
Is there any plan to have a way to get every shortcut currently registered? Maybe using the Context API or something like that?

I feel like displaying a list of keyboard shortcuts to the user is good UX. Maybe the options API should change a little bit, with something like name and/or description to return as representation, or a "scope" of the shortcut (does it come from the whole app? a precise component?)

For now, I'll use a wrapper around this lib doing that, but if the idea is interesting I’ll share code!

Also, why do you plan on dropping the hotkeys.js package?

@JohannesKlauss
Copy link
Owner Author

JohannesKlauss commented Oct 1, 2021

Regarding the list of all registered hotkeys, this question has came up in the past and I answered it here: #255 (comment)
In short I am against implementing this for two reasons:

  1. It holds global information in a modular hook
  2. It inverses ownership of information

So in short, you basically already know what keystrokes you passed to useHotkeys, even if it's dynamically assigned. In my answer in the link above I also showed an example on how one could create such a global map and assign specific strokes to several hotkeys hooks. This way you can even override the assigned hotkey and the useHotkeys hook will reassign it automatically.

Regarding dropping the hotkeys.js package it is just that there are too many bugs/incompatabilities with react, like scoping, portals, splitKeys, etc.

I hope this info helps you a bit :)

@devinhalladay
Copy link

I actually came here with the same question as @sonicolasj — something I loved about other hotkey libraries (which I otherwise hated working with) was the ability to easily scope hotkeys, and display a menu of all available hotkeys depending on what part of the app I'm in.

Your linked answer makes sense and would've been my solution as well. I wish it were otherwise, but I agree with you that it doesn't make sense to implement that as a feature because of the nature of hooks.

That said — since a documentation overhaul is part of v4, I think it would be great to include a prominent example of how to do this in the docs once the new version is ready :)

Thanks for your hard work and for opening up the discussion here!

@JohannesKlauss
Copy link
Owner Author

@devinhalladay Yes I agree completely and will update the docs accordingly :)

@sarensw
Copy link

sarensw commented Nov 10, 2021

One thing that I would consider is to change the way dependencies work. Currently, when there are no dependencies provided to the useHotkeys hook, those values will not be considered after they are updated. So as soon as one uses values from useState in the useHotkeys callback, then you have to add those values to the dependencies array. Took me a while to figure this out as I was questioning myself all the time why react doesn't change the value with the set method. (Does that make sense?)

I created a pen based on the simple example in the README which you can find here: https://codepen.io/sarensw/pen/VwzdRYV

Using Ctrl+K it it will aways be set to 1 as the original value for the counter is memorized. Using Ctrl+L, all works as expected.

@JohannesKlauss
Copy link
Owner Author

JohannesKlauss commented Nov 10, 2021

The reason for this is that we otherwise would need to unbind and bind the hotkey with every value change you are referencing inside the callback (which basically comes down to every render of the component).
There is a complete section inside the docs regarding that topic:
https://react-hotkeys-hook.vercel.app/docs/documentation/useHotkeys/setting-callback-dependencies

The way I see it is that the callback is working just like the callbacks for every other react hook (useEffect, useMemo, useCallback).

I that sense the value doesn't get memoised but stale, just like a useEffect would have a stale value if you don't add it to the dependency array.

So it is a tradeoff, but setting dependencies for hook callbacks is a very react way of handling stale state.
But I agree that there is definitely some improvement regarding relaying this info to the developer.

@sarensw
Copy link

sarensw commented Nov 11, 2021

Thanks for the explanation. I agree with you. If you could make this more visible in the README.md then I think all is good.

@n0mn0m
Copy link

n0mn0m commented Nov 11, 2021

@sarensw it's under parameters in the readme.

@liorp
Copy link

liorp commented Feb 8, 2022

Hello! Great work done here! Is there any plan to have a way to get every shortcut currently registered? Maybe using the Context API or something like that?

I feel like displaying a list of keyboard shortcuts to the user is good UX. Maybe the options API should change a little bit, with something like name and/or description to return as representation, or a "scope" of the shortcut (does it come from the whole app? a precise component?)

For now, I'll use a wrapper around this lib doing that, but if the idea is interesting I’ll share code!

Also, why do you plan on dropping the hotkeys.js package?

Hi! I’ve actually created a wrapper for this- would love to see this get merged to this package/docs. It stores your currently active hot keys in a context, and allows for easy display of them.

@JohannesKlauss
Copy link
Owner Author

JohannesKlauss commented Feb 15, 2022

Hey everybody,
I finally have the resources to work on version 4.
So if you have any specific features in mind, wishes, improvements, etc. now is the time to let me know :)

I can't give a timeline yet, but I hope to release a candidate around march.
If anybody has experience in writing documentation and is willing to help me out, I'd love to take it, because this is the tedious part as I learnt the last time.

@JohannesKlauss
Copy link
Owner Author

You can track the progress in the to do list above: #574 (comment)

I also realized that I need a context to support scoping. And since a lot of users where asking about currently registered hotkeys, I am thinking about adding that to the core package. Let me know what you think about that.

@RyKilleen
Copy link

RyKilleen commented Mar 10, 2022

I haven't seen mention of it so thought I'd throw it out here:

The biggest feature I'm looking for in a library like this is something like getPressedKeycodes(). I'd love to access an array of the currently pressed keys when attempting to let users define a custom shortcut command. Perhaps this is achieved some other way with your library, either in v3 or v4, but I couldn't identify a way to do it. Happy to make this a separate issue if that's useful!

@JohannesKlauss
Copy link
Owner Author

Hi @RyKilleen thanks for the suggestion. I will definitely add this to the roadmap for version 4. Sounds like a useful feature.

@felixmpaulus
Copy link

felixmpaulus commented May 28, 2022

Would love to have the ability to call a function on keyup and then on keydown to basically change the state while a key is pressed and revert it once the key is no longer pressed.

I tried adding two hooks for the same key combination, one on keyup, the other on keydown and have them execute different callbacks. But that did not work...
Thanks for your work! :)

@JohannesKlauss
Copy link
Owner Author

@felixmpaulus Huh, was under the impression that this was already possible, but you are absolutely right. I'll add that to the list

@RealSGII2
Copy link

I may be a tad bit late, and I hope this isn't out of the scope:
It'd be very nice to see a "key chord" mechanic, similar to Visual Studio or Visual Studio Code.

Essentially, you have to press multiple hot keys to run something, i.e. in Visual Studio Code, you can press Ctrl+K, then W to close all of the currently open tabs.

@JohannesKlauss
Copy link
Owner Author

@RealSGII2 thank you for the suggestion, but you can already implement this in user land. Just set a state flag to true when the first combination is pressed (and possibly set it to false after a given frame of time) and then check that flag inside the second hotkey callback

@davidslaby
Copy link

Hey guys, when do you expect the new version will be out? I'm worth to wait to scoping support. Thank you.

@davidslaby
Copy link

Is there any option to release an alpha version with already existing features (scoping is checked above), so we can use it before version 4 has been released?

@JohannesKlauss
Copy link
Owner Author

JohannesKlauss commented Jul 20, 2022

Hey @davidslaby I know it has been quite a while since the announcement. I got very limited time at my hands to push this project further (which also involves rewriting a good bit of the documentation and introduce versioning there).

A alpha release sounds like a great idea. Will probably be able to release one on friday or over the weekend.

@JohannesKlauss
Copy link
Owner Author

JohannesKlauss commented Jul 22, 2022

@devinhalladay and every body else who are eager to try out the new version:
A pre release is now available under either next or 4.0.0-1

I didn't had any time to rewrite the readme yet, so usage is visible in the tests here: https://github.com/JohannesKlauss/react-hotkeys-hook/blob/feature/version-4/tests/useHotkeys.test.tsx

Or just have a look at the source code here: https://github.com/JohannesKlauss/react-hotkeys-hook/blob/feature/version-4/src/useHotkeys.ts

Please let me know if you encounter any strange behavior and/or bugs.

Please note that there are quite some breaking changes involved, so please check the API changes above (#574 (comment)) first

I am also happy to say that I am nearing feature completeness (only portal functionality is missing).

If you want to use the new Scoping feature to group hotkeys together (and activate/deactivate scopes programmatically) you have to wrap your app with <HotkeysProvider>

@JohannesKlauss
Copy link
Owner Author

I am gathering feedback on this discussion thread: #788

@JohannesKlauss
Copy link
Owner Author

Version 4 is now officially released!

@JohannesKlauss JohannesKlauss unpinned this issue Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs more info Further information is requested v4 Issue regarding v4
Projects
None yet
Development

No branches or pull requests