feat: make navbar sticky on scroll#667
Conversation
📝 WalkthroughWalkthroughThis pull request adds sticky positioning to the header element by updating its Tailwind CSS classes. The header changes from relative to sticky positioning with Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
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)
📝 Coding Plan
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.
🧹 Nitpick comments (1)
src/components/shared/Header.jsx (1)
269-363: Consider simplifying or removing unused scroll-tracking logic.The
useEffectcontains substantial scroll-tracking code (updateHeaderStyles,updateAvatarStyles) that was designed to dynamically hide/show the header and animate the avatar. With the newsticky top-0approach, much of this logic may be redundant or could conflict with the simpler sticky behavior.If the avatar scaling animation on the home page is still desired, the logic can be retained but isolated. Otherwise, this could be significantly simplified to just set initial CSS variables without scroll listeners.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/shared/Header.jsx` around lines 269 - 363, The effect registers heavy scroll logic (updateHeaderStyles, updateAvatarStyles inside the useEffect) that is now largely redundant with the new sticky top-0 approach; either remove the scroll listeners and the functions entirely or keep only the avatar animation isolated: extract updateAvatarStyles and its dependency on isHomePage into a separate hook (or effect) that only runs on the home page, and simplify the original useEffect to set initial CSS variables (using avatarRef/headerRef and isInitial) without adding window scroll/resize listeners; ensure removal of window.addEventListener calls and corresponding removeEventListener cleanup when deleting the handlers, or move them to the isolated hook so updateHeaderStyles is removed and updateAvatarStyles remains only when isHomePage is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/shared/Header.jsx`:
- Around line 269-363: The effect registers heavy scroll logic
(updateHeaderStyles, updateAvatarStyles inside the useEffect) that is now
largely redundant with the new sticky top-0 approach; either remove the scroll
listeners and the functions entirely or keep only the avatar animation isolated:
extract updateAvatarStyles and its dependency on isHomePage into a separate hook
(or effect) that only runs on the home page, and simplify the original useEffect
to set initial CSS variables (using avatarRef/headerRef and isInitial) without
adding window scroll/resize listeners; ensure removal of window.addEventListener
calls and corresponding removeEventListener cleanup when deleting the handlers,
or move them to the isolated hook so updateHeaderStyles is removed and
updateAvatarStyles remains only when isHomePage is true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9e4e6f9-b49f-4f4c-ad56-f01d152cd441
📒 Files selected for processing (1)
src/components/shared/Header.jsx
2 participants
Addressed Issues:
Fixes #647
This PR addresses issue #647, resolving the behaviour where the Navbar hides/moves while navigating long pages. Previously, the
Headercomponent was adopting behaviour from an initial UI template that pushed the navigation out of view using calculated CSS variables attached to a scroll-trackinguseEffect.The navigation component is now permanently affixed across all environments and pages, resulting in:
Screenshots/Recordings:
Before:
After:
Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Checklist
Summary by CodeRabbit