Allow site key to be a promise #220
Conversation
There was a problem hiding this comment.
@TheSharpieOne @mtrias I'd like to know your thoughts on my comments.
| scope.$watch('required', validate); | ||
| } | ||
|
|
||
| var removeCreationListener = scope.$watch('key', function (key) { | ||
| // Abort if undefined or null | ||
| if (ng.isUndefined(key) || key === null) { | ||
| return; |
There was a problem hiding this comment.
This early return prevents the service from throwing an error when the key is undefined. Check the failing test.
| 'expired-callback': expired | ||
|
|
||
| // Accept a promise, or resolve immediately if the value is a bare string | ||
| $q.resolve(key).then(function (resolved) { |
There was a problem hiding this comment.
IMO, this should go in the service and not in the directive. The support for key-as-promise would be complete in this project, since ATM the service doesn't support a key as promise.. Also, it could make testing the directive simpler.
There was a problem hiding this comment.
Also, should we use $q.when instead of $q.resolve? The latter was added in Angular 1.4
There was a problem hiding this comment.
You may want to have the feature in the service as well as the directive.
$q.when should be used for the reason mentioned above, more support.
Relates to: #219
This modification makes possible to pass a promise as a site key.