feat: Improve mobile responsiveness with enhanced navigation and layout fixes#8
Conversation
…bile menu improvements, and theme provider updates
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit
WalkthroughReplaces the mobile navbar with a right-anchored Sheet drawer (with ESC-to-close and background scroll lock), adds a useLockScroll hook, honors prefers-reduced-motion, prevents horizontal overflow and adds iOS safe-area helpers, adjusts particle canvas CSS sizing, and configures Tailwind container padding. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant Nav as Navbar
participant Sheet as Sheet/Dialog
participant Hook as useLockScroll
participant Doc as document/body
U->>Nav: tap menu button
Nav->>Sheet: open()
Sheet->>Hook: enable lock (true)
Hook->>Doc: store scrollY, apply fixed position + hide overflow
U->>Sheet: interact / press ESC / tap close
Sheet->>Nav: onOpenChange(false)
Sheet->>Hook: enable lock (false)
Hook->>Doc: restore styles and scroll position
sequenceDiagram
autonumber
participant Theme as ThemeProvider
participant MQ as matchMedia
participant Doc as document.documentElement
Theme->>MQ: query("(prefers-reduced-motion)")
MQ-->>Theme: matches (true/false)
Theme->>Doc: add/remove "reduce-motion" class
Theme->>MQ: add change listener
MQ-->>Theme: change event
Theme->>Doc: update "reduce-motion" class
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (1)
🔇 Additional comments (5)
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 |
nabobery
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
app/globals.css(2 hunks)components/layout/navbar.tsx(4 hunks)components/theme/theme-provider.tsx(1 hunks)components/ui/particle-canvas.tsx(1 hunks)components/ui/sheet.tsx(1 hunks)lib/use-lock-scroll.ts(1 hunks)tailwind.config.ts(1 hunks)
…vigation in Navbar; enhance theme provider for better browser compatibility
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/layout/navbar.tsx (1)
52-64: Remove redundant ESC key handler.The Radix UI Sheet component already handles ESC key presses to close the dialog. This custom handler duplicates that built-in functionality and can be safely removed.
Apply this diff to remove the redundant code:
- // Handle ESC key to close menu - useEffect(() => { - const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape' && isMenuOpen) { - setIsMenuOpen(false) - } - } - - if (isMenuOpen) { - window.addEventListener('keydown', handleKeyDown) - return () => window.removeEventListener('keydown', handleKeyDown) - } - }, [isMenuOpen]) -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
components/layout/navbar.tsx(4 hunks)components/theme/theme-provider.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/theme/theme-provider.tsx
🔇 Additional comments (2)
components/layout/navbar.tsx (2)
76-76: Verify that overflow-visible doesn't cause layout issues.The
overflow-visibleclass on the nav element is unusual. While it's the default overflow behavior, explicitly setting it suggests there's a specific layout requirement. Ensure this doesn't cause unintended content overflow on smaller screens or with larger font sizes.Test the navbar at various viewport widths and with browser zoom at 200% to confirm the layout remains stable.
206-216: Verify onClick behavior with asChild Button pattern.When using
asChildon a Button, it passes props to its child component (Link). The onClick handler attempting to close the menu might not fire as expected since the Link will handle the click event for navigation. While the Sheet may close automatically on navigation, this pattern could be confusing.Consider moving the close handler directly to the Link component or verifying that the Sheet closes as expected:
<Button asChild className="w-full"> <Link href="#contact" onClick={() => setIsMenuOpen(false)} > Get in Touch </Link> </Button>
…bility; add SheetTitle and SheetDescription for mobile navigation
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
components/layout/navbar.tsx(7 hunks)
…ize early exit in Navbar useEffect for improved performance
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/layout/navbar.tsx (1)
106-118: Optional: Remove redundant ESC key handler.The Radix UI Sheet component already handles the ESC key natively to close the drawer. Since the Sheet is controlled (
open={isMenuOpen} onOpenChange={setIsMenuOpen}), the built-in ESC handler will triggeronOpenChange(false), making this custom handler redundant.Consider removing this effect to rely on Sheet's built-in keyboard handling:
- // Handle ESC key to close menu - useEffect(() => { - const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape' && isMenuOpen) { - setIsMenuOpen(false) - } - } - - if (isMenuOpen) { - window.addEventListener('keydown', handleKeyDown) - return () => window.removeEventListener('keydown', handleKeyDown) - } - }, [isMenuOpen]) -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
components/layout/navbar.tsx(7 hunks)
🔇 Additional comments (3)
components/layout/navbar.tsx (3)
29-77: Excellent reduced motion implementation.The reduced-motion handling properly synchronizes local state with the hook, provides an SSR-safe fallback with matchMedia, and includes defensive compatibility for older browsers. This correctly addresses the previous review concerns about state synchronization when users toggle their OS preference.
219-328: Excellent Sheet implementation with comprehensive accessibility.The mobile navigation drawer properly implements:
- Radix UI Sheet with controlled state
- Required
SheetTitleandSheetDescriptionfor screen readers (visually hidden withsr-only)- Correct ARIA attributes (
aria-label,aria-expanded) without the semantic conflicts from previous reviews- Reduced-motion guards on all animations
- Intuitive UX patterns (menu closes on navigation, theme toggle accessible)
This implementation successfully addresses all previous accessibility and semantic concerns.
129-129: Safe-area utilities are implemented. Confirmedsafe-areaandsafe-area-inset-rightclasses inapp/globals.css; no additional changes needed.
…prove readability
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
components/layout/navbar.tsx(7 hunks)
…ference for improved accessibility
1 participant
Summary
This pull request addresses mobile responsiveness and accessibility improvements, specifically targeting the requirements outlined in issue #3. The changes focus on ensuring proper central alignment and responsive behavior across mobile devices.
Key Changes
Mobile Navigation Enhancement
Layout & Responsive Design
overflow-x: hiddenand proper width constraintsAccessibility Improvements
prefers-reduced-motionsupport for better accessibilityTechnical Enhancements
useLockScrollhook) for modal/drawer componentsIssue Resolution
This PR successfully addresses issue #3: Improve Mobile Responsiveness: Ensure Central Alignment by implementing:
The changes ensure that all content and elements are properly centered and accessible across different mobile devices and screen sizes, meeting the core requirements specified in the original issue.