Feature/layout signin signup#81
Merged
Merged
Conversation
…of its parent, and use google identity services api instead of custom interface for typing
Contributor
|
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughModernizes auth UIs: introduces PanelHeader/PanelError, consolidates form state and errors, updates globals/input utilities, refactors Google Identity Services button for guarded inline init and responsive sizing, and adjusts MFA verification layout. ChangesAuth UI Panel Components and State Refactoring
Sequence Diagram(s)sequenceDiagram
participant ResizeObserver as ResizeObserver
participant GoogleLoginButton as GoogleLoginButton
participant GoogleScript as GIS Script
participant GoogleAccounts as google.accounts.id
participant BackendAuth as Backend Auth
ResizeObserver->>GoogleLoginButton: report containerWidth
GoogleLoginButton->>GoogleScript: reuse or inject GIS script tag
GoogleScript-->>GoogleLoginButton: script loaded
GoogleLoginButton->>GoogleAccounts: id.initialize(callbackRef)
GoogleAccounts-->>GoogleLoginButton: renderButton into gsi-target-btn (with width)
GoogleAccounts->>BackendAuth: POST credential token
BackendAuth-->>GoogleLoginButton: response (mfaRequired / success)
sequenceDiagram
participant User
participant LoginPage
participant Backend as Backend Auth
participant GoogleButton as GoogleLoginButton
User->>LoginPage: submit email/password
LoginPage->>LoginPage: validate inputs
LoginPage->>Backend: POST /api/users/login
Backend-->>LoginPage: mfaRequired or success/error
LoginPage->>LoginPage: set mfaPending or error
User->>GoogleButton: click Google sign-in
GoogleButton->>Backend: POST /api/auth/google (via credential)
Backend-->>GoogleButton: mfaRequired or success
GoogleButton->>LoginPage: update state or redirect
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/google-login-button.tsx (1)
59-111: ⚖️ Poor tradeoffConsider splitting one-time init from width-driven re-render.
Because this effect depends on
containerWidth,id.initializeandrenderButtonre-run on every clamped width change. During a drag-resize the ResizeObserver fires frequently, re-rendering the Google button repeatedly (visible flicker). Consider initializing once and only re-runningrenderButtonwhencontainerWidthchanges, and/or debouncing the observer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/google-login-button.tsx` around lines 59 - 111, The effect currently calls window.google.accounts.id.initialize and renderButton on every containerWidth change causing flicker; split concerns by initializing the Google ID client once (move window.google.accounts.id.initialize into a mount-only effect tied to window.google/accounts/script load and handleBackendAuth) and create a separate effect that only calls window.google.accounts.id.renderButton (or a small wrapper like renderGoogleButton) when containerWidth changes, optionally debouncing the render call from the ResizeObserver; ensure you still add/remove the script load listener and clean up debounced timers so script, renderButton, handleBackendAuth, containerWidth and disposed logic are preserved and not re-run unnecessarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(ui)/signup/page.tsx:
- Around line 164-174: Replace the hardcoded color utility and duplicate link
styling: in the block rendering the "Already have an account?" paragraph (the
surrounding div / p and the Link component), remove the `text-gray-600` class
and use the theme muted token (e.g., apply the `--foreground-muted` token via
your theme utility or a text color class that maps to that token), and swap the
Link's custom classes (`text-brand hover:text-brand-highlight font-medium
transition-colors`) for the shared `input-link` utility so the link styling is
consistent with globals.css; target the p element and the Link component when
making these changes.
In `@src/app/globals.css`:
- Around line 282-289: input-error currently sets only
focus:outline-error-highlight which doesn't override input-base's
focus-visible:outline-brand; update the `@utility` input-error rule (the
input-error utility) to explicitly set the focus-visible outline classes (e.g.,
include focus-visible:outline-2 and focus-visible:outline-error-highlight) so
keyboard/focus-visible state shows the error outline; ensure you replace or
augment the existing focus:outline-* class in input-error with focus-visible::*
to correctly override input-base's focus-visible styles.
In `@src/components/google-login-button.tsx`:
- Around line 119-122: The dynamic Tailwind arbitrary class using containerWidth
in className on the gsi-target-btn div will never be generated by Tailwind and
must be replaced with a static approach: remove the
`[&>div>iframe]:w-[${containerWidth}px]!` template literal and instead set the
iframe width via an inline style or by directly updating the iframe element’s
style when you know containerWidth; reference the gsi-target-btn element and
apply style.width. Also avoid re-running renderGoogleButton on every
containerWidth change: initialize Google’s button once (call renderGoogleButton
in a useEffect with empty deps or an init flag) and handle size updates
separately inside the ResizeObserver callback (debounced) to only adjust iframe
width, so the init step is not repeated frequently.
---
Nitpick comments:
In `@src/components/google-login-button.tsx`:
- Around line 59-111: The effect currently calls
window.google.accounts.id.initialize and renderButton on every containerWidth
change causing flicker; split concerns by initializing the Google ID client once
(move window.google.accounts.id.initialize into a mount-only effect tied to
window.google/accounts/script load and handleBackendAuth) and create a separate
effect that only calls window.google.accounts.id.renderButton (or a small
wrapper like renderGoogleButton) when containerWidth changes, optionally
debouncing the render call from the ResizeObserver; ensure you still add/remove
the script load listener and clean up debounced timers so script, renderButton,
handleBackendAuth, containerWidth and disposed logic are preserved and not
re-run unnecessarily.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d153a9ee-792b-44e4-b298-874d0a1e913f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
docs/nae-todo.txtpackage.jsonsrc/app/(ui)/login/page.tsxsrc/app/(ui)/signup/page.tsxsrc/app/globals.csssrc/components/google-login-button.tsxsrc/components/mfa-challenge.tsxsrc/components/mfa-verify-form.tsxsrc/components/nae-input.tsxsrc/components/nae-set-password.tsxsrc/components/panel-error.tsxsrc/components/panel-header.tsxsrc/types/global.d.tstsconfig.json
💤 Files with no reviewable changes (1)
- src/types/global.d.ts
Owner
Author
|
@coderabbitai - Can you propose a fix for |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
cat src/components/google-login-button.tsxLength of output: 4287
Root CausesDouble React Strict Mode double-init: Strict Mode does mount → unmount → remount in dev. The
Proposed FixThree targeted changes:
'use client';
import { AuthLoginResponse, useAuth } from '`@/context-providers/auth-context-provider`';
import { useRouter } from 'next/navigation';
import { useCallback, useEffect, useRef, useState } from 'react';
interface GoogleLoginButtonProps {
redirect?: boolean;
disabled?: boolean;
callback?: (res: AuthLoginResponse) => void;
onLoginAttempt?: () => void;
onLoginError?: () => void;
}
// Module-level guard: survives React Strict Mode's unmount→remount cycle.
// GSI's initialize() must only be called once per page load.
let gsiInitialized = false;
const GSI_SCRIPT_SRC = 'https://accounts.google.com/gsi/client';
export default function GoogleLoginButton({
redirect = false,
disabled = false,
callback,
onLoginAttempt,
onLoginError,
}: GoogleLoginButtonProps) {
const { loggingIn, loginViaGoogle } = useAuth();
const router = useRouter();
const containerRef = useRef<HTMLDivElement>(null);
const [containerWidth, setContainerWidth] = useState<number>(384);
const [debouncedWidth, setDebouncedWidth] = useState<number>(384);
const [gsiReady, setGsiReady] = useState(false);
// Keep a stable ref to handleBackendAuth so the GSI credential callback
// always invokes the latest version without requiring re-initialization.
const handleBackendAuth = useCallback(async (token: string) => {
if (onLoginAttempt) onLoginAttempt();
try {
const res = await loginViaGoogle(token);
if (callback) callback(res);
if (redirect && !res.data.mfaRequired) {
router.replace('/dashboard');
}
} catch {
console.error('Error logging in via Google');
if (onLoginError) onLoginError();
}
}, [loginViaGoogle, callback, onLoginAttempt, onLoginError, redirect, router]);
const handleBackendAuthRef = useRef(handleBackendAuth);
useEffect(() => { handleBackendAuthRef.current = handleBackendAuth; }, [handleBackendAuth]);
// Effect 1 — Track container width via ResizeObserver (unchanged).
useEffect(() => {
if (!containerRef.current) return;
const observer = new ResizeObserver((entries) => {
for (const entry of entries) {
const width = Math.min(Math.max(entry.contentRect.width, 240), 400);
setContainerWidth(width);
}
});
observer.observe(containerRef.current);
return () => observer.disconnect();
}, []);
// Effect 2 — Debounce containerWidth (150 ms).
// Prevents renderButton from firing on every intermediate resize tick.
useEffect(() => {
const timer = setTimeout(() => setDebouncedWidth(containerWidth), 150);
return () => clearTimeout(timer);
}, [containerWidth]);
// Effect 3 — Inject the GSI script once (idempotent check).
useEffect(() => {
if (document.querySelector(`script[src="${GSI_SCRIPT_SRC}"]`)) return;
const script = document.createElement('script');
script.src = GSI_SCRIPT_SRC;
script.async = true;
script.defer = true;
document.body.appendChild(script);
}, []);
// Effect 4 — Initialize GSI exactly once, even under React Strict Mode.
// Uses the module-level `gsiInitialized` flag; does NOT reset on cleanup
// so Strict Mode's second mount finds it already set and skips the call.
useEffect(() => {
const clientId = process.env.NEXT_PUBLIC_GOOGLE_CLIENT_ID;
if (!clientId) return;
const doInit = () => {
if (gsiInitialized || !window.google?.accounts) return;
gsiInitialized = true;
window.google.accounts.id.initialize({
client_id: clientId,
callback: (response: google.accounts.id.CredentialResponse) => {
if (response.credential) {
// Always calls the latest handler via ref — no re-init needed.
handleBackendAuthRef.current(response.credential);
}
},
});
setGsiReady(true);
};
const script = document.querySelector(`script[src="${GSI_SCRIPT_SRC}"]`) as HTMLScriptElement | null;
if (window.google?.accounts) {
doInit();
} else {
script?.addEventListener('load', doInit);
return () => script?.removeEventListener('load', doInit);
}
}, []); // Intentionally empty — must only run once
// Effect 5 — Re-render the button only when width changes (or GSI becomes ready).
// initialize() is never called here.
useEffect(() => {
if (!gsiReady || !window.google?.accounts) return;
const targetDiv = document.getElementById('gsi-target-btn');
if (!targetDiv) return;
window.google.accounts.id.renderButton(targetDiv, {
type: 'standard',
theme: 'outline',
size: 'large',
text: 'signin_with',
logo_alignment: 'center',
width: Math.floor(debouncedWidth),
});
}, [gsiReady, debouncedWidth]);
return (
<div
ref={containerRef}
className='relative w-full max-w-sm mx-auto flex justify-center'
>
<div
id="gsi-target-btn"
className={`w-full flex justify-center [&>div]:w-full! [&>div>iframe]:w-[${debouncedWidth}px]!`}
></div>
{(loggingIn || disabled) && (
<div className='absolute top-0 left-0 w-full h-full bg-page/80 cursor-not-allowed'></div>
)}
</div>
);
}Is the Strict Mode warning worth fixing separately?No — the module-level One caveat to note: the module-level flag means if Debounce responsiveness150 ms is the sweet spot here — a typical window resize event fires at ~60 fps (≈17 ms intervals), so without debouncing you'd rebuild the iframe dozens of times during a drag. At 150 ms the button resizes about 100–150 ms after the user stops dragging, which is imperceptible during the interaction itself.
The requested changes have been implemented and a pull request has been created: View PR |
📝 CodeRabbit Chat: Implement requested code changes
…renes/nextjs-auth into feature/layout-signin-signup
… attribute altogether - the width of the button is generated programatically and no styles are needed
This was referenced Jun 6, 2026
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
New Features
Style
Chores