-
Notifications
You must be signed in to change notification settings - Fork 350
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
Configure or Handle Timeout on Popup Authentication #92
Comments
I agree that will be super useful. Do you want to send a PR with that? If you can't, I'll add our near term backlog to make sure we work on this. |
Same here, I feel the 30s timeout is way too low :( I would really liked there to be a way to configure custom timeout + way to close popup on |
@mwillbanks @Enngage I'd love your feedback on #133 🎉 |
@luisrudge awesome, thank you! Could you possibly share the reasoning behind the default 30sec timeout? Many users fail to fill in their credentials in such short time. I like that you can close the popup from within the error using |
@Enngage I'm not sure where you saw a 30s default. I think we always used a 60s timeout. |
You can fetch the pop-up today it's a hack but works with most browsers: |
@luisrudge I thought I saw 30 seconds somewhere, but maybe I confused it with something else :-) Either way the PR looks good and would definitely help accomplish both my issues I have currently with this package. Thank you! |
Yes, I can get the reference of the popup in the code, but there's no API that would expose the popup before the code actually runs. For example, how would you get a reference to the popup with this code: We could change it to be something like this:
But then we'd be penalizing the most used scenario for a niche use case. |
When using loginWithPopup:
An issue arrises that when an error occurs such as leaving the window open until it times out there is not a recourse to shut the popup window, possibly this should be a parameter with the
getTokenWithPopup
where a parameter such asclosePopupOnError
or otherwise would exist. We should also likely be able to control the timeouts associated with this rather than the hard coding to 30s inside of the util script.This is a fairly large blocker for us but I am not certain how to handle this other than forking the current code and re-integrating it that way.
The text was updated successfully, but these errors were encountered: