Skip to content

fix: navbar active state tracking and mobile menu behavior#44

Open
rohans02 wants to merge 2 commits intoDjedAlliance:mainfrom
rohans02:fix/navbar-active-state
Open

fix: navbar active state tracking and mobile menu behavior#44
rohans02 wants to merge 2 commits intoDjedAlliance:mainfrom
rohans02:fix/navbar-active-state

Conversation

@rohans02
Copy link

@rohans02 rohans02 commented Mar 3, 2026

Addressed Issues:

Fixes #36

Changes:

src/App.tsx

  • Added activeSection / setActiveSection React state (useState<string>('home'))
  • Replaced threshold: 0.8 with { threshold: 0.1, rootMargin: '-20% 0px -60% 0px' } — the Dapps section is taller than the viewport so 80% visibility was never achieved, meaning the observer never fired for it
  • Replaced the switch/case block that used document.getElementsByClassName('active')[0].classList.remove('active') (unsafe DOM mutation, throws on fast scroll when no .active element exists) with a clean setActiveSection(sectionId) call
  • Replaced manual document.querySelector calls with a sectionMap-driven loop; added proper observer cleanup in the useEffect return

src/components/pages/header/Header.tsx

  • Added HeaderProps interface (activeSection: string, setActiveSection)
  • Header now receives active state as props instead of managing it via direct DOM
  • Active class applied conditionally in JSX: className={`navbar-link${activeSection === item.section ? ' active' : ''}`}
  • handleActive() now calls setIsOpen(false) — mobile menu closes on nav link tap
  • Nav items extracted to a navItems array to eliminate repetition

Screenshots/Recordings:

Djed-navbar-resolve.mp4
Djed-mobile-navbar-resolve-1.mp4

Checklist

  • My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • My code follows the project's code style and conventions.
  • If applicable, I have made corresponding changes or additions to the documentation.
  • If applicable, I have made corresponding changes or additions to tests.
  • My changes generate no new warnings or errors.
  • I have joined the Stability Nexus's Discord server and I will share a link to this PR with the project maintainers there.
  • I have read the Contribution Guidelines.
  • Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.

⚠️ AI Notice - Important!

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.

Summary by CodeRabbit

New Features

  • Header navigation now intelligently tracks your position on the page and automatically highlights the active section as you scroll.
  • Enhanced navigation with improved state management and visual feedback for active links.
  • Mobile menu automatically closes after selecting a navigation item for a cleaner user experience.

@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

This pull request refactors the navbar active state tracking system from direct DOM manipulation to React state-driven approach. It introduces a centralized activeSection state in App.tsx with IntersectionObserver-based section tracking, passes this state to the Header component, and replaces hard-coded navigation links with dynamic rendering based on active section matching.

Changes

Cohort / File(s) Summary
App State Management
src/App.tsx
Introduces activeSection state and replaces direct DOM class manipulation with setActiveSection callback. Expands IntersectionObserver configuration (threshold and rootMargin adjustments) to observe all sections via sectionMap. Properly cleans up observer subscriptions on unmount. Propagates activeSection to Header component via new props.
Header Component Refactor
src/components/pages/header/Header.tsx
Converts Header to typed, prop-driven component with new HeaderProps interface. Replaces DOM-based active state management with activeSection prop and setActiveSection callback. Updates handleActive to delegate state changes via callback, closes mobile menu on navigation, and replaces hard-coded navigation links with dynamic navItems array mapping that applies active class conditionally.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 The nav now knows which section's in sight,
With state and observers, the tracking's just right!
No more DOM tricks or menu that sticks—
React state flows smooth through navigation tricks! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: fixing navbar active state tracking and mobile menu behavior, which are the core objectives.
Linked Issues check ✅ Passed All three objectives from issue #36 are met: IntersectionObserver config adjusted to detect tall sections, mobile menu closes on nav link tap via setIsOpen(false), and active state moved from DOM manipulation to React state.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #36: activeSection state management, IntersectionObserver config updates, Header prop-driven approach, and navItems array extraction are all required for the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/App.tsx (1)

34-38: Avoid duplicating section metadata across App and Header.

sectionMap here and navItems in Header.tsx encode the same section ids independently. Consider a shared constant to prevent drift-related nav/observer mismatches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 34 - 38, The section ids are duplicated between the
sectionMap object in App (sectionMap) and navItems in Header (navItems); extract
a shared constant (e.g., SECTION_IDS or NAV_SECTIONS) exported from a single
module and import it into both App and Header so both use the same source of
truth; update App to replace sectionMap with the imported constant (or a derived
Record if mapping needed) and update Header to consume the same constant so
navItems and any scroll/observer logic reference identical ids.
🤖 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/App.tsx`:
- Around line 34-38: The section ids are duplicated between the sectionMap
object in App (sectionMap) and navItems in Header (navItems); extract a shared
constant (e.g., SECTION_IDS or NAV_SECTIONS) exported from a single module and
import it into both App and Header so both use the same source of truth; update
App to replace sectionMap with the imported constant (or a derived Record if
mapping needed) and update Header to consume the same constant so navItems and
any scroll/observer logic reference identical ids.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da4bec7 and 6ae0d12.

📒 Files selected for processing (2)
  • src/App.tsx
  • src/components/pages/header/Header.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Fix navbar active state tracking and mobile menu behavior

1 participant