Skip to content

Conversation

@jfturcot
Copy link
Contributor

I needed a different event for when a user manually logs out, this would work for me without breaking anything already in the library.

@andreareginato
Copy link
Collaborator

Actually, I think this could be the real logout event. If the logout is fired right now on "login" there is a logic mistake. Do you think you can we can clean up the logic before the merge?

  • logout event only when the user manually log out.

@jfturcot
Copy link
Contributor Author

I agree with you, but if the oauth:logout event is moved to another function, it might break some people's code already using it at page refresh. That is why I called this event tokenDestroy to make sure legacy code would still work until you decide to have a major version change.

@andreareginato
Copy link
Collaborator

You have a good point. The thing is that people using the logout event will expect the real logout. I think this is acceptable "braking update".

@jfturcot
Copy link
Contributor Author

How about this?

@andreareginato
Copy link
Collaborator

If the README is updated and the tests are passing, I think it's perfect. Can you confirm both of them?

@jfturcot
Copy link
Contributor Author

The tests are passing and I don't see any information in the README about the events. There is an Events section on the website, I could add the oauth:loggedOut event documentation there since the oauth:logout documentation there wouldn't change. I guess that would be a different pull request, I never sent a pull request for the gh-pages branch, but I could do it if you wish.

@andreareginato
Copy link
Collaborator

Would ne perfect to integrate it. Palese send it.
Il 29/ott/2014 19:51 "Jean-Francois Turcot" notifications@github.com ha
scritto:

The tests are passing and I don't see any information in the README about
the events. There is an Events section on the website, I could add the
oauth:loggedOut event documentation there since the oauth:logout
documentation there wouldn't change. I guess that would be a different pull
request, I never sent a pull request for the gh-pages branch, but I could
do it if you wish.


Reply to this email directly or view it on GitHub
#45 (comment)
.

andreareginato added a commit that referenced this pull request Dec 3, 2014
Broadcast event oauth:tokenDestroy after a logout
@andreareginato andreareginato merged commit 9fcb16a into angularjs-oauth:master Dec 3, 2014
@andreareginato
Copy link
Collaborator

Just missing the docs in the gh-pages branch. Whenever you have time :)
Thanks a lot @jfturcot

@jfturcot
Copy link
Contributor Author

jfturcot commented Dec 3, 2014

Oh right, sorry, I will get on that once I get a minute later today. Thanks for merging this.

@jfturcot jfturcot deleted the token_destroy_event branch December 3, 2014 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants