Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Typeahead - do not preselect first match #2103

Closed
wants to merge 3 commits into from
Closed

Typeahead - do not preselect first match #2103

wants to merge 3 commits into from

Conversation

xelibrion
Copy link

This pull request addresses issue #908

@mrzepinski
Copy link

+1

@xelibrion
Copy link
Author

Clarification - this pull request implements functionality with a contract proposed by @pkozlowski-opensource here

@davelosert
Copy link

+1

@pkozlowski-opensource
Copy link
Member

@xelibrion could you please squash commits and remove commits that are not essential to adding the required functionality (you can send those are as a separate pull requests).

Small, focused PRs are much easier to review and got much more chance of getting merged fast.

@xelibrion
Copy link
Author

@pkozlowski-opensource done

@pkozlowski-opensource
Copy link
Member

@xelibrion this starts to look good, thnx. I've left one comment (other one is to be ignored). I need to have another look at the code / tests but it should land soon.

One more thing I was wondering about, from the functional point of view: all the implementations I've checked (typeahead.js, angular-ui autocomplete) will go through the "unselected" state when you use up/down arrows. I'm not sure if this matters, though. WDYT?

@xelibrion
Copy link
Author

@pkozlowski-opensource not sure that I get you're saying about "unselected" state. Is it related to their onSelectCallback implementation?

I fixed indentation in a couple of places, please let me know if you're happy with the diff, I'll squash these commits in one - I didn't want to overwrite first commit for now as there are a few comments on that one.

@wesleycho
Copy link
Contributor

Can you update this PR based on current master?

@chrisirhc
Copy link
Contributor

Closing as this was addressed via #2916 . Thanks for your work though.

@chrisirhc chrisirhc closed this Apr 6, 2015
@chrisirhc chrisirhc removed this from the 0.13.1 milestone Apr 6, 2015
@karianna karianna added this to the 0.13.0 milestone Apr 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants