Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Abort current implicit flow #537

Merged
merged 2 commits into from
Jul 17, 2019

Conversation

enricodeleo
Copy link
Contributor

Allow aborting the current implicit flow. This allows restarting an
implicit flow within SPAs.

Allow aborting the current implicit flow. This allow restarting an
implicit flow within SPAs.
@jeroenheijmans
Copy link
Collaborator

Changes look good to me. I'm curious though, what is a practical scenario where you'd need this? Haven't encountered the need for this myself yet.

@enricodeleo
Copy link
Contributor Author

Thanks for your review @jeroenheijmans.

Basically when using a SPA (for example a cordova app) if an user abort the flow by closing the browser or going back with the ios "back to app" button he cannot re-initialize (hence open the browser again) unless he kills the app.

@jeroenheijmans
Copy link
Collaborator

Ah, ok, makes sense.

Thinking some more on your PR, the name might be confusing without context. That is, when I'd see abort...() as a method name, I think a running process or promise chain might get aborted/cancelled. No such thing would happen with your proposal. So possibly resetInImplicitFlow() or some such might be a better name?

Possibly update the comment as well to something like "Forcefully resets the service's tracking of whether the implicit flow is currently doing work".

But both are minor suggestions, ofc.

@manfredsteyer manfredsteyer merged commit 5e28f54 into manfredsteyer:master Jul 17, 2019
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.

4 participants