-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat(autocomplete simple): react component for autocomplete - simple variation #191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on this component 👍
6368615
to
b3a2924
Compare
I don't understand why we don't focus the list and focus elements on keyboard navigation -> check what have been done in angularjs on dropdown. |
We can discuss it in our meeting today! |
Just for visibility purposes, we discussed and the reason why we need the |
Awesome, I can definitely do that! |
@FlorentDinet fixed ! thanks for the catch ! |
ok, last thing I saw is that we're kind of "stuck" in the field with keyboard navigation:
I'd understand if tab was used to navigate between suggestions but that's not the case Anyway it's not blocking, just icing on the cake |
@FlorentDinet got it, on my end I see that as a blocker, it is definitely not a good UX. It was a quick fix, so there it is! would you mind trying again ? |
👌 |
Created the first version of the autocomplete component. it supports:
Simple usage
![QHePgrkFJV](https://user-images.githubusercontent.com/13719066/66750498-e49f7b80-ee8c-11e9-88b9-b9713495c5fc.gif)
Keyboard navigation
![BrMiLlz37o](https://user-images.githubusercontent.com/13719066/66751172-aacf7480-ee8e-11e9-9e14-4cefc2a4770d.gif)
No results
![BRTzGGX353](https://user-images.githubusercontent.com/13719066/66750548-0b5db200-ee8d-11e9-93ee-f10cd06d92e0.gif)
Note: this variation may need css changes since the
no results
label is a little bit more gray on the mockups.Leave and come back
![96ohhIcDx4](https://user-images.githubusercontent.com/13719066/66816530-86c96d00-ef3a-11e9-9299-179f2e795b87.gif)
Side effects:
isHighlighted
and control the boolean from the outsideDropdown
needs that reference to display the popover. Also, TextField did not allow to overrideonFocus
andonBlur
, so now, when theonFocus
andonBlur
are executed inside the component, they will check whether there are functions passed in as props and they will be executed first.inputRef
was also added since it is necessary to have a reference to the actual input, and not the wrapper astextFieldRef
did.