cheminfo / nmrium

React component to display and process nuclear magnetic resonance (NMR) spectra.
https://docs.nmrium.org
MIT License
55 stars 25 forks source link

use react-kbs for shortcut #1408

Open lpatiny opened 2 years ago

lpatiny commented 2 years ago

Please check how the following library could be used to manage the shortcuts:

https://github.com/zakodium/react-kbs

We should not forget that many NMRium components may be present in the same page.

hamed-musallam commented 2 years ago

@lpatiny I tried to implement but I face a problem when pressing shift + number for example ( shift + 1 ) it will not work and if we take the Alice, for example ( ! ) this will lead to another problem because the keyboard layout will be different between mac and windows and also we have to consider keyboard language.

hamed-musallam commented 2 years ago

@lpatiny where going to use in a global context and not local to the specific component because in any way we will lose the focus on the component once edit and click the button .... etc,

that means we will keep the same way of handling the problem by setting a flag in the global state to whether the mouse is over the Displayer or not and check this when we attend to do action.

so I think after I took a look at our code, maybe this will make the code nicer but will not change a lot we will still follow the old way and we can make a meeting for further discussion

targos commented 2 years ago

react-kbs v2 works better when the focus is on a div, like in nmrium

lpatiny commented 1 year ago

@hamed-musallam Could you make a PR that shows the error ? For example the the shortcuts '1', '2', ...

We would need to test in on swiss and franch keyboards.

hamed-musallam commented 1 year ago

@lpatiny

the demo should be at the side of react-kbs project

lpatiny commented 1 year ago

react-kbs is used in some other big react projects and we are wondering was is the problem in your case. So the PR should be in this project.

hamed-musallam commented 1 year ago

this is the problem that i have https://github.com/cheminfo/nmrium/issues/1408#issuecomment-1058965013

hamed-musallam commented 1 year ago

the other projects that you mention seem not to have the same use case

for example

1- press "2" to save the current preferences and if you press again it reapplies the preferences that you saved 2- press on shift + 2 to reset the preferences (what I get is @ on the English keyboard layout and " in the German keyboard layout) and the same for most of the numbers which will not work if the keyboard layout is different

hamed-musallam commented 1 year ago

@lpatiny

we can add the demo here https://zakodium-oss.github.io/react-kbs/

hamed-musallam commented 1 year ago

@lpatiny I tried to implement but I face a problem when pressing shift + number for example ( shift + 1 ) it will not work and if we take the Alice, for example ( ! ) this will lead to another problem because the keyboard layout will be different between mac and windows and also we have to consider keyboard language.

I was wrong in this example, because ! is the same for German and English keyboard layouts but shift+ 2 is not, and the same for the others shift + 3,4 .... etc

targos commented 1 year ago

https://github.com/zakodium-oss/react-kbs/releases/tag/v2.1.0 adds support for defining shortcuts with code instead of key.