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

feat(typeahead): add typeahead directive #169

Merged
merged 1 commit into from
Mar 1, 2013
Merged

Conversation

pkozlowski-opensource
Copy link
Member

Voila! Typeahead in under 200 LOC!

This version works with the select's syntax, understands promises etc. In short it does everything that Bootstrap's version does + is customizable and well integrated with AngularJS.

Please, review it carefully - all the tests are passing and things seems to work OK but it could be probably simplified in places.

@ccollie
Copy link

ccollie commented Feb 23, 2013

I know its not specifically Bootstrap, but have you seen
http://twitter.github.com/typeahead.js/

@pkozlowski-opensource
Copy link
Member Author

@ccollie yes, I did. In fact the goal of this directive is to be as powerful as the typeahead.js but being well rooted in the AngularJS world. Have you seen the code size of typeahead.js? It goes into hundreds and hundreds of LOC. On top of this it would have hard time understanding promises.

This version is 200 LOC and can take advantage of $http, its caching and $q promises.

@ccollie
Copy link

ccollie commented Feb 23, 2013

Nice! The thing i liked about typeahead.js is the support for multiple datasets per input (i need that yesterday). But you are absolutely correct about size.

@pkozlowski-opensource
Copy link
Member Author

Well, with this version it is you who query data so you can consult as many data sets as you want (even combining static arrays with $http results). So it is extremely flexible - there are no limits here, really.

As soon as it is merged and released I need to write a blog post showing how this can be used to achieve things that are covered by the Twitter's counterpart.

@snrmwg
Copy link

snrmwg commented Feb 23, 2013

Sorry, but I can't get it to work. I want to use it in a form with a lookup field. In the field I want to store internally the primary key value of the selection. But I want to display a label field of the selection.

I already looked into your tests where you have similar construct with state code and label. But I can't get it to work in a real form. How should my model look like?

Only the selection key:

$scope.adress={
   name:'...',
   street:'...',
   city_id:123
}

Or with city as sub-hash:

$scope.adress={
   name:'...',
   street:'...',
   city: {
     id:123,
     label: 'New York'
  }
}

My source will be a JSON API but for testing I declare a fixed array like this:

$scope.cities = [
   {id: 123, label: 'New York'},
   {id: 456, label: 'Munich'}
]

In my form I have something like this:

input(type="text", ng-model="customer.city_id", typeahead="city.id as city.label for city in cities | filter:$viewValue")

Could you please tell how what I'm doing wrong?

@pkozlowski-opensource
Copy link
Member Author

@snrmwg thnx so much for looking into this, early feedback is what we need!

You are right, looking closer into your use case there is no easy way to cover this with the current implementation. I though that people could write a custom formatter but this causes other problems and is not very user friendly.

Anyway, here is the plunker with an updated version (didn't push changes to the repo yet): http://plnkr.co/edit/OAh3Z7?p=preview

Could you give it a try and let us know how it feels? If this works for you I would be more confident in pushing changes I've made.

As for your question regarding what to bind (full object or an id) I think that the full object is the way to go. The trouble with binding id only is that you can't render user-friendly value to your users (you would have to look it up from DB or something).

Once again, thnx for your feedback so far, would be keen on hearing more!

@bekos
Copy link
Contributor

bekos commented Feb 27, 2013

A feature that twitter's typeahead promotes (and my in my opinion correctly) is the ability to prefetch data from URL and search in client's browser.

@pkozlowski-opensource Do you think you can provide this ability from the typeahead's configuration?

@pkozlowski-opensource
Copy link
Member Author

@bekos The great thing about native AngularJS directives is that you can utilize the whole power of AngularJS and all its services. Since with this implementation you can retrieve data as you like (using $http, local storage etc.) you are in total control of how and what data you pre-fetch. Which means that even today you could pre-fetch data and have it working with the implementation from this PR (!). I wanted this directive to be as light as possible and give users as much power as possible.

I need to write a blog post about this directive showing how to get all the functionality of the typeahead.js with this directive. Just need to convince @angular-ui/bootstrap guys to review this PR so it can be merged :-)

If you want to give it a try ASAP I can give you a hand and assist with any issue you might face.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 28, 2013

Looks great, Pawel! I only think perhaps we should add a complicated example with $http, or some sort of promises, in addition to the simple states example.

@pkozlowski-opensource pkozlowski-opensource merged commit 6a97da2 into master Mar 1, 2013
@kloy
Copy link

kloy commented Mar 1, 2013

@pkozlowski-opensource I have been trying to get this to work but am receiving the error "Cannot read property 'length' of undefined" line 71 which is if (matches.length > 0) { in getMatchesAsync(). Any ideas? My input looks like

<input placeholder="State" ng-model="states" typeahead="state for state in states" />

@pkozlowski-opensource
Copy link
Member Author

@kloy strange, line 71 doesn't correspond to the instruction you are mentioning. I've just merged this into master so be sure the reference the latest version.

One thing that looks strange is that you are using the same variable for the ng-model and collection of items which is odd. I guess states should be your collection of items for autocomplere, while ng-model should point to a variable where you want to store a selection.

Also, I guess you would like to filter your matches based on what users are typing so:

<input placeholder="State" ng-model="mySelectedState" typeahead="state for state in states | filter:$viewvalue" />

From the error it would look like the states variable is undefined on a scope. Please send a plunker with a reproduce scenario and I will be happy to help.

@kloy
Copy link

kloy commented Mar 1, 2013

@pkozlowski-opensource Thanks for the quick reply. The line is 78 actually, sorry about that. My ng-model actually is ng-model="foo.states" which is a shared model for the form. I tried adding in the filter and am receiving the same problem.

Also, am using the latest code from master.

@kloy
Copy link

kloy commented Mar 1, 2013

@pkozlowski-opensource I'm going to see if I can get my expression working properly in a select. I think I might have something dumb I'm missing.

@pkozlowski-opensource
Copy link
Member Author

@kloy Here is the plunker working with the latest version from the master:
http://plnkr.co/edit/JXduWk?p=preview

Try to modify it to expose your issue. I need to see more of your code to confirm if this is a bug or something else in your code.

@kloy
Copy link

kloy commented Mar 1, 2013

@pkozlowski-opensource I had done something dumb by placing my ng-controller on the wrong parent element when merging some code together and didn't realize my scope was misbehaving. I appear to have it working now.

@pkozlowski-opensource
Copy link
Member Author

@kloy Glad you have it working!

@kloy
Copy link

kloy commented Mar 1, 2013

@pkozlowski-opensource I am running into what I believe is a bug when using an array of objects.

Example can be found at http://plnkr.co/edit/hvyZud

When tabbing or clicking the item, the input is not being changed to the selected item.

@kloy
Copy link

kloy commented Mar 1, 2013

@pkozlowski-opensource "state as state.label for state in states | filter:$viewValue" Fixes the tab/select issue, but of course populates the model with the entire object.

@pkozlowski-opensource
Copy link
Member Author

@kloy looking at it right now. It indeed looks like a bug (!).

@pkozlowski-opensource
Copy link
Member Author

@kloy OK, this is a corner case really (one could argue it is a bug). While it would be possible to display a correct label upon selection the trouble is with the initial display. Let's say you would have your model initially pointing to a state code. Then you need to have a bit of gymnastic to display an initial input value (which might be next to impossible if you do auto-complete from data on the server side).

Need to sleep on this one, could you open a separate issue for it?

@kloy
Copy link

kloy commented Mar 1, 2013

@pkozlowski-opensource Sure, however supports this expression.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants