refactor: refine hero UI, header, and light mode visibility#21
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 39 minutes and 14 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. 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 (3)
📝 WalkthroughWalkthroughAdds scroll-aware active-section highlighting to the navbar with a framer-motion indicator, introduces an animated ChangesLanding Page UI Refresh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@components/navbar.tsx`:
- Around line 102-115: The fallback navigation in navbar.tsx drops the hash by
pushing only “/”, which breaks section links from other pages. Update the
navigation logic in the relevant click handler so it preserves the full href
(including the “#section”) or only intercepts same-page hash clicks, and keep
the existing scroll behavior tied to the targetId lookup and
isClickScrolling/current activeSection updates.
- Around line 274-287: The mobile theme toggle button in Navbar is icon-only and
lacks an accessible name, so add an aria-label to the button in the theme toggle
block so assistive tech can announce it properly. Update the button used with
setTheme and setIsOpen(false) to expose a clear label like “Toggle theme” while
keeping the existing icon rendering and click behavior unchanged.
In `@features/landing/hero.tsx`:
- Around line 135-138: The anchor in hero.tsx is triggering both the onClick
handler and the default href fragment jump, which can cause a second scroll.
Update the link around scrollToUniversities to prevent the default navigation
behavior in the click handler while keeping the existing smooth scroll logic, so
the href does not fire after scrollIntoView.
🪄 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: dc4d5d38-5b58-48fb-a8b4-33d5d1df1edf
📒 Files selected for processing (4)
app/globals.csscomponents/navbar.tsxfeatures/landing/hero.tsxfeatures/landing/universities.tsx
| } else { | ||
| // If element not found, navigate to home and then scroll | ||
| router.push("/"); | ||
| setTimeout(() => { | ||
| const el = document.getElementById(targetId); | ||
| if (el) { | ||
| isClickScrolling.current = true; | ||
| setActiveSection(href); | ||
| el.scrollIntoView({ behavior: "smooth", block: "start" }); | ||
| setTimeout(() => { | ||
| isClickScrolling.current = false; | ||
| }, 800); | ||
| } | ||
| }, 100); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the hash when navigating from other pages.
This fallback pushes / and then hopes the target exists 100ms later, so section links from /about or /subjects can land on the home page without ever scrolling. Push href itself (or only intercept same-page hashes) so the #section survives navigation.
🤖 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 `@components/navbar.tsx` around lines 102 - 115, The fallback navigation in
navbar.tsx drops the hash by pushing only “/”, which breaks section links from
other pages. Update the navigation logic in the relevant click handler so it
preserves the full href (including the “#section”) or only intercepts same-page
hash clicks, and keep the existing scroll behavior tied to the targetId lookup
and isClickScrolling/current activeSection updates.
| <button | ||
| onClick={() => { | ||
| setTheme(theme === "dark" ? "light" : "dark"); | ||
| setIsOpen(false); | ||
| }} | ||
| className="flex items-center justify-center rounded-xl border border-white/20 px-4 py-3 text-[#F4F5F7] transition-all duration-200 hover:bg-white/10" | ||
| className="flex w-full items-center justify-center rounded-xl border border-border px-4 py-3 text-foreground/90 transition-all duration-200 hover:bg-black/5 hover:text-foreground dark:border-white/15 dark:text-white/90 dark:hover:bg-white/5 dark:hover:text-white focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#1D4ED8]/50 dark:focus-visible:ring-white/50" | ||
| > | ||
| {mounted && | ||
| (theme === "dark" ? ( | ||
| <Sun className="h-5 w-5" /> | ||
| <Sun className="h-[18px] w-[18px]" /> | ||
| ) : ( | ||
| <Moon className="h-5 w-5" /> | ||
| <Moon className="h-[18px] w-[18px]" /> | ||
| ))} | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add an accessible name to the mobile theme toggle.
Unlike the desktop toggle, this icon-only button has no aria-label, so assistive tech will announce it as an unlabeled button.
Suggested fix
<button
onClick={() => {
setTheme(theme === "dark" ? "light" : "dark");
setIsOpen(false);
}}
+ aria-label="Toggle theme"
className="flex w-full items-center justify-center rounded-xl border border-border px-4 py-3 text-foreground/90 transition-all duration-200 hover:bg-black/5 hover:text-foreground dark:border-white/15 dark:text-white/90 dark:hover:bg-white/5 dark:hover:text-white focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#1D4ED8`]/50 dark:focus-visible:ring-white/50"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={() => { | |
| setTheme(theme === "dark" ? "light" : "dark"); | |
| setIsOpen(false); | |
| }} | |
| className="flex items-center justify-center rounded-xl border border-white/20 px-4 py-3 text-[#F4F5F7] transition-all duration-200 hover:bg-white/10" | |
| className="flex w-full items-center justify-center rounded-xl border border-border px-4 py-3 text-foreground/90 transition-all duration-200 hover:bg-black/5 hover:text-foreground dark:border-white/15 dark:text-white/90 dark:hover:bg-white/5 dark:hover:text-white focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#1D4ED8]/50 dark:focus-visible:ring-white/50" | |
| > | |
| {mounted && | |
| (theme === "dark" ? ( | |
| <Sun className="h-5 w-5" /> | |
| <Sun className="h-[18px] w-[18px]" /> | |
| ) : ( | |
| <Moon className="h-5 w-5" /> | |
| <Moon className="h-[18px] w-[18px]" /> | |
| ))} | |
| </button> | |
| <button | |
| onClick={() => { | |
| setTheme(theme === "dark" ? "light" : "dark"); | |
| setIsOpen(false); | |
| }} | |
| aria-label="Toggle theme" | |
| className="flex w-full items-center justify-center rounded-xl border border-border px-4 py-3 text-foreground/90 transition-all duration-200 hover:bg-black/5 hover:text-foreground dark:border-white/15 dark:text-white/90 dark:hover:bg-white/5 dark:hover:text-white focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#1D4ED8`]/50 dark:focus-visible:ring-white/50" | |
| > | |
| {mounted && | |
| (theme === "dark" ? ( | |
| <Sun className="h-[18px] w-[18px]" /> | |
| ) : ( | |
| <Moon className="h-[18px] w-[18px]" /> | |
| ))} | |
| </button> |
🤖 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 `@components/navbar.tsx` around lines 274 - 287, The mobile theme toggle button
in Navbar is icon-only and lacks an accessible name, so add an aria-label to the
button in the theme toggle block so assistive tech can announce it properly.
Update the button used with setTheme and setIsOpen(false) to expose a clear
label like “Toggle theme” while keeping the existing icon rendering and click
behavior unchanged.
| <a | ||
| href="#Universities" | ||
| onClick={scrollToUniversities} | ||
| className="group relative inline-flex items-center justify-center gap-2 overflow-hidden rounded-xl bg-[#1D4ED8] px-8 py-4 font-semibold text-white shadow-lg shadow-blue-500/25 transition-all duration-300 hover:bg-[#1E40AF] hover:shadow-blue-500/40 hover:-translate-y-0.5" | ||
| className="group relative inline-flex items-center justify-center gap-2 overflow-hidden rounded-xl bg-[#1D4ED8] px-8 py-3.5 font-semibold text-white shadow-lg shadow-blue-500/25 transition-all duration-300 hover:bg-[#1E40AF] hover:shadow-blue-500/40 hover:-translate-y-0.5 active:scale-[0.98] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#1D4ED8] focus-visible:ring-offset-2 focus-visible:ring-offset-background" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Prevent the anchor's default jump here.
scrollToUniversities already calls scrollIntoView, but this <a> still follows href="#Universities" afterward. That can cause a second jump and interrupt the smooth scroll in some browsers.
Suggested fix
<a
href="`#Universities`"
- onClick={scrollToUniversities}
+ onClick={(e) => {
+ e.preventDefault();
+ scrollToUniversities();
+ }}
className="group relative inline-flex items-center justify-center gap-2 overflow-hidden rounded-xl bg-[`#1D4ED8`] px-8 py-3.5 font-semibold text-white shadow-lg shadow-blue-500/25 transition-all duration-300 hover:bg-[`#1E40AF`] hover:shadow-blue-500/40 hover:-translate-y-0.5 active:scale-[0.98] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#1D4ED8`] focus-visible:ring-offset-2 focus-visible:ring-offset-background"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a | |
| href="#Universities" | |
| onClick={scrollToUniversities} | |
| className="group relative inline-flex items-center justify-center gap-2 overflow-hidden rounded-xl bg-[#1D4ED8] px-8 py-4 font-semibold text-white shadow-lg shadow-blue-500/25 transition-all duration-300 hover:bg-[#1E40AF] hover:shadow-blue-500/40 hover:-translate-y-0.5" | |
| className="group relative inline-flex items-center justify-center gap-2 overflow-hidden rounded-xl bg-[#1D4ED8] px-8 py-3.5 font-semibold text-white shadow-lg shadow-blue-500/25 transition-all duration-300 hover:bg-[#1E40AF] hover:shadow-blue-500/40 hover:-translate-y-0.5 active:scale-[0.98] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#1D4ED8] focus-visible:ring-offset-2 focus-visible:ring-offset-background" | |
| <a | |
| href="`#Universities`" | |
| onClick={(e) => { | |
| e.preventDefault(); | |
| scrollToUniversities(); | |
| }} | |
| className="group relative inline-flex items-center justify-center gap-2 overflow-hidden rounded-xl bg-[`#1D4ED8`] px-8 py-3.5 font-semibold text-white shadow-lg shadow-blue-500/25 transition-all duration-300 hover:bg-[`#1E40AF`] hover:shadow-blue-500/40 hover:-translate-y-0.5 active:scale-[0.98] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[`#1D4ED8`] focus-visible:ring-offset-2 focus-visible:ring-offset-background" |
🤖 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 `@features/landing/hero.tsx` around lines 135 - 138, The anchor in hero.tsx is
triggering both the onClick handler and the default href fragment jump, which
can cause a second scroll. Update the link around scrollToUniversities to
prevent the default navigation behavior in the click handler while keeping the
existing smooth scroll logic, so the href does not fire after scrollIntoView.
🎉 Congratulations @nitinmohan18!Thank you for contributing to HyperLearningTech. Your pull request has been successfully merged into main. 📦 Merge Summary
🚀 Keep Contributing
Thank you for helping make HyperLearningTech better. Happy Coding! 🚀 |
RamuuXfree
left a comment
There was a problem hiding this comment.
Hi @nitinmohan18,
Thanks for your contribution! Overall, the Hero section improvements, spacing, typography, and other UI refinements look good.
I have one request for future PRs:
-
Please keep each PR focused on one specific change or feature. It makes reviewing, testing, and reverting changes much easier if needed.
-
Please revert the Header background to its original color:
background-color: #00008B; /* Tailwind */ sticky top-0 z-50 transition-all duration-300 bg-[#00008B] border-transparent
During testing, I noticed the updated Header color doesn't stay visually consistent across Light and Dark modes, whereas the original color works well in both themes.
-
Similarly, please restore the original Footer color. Since the Footer change is unrelated to the Hero/Header refinement, it would be better to submit it in a separate PR if you'd like to redesign it later.
The rest of your improvements look good. Thanks again for the effort!
@imuniqueshiv Could you please review this approach as well? Thank you.
Pull Request
Summary
Refined the Hero section and Header to deliver a more polished, premium, and responsive user experience. Improved visual hierarchy, typography, spacing, CTA consistency, and light mode visibility while preserving the existing design system.
Related Issue
Closes #18
Type of Change
What Changed?
Screenshots (UI Changes Only)
Attached updated screenshots of the Header and Hero section.
Testing
Checklist
main.npm run format:checkpasses.npm run lintpasses.npm run typecheckpasses.npm run buildpasses.Additional Notes
This PR focuses on refining the Hero and Header experience by improving visual consistency, responsiveness, accessibility, and overall UI quality without introducing functional changes.
Summary by CodeRabbit
New Features
Bug Fixes