Fix/signup card padding#510
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 51 minutes and 3 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces scroll-to-top behavior on route transitions, fixes authentication form vertical padding to address layout compression issues, refactors signup validation and error handling with safer Axios typing, and improves code quality through type safety upgrades and unused import removal. ChangesApp improvements and auth form refinements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Signup/Signup.tsx (1)
285-297:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUser-facing text error causes confusion.
The text "Don't have an account?" is incorrect for the signup page. Users are already on the page to create an account. The text should read "Already have an account?" since the link navigates to the login page for users who already have accounts.
🐛 Proposed fix
<p className={`${mode === "dark" ? "text-slate-500" : "text-gray-600"} text-sm`} > - Don't have an account? + Already have an account? <Link to="/login" className="ml-1 text-purple-400 hover:text-purple-300 transition-colors duration-300"🤖 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/pages/Signup/Signup.tsx` around lines 285 - 297, Replace the incorrect prompt string in the Signup component: in the paragraph that precedes the Link (the element containing the text "Don't have an account?" and the Link to="/login"), change the text to "Already have an account?" so the message matches the login link; update the string inside the <p> that currently uses the mode-based className and the Link to "/login".
🧹 Nitpick comments (2)
src/hooks/useGitHubData.ts (1)
115-115: ⚡ Quick winPrefer explicit typing over
unknownfor better type safety.While changing
anytounknownis a step toward type safety, accessing.value.itemsand.value.totalon lines 156–157 and 170–171 requires TypeScript to either accept unsafe property access onunknownor rely on type assertions. The actual return type offetchPaginatedis known:Promise<{ items: GitHubItem[], total: number }>.Consider using an explicit type for stronger compile-time guarantees:
✨ Proposed fix for explicit typing
- const requests: Promise<unknown>[] = []; + const requests: Promise<{ items: GitHubItem[], total: number }>[] = [];This ensures TypeScript can verify property access without assertions and provides better IntelliSense support.
🤖 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/hooks/useGitHubData.ts` at line 115, Change the requests array to use the explicit response type instead of Promise<unknown> so TypeScript can validate accesses to .value.items and .value.total; specifically type the array as Promise<{ items: GitHubItem[]; total: number }>[] (matching the known return shape of fetchPaginated) and update any related generics or the fetchPaginated signature used in useGitHubData so callers and the variable requests (and subsequent accesses in the useGitHubData function where .value.items and .value.total are read) have correct compile-time types.src/pages/Signup/Signup.tsx (1)
95-98: 💤 Low valueMisleading comment about cookie handling.
The comment "Include cookies for session" suggests explicit cookie configuration, but there's no
withCredentials: trueset in this axios call. While same-origin requests do send cookies automatically, the comment could mislead developers into thinking there's explicit session cookie handling configured here.Suggested clarification
const response = await axios.post( `${backendUrl}/api/auth/signup`, - formData, // Include cookies for session + formData, );Or make it more accurate:
const response = await axios.post( `${backendUrl}/api/auth/signup`, - formData, // Include cookies for session + formData, // Cookies sent automatically for same-origin );🤖 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/pages/Signup/Signup.tsx` around lines 95 - 98, The inline comment on the axios.post call in Signup.tsx is misleading: either remove or update it to reflect actual cookie behavior; to explicitly send cookies for cross-origin sessions add the axios option withCredentials: true to the axios.post call (axios.post(`${backendUrl}/api/auth/signup`, formData, { withCredentials: true })) or change the comment to something accurate like "Same-origin cookies sent automatically" if you intend no explicit credential flag.
🤖 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.
Outside diff comments:
In `@src/pages/Signup/Signup.tsx`:
- Around line 285-297: Replace the incorrect prompt string in the Signup
component: in the paragraph that precedes the Link (the element containing the
text "Don't have an account?" and the Link to="/login"), change the text to
"Already have an account?" so the message matches the login link; update the
string inside the <p> that currently uses the mode-based className and the Link
to "/login".
---
Nitpick comments:
In `@src/hooks/useGitHubData.ts`:
- Line 115: Change the requests array to use the explicit response type instead
of Promise<unknown> so TypeScript can validate accesses to .value.items and
.value.total; specifically type the array as Promise<{ items: GitHubItem[];
total: number }>[] (matching the known return shape of fetchPaginated) and
update any related generics or the fetchPaginated signature used in
useGitHubData so callers and the variable requests (and subsequent accesses in
the useGitHubData function where .value.items and .value.total are read) have
correct compile-time types.
In `@src/pages/Signup/Signup.tsx`:
- Around line 95-98: The inline comment on the axios.post call in Signup.tsx is
misleading: either remove or update it to reflect actual cookie behavior; to
explicitly send cookies for cross-origin sessions add the axios option
withCredentials: true to the axios.post call
(axios.post(`${backendUrl}/api/auth/signup`, formData, { withCredentials: true
})) or change the comment to something accurate like "Same-origin cookies sent
automatically" if you intend no explicit credential flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31c753c3-6fed-4939-b0e8-cc8b9657adea
⛔ Files ignored due to path filters (2)
backend/bun.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
src/App.tsxsrc/components/Navbar.tsxsrc/components/ScrollToTop.tsxsrc/components/__test__/Navbar.test.tsxsrc/hooks/useGitHubData.tssrc/pages/Login/Login.tsxsrc/pages/Signup/Signup.tsx
1 participant
Related Issue
Description
Resolved a layout layout bug on the authentication container card where components pressed flush against vertical boundaries. Replaced the generic shorthand padding rules (
p-6 sm:p-10) with decoupled explicit horizontal and vertical properties (px-6 py-8 sm:px-10 sm:py-12). This guarantees steady top and bottom visual clearance safety zones, ensuring that text elements and validation warnings preserve consistent spacing buffers without clipping card borders.How Has This Been Tested?
npm run lintlocally to confirm complete linter compliance.Type of Change
###Screenshot
Summary by CodeRabbit
New Features
Style
Refactor