fix: scroll to top #100
fix: scroll to top #100
Conversation
|
@Gunjan10-droid is attempting to deploy a commit to the coderuzumaki's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a root package.json to orchestrate concurrent dev for server and client, introduces a client start script for Vite, and updates Header.jsx to scroll to top when clicking the active route while preserving normal navigation for route changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Header as Header.jsx
participant Router as react-router-dom
participant Window as window
User->>Header: Click Nav Link (path: /resources)
Header->>Router: Evaluate current location.pathname
alt Same route clicked
Header->>Window: window.scrollTo({ top: 0, behavior: "smooth" })
note over Window: Scroll-to-top triggered
else Different route
Header->>Router: Navigate to target route
Router-->>User: Route changes as usual
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
client/package.json (1)
10-11: Clarify dev vs. start semantics (optional).Having both dev and start run Vite dev is fine for the root concurrently flow, but it’s unconventional (start is often “production start” via vite preview). Two options:
- Keep as-is and update root dev to call npm run dev --prefix client for consistency, or
- Make start run vite preview (prod preview) and keep dev for development; then adjust the root dev script accordingly.
Example (if choosing option 2):
- "preview": "vite preview", - "start": "vite" + "preview": "vite preview", + "start": "vite preview"If you want to keep start for dev, consider updating the root script to call client dev for consistency.
client/src/components/Header.jsx (2)
59-64: Close menus and respect reduced motion when scrolling to top.When clicking the active route in mobile, the drawer stays open. Also, prefer reduced motion if set. Update handleNavClick:
- const handleNavClick = (to) => { - if (location.pathname === to) { - - window.scrollTo({ top: 0, behavior: "smooth" }) - } - } + const handleNavClick = (to) => { + if (location.pathname === to) { + const prefersReducedMotion = window.matchMedia?.("(prefers-reduced-motion: reduce)")?.matches + window.scrollTo({ top: 0, behavior: prefersReducedMotion ? "auto" : "smooth" }) + } + // Close any open UI chrome after a nav click + setIsMobileMenuOpen(false) + setIsDropdownOpen(false) + }
65-106: Good coverage of all primary links; consider NavLink for active styles (optional).The onClick wiring meets the PR goal. If you later want automatic “active” styling and less manual path handling, swap Link for NavLink and derive isActive without passing strings into handleNavClick.
package.json (1)
2-5: Mark the root package as private to avoid accidental publish.Add "private": true at the root. Also optional: add an engines field to document supported Node/npm versions.
{ "name": "prepedge-ai", "version": "1.0.0", + "private": true, "description": "<h1 align=\"center\"> PrepEdge AI </h1>\r <p align=\"center\"> An AI enabled smart Interview Preparation Platform </p>", "main": "index.js",Also applies to: 16-26
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
client/package.json(1 hunks)client/src/components/Header.jsx(3 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/components/Header.jsx (2)
client/src/components/ScrollToTop.jsx (2)
useLocation(5-5)ScrollToTop(4-15)client/src/App.jsx (1)
App(20-31)
🔇 Additional comments (2)
client/src/components/Header.jsx (1)
2-2: LGTM: useLocation import and usage.Importing and wiring useLocation is correct for same-route detection.
Also applies to: 13-13
package.json (1)
6-9: No action required — client "start" and "dev" both runvite.
Root usesnpm start --prefix client; client/package.json defines both"start"and"dev"as"vite"and server provides"dev"as"nodemon". Optional: switch root tonpm run dev --prefix clientfor uniformity.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Hey @Gunjan10-droid ,
Thanks again for contributing. |
2 participants
This PR fixes the issue where clicking a navigation link (Home, Resources, etc.) while already on the same page did not reset the scroll position.
Fixes #86
Summary by CodeRabbit
New Features
Chores