Skip to content
This repository was archived by the owner on Mar 17, 2025. It is now read-only.

Minor fixes to $firebaseAuth #468

Merged
katowulf merged 1 commit into from
Nov 17, 2014
Merged

Minor fixes to $firebaseAuth #468

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/FirebaseAuth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@
},

// Authenticates the Firebase reference with a custom authentication token.
authWithCustomToken: function(authToken) {
authWithCustomToken: function(authToken, options) {
var deferred = this._q.defer();

this._ref.authWithCustomToken(authToken, this._onLoginHandler.bind(this, deferred));
this._ref.authWithCustomToken(authToken, this._onLoginHandler.bind(this, deferred), options);

return deferred.promise;
},
Expand Down Expand Up @@ -138,14 +138,14 @@
// Asynchronously fires the provided callback with the current authentication data every time
// the authentication data changes. It also fires as soon as the authentication data is
// retrieved from the server.
onAuth: function(callback) {
onAuth: function(callback, context) {
var self = this;

this._ref.onAuth(callback);
this._ref.onAuth(callback, context);
Copy link
Contributor

@katowulf katowulf Nov 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should be doing any internal error handling here, perhaps triggering events in the Angular scope. Hrm. Something to think about.

Copy link
Author

@jwngr jwngr Nov 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Their callback will already receive the errors and they can do what they want with them.

Copy link
Contributor

@katowulf katowulf Nov 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callbacks aren't terribly convenient in all architectures. An event broadcasting strategy can be pretty handy for things like this. It's not impossible or particularly hard to add a .run() method that implements this, if one is versed in Angular and AngularFire, but it's worth pondering as part of the API as lots of people utilize $emit and $broadcast as a basic pattern (particularly when talking to directives)

Copy link
Author

@jwngr jwngr Nov 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, add it to our discussion list for the meeting this week. Sounds like a reasonable idea.

Copy link
Contributor

@jamestalmage jamestalmage Nov 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #474 for an alternative idea.


// Return a method to detach the `onAuth()` callback.
return function() {
self._ref.offAuth(callback);
self._ref.offAuth(callback, context);
};
},

Expand Down