Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Dispatch event to root only if the key combo is registered as shortcut #1739

Merged

Conversation

sainthkh
Copy link
Contributor

@sainthkh sainthkh commented Oct 26, 2020

Closes TryGhost/Ghost#12294.

Cache registered shortcuts and check KeyboardEvent before dispatching event to the root.

Fixed the bug introduced with #1707.

I wish there were a way to test ctrl+v event. But unfortunately, native events are not executed by triggering event in the acceptance tests.


Got some code for us? Awesome 🎊!

Please include a description of your change & check your PR against this list, thanks!

  • There's a clear use-case for this code change
  • Commit message has a short title & references relevant issues
  • The build will pass (run ember test from the repo root - will be core/client if working from the submodule in Ghost).

More info can be found by clicking the "guidelines for contributing" link above.

@sainthkh sainthkh marked this pull request as ready for review October 26, 2020 11:50
Base automatically changed from master to main February 2, 2021 17:50
@ErisDS ErisDS self-assigned this Feb 16, 2021
@ErisDS
Copy link
Member

ErisDS commented Feb 16, 2021

@sainthkh Thank you for this PR! I apologise for the delay but this is on my list to review ASAP.

@ErisDS ErisDS assigned kevinansfield and unassigned ErisDS Aug 2, 2022
Closes TryGhost/Ghost#12294.

Cache registered shortcuts and check KeyboardEvent before dispatching event to the root.
@kevinansfield
Copy link
Member

Hey @sainthkh 👋 So sorry this was left for such a long time. I've tested and it works for the main use-case of copy/paste in the post tags input, thanks! 🎉

I'm going to merge but just wanted to note two concerns for future reference:

  1. we're trying to move away from keymaster and the shortcuts mixins, replacing all shortcut handling with ember-keyboard so there may be some places where we use the token input where there aren't any keymaster shortcuts registered
  2. the shortcuts cache util doesn't take the keymaster scope into account so it's possible there are some edge cases where it doesn't quite work as expected

Thanks again for the contribution 😊

@kevinansfield kevinansfield merged commit 35694e9 into TryGhost:main Aug 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ctrl/Cmd+V not working for tags in post settings menu
3 participants