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

feat(typeahead): add the typeahead directive #147

Closed
wants to merge 1 commit into from

Conversation

pkozlowski-opensource
Copy link
Member

After a lengthy discussion in #114 and several options for the syntax here is the finished PR for the typeahead directive.

The current version uses the same interface as the original one which works but feels very non-AngularJS. I was toying with different ideas but the only other thing that kind of made sense was string=based syntax similar to the one used by the select directive. The problem with this is that select's syntax is not very intuitive either.

The other issue is that atm I'm using unsafe binding of HTML (to support matches highlighting). This is definitively not something I would like to do but for me this is one more evidence that we need ngSanitize

Anyway, the current version supports all the bootstrap's options so if anyone has an idea of a better syntax we can (easily?) change things. It also has integration with $q so functions going for matchers can be async!

@ajoslin
Copy link
Contributor

ajoslin commented Feb 17, 2013

So I thought to myself this directive is doing a lot .. couldn't we factor out most of the logic to some sort of promise-chaining service? Then the typeahead directive could do just what it needs to do: display some results for the user.

So basically, we take $q and add some functions to it for autocomplete, and expose that as a service.

//Default: verbose version
$scope.items = autocomplete(states)
  .if(function() { return $scope.input.length > 1; })
  .filter($scope.input)
  .limitTo(10);
//Helper for the default 
$scope.items = autocomplete.typeahead(states, $scope.input, 1, 10);
//With my own custom filter eg fuzzy search
$scope.items = autocomplete(states)
  .then(function(results) {
    return fuzzySearch(results, $scope.input);
  })
  .limitTo(10);
//With http
$scope.items = autocomplete(
  $http.get("/items?input="+$scope.input).then(function(response) {
    //we might be able to automagically turn response into data internally
    return response.data; 
  })
)
.then(doMagicWithResults)
.limitTo(10)

Then the html...

<!-- here the ng-change is just an example way to trigger an update to $scope.items -->
<input ng-model="input" ng-change="updateItems()" typeahead="items">

The only thing is highlighting.. I guess it could find it out off of ng-model?

And the typeahead results thing could display a spinner perhaps if the items are not resolved yet.

Thoughts? This is just a brain dump of my idea lol. It may be over-engineered. And autocomplete is probably a bad name for the service.

@joshdmiller
Copy link
Contributor

@pkozlowski-opensource This is awesome!

@ajoslin Funny you should mention the promise-chaining. I messed a few weeks ago with a $q-based DSL to do something similar, along with an implementation of some of the caolan/async library. It was more generic than for autocomplete but I didn't have time to get much into it. Anyway, I like very much the chaining - it looks much cleaner.

@petebacondarwin
Copy link
Member

Sorry didn't get round to reviewing this today. Will look tomorrow.

On 17 February 2013 20:56, Andy Joslin notifications@github.com wrote:

So I thought to myself this directive is doing a lot .. couldn't we factor
out most of the logic to some sort of promise-chaining service? Then the
typeahead directive could do just what it needs to do: display some results
for the user.

So basically, we take $q and add some functions to it for autocomplete,
and expose that as a service.

//Default: verbose version$scope.items = autocomplete(states)
.if(function() { return $scope.input.length > 1; })
.filter($scope.input)
.limitTo(10);

//Helper for the default $scope.items = autocomplete.typeahead(states, $scope.input, 1, 10);

//With my own custom filter eg fuzzy search$scope.items = autocomplete(states)
.then(function(results) {
return fuzzySearch(results, $scope.input);
})
.limitTo(10);

//With http$scope.items = autocomplete(
$http.get("/items?input="+$scope.input).then(function(response) {
//we might be able to automagically turn response into data internally
return response.data;
})).then(doMagicWithResults).limitTo(10)

Then the html...

The only thing is highlighting.. I guess it could find it out off of
ng-model?

And the typeahead results thing could display a spinner perhaps if the
items are not resolved yet.

Thoughts? This is just a brain dump of my idea lol. It may be
over-engineered. And autocomplete is probably a bad name for the service.


Reply to this email directly or view it on GitHubhttps://github.com//pull/147#issuecomment-13695505.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 18, 2013

Yeah @pkozlowski-opensource I think we could go ahead and merge this, and maybe consider something else for a simpler API later.

@pkozlowski-opensource
Copy link
Member Author

Actually I was pondering an idea of the following api (once again, inspired by the <select> directive):

<input ng-model="myModel" typeahead="[model to select] as [label to display] for value in [array | myFunction($taValue)">

Of course as with select people would have an option of specifying a simpler syntax, in the extreme case reduced to:

<input ng-model="myModel" typeahead="[array]">

Now, if people would like to use custom filtering, ordering etc. they would just have to write a function in a controller for this.

For the higlight I'm more and more leaning toward allowing people to provide they own template string for each item.

I guess this all would be super-cool but would require a bit more work, which is not a problem in itself. I'm just slightly worried about this not-so-intuitive syntax of select. But thinking more about this - it is hard to come up with something simpler (!).

@darwin
Copy link

darwin commented Feb 19, 2013

I haven't used typeahead yet. But just noticed this announcement today:
http://engineering.twitter.com/2013/02/twitter-typeaheadjs-you-autocomplete-me.html

Is this new twitter code something completely different from:?
http://twitter.github.com/bootstrap/javascript.html#typeahead

@pkozlowski-opensource
Copy link
Member Author

@darwin Thnx for the link. No, the 2 are not the same, the new thing they've open-sourced finally looks like a decent (functionally speaking!) implementation. But I'm SHOCKED how much code they've put into this... https://github.com/twitter/typeahead.js/tree/master/src/js

I think that that the version in this repo will be better integrated with AngularJS ($q and use of native AngularJS templates for matched items) and written using fraction of their code (<200LOC).

@pkozlowski-opensource
Copy link
Member Author

OK, another (last!) stab at the typeahead syntax, this time making it really AngularJS-like and not at all Bootstrap-JS-like:

https://gist.github.com/pkozlowski-opensource/4998969

The only downside I can see is that it has a bit of noise for the simplest possible use case (items from an array). On the other hand it is much simpler for more elaborate use cases.

BTW: I think that with this proposal we would be exactly matching functionality of http://engineering.twitter.com/2013/02/twitter-typeaheadjs-you-autocomplete-me.html but reusing the whole AngularJS infrastructure ($q, $http + caching, templates etc.)

@joshdmiller
Copy link
Contributor

I like it!

Regarding typeahead.js, I took one look at their code and my jaw dropped. I must be missing something, because 13 javascript files seems like an awful lot...

@pkozlowski-opensource
Copy link
Member Author

@joshdmiller yeh, I guess guys at Twitter had to re-create the whole infrastructure for communicating with a back-end, caching etc. Plus 2-way data binding cuts down lines of ode considerably... I don't know, need to dig into their code more as I might be missing something as well.

@GuillaumeBiton
Copy link

Hi Pawel,

I have created a jsbin to test. There is for the moment issues when trying to deal with array of objects.

Examples

For the syntax, this is great.

@pkozlowski-opensource
Copy link
Member Author

Since the feedback for the new syntax is generally positive I'm going to close this PR and work toward the proposed solution. Hopefully this should simplify the implementation as well.

@GuillaumeBiton as you can see this is still very much WIP and we are pondering different APIs. I will have a look at your examples when testing new PR.

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.

6 participants