Fix Signup page design, logo visibility, and add signin redirect #193
Conversation
…o signin page if already have an account
WalkthroughRefactors Signup page to integrate ThemeContext-driven theming, reworks the UI layout and inputs, adds loading/error/success messaging, and updates form submission to post to the backend and navigate to /login on success. The login link is present. No exported/public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as SignupPage
participant T as ThemeContext
participant B as Backend API
participant R as Router
U->>S: Open /signup
S->>T: Read mode/theme
T-->>S: Theme values
U->>S: Submit form
S->>S: Set isLoading=true
S->>B: POST /api/auth/signup (formData)
alt Success
B-->>S: { message, success:true }
S->>S: Show success message
S->>R: navigate("/login")
else Error
B-->>S: { message, success:false } / error
S->>S: Show error message
end
S->>S: Set isLoading=false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @Riyam111 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![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 (5)
src/pages/Signup/Signup.tsx (5)
33-49: Make submit flow more robust: clear stale messages, build URL safely, rely on HTTP status, and type the error.
- Clear
messageat submit start.- Build the endpoint with a fallback to relative path when backendUrl is empty.
- Don't rely on exact server message text; use HTTP status for navigation.
- Type the error to extract Axios response safely.
Apply this diff:
const handleSubmit = async (e: FormEvent) => { e.preventDefault(); - setIsLoading(true); + setIsLoading(true); + setMessage(""); try { - const response = await axios.post(`${backendUrl}/api/auth/signup`, formData); - setMessage(response.data.message); - - if (response.data.message === "User created successfully") { - navigate("/login"); - } - } catch (error: any) { - setMessage(error.response?.data?.message || "Something went wrong"); + const url = (backendUrl ? `${backendUrl}` : "") + "/api/auth/signup"; + const response = await axios.post(url, formData); + const msg = response.data?.message ?? "Account created"; + setMessage(msg); + if (response.status >= 200 && response.status < 300) { + navigate("/login"); + } + } catch (err) { + const error = err as import("axios").AxiosError<{ message?: string }>; + setMessage(error.response?.data?.message ?? "Something went wrong"); } finally { setIsLoading(false); } };
60-81: Respect reduced motion and avoid intercepting pointer events on decorative elements.Decorative animated blobs should not trap pointer events and should respect users who prefer reduced motion.
Apply this diff:
- <div className="absolute inset-0"> + <div className="absolute inset-0 pointer-events-none" aria-hidden="true"> @@ - } rounded-full blur-3xl opacity-30 animate-pulse`} + } rounded-full blur-3xl opacity-30 motion-safe:animate-pulse motion-reduce:animate-none`} @@ - } rounded-full blur-3xl opacity-30 animate-pulse`} + } rounded-full blur-3xl opacity-30 motion-safe:animate-pulse motion-reduce:animate-none`} @@ - } rounded-full blur-3xl opacity-30 animate-pulse`} + } rounded-full blur-3xl opacity-30 motion-safe:animate-pulse motion-reduce:animate-none`} @@ - } rounded-full blur-2xl opacity-20 animate-pulse delay-1000`} + } rounded-full blur-2xl opacity-20 motion-safe:animate-pulse motion-reduce:animate-none delay-1000`}
124-137: Add accessibility and autofill hints to inputs.Improve a11y and UX with proper autocomplete hints, labels (or aria-labels), and sensible constraints.
Apply these diffs:
- <input + <input type="text" name="username" placeholder="Enter your username" value={formData.username} onChange={handleChange} required + autoComplete="username" + aria-label="Username" + autoCapitalize="none" + autoCorrect="off" + spellCheck={false} className={`w-full pl-4 pr-4 py-4 rounded-2xl focus:outline-none transition-all ${- <input + <input type="email" name="email" placeholder="Enter your email" value={formData.email} onChange={handleChange} required + autoComplete="email" + aria-label="Email address" + autoCapitalize="none" + autoCorrect="off" + spellCheck={false} className={`w-full pl-4 pr-4 py-4 rounded-2xl focus:outline-none transition-all ${- <input + <input type="password" name="password" placeholder="Enter your password" value={formData.password} onChange={handleChange} required + autoComplete="new-password" + aria-label="Password" + minLength={8} className={`w-full pl-4 pr-4 py-4 rounded-2xl focus:outline-none transition-all ${Optional (further improvement): add visible elements for best a11y rather than relying solely on placeholders/aria-labels.
Also applies to: 139-151, 153-165
176-176: Make status messaging accessible and less brittle.
- Use
aria-liveandrole="status"so screen readers announce feedback.- Prefer case-insensitive "success" substring over exact "successfully" to reduce brittleness.
Apply this diff:
- {message && ( - <div - className={`mt-6 p-4 rounded-2xl text-center text-sm font-medium ${ - message.includes("successfully") + {message && ( + <div + role="status" + aria-live="polite" + className={`mt-6 p-4 rounded-2xl text-center text-sm font-medium ${ + message.toLowerCase().includes("success") ? "bg-green-500/20 text-green-300 border border-green-500/30" : "bg-red-500/20 text-red-300 border border-red-500/30" }`} >Also applies to: 178-184
208-213: Prevent the bottom gradient overlay from capturing pointer events.Mark the overlay as decorative to avoid any chance it intercepts clicks near the page bottom.
Apply this diff:
- <div + <div className={`${ mode === "dark" ? "from-slate-900" : "from-slate-100" - } absolute bottom-0 left-0 w-full h-20 bg-gradient-to-t to-transparent`} + } absolute bottom-0 left-0 w-full h-20 bg-gradient-to-t to-transparent pointer-events-none`} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/pages/Signup/Signup.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/pages/Signup/Signup.tsx (2)
src/context/ThemeContext.tsx (3)
ThemeContext(10-10)ThemeContextType(49-49)ThemeContextType(5-8)src/pages/Login/Login.tsx (2)
e(28-44)e(23-26)
🔇 Additional comments (3)
src/pages/Signup/Signup.tsx (3)
52-58: Design consistency LGTM.The theme-aware gradient background aligns with the Login page and the PR objective for branding consistency.
189-204: Sign-in link UX LGTM.Clear navigation for returning users (“Already have an account? Sign in here”) meets the issue requirement. Styling is consistent with the Login page.
86-88: Asset path validated & alt text improvedThe logo file exists at
public/crl-icon.png, so the absolute path is correct. Updated thealtattribute to “GitHubTracker logo” for better accessibility.- <img src="/crl-icon.png" alt="Logo" className="w-14 h-14 object-contain" /> + <img src="/crl-icon.png" alt="GitHubTracker logo" className="w-14 h-14 object-contain" />
| import { ThemeContext } from "../../context/ThemeContext"; | ||
| import type { ThemeContextType } from "../../context/ThemeContext"; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid unsafe ThemeContext cast; provide a safe fallback for mode.
ThemeContext is created with a nullable default (see src/context/ThemeContext.tsx). Casting and destructuring without a guard risks runtime crashes if the Provider is ever omitted. Use a nullable context and a sane fallback.
Apply this diff:
- const themeContext = useContext(ThemeContext) as ThemeContextType;
- const { mode } = themeContext;
+ const themeContext = useContext(ThemeContext);
+ const mode = themeContext?.mode ?? "light";If you adopt the above, you can remove the now-unused type-only import:
-import type { ThemeContextType } from "../../context/ThemeContext";Run this quick check to confirm the app wraps routes with a Theme Provider:
Also applies to: 25-26
🏁 Script executed:
#!/bin/bash
# Verify ThemeContext provider usage exists somewhere in the app.
rg -n -C2 -g '!**/node_modules/**' -P '(ThemeProvider|<ThemeContext\.Provider\b|ThemeContext\.Provider\b)'Length of output: 950
🏁 Script executed:
#!/bin/bash
# Print the beginning of ThemeContext file to inspect Provider component wrapper
sed -n '1,100p' src/context/ThemeContext.tsxLength of output: 1416
🏁 Script executed:
#!/bin/bash
# Check for ThemeWrapper usage in the app entry points or components
rg -n -C2 -g '!**/node_modules/**' 'ThemeWrapper'Length of output: 1678
Handle nullable ThemeContext safely; remove unsafe cast and redundant import
Verified that ThemeWrapper wraps the entire app (in both App.tsx and main.tsx), so useContext(ThemeContext) should never be null at runtime—but it’s still best practice to guard against a missing provider and eliminate the unnecessary cast.
• In src/pages/Signup/Signup.tsx, replace:
- const themeContext = useContext(ThemeContext) as ThemeContextType;
- const { mode } = themeContext;
+ const themeContext = useContext(ThemeContext);
+ const mode = themeContext?.mode ?? "light";• Remove the now-unused type-only import:
- import type { ThemeContextType } from "../../context/ThemeContext";Also apply the same change on lines 25–26 if this pattern repeats.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ThemeContext } from "../../context/ThemeContext"; | |
| import type { ThemeContextType } from "../../context/ThemeContext"; | |
| // src/pages/Signup/Signup.tsx | |
| import { ThemeContext } from "../../context/ThemeContext"; | |
| function Signup() { | |
| // … | |
| const themeContext = useContext(ThemeContext); | |
| const mode = themeContext?.mode ?? "light"; | |
| // … | |
| } |
🤖 Prompt for AI Agents
In src/pages/Signup/Signup.tsx around lines 4 to 6 (and similarly on lines 25–26
if repeated), the code currently imports ThemeContext and also imports
ThemeContextType then uses an unsafe non-null cast; remove the redundant
type-only import (ThemeContextType) and replace the non-null cast by calling
useContext(ThemeContext) and defensively handling a possible null (e.g., check
the returned value and either throw a clear error, return a fallback theme, or
guard rendering until the context is available) so the component no longer
relies on a non-null assertion.
| import { ThemeContext } from "../../context/ThemeContext"; | ||
| import type { ThemeContextType } from "../../context/ThemeContext"; | ||
|
|
||
| const backendUrl = import.meta.env.VITE_BACKEND_URL; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Provide a safe fallback when VITE_BACKEND_URL is not set.
Without a fallback, the code may post to the literal string "undefined/api/auth/signup". Default to empty string so relative URLs still work (useful in dev or same-origin deployments).
Apply this diff:
-const backendUrl = import.meta.env.VITE_BACKEND_URL;
+const backendUrl = import.meta.env.VITE_BACKEND_URL || "";Optional: verify the env var is defined in your repo:
🏁 Script executed:
#!/bin/bash
rg -n 'VITE_BACKEND_URL'Length of output: 247
Add a safe fallback for VITE_BACKEND_URL
To prevent the app from posting to "undefined/api/auth/signup" (or login), default the backend URL to an empty string when the env var isn’t set.
• Update in src/pages/Signup/Signup.tsx
• Also update in src/pages/Login/Login.tsx for consistency
--- a/src/pages/Signup/Signup.tsx
+++ b/src/pages/Signup/Signup.tsx
@@ -7,1 +7,1 @@
-const backendUrl = import.meta.env.VITE_BACKEND_URL;
+const backendUrl = import.meta.env.VITE_BACKEND_URL || "";--- a/src/pages/Login/Login.tsx
+++ b/src/pages/Login/Login.tsx
@@ -7,1 +7,1 @@
-const backendUrl = import.meta.env.VITE_BACKEND_URL;
+const backendUrl = import.meta.env.VITE_BACKEND_URL || "";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const backendUrl = import.meta.env.VITE_BACKEND_URL; | |
| const backendUrl = import.meta.env.VITE_BACKEND_URL || ""; |
🤖 Prompt for AI Agents
In src/pages/Signup/Signup.tsx around line 7, import.meta.env.VITE_BACKEND_URL
may be undefined causing requests to go to "undefined/..."; change the
assignment to provide a safe fallback (e.g. const backendUrl =
import.meta.env.VITE_BACKEND_URL ?? ""; or || "") so the app uses an empty
string when the env var is missing; apply the same change in
src/pages/Login/Login.tsx for consistency.
2 participants
Changes
Why
Related Issue
Summary by CodeRabbit
New Features
Improvements