🐛 fix: reset scroll position to top on route change and fix lint issues#501
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
More reviews will be available in 43 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements a fix for automatic page scroll-to-top on route navigation (addressing issue ChangesRoute Navigation and Form Quality Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @atharv96k 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 (3)
src/components/Navbar.tsx (1)
4-4: 💤 Low valueAdd spacing before closing brace in import statement.
The import statement is missing a space before the closing brace:
X}should beX }for consistent formatting.✨ Formatting fix
-import { Moon, Sun, Menu, X} from "lucide-react"; +import { Moon, Sun, Menu, X } from "lucide-react";🤖 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/Navbar.tsx` at line 4, The import list in Navbar.tsx has inconsistent spacing before the closing brace—update the import statement that currently lists Moon, Sun, Menu, X to include a space before the closing brace (change the token sequence ending with "X}" to "X }") so the import reads with consistent spacing around braces for the symbols Moon, Sun, Menu, and X.src/components/__test__/Navbar.test.tsx (1)
3-3: 💤 Low valueAdd spacing before closing brace in import statement.
The import statement is missing a space before the closing brace:
vi}should bevi }for consistent formatting.✨ Formatting fix
-import { describe, it, expect, vi} from 'vitest' +import { describe, it, expect, vi } from 'vitest'🤖 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/__test__/Navbar.test.tsx` at line 3, The import line importing describe, it, expect, and vi has no space before the closing brace; update the import statement that lists the symbols (describe, it, expect, vi) so the closing brace is spaced as "vi }" to match project formatting conventions.src/hooks/useGitHubData.ts (1)
115-115: ⚡ Quick winAvoid losing type info by typing
requestsas thefetchPaginatedreturn type (src/hooks/useGitHubData.ts:115).
fetchPaginatedreturns{ items: GitHubItem[]; total: number }, andtsc --noEmitsucceeds, so the current.items/.totalaccess is accepted.Promise<unknown>[]unnecessarily erases that type; use thefetchPaginatedreturn type instead (e.g.,Promise<{ items: GitHubItem[]; total: number }>[]/ReturnType<typeof fetchPaginated>[]).Proposed fix
- const requests: Promise<unknown>[] = []; + const requests: Promise<{ items: GitHubItem[], total: number }>[] = [];🤖 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, The variable requests is currently typed as Promise<unknown>[] which erases the actual shape returned by fetchPaginated; change its type to the fetchPaginated return type (for example Promise<{ items: GitHubItem[]; total: number }>[] or ReturnType<typeof fetchPaginated>[] ) so callers that read .items and .total keep proper types—update the declaration of requests in useGitHubData (variable name requests) to use that precise return type.
🤖 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/pages/Signup/Signup.tsx`:
- Around line 200-212: The username input (and other fields using formData,
handleChange, errors and mode) lacks a programmatic label and proper ARIA error
association; add a <label htmlFor="username"> linked to the input's
id="username", set input's aria-describedby to an id for the error message
(e.g., username-error) and ensure the error <p> has that id plus role="alert"
and aria-live="assertive" so screen readers announce validation; repeat the same
pattern for email and password fields, keeping existing props
(value={formData...}, onChange={handleChange}, required) and preserving the
mode-based className logic.
- Around line 95-98: The comment "Include cookies for session" on the axios.post
in Signup.tsx is misleading because the call only passes url and formData;
either remove that comment or explicitly enable credentials: update the
axios.post call that sends formData to include an axios config with
withCredentials: true (or ensure a global axios default is set to
withCredentials: true) so cookies are actually sent; locate the axios.post
invocation in the Signup component and apply one of these two fixes to make the
behavior and comment consistent.
---
Nitpick comments:
In `@src/components/__test__/Navbar.test.tsx`:
- Line 3: The import line importing describe, it, expect, and vi has no space
before the closing brace; update the import statement that lists the symbols
(describe, it, expect, vi) so the closing brace is spaced as "vi }" to match
project formatting conventions.
In `@src/components/Navbar.tsx`:
- Line 4: The import list in Navbar.tsx has inconsistent spacing before the
closing brace—update the import statement that currently lists Moon, Sun, Menu,
X to include a space before the closing brace (change the token sequence ending
with "X}" to "X }") so the import reads with consistent spacing around braces
for the symbols Moon, Sun, Menu, and X.
In `@src/hooks/useGitHubData.ts`:
- Line 115: The variable requests is currently typed as Promise<unknown>[] which
erases the actual shape returned by fetchPaginated; change its type to the
fetchPaginated return type (for example Promise<{ items: GitHubItem[]; total:
number }>[] or ReturnType<typeof fetchPaginated>[] ) so callers that read .items
and .total keep proper types—update the declaration of requests in useGitHubData
(variable name requests) to use that precise return type.
🪄 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: e9c03e3f-75d6-4fcc-a7f5-9ac7d25b1063
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
src/App.tsxsrc/components/Navbar.tsxsrc/components/ScrollToTop.tsxsrc/components/__test__/Navbar.test.tsxsrc/hooks/useGitHubData.tssrc/pages/Signup/Signup.tsx
| const response = await axios.post( | ||
| `${backendUrl}/api/auth/signup`, | ||
| formData, // Include cookies for session | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether credentials are already configured globally and compare auth request patterns.
rg -n -C3 'axios\.create\(|axios\.defaults\.withCredentials|withCredentials\s*:\s*true'
rg -n -C2 '/api/auth/signup|/api/auth/login|/api/auth'Repository: GitMetricsLab/github_tracker
Length of output: 881
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "withCredentials" -S .
rg -n "axios\.create\(" -S src backend .
rg -n "axios\.defaults\.withCredentials" -S src backend .
rg -n -C3 "Include cookies" src/pages/Signup/Signup.tsx
sed -n '70,120p' src/pages/Signup/Signup.tsx
sed -n '1,120p' src/pages/Login/Login.tsx
sed -n '1,120p' backend/server.js
ls -la src | headRepository: GitMetricsLab/github_tracker
Length of output: 151
🏁 Script executed:
#!/bin/bash
set +euo pipefail
rg -n "withCredentials" -S src backend || true
rg -n "axios\.create\(" -S src backend || true
rg -n "axios\.defaults\.withCredentials" -S src backend || true
rg -n "axios\.defaults" -S src backend | head -n 50
sed -n '1,220p' src/pages/Signup/Signup.tsx
sed -n '1,140p' src/pages/Login/Login.tsx
fd -t f "axios*.ts" src backend -a || true
fd -t f -g "*axios*" src backend || true
rg -n "from 'axios'|require\\(['\"]axios['\"]\\)" -S src backend || trueRepository: GitMetricsLab/github_tracker
Length of output: 14360
Fix misleading cookies comment on signup axios request
src/pages/Signup/Signup.tsx’s /api/auth/signup axios.post only passes url and data; there’s no withCredentials (and no global axios credentials config found—only a withCredentials: false elsewhere). Either remove the “Include cookies for session” comment or add explicit axios config.
✅ If cookies are required for this route
const response = await axios.post(
`${backendUrl}/api/auth/signup`,
- formData, // Include cookies for session
+ formData,
+ { withCredentials: true },
);🤖 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 comment "Include
cookies for session" on the axios.post in Signup.tsx is misleading because the
call only passes url and formData; either remove that comment or explicitly
enable credentials: update the axios.post call that sends formData to include an
axios config with withCredentials: true (or ensure a global axios default is set
to withCredentials: true) so cookies are actually sent; locate the axios.post
invocation in the Signup component and apply one of these two fixes to make the
behavior and comment consistent.
| <input | ||
| type="text" | ||
| name="username" | ||
| placeholder="Enter your username" | ||
| value={formData.username} | ||
| onChange={handleChange} | ||
| required | ||
| className={`w-full pl-12 pr-4 py-4 rounded-2xl border focus:outline-none focus:ring-2 focus:ring-gray-400 focus:border-transparent transition-all duration-300 ${mode === "dark" ? "bg-white/10 border-white/20 text-white placeholder-gray-400" : "bg-gray-100 border-gray-300 text-black placeholder-gray-400"}`} | ||
| /> | ||
| </div> | ||
| {errors.username && <p className="text-red-500 text-sm mt-2">{errors.username}</p>} | ||
| {errors.username && ( | ||
| <p className="text-red-500 text-sm mt-2">{errors.username}</p> | ||
| )} |
There was a problem hiding this comment.
Add explicit labels and error associations for all form fields.
The inputs rely on placeholders only. Please add programmatic labels (label + htmlFor) and connect inline errors with aria-describedby/role="alert" so screen-reader users get field context and validation feedback reliably.
♿ Suggested pattern (apply to username/email/password)
+ <label htmlFor="signup-username" className="sr-only">Username</label>
<input
+ id="signup-username"
type="text"
name="username"
placeholder="Enter your username"
value={formData.username}
onChange={handleChange}
required
+ aria-invalid={Boolean(errors.username)}
+ aria-describedby={errors.username ? "signup-username-error" : undefined}
className="..."
/>
- {errors.username && (
- <p className="text-red-500 text-sm mt-2">{errors.username}</p>
- )}
+ {errors.username && (
+ <p id="signup-username-error" role="alert" className="text-red-500 text-sm mt-2">
+ {errors.username}
+ </p>
+ )}Also applies to: 220-232, 240-265
🤖 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 200 - 212, The username input (and
other fields using formData, handleChange, errors and mode) lacks a programmatic
label and proper ARIA error association; add a <label htmlFor="username"> linked
to the input's id="username", set input's aria-describedby to an id for the
error message (e.g., username-error) and ensure the error <p> has that id plus
role="alert" and aria-live="assertive" so screen readers announce validation;
repeat the same pattern for email and password fields, keeping existing props
(value={formData...}, onChange={handleChange}, required) and preserving the
mode-based className logic.
|
🎉🎉 Thank you for your contribution! Your PR #501 has been merged! 🎉🎉 |
2 participants
Related Issue
Closes: #497
Description
Implemented a global ScrollToTop utility component that listens to navigation events using react-router-dom's useLocation hook. This ensures that the window viewport resets automatically to the top coordinates (0, 0) upon route changes, resolving the bug where navigating via footer quick links kept the user stuck at the bottom of the newly loaded page.
Additionally, resolved pre-existing strict linter errors (@typescript-eslint/no-unused-vars and @typescript-eslint/no-explicit-any) across Navbar.tsx, Navbar.test.tsx, useGitHubData.ts, and Signup.tsx to ensure npm run lint passes successfully.
How Has This Been Tested?
Manual testing: Navigated to the bottom of the page, clicked "About" and "Contact Us" links from the footer, and verified that the viewport automatically scrolls to the top.
Edge case testing: Scrolled down on the /about page and clicked the "About" link again; verified that it successfully scrolls to the top by tracking the full location object inside the useEffect dependency array.
Ran local verification checks: Executed npm run lint locally and confirmed that all errors have been resolved (0 errors remaining).
Type of Change
[x] Bug fix
[ ] New feature
[x] Code style update
[ ] Breaking change
[ ] Documentation update
Screenshots (if applicable)
Summary by CodeRabbit
New Features
Refactor