FIX: show loading spinner when redirecting to discourse connect #34135
+14
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On "slow" computers and/or in production builds of the client-side application, the redirection to discourse connect (via window.location) might happen _after_ the login/signup route is done loading and the login/signup template is being shown. Since when discourse connect is enabled, no other auth "provider" is allowed, we "flashed" a screen that indicated there was no "login method" configured. In order to fix this, we rely on the "isRedirectingToExternalAuth" boolean and ensure it's set to "true" when discourse connect is enabled. This is unfortunately ~~impossible~~ very hard to test as it depends on how fast the browser running the test is...
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a UX issue where a "no login method configured" screen briefly flashes before redirecting to discourse connect on slower systems. The fix ensures a loading spinner is shown during the redirection process.
- Modified the
isRedirectingToExternalAuthcontroller property to include discourse connect scenarios - Changed
isRedirectingto a private field (#isRedirecting) for better encapsulation
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/assets/javascripts/discourse/app/routes/signup.js | Updated to show loading spinner when discourse connect is enabled and made isRedirecting field private |
| app/assets/javascripts/discourse/app/routes/login.js | Applied identical changes to login route for consistency |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On "slow" computers and/or in production builds of the client-side application, the redirection to discourse connect (via window.location) might happen after the login/signup route is done loading and the login/signup template is being shown.
Since when discourse connect is enabled, no other auth "provider" is allowed, we "flashed" a screen that indicated there was no "login method" configured.
In order to fix this, we rely on the "isRedirectingToExternalAuth" boolean and ensure it's set to "true" when discourse connect is enabled.
This is unfortunately
impossiblevery hard to test as it depends on how fast the browser running the test is...The only way I was able to reproduce this issue locally was to throttle both the CPU and network in Chrome's performance tab.
I also renamed
isRedirectingto#isRedirectingto better indicate this is a private variable.BEFORE
before.mov
AFTER
after.mov