sickdyd / react-search-autocomplete

A search box that filters the provided array of objects
https://sickdyd.github.io/react-search-autocomplete
MIT License
218 stars 82 forks source link

Add arrow key navigation and enter key selection #34

Closed harveytoro closed 2 years ago

harveytoro commented 2 years ago

My approach to adding arrow key navigation and enter key selection (should resolve #26 )

Also Thank You for creating such a useful component.

sickdyd commented 2 years ago

@harveytoro Thank you for you PR. There are a few issues:

harveytoro commented 2 years ago

@harveytoro Thank you for you PR. There are a few issues:

  • the code breaks the mouse hover highlighting
  • ideally the key pressing behavior should mimic what happens on Google (try to use the arrows there and see how it works)
  • when using up and down the location of the cursor in the input box jumps (maybe fixable with event.preventDefault(), not sure if it breaks other behaviors though)
  • please use Prettier to fix the code formatting
  • please write some tests for the functionality

@sickdyd thanks for the review, have committed some changes:

sickdyd commented 2 years ago

@harveytoro Thanks!

On mouse hover, can you describe what you are seeing? I am unsure how it is broken

Sorry, I should have been clearer: when using keys and mouse hover the highlighted items can be 2, but there should be only one otherwise the UI becomse confusing: if I hit return, which element will be selected? Not sure how much work this entails to fix. See the gif:

double

Having some issue with getting fireEvent.keyDown(inputElement, {key: 'ArrowDown', code: 'ArrowDown'}) to work at the moment so will have to work on the tests

I will take a look into this as soon as I have time.

sickdyd commented 2 years ago

@harveytoro Closing because I made my own implementation that fixes some of the issues we had on this PR. Sorry about that, but it was quicker to just rebuild the feature from scratch.

https://github.com/sickdyd/react-search-autocomplete/pull/54