3
\$\begingroup\$

I am writing a Type-ahead / Autocomplete search with angular using directive and filter service. I am almost there but need your expert opinions on things which I am planning to achieve.

Git Repository

@Todos:

  1. Look and feel

  2. Allow navigation using keyboard

  3. Enter and Esc key handling

Could anyone review and suggest more optimize ways of writing this?

Also, shall I bind events within a parent controller scope or directive scope? Here is how my directive looks like:

var modServiceDef = angular.module('modSerDefinition', []);

modServiceDef.controller('modDefCtrl', ['$scope', 'myFactory', function($scope,  myFactory) {
    myFactory.getData().then(function (httpData) {
            $scope.dataToPopulate = httpData;
        },function(httpData) {
            console.log('name retrieval failed.');
        });
    }
]); 


modServiceDef.directive('typeAhead',[function() {
    return {
        scope : {
            cinfo:'=datalist',
        },
        link:function(scope,element,attr) {
            scope.visible = true;
            scope.updateSearchTerm = function(selName) {
                scope.sterm = selName;
            }
        },
        templateUrl:'templates/typeahead.tmpl'
    }
}]); 

modServiceDef.factory('myFactory', function($http,$q) {
    var factory = {};
    var def = $q.defer();
    factory.getData = function() {
        $http.get('https://raw.githubusercontent.com/dominictarr/random-name/master/first-names.json')
            .success(function(res){ 
                factory.data = res;
                def.resolve(res);
            })
            .error(function(err){
                def.reject(err);
            })
            return def.promise;
        }
    return factory;
});

HTML:

<type-ahead datalist="dataToPopulate"></type-ahead>

Template:

<input type='search' id='idSearch' ng-model='sterm' placeholder='Enter Search Term' />
<ul ng-if='sterm.length'>
  <a href='#'>
    <li ng-model='selName' ng-click='updateSearchTerm(data)'  ng-repeat="data  in cinfo| filter:sterm as results track by $index " class='listdata'>
      {{data}}
    </li>
  </a>
</ul>
\$\endgroup\$
3
\$\begingroup\$

There are some points you could improve:

  • App structure: I would put the typeahead directive in an own module that you can include as a dependency to your app. angular.module('demoApp', ['pdTypeAhead']).
  • Click events: I think the best place is in the directive controller because they're there for your typeahead logic.
  • Keyboard binding: You can add the mousetrap.js keyboard binding in a directive and add it in your typeahead link method. It's a bit tricky because you need to change the typeahead restrict to attribute (otherwise there is a problem with infinite digest loop.).
  • Your todos:

    • look & feel: Twitter Bootstrap is a good starting point to do your styling.
    • Keyboard navigation: You could do it with pure js but I think mousetrap.js is easier to use.
    • Enter + Escape handling: Also use mousetrap. For event handling you could use $scope.$brodcast('eventName', data) and catch it in your directive's controller with $scope.$on('eventName', function(data) { ... });
    • Responsiveness: If you'd type the letter a in your code it took pretty long to show the result (> 3000 matches). I think it's OK to limit the result to 100 or lower matches with a limitTo filter and the result is immediately visible.

You can find my code in this fork. (If you like I could do a pull request in your repo.)

There are still the following possible improvements in the updated code:

  • Controller of the typeahead directive could be in it's own file. Improves readability.
  • It's probably possible to put code from the typeahead controller into a service for refactoring.
  • Escape key handling could be improved: After a click on a name you can't undo that click with a press on escape. But if you select with enter you can undo with escape.
  • Directive template typeahead.tmpl could be moved to the components folder of the directive js/components/typeahead.
  • Typeahead controller: Use controllerAs instead of $scope (if possible). Best practice, see style guide.
  • Check if filtering is also possible directly in ng-repeat? Should be possible and better. I've just moved it to js for easier debugging.
  • Add options to typeahead, e.g. a way to enable or disable autofocus of search input directly in markup e.g. <div type-ahead datalist="dataToPopulate" auto-focus="off">
  • Add a config provider for options service so you could add the options application wide inside of angular.module(...).config(...). This post is helpful for creating configurable directives.
  • Add ng-blur to close the typeahead if clicked outside.
  • Add unit tests for the directive.
\$\endgroup\$
1
  • \$\begingroup\$ please do the pull request . i will take it up then \$\endgroup\$
    – Robin
    Jun 21 '15 at 6:17
2
\$\begingroup\$

You could use the implicit version of deps management in Angular. It saves you a few keystrokes, just make sure the dependency variable name is the same as the dependency name. If you plan to minify, just run it through ng-annotate.

modServiceDef.controller('modDefCtrl', function($scope,  myFactory) {
  ...
}); 

I'm not sure why your code appears to recycle promises in myFactory when $http.get returns a promise already. Also, logging the http error is the responsibility of myFactory and not your controller, so you console.log should be there. I suppose changing it to this would be better:

modServiceDef.factory('myFactory', function($http,$q) {
  return {
    getData: function() {
      return $http.get('https://raw.githubusercontent.com/dominictarr/random-name/master/first-names.json')
        .error(function(){
          console.log('name retrieval failed.');
        });
    }
  };
});

Other than those, I think your code is good.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.