Skip to content

feat(auth): add last used login tracking and badge#302

Open
geraldpeng6 wants to merge 2 commits intoimport-ai:devfrom
geraldpeng6:main
Open

feat(auth): add last used login tracking and badge#302
geraldpeng6 wants to merge 2 commits intoimport-ai:devfrom
geraldpeng6:main

Conversation

@geraldpeng6
Copy link
Copy Markdown

  • persist last login method in localStorage
  • render last used badge on login options
  • record last used for email/phone/otp and oauth
  • add i18n label for last used

geraldpeng6 and others added 2 commits January 20, 2026 13:20
- persist last login method in localStorage
- render last used badge on login options
- record last used for email/phone/otp and oauth
- add i18n label for last used
Copy link
Copy Markdown

@omnibox-bot omnibox-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: feat(auth) — add last used login tracking and badge

Critical Issues

1. verify-otp.tsx — Duplicate/incorrect setLastLoginMethod call

Two setLastLoginMethod calls were added to verify-otp.tsx:

  • One hardcodes setLastLoginMethod('email') unconditionally (~line 82)
  • One correctly uses setLastLoginMethod(isPhoneVerification ? 'phone' : 'email') (~line 128)

If both are inside the same handleComplete handler, the hardcoded 'email' fires first for all users — phone OTP users will be mislabeled and see the badge on the wrong method on next login. Remove the hardcoded call and keep only the ternary version.

Also missing: The verifyMagicLink path calls setGlobalCredential and navigates away, but has no setLastLoginMethod call. Magic link logins won't update the last-used badge.


2. util.tsxlocalStorage calls lack try-catch

The new setLastLoginMethod and getLastLoginMethod functions call localStorage without try-catch:

export function setLastLoginMethod(method: LastLoginMethod): void {
  localStorage.setItem(LAST_LOGIN_METHOD_KEY, method); // throws SecurityError in Safari private mode
}

export function getLastLoginMethod(): LastLoginMethod | null {
  const method = localStorage.getItem(LAST_LOGIN_METHOD_KEY); // throws in Safari private mode

localStorage.setItem/getItem throws a SecurityError in Safari private/incognito browsing. Since setLastLoginMethod is called directly after setGlobalCredential in the auth flow, a storage error here can abort authentication after the server has already accepted the user.

The project already has an established pattern for best-effort localStorage in src/hooks/auth-config-context.tsx:

const setCachedConfig = (config: AuthConfig) => {
  try {
    localStorage.setItem(STORAGE_KEY, JSON.stringify(config));
  } catch {
    // ignore storage error
  }
};

Both new functions should follow this same pattern. A failure to record the last-used method — a non-critical UX feature — must never propagate into and abort the authentication flow.


Important Issues

3. login/index.tsx — Broken toggle UX when last-used matches current contact method

const showEmailOption = contactMethod !== 'email' || lastLoginMethod === 'email';
const showPhoneOption = contactMethod !== 'phone' || lastLoginMethod === 'phone';

When contactMethod === 'email' and lastLoginMethod === 'email':

  • showEmailOption = true (because lastLoginMethod === 'email')
  • showPhoneOption = true (because contactMethod !== 'phone' is always true)

Both buttons render simultaneously. The Email button's onClick calls setContactMethod('email'), which is a no-op since you're already on email. Users see a clickable button that does nothing.

The original code showed only one "switch to" button at a time (mutual exclusion). The new logic needs to preserve that toggle behavior while independently displaying the badge on whichever button is visible.


Suggestions

4. en.json — All-caps baked into translation value

"last_used": "LAST USED"

All-caps is a presentation concern and should be handled via CSS (uppercase Tailwind class), not embedded in the translation string. The Chinese translation "上次登录" is sentence case — the two locales are inconsistent. Consider storing "Last used" and adding uppercase to the badge's Tailwind class.


Strengths

  • Clean LastLoginMethod type with runtime validation via the VALID_LAST_LOGIN_METHODS whitelist in getLastLoginMethod — good defensive practice
  • Consistent placement of setLastLoginMethod immediately after setGlobalCredential across all OAuth providers (apple, google, wechat)
  • pointer-events-none on the badge correctly prevents it from interfering with button clicks
  • i18n covered for both EN and ZH

@LucienShui LucienShui changed the base branch from main to dev March 21, 2026 16:48
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.

3 participants