Fix signup category visibility and alignment issue#322
Merged
Sachinchaurasiya360 merged 1 commit intoMay 21, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR consolidates recruiter-specific authentication into unified login/register pages. Recruiter routes now redirect with a ChangesUnified Auth with Role Selection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. 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: 2
🧹 Nitpick comments (1)
client/src/main.tsx (1)
12-12: 💤 Low valueConsider removing the type assertion for better type safety.
The
as stringassertion is imprecise becauseimport.meta.env.VITE_GOOGLE_CLIENT_IDcould beundefined. While the||fallback handles this correctly at runtime, the assertion masks the actual type and could lead to confusion during maintenance.♻️ Optional type-safe alternative
-const googleClientId = import.meta.env.VITE_GOOGLE_CLIENT_ID as string || 'missing_client_id' +const googleClientId = import.meta.env.VITE_GOOGLE_CLIENT_ID || 'missing_client_id'TypeScript will infer the correct type (
string) from the fallback without the assertion.🤖 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 `@client/src/main.tsx` at line 12, The declaration of googleClientId uses a type assertion ("as string") that hides the actual possibly-undefined type of import.meta.env.VITE_GOOGLE_CLIENT_ID; remove the "as string" assertion and either rely on TypeScript's type inference from the fallback or explicitly annotate the constant as a string (i.e., update the googleClientId declaration) so the code reflects the runtime fallback and preserves correct typing without masking undefined.
🤖 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 `@client/src/module/auth/LoginPage.tsx`:
- Around line 117-137: Replace the two raw <button> elements with the shared
Button component (Button) and keep the existing onClick handlers
(setRole("STUDENT") / setRole("RECRUITER")) and role-based styling by mapping
the current conditional classes into the Button's variant/size props and/or
className prop; add the Button import and ensure the "selected" state uses the
same bg/text classes when role === "STUDENT"/"RECRUITER" and preserve the
left-border for the second button via className (e.g., border-l styling) so
behavior and visuals match the original.
- Around line 14-15: Replace the native <button> usage in LoginPage with the
reusable Button component (import Button from
client/src/components/ui/button.tsx) and apply the appropriate variant/props
used elsewhere in the app; locate the buttons in the LoginPage render (the
elements controlling role selection and submission) and swap them to use Button
while preserving onClick handlers and accessibility attributes. Also ensure role
state (initialRole, role, setRole) stays in sync with the URL by adding a
useEffect in LoginPage that reads searchParams.get("role") and calls setRole
when the query param changes so the component and API payload reflect the
current URL.
---
Nitpick comments:
In `@client/src/main.tsx`:
- Line 12: The declaration of googleClientId uses a type assertion ("as string")
that hides the actual possibly-undefined type of
import.meta.env.VITE_GOOGLE_CLIENT_ID; remove the "as string" assertion and
either rely on TypeScript's type inference from the fallback or explicitly
annotate the constant as a string (i.e., update the googleClientId declaration)
so the code reflects the runtime fallback and preserves correct typing without
masking undefined.
🪄 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: aaeffd8f-201f-4fbe-8691-04957dc6dc23
⛔ Files ignored due to path filters (1)
client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
client/src/App.tsxclient/src/main.tsxclient/src/module/auth/LoginPage.tsxclient/src/module/recruiter/auth/RecruiterLoginPage.tsx
Comment on lines
+14
to
+15
| const initialRole = searchParams.get("role") === "RECRUITER" ? "RECRUITER" : "STUDENT"; | ||
| const [role, setRole] = useState<"STUDENT" | "RECRUITER">(initialRole); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether query-param role is synchronized after initialization.
rg -n 'roleFromQuery|initialRole|useState<"STUDENT" \| "RECRUITER">|useEffect\(' client/src/module/auth/LoginPage.tsxRepository: Sachinchaurasiya360/InternHack
Length of output: 244
🏁 Script executed:
# Get the full LoginPage component to understand role usage and routing context
head -200 client/src/module/auth/LoginPage.tsx | cat -nRepository: Sachinchaurasiya360/InternHack
Length of output: 10240
🏁 Script executed:
# Check how searchParams is imported/used and if it's reactive
rg -B5 -A5 'searchParams' client/src/module/auth/LoginPage.tsxRepository: Sachinchaurasiya360/InternHack
Length of output: 726
🏁 Script executed:
# Check if there are similar patterns in the codebase for syncing query params with state
rg -A10 'useEffect.*searchParams|useEffect.*query' client/src --type tsx | head -50Repository: Sachinchaurasiya360/InternHack
Length of output: 102
Replace native buttons with the reusable Button component per coding guidelines.
Lines 117–136 use native <button> elements instead of the Button component from client/src/components/ui/button.tsx. Per guidelines, all new buttons must use the reusable Button component with proper variants/modes.
Additionally, consider syncing the role state when the role query param changes. Currently useState(initialRole) only captures the initial value; if the URL param changes while the component stays mounted, the state and API payload can drift from the URL. Add a useEffect hook to sync state to the query param.
Suggested fixes
Replace the native buttons with the Button component:
- <button
- type="button"
- onClick={() => setRole("STUDENT")}
- className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer ${role === "STUDENT"
- ? "bg-lime-400 text-stone-950"
- : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50"
- }`}
- >
- Student
- </button>
- <button
- type="button"
- onClick={() => setRole("RECRUITER")}
- className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer border-l border-stone-300 dark:border-white/10 ${role === "RECRUITER"
- ? "bg-lime-400 text-stone-950"
- : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50"
- }`}
- >
- Recruiter
- </button>
+ <Button variant={role === "STUDENT" ? "primary" : "secondary"} onClick={() => setRole("STUDENT")} className="text-sm font-bold">Student</Button>
+ <Button variant={role === "RECRUITER" ? "primary" : "secondary"} onClick={() => setRole("RECRUITER")} className="text-sm font-bold">Recruiter</Button>Optionally, sync role state with query param:
-import { useState } from "react";
+import { useEffect, useState } from "react";
...
- const initialRole = searchParams.get("role") === "RECRUITER" ? "RECRUITER" : "STUDENT";
- const [role, setRole] = useState<"STUDENT" | "RECRUITER">(initialRole);
+ const roleFromQuery = searchParams.get("role") === "RECRUITER" ? "RECRUITER" : "STUDENT";
+ const [role, setRole] = useState<"STUDENT" | "RECRUITER">(roleFromQuery);
+
+ useEffect(() => {
+ setRole(roleFromQuery);
+ }, [roleFromQuery]);🤖 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 `@client/src/module/auth/LoginPage.tsx` around lines 14 - 15, Replace the
native <button> usage in LoginPage with the reusable Button component (import
Button from client/src/components/ui/button.tsx) and apply the appropriate
variant/props used elsewhere in the app; locate the buttons in the LoginPage
render (the elements controlling role selection and submission) and swap them to
use Button while preserving onClick handlers and accessibility attributes. Also
ensure role state (initialRole, role, setRole) stays in sync with the URL by
adding a useEffect in LoginPage that reads searchParams.get("role") and calls
setRole when the query param changes so the component and API payload reflect
the current URL.
Comment on lines
+117
to
+137
| <button | ||
| type="button" | ||
| onClick={() => setRole("STUDENT")} | ||
| className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer ${role === "STUDENT" | ||
| ? "bg-lime-400 text-stone-950" | ||
| : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50" | ||
| }`} | ||
| > | ||
| Student | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={() => setRole("RECRUITER")} | ||
| className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer border-l border-stone-300 dark:border-white/10 ${role === "RECRUITER" | ||
| ? "bg-lime-400 text-stone-950" | ||
| : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50" | ||
| }`} | ||
| > | ||
| Recruiter | ||
| </button> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the shared Button component for the new role toggle controls.
These are newly added buttons in a TSX file, so they should use the reusable Button component to keep behavior/styles consistent.
Refactor sketch
+import { Button } from "../../components/ui/button";
...
- <button
+ <Button
type="button"
onClick={() => setRole("STUDENT")}
- className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer ${role === "STUDENT"
- ? "bg-lime-400 text-stone-950"
- : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50"
- }`}
+ variant={role === "STUDENT" ? "primary" : "secondary"}
+ size="md"
+ aria-pressed={role === "STUDENT"}
+ className="rounded-none border-0"
>
Student
- </button>
- <button
+ </Button>
+ <Button
type="button"
onClick={() => setRole("RECRUITER")}
- className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer border-l border-stone-300 dark:border-white/10 ${role === "RECRUITER"
- ? "bg-lime-400 text-stone-950"
- : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50"
- }`}
+ variant={role === "RECRUITER" ? "primary" : "secondary"}
+ size="md"
+ aria-pressed={role === "RECRUITER"}
+ className="rounded-none border-0 border-l border-stone-300 dark:border-white/10"
>
Recruiter
- </button>
+ </Button>As per coding guidelines, client/src/**/*.tsx: Use the reusable Button component from client/src/components/ui/button.tsx for all new buttons with variants/modes/sizes.
📝 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.
Suggested change
| <button | |
| type="button" | |
| onClick={() => setRole("STUDENT")} | |
| className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer ${role === "STUDENT" | |
| ? "bg-lime-400 text-stone-950" | |
| : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50" | |
| }`} | |
| > | |
| Student | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => setRole("RECRUITER")} | |
| className={`py-2.5 text-sm font-bold transition-colors border-0 cursor-pointer border-l border-stone-300 dark:border-white/10 ${role === "RECRUITER" | |
| ? "bg-lime-400 text-stone-950" | |
| : "bg-white dark:bg-stone-900 text-stone-600 dark:text-stone-400 hover:text-stone-900 dark:hover:text-stone-50" | |
| }`} | |
| > | |
| Recruiter | |
| </button> | |
| </div> | |
| <Button | |
| onClick={() => setRole("STUDENT")} | |
| variant={role === "STUDENT" ? "primary" : "secondary"} | |
| size="md" | |
| aria-pressed={role === "STUDENT"} | |
| className="rounded-none border-0" | |
| > | |
| Student | |
| </Button> | |
| <Button | |
| onClick={() => setRole("RECRUITER")} | |
| variant={role === "RECRUITER" ? "primary" : "secondary"} | |
| size="md" | |
| aria-pressed={role === "RECRUITER"} | |
| className="rounded-none border-0 border-l border-stone-300 dark:border-white/10" | |
| > | |
| Recruiter | |
| </Button> | |
| </div> |
🤖 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 `@client/src/module/auth/LoginPage.tsx` around lines 117 - 137, Replace the two
raw <button> elements with the shared Button component (Button) and keep the
existing onClick handlers (setRole("STUDENT") / setRole("RECRUITER")) and
role-based styling by mapping the current conditional classes into the Button's
variant/size props and/or className prop; add the Button import and ensure the
"selected" state uses the same bg/text classes when role ===
"STUDENT"/"RECRUITER" and preserve the left-border for the second button via
className (e.g., border-l styling) so behavior and visuals match the original.
Abfa41
added
good first issue
Collaborator
|
@Darshan200531 bro, could you please add before and after screenshots of the UI?? It will help us in reviewing faster.. |
Author
|
Before :- After :- |
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
3 participants
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.
Changes Made
Summary by CodeRabbit
New Features
Improvements
Bug Fixes