0

I'm attempting to create a directive that binds specific keypresses to functions specified in the scope of a controller, but all of the callback functions seem to be overridden by the last callback function in the object containing the bindings. I've tried using keymaster.js and mousetrap.js for binding the events with the same results.

Code in a JSFiddle

The Javascript code:

angular.module('app', ['directives', 'controllers']);

angular.module('directives', [])
.directive('keypress', [function () {
    return function (scope, element, attrs) {
        // console.log(scope, element, attrs);
        var attribute = scope.$eval(attrs.keypress || '{}');
        for (var k in attribute) {
            console.log('binding ' + k + ' as ' + attribute[k]);
            Mousetrap.bind(k, function() { return attribute[k](scope, element); });
        }
    };
}]);

angular.module('controllers', [])
.controller('TodoController', function($scope) {
    $scope.shortcuts = {
        'w': function () { console.log('w'); },
        's': function () { console.log('s'); },
        'a': function () { console.log('a'); },
        'd': function () { console.log('d'); }
    };
});

The HTML File:

<html>
  <head>
    <script src="http://ajax.googleapis.com/ajax/libs/angularjs/1.0.3/angular.js"></script>
    <script src="https://raw.github.com/ccampbell/mousetrap/master/mousetrap.min.js"></script>
    <script src="/javascript/app.js"></script>
  </head>
  <body>
    <div ng-app="app">
      <div ng-controller='TodoController' keypress='shortcuts'>Foo</div>
    </div>
  </body>
</html>

Why is 'd' always written to the console, regardless of whether I press 'w', 'a', 's', or 'd'?

iand675
  • 1,118
  • 1
  • 11
  • 23

1 Answers1

6

You've fallen for a common trap: variables in JavaScript are always function scoped. When you do this:

for (var k in attribute) {
    console.log('binding ' + k + ' as ' + attribute[k]);
    Mousetrap.bind(k, function() { return attribute[k](scope, element); });
}

With that bind(), you're creating four closures that all close over the variable k—but they all close over the same variable. You don't get a new one for each run of the loop. The console.log works perfectly because the value of k is used immediately. The closure doesn't evaluate k until it's actually run, and by then, its value has changed to whatever it was when the loop finished.

Depending on your target audience, the easiest way by far to fix this is to use let instead of var. let does block scoping (which works about how you'd expect), but is a fairly recent invention, and I'm not sure how well it's supported.

Otherwise, to get a new scope, you need a new function:

for (var k in attribute) {
    (function(k) {
        console.log('binding ' + k + ' as ' + attribute[k]);
        Mousetrap.bind(k, function() { return attribute[k](scope, element); });
    })(k);
}

This passes the outer k to the function's inner k, which will be a different variable every time. You could also split this out into a little factory function, but for something this tiny, I wouldn't bother.

Eevee
  • 47,412
  • 11
  • 95
  • 127
  • Perfect, thanks so much! So by defining and subsequently calling that anonymous function, we're actually retaining the *value* of k, rather than a reference? – iand675 Jan 15 '13 at 03:27
  • @iand675: it's not reeeally about value vs reference, but about what creates distinct variables. each iteration of the loop uses the same variable. each call of the anonymous function creates a _new_ variable—but they happen to all have the same name. – Eevee Jan 15 '13 at 03:38