search

janpantel / angular-sails

An angular module for using the sails socket.io api
MIT License
307 stars 56 forks source link

Issue #60 - `off` with function doesn't work #79

Closed cheplv closed 9 years ago

cheplv commented 9 years ago

Usage: var eventType = "message"; var eventTypeFn = $sails.on(eventType); $scope.$on('$destroy', function() { ...... $sails.off(eventType, eventTypeFn); ..... });

TheSharpieOne commented 9 years ago

The usage would require a function be passed as the second argument, but that is just missing from the comment and the PR looks fine. One thing I would note is that typically you would pass the original function as the second argument to off, and not what on returned. If you have to use what on returned, I'd say it should be more like angular's $watch and what on returned would be a function, which when called removes the listener.

TheSharpieOne commented 9 years ago

Can you add some tests for this? (on a function/spy, expect it to be there (trigger the event, make sure the spy is called). off the thing that was oned, expect it to not be there (trigger the event, make sure the spy is not called))

cheplv commented 9 years ago

Sorry - in description was missed callback i.e. usage will be as example: function eventMessageHandler(msg) { .... } var eventType = "message"; var eventTypeFn = $sails.on(eventType, eventMessageHandler);

As I see there is created anonymous function which needs to be passed to $sails.off. This style of change also can be apply'ed to v2.x - there is same problem as I see

cheplv commented 9 years ago

$sails.off must be called directly. Hope this will solve issue