Fix returning callback return value.

[theclapp]

Propagate the return value of callbacks correctly, so that if you return “false”, the “default action” is not executed and the event ceases to propagate.

[theclapp]

This is my first PR ever, I hope I did it correctly. I could not figure out how to not include the update to .gitignore; sorry about that.

I wrote and submitted this PR and then browsed the others (currently two), and it appears that #37 is for similar functionality using a different method. I think I did it the right way.

Side note: at one point you say

         if (shouldExecute) {
            wrapApply(_callback.apply(this, arguments));

This is wrong (I think). wrapApply returns a closure. The only reason this works is because the call to wrapApply actually calls the callback. (I fixed this in this PR.)

[chieffancypants]

Can you show me an example code snippet that uses this? Seems like a good idea in theory, but I'd like to understand the usecase. Thanks!

Also don't worry about the other commit, I'll just cherry-pick the first one.

[theclapp]

Here it is in my app: https://github.com/theclapp/ntla/blob/master/app.js#L376

If that's too much context: I found that the "0" I'm using was propagated to my Codemirror editor window after I focused it (in the hotkey callback), which I didn't want. Returning "false" in the callback only works with the given patch.

[chieffancypants]

This can be accomplished by preventing default action from taking place in the browser (the default action being typing the letter into the input). See this response from a previous issue. The callback will pass you the event, which you can use to event.preventDefault().

That said, I'm still interested in merging this for other potential uses and modify the tests accordingly. I'm a bit busy at the moment on other things, so hopefully this solves your issue as it could be a bit before I'm able to focus on it.

Thanks again!

[theclapp]

Thanks for the response, Wes. Would it make it easier if I added this to the tests on my end? (Though I understand that even given a perfect PR (for whatever value of perfect you care to define) you still might not have time to do anything with it. :)

[chieffancypants]

Sure, that would definitely be helpful!

[margru]

I vote for this solution instead of mine #37 since this is more general.

[czema]

I was about to write this patch and saw this pull request. Even though you can always to do event.preventDefault(), returning false from an event handler is a common alternative. It also simplifies the callback, especially for controller methods which weren't originally designed with an event parameter.

$scope.func = function() {
    ...
    return false;
}

[theclapp]

I will not be able to update this PR any time soon. I invite interested parties to reproduce it in their own branch and submit it themselves. It's only 12 lines of changes, not counting .gitignore and blank lines.