typeahead pull request

[GuillaumeBiton]

This is a typeahead directive purpose.

you can see it in action here :
http://jsfiddle.net/guillaumebiton/hNxF7/

Sorry but I need help for unit tests. never use. I am very interesting to learn how to set them.

[petebacondarwin]

Hi @GuillaumeBiton,
This is cool. It works very nicely in the demo.

• You have done the right thing putting the template separately and providing a demo file.
• You need to tidy up the javascript to get rid of the JS lint errors: See https://travis-ci.org/angular-ui/bootstrap/builds/3997227
• Take a look at some of the other directives in the project for an idea of how to set up unit tests. The general idea is that you compile a string of HTML containing your directive; then interact with it either by changing the scope or by simulating DOM events; then check that the correct DOM and model are updated correctly.
• I was interested to see that you are using a data-source attribute and then $element[0].dataset.source to access the expression. I guess this was to keep it in line with the usage of the original BootStrap component? @angular-ui/bootstrap : What is our project's view on this?
• Also do you intend to support all the other options available on the original component? I.E. items, minLength, matcher, sorter, etc...

[GuillaumeBiton]

Hi,

I need some help unfortunately:

• don't know how to replace eval() to get the source.
• Do I have to put the controller outside the directive like template ?
• You're right, I keep the data-source attributes to keep the way bootstrap does. We could use a angularjs model instead to do it as the angular way.
• I can try to support all features of the original component.

[petebacondarwin]

Hi @GuillaumeBiton. Great! We are getting there.

Eval

Eval is indeed evil!! Instead in Angular we eval things against a scope ($scope.$eval). This is much less evil :-) This will also allow you to move your source array onto a scope and define it in a controller.

Even better, we ought to watch this source expression for changes. On top of that I think we should not be tying ourselves to using the dataset. AngularJS will automatically match source="" and data-source="" attributes on the element and convert them to a canonical form, which in this case is source.

The easiest way of achieving this binding (i.e. watching and evaluating) is to add a property to the scope field on the directive definition object:

  ...
  scope: {
    'source': '='
  },
  ...

Angular will now keep the value of the expression in the source attribute in synch with the value of scope.source inside the directive.

Controller

At the moment it is no problem to have the controller declared inline inside the directive, but when you come to test, it can be easier to declare it separately, so that you can unit test the controller in isolation.

Finally

Can you explain what this is for in the template?

 on-keyup-fn="handleKeypress"

[GuillaumeBiton]

@petebacondarwin, I don't know how to thank you.

Thanks for the explanation on eval. It makes me understand lot of things that I have tried before with angular and directives.

In template

on-keyup-fn="handleKeypress"

this is a mistake. I forgot to delete these which was used in a first implementation attempt to bind the keyup events.

[petebacondarwin]

This is very nice. Good to merge but first one last thing. Can you move the array list in the demo into a controller in a separate file, demo.js? See the other modules for examples.

[pkozlowski-opensource]

@GuillaumeBiton This looks awesome, thnx so much for this pull request!

At the moment I'm on something else but later today Peter or me will try to merge it into a separate branch and play with it. I will also try to stub some tests so you can extend them in a separate PR. There are other enhancements that can be added but this is a very good start!

[GuillaumeBiton]

One thing interesting with the typeahead jquery plugin of Bootstrap is that you can override some methods like matcher or sorter methods.

It could be interesting to do the same with the angular version but I don't really know how to do it.
is it possible to override the directive scope with the parent scope ? I know that child scope inherit from the parent scope but angular first looks at the child scope. So I will have to test for options in the parent scope before sets the default ?

[pkozlowski-opensource]

@GuillaumeBiton All those things are possible and really easy - it is enough to specify an attribute that would accept an expression evaluating to a function. Then, we can get a hand of the actual function instance by invoking $eval on Scope: http://docs.angularjs.org/api/ng.$rootScope.Scope

As soon as this is done we could use this function instance as a callback - basically invoking it any time we need to. I don't know if this explanation makes any sense but we will definitively have some iterations on this code. I would also like to add some unit tests before we add more features. With unit tests we can see how the directive behaves under different browsers.

[pkozlowski-opensource]

@petebacondarwin starting to look into this PR right now, please tell me if you were planning to do so as well so I can switch to something else.

[pkozlowski-opensource]

@GuillaumeBiton I think that I might have been a bit over-enthusiastic :-) This PR will require some more work. As I've said this is a good start but in reality it will have to be more complex that I've initially thought.

The biggest problem, as for now, is that the implementation has an input buried in a template so I can't set any attributes on a input (for example - validation). This directive probably should be turned into an attribute one so I could use it on any existing input.

The other problem - steaming from the first one - is that ng-model is bound to a transcluded scope so capturing model changes is not easy. On top of this the name of the ng-model is hard-coded to query at the moment. Generally speaking we need a better integration with the NgModelController.

@GuillaumeBiton I know that the above might sound a bit cryptic so I will be toying with the code / ideas a bit and try to push into a separate branch in this repo. Feel free to ping me or @petebacondarwin if you need more info but for now it can't be merged as-is.

[pkozlowski-opensource]

@GuillaumeBiton I've hacked together an alternative approach where we can keep inputs (and ng-model) outside of a template. Please have a look and let me know WDYT.

[GuillaumeBiton]

@petebacondarwin, @pkozlowski-opensource,

With your advices and the idea from pull request #57, I have modify the code. It's much simpler, allow to override some methods.

So here in action : http://jsfiddle.net/guillaumebiton/sFNhJ/

Now I have to deal with these questions :

• is it necessary to allows ng-model in the input ? maybe to be able to made a query string interacts with another component ?
• as @pkozlowski-opensource shows, it's easy to add an attribute to define a specific source. what's your opinion ? the original allows data-source or source when calling the jquery module.
• I have an issue using yeoman that I don't reproduce on jsfiddle. do you use yeoman and do you have an issue ?

[pkozlowski-opensource]

@GuillaumeBiton Just had a quick look. I will comment on some code fragments in the PR, but for me the main issue is that we don't have access to the input element as it is part of the template. This means that I can't:

• specify value of the ng-model
• specify any other attributes (for example - validation).

I will let @petebacondarwin comment more but IMHO this directive should be an attribute on an input.

As for yeoman - no, we are not using it at the moment, our build is based on Grunt.js + Testacular.

BTW: Travis build is failing (lint issues). You should run grunt (or grunt.cmd on windows) to check if things are building correctly.

[pkozlowski-opensource]

@GuillaumeBiton I've just opened the issue #114 to discuss syntax of this directive. Your PR is a great inspiration but we need to discuss things a bit more before things get merged. Please join the discussion in #114!