Suggestion: Allow loginConfirmed and loginCancelled from broadcasted event

[christianrondeau]

It's a breaking change but one I would personally love :)

Instead of referencing authService, you could provide the loginConfirmed and loginCancelled callbacks directly on the event arguments of the broadcast. That would remove a dependency. Example:

$rootScope.$on('event:auth-loginRequired', function (args) {
    signin().then(
        function() {
            args.loginConfirmed();
        },
        function() {
            args.loginCancelled();
        });
});

Let me know if you'd like me to suggest a pull request!

[jameskleeh]

+1

[witoldsz]

This looks nice, I wish I did not consider it earlier. But now, what can I do? Should I break the API compatibility and force every one to update theirs code? Maybe once AngularJS 2.0 arrives, the new version of this module could apply this pattern, I like it.

[jameskleeh]

Introducing a new major version doesn't force others to update to it

[witoldsz]

Yes, but I don't feel this one little convenience is enough to release a new major version.

[christianrondeau]

I can still provide a pull request if you are interested in releasing a new version.

Another approach would be to create a second event; it bloats the code but won't break existing implementations.

What do you think?

[witoldsz]

Pull request does not hurt :) You can open one, I won't pull it now, but I will keep an eye on it, so when new major version would be prepared, I could merge it.

[christianrondeau]

With the breaking change or the new event? Even though I prefer the breaking change, it's up to you.

[witoldsz]

Braking change I guess, no need to keep them both...

[christianrondeau]

I just made available a pull request for this (#70). I think it needs a peer review, otherwise feel free to pull if you're happy with it!