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

Prevent ESC, KEY_RIGHT, KEY_LEFT propagation when keyboardNav is enabled #2399

Conversation

karendolan
Copy link
Contributor

This pull prevents key down event propagation on ESC, KEY_RIGHT, KEY_LEFT keys in Shepherd, when the Shepherd keyboardNavigation is enabled in a shepherd tour.

The reason: our underlying web application, like may others, has a listener for ESC, KEY_RIGHT, KEY_LEFT key press to enable shortcut actions. When the Shepherd "onboarding" tour is played and the user uses keyboardNav, our underlying web application also receives the KEY_RIGHT, KEY_LEFT, and ESC events and also handles them. This causes havoc and confusion with the underlying web application doing things that the user did not intend while they were only trying to navigate the Shepherd tour.

@RobbieTheWagner
Copy link
Member

@karendolan thanks for the PR, this looks great! I am a bit confused on the testing coverage though. Can you please explain how it works?

@karendolan
Copy link
Contributor Author

karendolan commented Jul 19, 2023

@karendolan thanks for the PR, this looks great! I am a bit confused on the testing coverage though. Can you please explain how it works?

Hi @RobbieTheWagner I tweaked the test to improve it and added comments. The test adds an event listener to the body, i.e. parent container, to check if the keydown event trickles up to it.

  • In the first test, "keyboard navigation is true", the keydown event should not get to the parent. Because the user's intent is to those keys to navigate Shepherd.
  • In the second test, "keyboard navigation is false", the keydown event should get to the parent because they keys are not being used to navigate Shepherd, they are being used for their default behavior, which is what ever the parent is doing with them.

@RobbieTheWagner RobbieTheWagner merged commit 38fe9b3 into shepherd-pro:master Jul 27, 2023
5 checks passed
karendolan added a commit to karendolan/shepherd that referenced this pull request Jan 2, 2024
chuckcarpenter pushed a commit that referenced this pull request Jan 21, 2024
…is enabled (#2562)

Related to #2399, stop key event propagation and default behavior when navigation is enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants