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

Add popup config option to prevent closing the popup #1319

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ashwsn
Copy link

@ashwsn ashwsn commented Nov 8, 2024

Changes

This PR adds a new optional boolean suppressPopupClose field to the PopupConfigOptions interface. When true, this prevents the runPopup util func used by loginWithPopup from closing the popup window after receiving the authorization_response message.

There are two main ways this might be used:

  1. In a Chrome extension, to prevent the popup from closing too early and interrupting the login flow. See Login popup closes before flow is complete, making login impossible in Chrome extensions #1318 for a discussion of why this is needed (tl;dr: closing the popup also closes the extension itself). The popup can then be closed after completion of the login flow, once the app is in a state where it would be okay for the code to be immediately terminated
  2. To add a post-login message to the popup. For example, "You're signed in! Click here to return to the app". Once loginWithPopup is complete, you could change the href of the popup to whatever post-login page you want.

(Note: in both scenarios, it is assumed the popup is also being passed in the config options, so that it can be managed outside of loginWithPopup)

I've also added a section to the FAQs that discusses using this option for login in Chrome extensions (as well as a brief discussion of logout for extensions, which is not directly impacted by this change but is related and was lacking documentation)

References

Closes #1318

Testing

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of the platform/language

Checklist

@ashwsn ashwsn requested a review from a team as a code owner November 8, 2024 15:59
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.

Login popup closes before flow is complete, making login impossible in Chrome extensions
1 participant