fix(frontend): restore login flow loading states#1852
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a typed two‑step email→continue→password flow with saved‑session checking and Turnstile CAPTCHA lifecycle/state; centralizes domain-check loading state; updates E2E helpers/tests to use the intermediate step; adjusts post‑login redirect matching; and changes Playwright frontend startup to a local env command. (50 words) Changes
Sequence DiagramsequenceDiagram
actor User
participant LoginPage as Login Component
participant DomainAPI as Domain Check API
participant AuthAPI as Auth API
participant Turnstile as CAPTCHA Widget
participant Router as Router
User->>LoginPage: Enter email
LoginPage->>DomainAPI: Check domain (has_sso?)
DomainAPI-->>LoginPage: Respond has_sso
LoginPage->>LoginPage: Update isEmailStepBusy, enable/disable Continue
User->>LoginPage: Click Continue
LoginPage->>LoginPage: Reveal password field
User->>LoginPage: Enter password
User->>Turnstile: Complete CAPTCHA
Turnstile-->>LoginPage: Return token
LoginPage->>AuthAPI: Submit credentials + token
alt Auth success
AuthAPI-->>LoginPage: Success
LoginPage->>Router: await router.replace('/apps' or '/dashboard')
Router-->>User: Redirect
else Auth failure
AuthAPI-->>LoginPage: Error
LoginPage->>Turnstile: Reset widget / clear token
LoginPage->>LoginPage: Update flags / show errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e7364923b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/login.vue (1)
505-514: Extract repeated spinner markup into a reusable component/helper.The same SVG block is duplicated across multiple buttons. Centralizing it will reduce drift and simplify future style/test updates.
Also applies to: 545-554, 601-610, 681-690, 755-764
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/login.vue` around lines 505 - 514, Create a reusable spinner component (e.g., LoadingSpinner) that encapsulates the SVG and accepts class/name props (or simply forwards attributes) and the data-test value; then replace each duplicated SVG block in src/pages/login.vue (the repeated spinner used with v-if="isLoading") with the new component (e.g., <LoadingSpinner v-if="isLoading" />), import/register the component in the Login component, and ensure the component forwards classes/attributes so existing styling and data-test="loading" continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/login.vue`:
- Around line 347-350: The branch that handles a falsy result from
autoAuth(route) (where logSession is falsy) returns early without calling
hideLoader(), which can leave the global loader visible; update the conditional
in the login flow so that before returning when !logSession you call
hideLoader() (and ensure isLoading.value is set appropriately), or wrap the
autoAuth(route) call and subsequent logic in a try/finally that calls
hideLoader() to guarantee the loader is hidden; look for symbols autoAuth,
logSession, isLoading.value and hideLoader to apply the fix.
- Around line 373-427: The try/finally in checkLogin() resets
isCheckingSavedSession but doesn't catch errors causing loader/UI
inconsistencies; wrap the existing try body with a catch that logs the error (or
shows a toast), sets isLoading.value = false, calls hideLoader(), and ensures
any other UI flags (e.g., hasQuerySession.value) are left consistent before
rethrowing or swallowing as appropriate; specifically modify the function
checkLogin() to add a catch block that handles errors from
supabase.auth.getClaims(), getSession(), exchangeCodeForSession(),
checkAuthUser(), and checkMagicLink() and performs the cleanup (isLoading.value
= false, hideLoader(), optionally toast.error or console.error) before the
finally block runs to reset isCheckingSavedSession.value.
---
Nitpick comments:
In `@src/pages/login.vue`:
- Around line 505-514: Create a reusable spinner component (e.g.,
LoadingSpinner) that encapsulates the SVG and accepts class/name props (or
simply forwards attributes) and the data-test value; then replace each
duplicated SVG block in src/pages/login.vue (the repeated spinner used with
v-if="isLoading") with the new component (e.g., <LoadingSpinner v-if="isLoading"
/>), import/register the component in the Login component, and ensure the
component forwards classes/attributes so existing styling and
data-test="loading" continue to work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11230ed6-4152-410e-b324-6501635382bb
📒 Files selected for processing (3)
playwright/e2e/auth.spec.tsplaywright/support/commands.tssrc/pages/login.vue
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4b24ea70c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/pages/login.vue (3)
582-591: Consider extracting the spinner SVG into a reusable component.The same inline spinner SVG is repeated five times across the template. Extracting it would reduce duplication and simplify future styling changes.
Example extraction
Create a small component or use a shared template fragment:
<!-- components/LoadingSpinner.vue --> <template> <svg class="inline-block mr-3 -ml-1 w-5 h-5 text-white align-middle animate-spin" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" > <circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" /> <path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z" /> </svg> </template>Then replace inline SVGs with
<LoadingSpinner v-if="isLoading" />.Also applies to: 622-631, 685-694, 772-781, 846-855
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/login.vue` around lines 582 - 591, Extract the repeated inline spinner SVG into a reusable Vue component (e.g., LoadingSpinner) and replace each inline SVG instance that uses v-if="isLoading" (the SVG block present in src/pages/login.vue and repeated at the other locations) with <LoadingSpinner v-if="isLoading" />; ensure the new component preserves the original classes (inline-block mr-3 -ml-1 w-5 h-5 text-white align-middle animate-spin), viewBox, and child elements so styling/behavior is identical, then import/register LoadingSpinner in the parent component(s).
507-523:nextLogin()is not awaited, unlike other call sites.At line 522,
nextLogin()is called withoutawait, while all other invocations (lines 163, 345, 436, 482) await it. This inconsistency could cause the function to return before navigation completes.Proposed fix
hasQuerySession.value = false querySessionAccessToken.value = '' querySessionRefreshToken.value = '' - nextLogin() + await nextLogin() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/login.vue` around lines 507 - 523, The acceptQuerySession function calls nextLogin() without awaiting it, causing inconsistent behavior compared to other call sites; update acceptQuerySession to await nextLogin() after successful session set (ensure isLoading.value is handled appropriately around the await) so navigation completes before the function returns—locate the acceptQuerySession function and change the final call to await nextLogin() to match the other usages where nextLogin is awaited.
64-70: Consider typingwindow.turnstileto avoid implicitany.
window.turnstileis accessed without a type declaration. If strict TypeScript is enabled, this may produce a warning.Optional fix
Add a type declaration in a
.d.tsfile or inline:declare global { interface Window { turnstile?: unknown } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/login.vue` around lines 64 - 70, The code reads window.turnstile without a type, causing implicit any under strict TS; add a type declaration for turnstile (e.g., augment the global Window interface with turnstile?: unknown or the appropriate type) in a .d.ts or module file so references in the captcha init logic (captchaInitTimeout, clearCaptchaInitTimeout, and the check `window.turnstile`) are properly typed; alternatively, narrow or cast `window` access at the use site to the declared type to satisfy the compiler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/login.vue`:
- Around line 582-591: Extract the repeated inline spinner SVG into a reusable
Vue component (e.g., LoadingSpinner) and replace each inline SVG instance that
uses v-if="isLoading" (the SVG block present in src/pages/login.vue and repeated
at the other locations) with <LoadingSpinner v-if="isLoading" />; ensure the new
component preserves the original classes (inline-block mr-3 -ml-1 w-5 h-5
text-white align-middle animate-spin), viewBox, and child elements so
styling/behavior is identical, then import/register LoadingSpinner in the parent
component(s).
- Around line 507-523: The acceptQuerySession function calls nextLogin() without
awaiting it, causing inconsistent behavior compared to other call sites; update
acceptQuerySession to await nextLogin() after successful session set (ensure
isLoading.value is handled appropriately around the await) so navigation
completes before the function returns—locate the acceptQuerySession function and
change the final call to await nextLogin() to match the other usages where
nextLogin is awaited.
- Around line 64-70: The code reads window.turnstile without a type, causing
implicit any under strict TS; add a type declaration for turnstile (e.g.,
augment the global Window interface with turnstile?: unknown or the appropriate
type) in a .d.ts or module file so references in the captcha init logic
(captchaInitTimeout, clearCaptchaInitTimeout, and the check `window.turnstile`)
are properly typed; alternatively, narrow or cast `window` access at the use
site to the declared type to satisfy the compiler.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba8f9a2074
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
1 participant
Summary (AI generated)
Motivation (AI generated)
The new login flow was dropping visible pending states during session bootstrap and TOTP verification, which made the page appear idle while it was still authenticating or redirecting. That caused confusing flashes of the login form for already-authenticated users and allowed submit actions before Turnstile was ready.
Business Impact (AI generated)
This reduces friction and ambiguity during sign-in, especially for returning users and accounts using 2FA or captcha. A clearer login flow lowers the risk of duplicate submissions, perceived instability, and support noise around authentication regressions.
Test Plan (AI generated)
bunx eslint src/pages/login.vue playwright/support/commands.ts playwright/e2e/auth.spec.tsbunx playwright test playwright/e2e/auth.spec.ts playwright/e2e/sso-login.spec.ts --project=chromiumGenerated with AI
Summary by CodeRabbit
Bug Fixes
User Experience
Tests
Chores