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

Remove previous keydown listener #177

Merged

Conversation

marsimeau
Copy link
Contributor

This PR fixes a bug when selecting items using the keyboard.

Bug

To replicate the bug on the demo app of the current release:

  • Write "Food"
  • Press ArrowDown on the keyboard three times to land on "Food Colouring - Red"
  • Erase one character to be left with "Foo" (this resets the navigation)
  • Press Tab to lend on the first item "Food Colouring - Green"
  • Press Enter to select the entry
  • Bug: the fourth item, "Black-footed ferret", is selected instead

Explanation

The navigation controller does not remove the previously set navigate keydown listener after the reset. On keydown, only the first listener added updates its currentFocus or selects the item. This causes the navigation to use the wrong index on selection with the keyboard after the list is reset.

Fix

Remove previous keydown listener on navigation init.

Notes

I wasn't sure how to reference the previous handler function. I attached it to the inputField under a rather descriptive property.

@TarekRaafat
Copy link
Owner

Hello @marsimeau,

I like this one, good catch!

Thanks for your neat and detailed explanation and replication guide. I was able to replicate it immediately!

Cheers, Have a nice day! :)

@TarekRaafat TarekRaafat merged commit b60c5b8 into TarekRaafat:master Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants