search

omridevk / ng-keyboard-shortcuts

Dead Simple Keyboard Shortcuts Management for Angular
https://ng-keyboard-shortcuts.vercel.app/
147 stars 34 forks source link

BUG: Onclick events bugged #53

Closed takashi1kun closed 4 years ago

takashi1kun commented 4 years ago

Describe the bug After executing a shortcode onclick events become unresponsive nad you need to click 2 times to restore to normal behavior

To Reproduce Steps to reproduce the behavior:

  1. use a shortcut
  2. Click on button
  3. it doesn't work
  4. you need to push the button again

Expected behavior for a shortcut to not alter onclick behavior

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

omridevk commented 4 years ago

Thank you for reporting the issue, I'll look into it asap

takashi1kun commented 4 years ago

Thank you for reporting the issue, I'll look into it asap

Thanks, after more testing i discovered that this doesn't apply only to buttons, but to near every onclick event like exiting a dialog touching the overlay backdrop, etc. I also discovered that this only happens with a shortcut that has the preventDefault property on true, but for example doesn't block exiting from a dialog with only a single "esc" so it seems to only affect mouse clicks but that is not all, for example if in a dialog you have a form with a single field and a button with type submit and that button is also the button that closes the dialog, when you press enter on that field it should execute the submit button, but it doesn't the first time, so is not exactly the mouse itself as much as it is the onclick events

takashi1kun commented 4 years ago

@omridevk here i replicated the behavior in a modified version of the demo:
https://codesandbox.io/s/ng-keyboard-shortcuts-example-x00v3?fontsize=14&hidenavigation=1&theme=dark if you click the button without any shortcut it would work as normal, but if you push ctrl + f that is a shortcut with preventDefault, you would need to push the button two times for it to work.

omridevk commented 4 years ago

@takashi1kun Thanks a lot for the effort and for providing an example, I assume this has something to do with the help modal component who might be preventing clicks from happening. Hopefully i'll release a fix for this and a new version this weekend.

takashi1kun commented 4 years ago

might be preventing clicks from happening

I'm not so sure about that, in my project i'm not using the help component because i already had one for other things, so instead of using the shortcuts's help component i just hand added the shortcuts in my own help dialog.

omridevk commented 4 years ago

@takashi1kun Unrelated question When you built you help component, how did make sure all the shortcuts are displayed in the help menu? because the library also provides a service and helps you build/integrate with existing help menu/screens. See: https://github.com/omridevk/ng-keyboard-shortcuts#service You can subscribe the shortcuts Observable to get a list of all active shortcuts in your entire app.

omridevk commented 4 years ago

Okay, I found the root cause: https://github.com/omridevk/ng-keyboard-shortcuts/blob/master/projects/ng-keyboard-shortcuts/src/lib/ng-keyboard-shortcuts.service.ts#L135

Seems to be a regression due to the bug we had in Firefox. I will see how I can resolve this issue without affecting the previous one.

takashi1kun commented 4 years ago

@takashi1kun Unrelated question When you built you help component, how did make sure all the shortcuts are displayed in the help menu?

I didn't, i added them manually, in the sense that i hard coded them in the html.

omridevk commented 4 years ago

I see, you can have a look at how I built the help component, as it may be easier to maintain if you keep one single source of truth and not have to update the HTML if you change/add/modify existing shortcuts.

omridevk commented 4 years ago

this means that you can use this version: https://github.com/omridevk/ng-keyboard-shortcuts/releases/tag/8.2.0 until I resolve the issue.

omridevk commented 4 years ago

@takashi1kun Sorry for the delay, I will try to release a fix soon, or revert the PR the caused this, as this is regression and must be handled. Thanks for the patience.

takashi1kun commented 4 years ago

@omridevk Thank you very much, for the moment i am using the version you suggested.

lambdadbmal commented 4 years ago

I have a same issue on Chrome 80.0.3987.132 (Mac OS) + Angular 8.2.14 + ng-keyboard-shortcuts 8.2.5. Unfortunately, ng-keyboard-shortcuts 8.2.0 is not suit for my situation. I am waiting for next version. Thank you for your very useful software!

omridevk commented 4 years ago

I am going to revert the firefox as it seems to be a bug in Firefox itself: https://bugzilla.mozilla.org/show_bug.cgi?id=1487102 so I'll re-open the firefox bug and keep track there.

omridevk commented 4 years ago

closing will be fixed in 8.2.6 will be released in the next hour or so. feel free to update. Reopen if issue persist please.