Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Updates the authentication UI layout for mobile/desktop modals and refactors the sign-in success path so success feedback is shown consistently after authentication.
Changes:
- Adjust auth modal
Cardsizing/padding and surrounding layout containers for more consistent rendering. - Move success toast + redirect handling to run after successful sign-in regardless of redirect presence.
- Make the auth page/modal wrappers fill available width/height.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/components/pages/auth/MobileAuthModal.tsx | Updates mobile modal card sizing (fixed height) and padding reset. |
| src/components/pages/auth/DesktopAuthModal.tsx | Updates desktop modal card sizing (fixed height), padding reset, and ensures inner container fills height. |
| src/components/pages/auth/AuthenticationModal.tsx | Refactors success handling + tweaks layout wrapper sizing; adds Biome suppression comment. |
| src/app/(internal)/(auth)/auth/page.tsx | Makes auth page container fill available width/height. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -42,20 +42,21 @@ export default function AuthenticationModal() { | |||
| variant: "destructive", | |||
| }); | |||
| throw new Error(error); | |||
There was a problem hiding this comment.
In the error branch you toast an error and then throw new Error(error), but the outer catch also toasts, which will likely result in duplicate error toasts for the same failure. Prefer a single error-reporting path (e.g., throw and let the catch toast, or toast and return without throwing).
| throw new Error(error); | |
| return; |
| if (redirect) { | ||
| router.push(redirect); | ||
| } else { | ||
| toast({ | ||
| title: "Success", | ||
| description: "Signed in successfully", | ||
| variant: "success", | ||
| }); | ||
| if (redirect) { | ||
| router.push(redirect); | ||
| } else { | ||
| router.push("/"); | ||
| } | ||
| router.push("/"); |
There was a problem hiding this comment.
redirect comes directly from a query string and is passed to router.push without validation. This can enable open-redirect behavior (e.g., redirect=https://evil.com) depending on how Next handles the value. Consider only allowing relative internal paths (e.g., redirect must start with / and must not start with //), otherwise fall back to /.
| // biome-ignore lint/suspicious/noExplicitAny: <explanation> | ||
| } catch (error: any) { | ||
| toast({ |
There was a problem hiding this comment.
The Biome suppression comment contains a placeholder (<explanation>) and the catch (error: any) is being silenced rather than handled. Please replace the placeholder with a real justification, or preferably remove the suppression by typing the catch param as unknown and safely deriving a message for the toast.
| className="w-full" | ||
| > | ||
| <Card className="w-full min-h-[500px] !shadow-none rounded-3xl overflow-hidden border-t border-l border-r md:border-0 mt-96"> | ||
| <Card className="w-full h-[600px] !shadow-none rounded-3xl overflow-hidden border-t border-l border-r md:border-0 p-0"> |
There was a problem hiding this comment.
With the Card now having a fixed height (h-[600px]) and overflow-hidden, the wallet provider list can become inaccessible if it exceeds the available vertical space (there’s no overflow-y-auto on the bottom panel like the desktop modal has). Consider adding an internal scroll container for the wallet options area or removing overflow-hidden/fixed height so content can be reached on smaller screens.
No description provided.