Fix mobile app nav expansion button

[MikeShi42]

Fixes #1430

On small screens (width < 900px), the nav expansion button now works correctly by:

• Using a separate isMobileNavOpen state for mobile screens
• Showing an overlay backdrop when mobile nav is expanded
• Auto-closing the nav when navigating to a new page
• Maintaining the existing behavior for desktop screens

Generated with Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: 8d78844

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
hyperdx-v2-oss-app	Ready	Preview	Comment	Dec 2, 2025 11:13pm

[claude]

PR Review

✅ No critical issues found.

The implementation is clean and follows good patterns:

• ✅ Uses separate state (isMobileNavOpen) for mobile navigation without affecting desktop behavior
• ✅ Properly auto-closes mobile nav on route changes
• ✅ Uses Mantine's Overlay component for the backdrop (following project's Mantine-first approach)
• ✅ Correct z-index layering (overlay at 99, nav at 100)
• ✅ Proper conditional logic for small screen detection
• ✅ TypeScript typing is correct (boolean state)
• ✅ No security issues or unvalidated inputs
• ✅ Follows existing code patterns and conventions

The code is production-ready. 🚀

[MikeShi42]

@claude can you instead just default isPreferCollapsed to be isSmallScreen? So that mobile users by default will see a collapsed nav, but if they want to expand it, they can and we respect their preference? No need to include a whole new isMobileNavOpen state

[claude]

PR Review

⚠️ Race condition with window size and localStorage initialization

Issue: isSmallScreen is calculated using width from useWindowSize(), which is undefined on first render. The fallback (width ?? 1000) < 900 = false becomes the localStorage default value before the actual window size is known.

Impact: On mobile devices, the first load will set isNavCollapsed localStorage to false, then persist that value. Users won't get the mobile-optimized collapsed nav on subsequent visits.

Fix: Either use Mantine's getInitialValueInEffect option to defer localStorage initialization, or revert to the original isCollapsed = isSmallScreen || isPreferCollapsed logic where mobile is always collapsed.

[github-actions]

E2E Test Results

✅ All tests passed • 46 passed • 3 skipped • 257s

Status	Count
✅ Passed	46
❌ Failed	0
⚠️ Flaky	0
⏭️ Skipped	3

View full report →

[MikeShi42]

@claude okay let's just set defaultValue to undefined instead, and then use isCollapsed = isPreferCollapsed != null ? isPreferCollapsed : (isSmallScreen || false) to check if the user has defined a preference. If they have, we will respect their collapse preference. Otherwise, we'll default to whether it's a small screen or not. For the initial render, we'll default to false.

[claude]

PR Review

❌ Critical Issues:

• PR description doesn't match implementation → The description claims to add:

  • Separate isMobileNavOpen state for mobile screens
  • Overlay backdrop when mobile nav is expanded
  • Auto-closing nav on navigation

  None of these are present in the actual code changes.

• Logic error in line 412-413 → The ternary condition isPreferCollapsed \!= null ? isPreferCollapsed : isSmallScreen || false is redundant. isSmallScreen is already a boolean, so || false does nothing. Should be: isPreferCollapsed \!= null ? isPreferCollapsed : isSmallScreen

⚠️ Design concerns:

• The fix may not solve the issue → The change allows toggling isPreferCollapsed between true/false/undefined, but on small screens clicking the button (line 644) will set isPreferCollapsed to true, which per line 412-413 takes precedence and keeps the nav collapsed. The user would need to click twice to expand it.

• localStorage default change → Changing defaultValue from false to undefined may affect existing users who have this value already stored.

Recommended fixes:

1. Either update PR description to match actual changes, or implement the features described
2. Fix the redundant || false on line 413
3. Test the toggle behavior on mobile - the logic may need adjustment for proper expand/collapse on small screens

[MikeShi42]

@claude please fix lint error

  Error: src/AppNav.tsx(644,51): error TS2345: Argument of type '(v: boolean) => boolean' is not assignable to parameter of type 'boolean | ((prevState: boolean | undefined) => boolean | undefined) | undefined'.
    Type '(v: boolean) => boolean' is not assignable to type '(prevState: boolean | undefined) => boolean | undefined'.
      Types of parameters 'v' and 'prevState' are incompatible.
        Type 'boolean | undefined' is not assignable to type 'boolean'.
          Type 'undefined' is not assignable to type 'boolean'.

[claude]

PR Review

❌ Implementation doesn't match PR description → The PR body describes features (isMobileNavOpen state, overlay backdrop, auto-closing on navigation) that are not implemented in the code

❌ Logic bug in toggle handler → onClick={() => setIsPreferCollapsed((v: boolean | undefined) => \!v)} will toggle undefined → true → false → true, but should set to explicit boolean value based on current collapsed state

⚠️ Missing visual feedback → The collapse/expand button shows the same icon regardless of nav state (no rotation or icon change)

⚠️ Incomplete mobile UX → PR description mentions overlay backdrop and auto-close on navigation but these aren't implemented

Suggested fix:

onClick={() => setIsPreferCollapsed(\!isCollapsed)}

This ensures the toggle always sets the opposite of the current computed state, not just negating the stored preference.