fix: restore mobile sidebar accessibility#432
Conversation
Walkthrough
ChangesMain layout mobile sidebar trigger
🎯 2 (Simple) | ⏱️ ~10 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/(main)/layout.tsx (2)
56-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType
MainContentas a TypeScript FC for consistency.Lines 56-71 define
MainContentas a plain function; this should follow the TS FC typing convention used by the repo guideline.Suggested change
-function MainContent({ children }: { children: React.ReactNode }) { +const MainContent: React.FC<{ children: React.ReactNode }> = ({ children }) => { return ( <main className={cn( "flex flex-1 flex-col gap-4 lg:gap-6 transition-all duration-300", `${GeistSans.variable} ${GeistMono.variable}` )} > <div className="md:hidden p-2"> <SidebarTrigger /> </div> {children} </main> ); -} +};As per coding guidelines, "
**/*.tsx: Use functional components with TypeScript FC types."🤖 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 `@app/`(main)/layout.tsx around lines 56 - 71, MainContent is currently declared as a plain function; update it to use the repo’s TypeScript FC convention for consistency. Adjust the MainContent component definition in the layout component to be typed as a React FC while keeping its existing children prop and JSX behavior unchanged.Source: Coding guidelines
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicate sidebar imports.
Line 13 adds a second import from
@/components/ui/sidebar; merge it into the existing Line 8 import to keep import grouping/order consistent.Suggested change
-import { SidebarInset, SidebarProvider, useSidebar } from "`@/components/ui/sidebar`"; +import { SidebarInset, SidebarProvider, SidebarTrigger, useSidebar } from "`@/components/ui/sidebar`"; ... -import { SidebarTrigger } from "`@/components/ui/sidebar`";As per coding guidelines, "
**/*.{ts,tsx}: Order imports: React/Next imports first, then components, then utilities."🤖 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 `@app/`(main)/layout.tsx at line 13, There is a duplicate import from "`@/components/ui/sidebar`" in the layout component; consolidate it into the existing sidebar import to keep imports grouped and ordered correctly. Update the imports in app/(main)/layout.tsx so the SidebarTrigger symbol is merged with the other sidebar exports in the same import statement, preserving the React/Next, components, utilities ordering guideline.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@app/`(main)/layout.tsx:
- Around line 56-71: MainContent is currently declared as a plain function;
update it to use the repo’s TypeScript FC convention for consistency. Adjust the
MainContent component definition in the layout component to be typed as a React
FC while keeping its existing children prop and JSX behavior unchanged.
- Line 13: There is a duplicate import from "`@/components/ui/sidebar`" in the
layout component; consolidate it into the existing sidebar import to keep
imports grouped and ordered correctly. Update the imports in
app/(main)/layout.tsx so the SidebarTrigger symbol is merged with the other
sidebar exports in the same import statement, preserving the React/Next,
components, utilities ordering guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 535069b7-8930-49f5-8213-b37db1cf0862
📒 Files selected for processing (1)
app/(main)/layout.tsx
|
Hi @nndn ,could you please review this. |
1 participant
Summary
This PR restores access to the mobile sidebar by rendering
SidebarTriggerfrom the shared application layout.Problem
On viewports below 768px, the sidebar is rendered as a
Sheet, but no visible trigger is available to open it, making navigation inaccessible.Solution
SidebarTriggerinapp/(main)/layout.tsxmd:hiddenTesting
Verified on:
Fixes #431
Summary by CodeRabbit