Fix: Prevent full page reload when clicking avatar/logo #865
Conversation
📝 WalkthroughWalkthroughThese changes fix full-page reloads that occurred when clicking navbar elements. The frontend navbar now uses React Router Links instead of anchor elements for logo and settings navigation. The landing page wraps its HomePage content with an identifiable div and implements hash-based smooth scrolling with offset compensation for sticky navbars. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
landing-page/src/Pages/Landing page/Navbar.tsx (1)
198-206: Hash targets#featuresand#aboutare missing from the codebase.The mobile navigation correctly implements hash-based scroll behavior and properly closes the menu on link clicks. However, the navigation will fail silently for
#featuresand#aboutbecause these elements lack correspondingidattributes:
#homeis properly defined in App.tsx ✓#featuresis not defined anywhere in the codebase#aboutis not defined anywhere in the codebaseAdd
id="features"to the features section (likelylanding-page/src/components/ui/Features.tsx) andid="about"to the about section (likelylanding-page/src/Pages/HowItWorks/HowItWorks.tsxor equivalent) to enable working navigation.
🧹 Nitpick comments (1)
landing-page/src/Pages/Landing page/Navbar.tsx (1)
30-38: Scroll offset implementation works well; consider making it dynamic.The 90px offset correctly compensates for the sticky navbar height, and using
window.scrollToprovides smooth, controlled scrolling behavior.For better maintainability, consider calculating the offset dynamically based on the actual navbar height rather than hardcoding the value. This would prevent issues if the navbar height changes in the future.
💡 Optional: Calculate offset dynamically
- // Scroll with offset to account for sticky navbar - const offset = 90; + // Calculate offset dynamically based on navbar height + const navbar = document.querySelector('nav'); + const offset = navbar ? navbar.offsetHeight + 10 : 90; // +10 for padding const elementTop = element.getBoundingClientRect().top + window.scrollY; const targetScroll = elementTop - offset;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/Navigation/Navbar/Navbar.tsxlanding-page/src/App.tsxlanding-page/src/Pages/Landing page/Navbar.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
frontend/src/constants/routes.ts (1)
ROUTES(1-12)
🔇 Additional comments (4)
landing-page/src/App.tsx (1)
18-27: LGTM! Proper anchor target for hash-based navigation.The addition of
id="home"provides the necessary target for the smooth scroll navigation implemented in the landing page Navbar. This change coordinates well with the hash-based navigation links (to="#home") and offset scrolling logic.landing-page/src/Pages/Landing page/Navbar.tsx (1)
151-151: LGTM! Hash-based navigation aligns with the home anchor.The change from
to="/"toto="#home"withisScrollLink={true}properly implements in-page scrolling that coordinates with theid="home"target added in App.tsx. This provides smooth scrolling without triggering route changes.frontend/src/components/Navigation/Navbar/Navbar.tsx (2)
25-28: Excellent fix for SPA navigation! Verify route definitions exist.The replacement of anchor tags with React Router
Linkcomponents directly addresses the PR objective of preventing full page reloads. The navigation will now stay within the SPA without triggering white flashes.However, ensure that the routes
/homeand/settingsare properly defined in your routing configuration.#!/bin/bash # Search for route definitions to verify /home and /settings routes exist # Look for Routes/Route components that define 'home' and 'settings' paths rg -n --type=tsx --type=ts -C5 'path.*["\x27](home|settings)["\x27]' # Also check for route definitions using ROUTES constants rg -n --type=tsx --type=ts -C5 'ROUTES\.(HOME|SETTINGS)'Also applies to: 87-93
9-9: The importimport { Link } from 'react-router'is correct. The frontend package uses React Router v7.6.2, where'react-router'is the primary package and the correct import source for theLinkcomponent. This is consistent with other components in the frontend (e.g., AppSidebar.tsx).
1 participant
Problem
Clicking the avatar or logo triggered a full page reload (white flash)
because navigation used instead of React Router navigation.
Fix
Replaced with so navigation stays inside the SPA
No reload, smoother UX
Consistent with other internal routes
How to test
1️⃣ Open app
2️⃣ Click the avatar or logo
3️⃣ Navigation happens instantly — no white flash
Fixes: #864
Before:
Screen.Recording.2025-12-27.at.9.02.21.AM.mov
After:
Screen.Recording.2025-12-27.at.9.32.16.AM.mov
Summary by CodeRabbit
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.