fix(web): Resolve mobile overflow in settings layout and navigation - #165
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughLayout and responsive updates across settings, integrations, and app root: added Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/components/settings/settings-nav.tsx (1)
59-67: Addaria-currenton active mobile links for better screen-reader context.Active state is currently visual; adding
aria-current="page"improves nav accessibility with minimal change.♿ Suggested update
<Link key={item.href} href={item.href} + aria-current={isActive ? "page" : undefined} className={cn( "relative flex shrink-0 items-center justify-center gap-2 whitespace-nowrap rounded-md px-3 py-1.5 text-sm font-medium transition-colors", isActive ? "text-foreground" : "text-muted-foreground hover:text-foreground", )} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/components/settings/settings-nav.tsx` around lines 59 - 67, The Link elements rendering each nav item (the JSX using Link with key={item.href}, href={item.href}, className={cn(...)} and the isActive conditional) need an accessibility attribute: when isActive is true add aria-current="page" to the Link to expose the active state to screen readers; update the Link props so aria-current={isActive ? "page" : undefined} (or only include the attribute when isActive) so mobile active links announce current page.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/components/settings/settings-nav.tsx`:
- Around line 59-67: The Link elements rendering each nav item (the JSX using
Link with key={item.href}, href={item.href}, className={cn(...)} and the
isActive conditional) need an accessibility attribute: when isActive is true add
aria-current="page" to the Link to expose the active state to screen readers;
update the Link props so aria-current={isActive ? "page" : undefined} (or only
include the attribute when isActive) so mobile active links announce current
page.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(protected)/settings/layout.tsxapps/web/components/integrations/integration-row.tsxapps/web/components/integrations/primary-calendar-selector.tsxapps/web/components/integrations/primary-task-list-selector.tsxapps/web/components/settings/settings-nav.tsx
Summary
How to test
Related
Summary by CodeRabbit