Skip to content

Cancel Google One Tap on login unmount#26

Open
TheShahnawaaz wants to merge 2 commits into
mainfrom
codex/fix-onetap-cleanup
Open

Cancel Google One Tap on login unmount#26
TheShahnawaaz wants to merge 2 commits into
mainfrom
codex/fix-onetap-cleanup

Conversation

@TheShahnawaaz
Copy link
Copy Markdown
Owner

Summary

  • Reuse one cleanup function for Google One Tap cancellation
  • Ensure One Tap is cancelled when the Google script was already loaded before the login page mounted

Verification

  • npm run build in frontend

Copilot AI review requested due to automatic review settings April 24, 2026 10:14
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cab-management-system Ready Ready Preview, Comment Apr 24, 2026 10:56am
cab-management-system-b59a Ready Ready Preview, Comment Apr 24, 2026 10:56am

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the frontend Login page’s Google One Tap integration to ensure the One Tap prompt is properly cancelled on unmount, including when the Google script is already present before the page mounts.

Changes:

  • Extracted a reusable cancelGoogleOneTap cleanup function.
  • Returned the cleanup function from the useEffect even when the Google script is already present (previously returned early without cleanup).

Comment thread frontend/src/pages/Login.tsx Outdated
Comment on lines 102 to 105
if (existingScript) {
initializeGoogleOneTap();
return;
return cancelGoogleOneTap;
}
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

existingScript only indicates the tag exists, not that window.google.accounts.id is ready. If the script is still loading, initializeGoogleOneTap() returns early and One Tap will never be initialized because no load handler is attached in this branch. Consider checking for window.google?.accounts?.id and, if missing, attaching a one-time load listener to the existing script (or polling) before calling initializeGoogleOneTap().

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Addressed in 6d1a4a5 by checking whether google.accounts.id is ready before initialization and attaching a one-time load listener when the existing script is still loading.

Comment thread frontend/src/pages/Login.tsx Outdated
Comment on lines +107 to +114
@@ -105,11 +111,7 @@ export default function Login() {
script.onload = initializeGoogleOneTap;
document.head.appendChild(script);

return () => {
const googleClient = (window as Window & { google?: GoogleOneTapClient })
.google;
googleClient?.accounts?.id?.cancel();
};
return cancelGoogleOneTap;
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The cleanup only calls google.accounts.id.cancel(). If the component unmounts before the script finishes loading, script.onload can still fire later and run initializeGoogleOneTap() (with stale closures), potentially prompting One Tap when the Login page is no longer mounted. Consider clearing the onload handler (or using addEventListener + removeEventListener) and/or guarding initializeGoogleOneTap() with an isMounted/cancelled flag set by the cleanup.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Addressed in 6d1a4a5 by using a cancelled guard and removing the load listener during cleanup, so a late script load cannot initialize One Tap after the login page unmounts.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

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.

2 participants