App shell: prototype-style sidebar for dashboard, explore, start - #2
Conversation
…l only) Collapsible sidenav on the elevated card surface: logo, member nav with icons and active states, 232px to 64px icon rail persisted in localStorage, hidden below 900px. Pages are wrapped only; no wallet, data, or logic changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughChangesApp shell navigation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Page
participant AppShell
participant AppSidenav
participant localStorage
Page->>AppShell: Render page content as children
AppShell->>AppSidenav: Render shared sidebar
AppSidenav->>localStorage: Load persisted collapsed state
AppSidenav->>AppSidenav: Match current pathname and render active item
AppSidenav->>localStorage: Persist toggle changes
AppShell-->>Page: Render sidebar with page content
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/web/components/app-shell/app-shell.tsx (1)
5-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
min-w-0to the flex child.By default, flex children have
min-width: auto, meaning they cannot shrink below the intrinsic width of their content. If thechildrencontain wide elements (like code blocks, data tables, or long strings without wrapping), it will force this wrapper to expand beyond the viewport and break the layout. Addingmin-w-0prevents this and allows internal content to truncate or scroll properly.♻️ Proposed refactor
export function AppShell({ children }: { children: ReactNode }) { return ( <div className="flex min-h-screen"> <AppSidenav /> - <div className="flex-1">{children}</div> + <div className="flex-1 min-w-0">{children}</div> </div> ); }🤖 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 `@packages/web/components/app-shell/app-shell.tsx` around lines 5 - 12, Update the flex child wrapper next to AppSidenav in AppShell to include the Tailwind min-w-0 utility, allowing children content to shrink within the available viewport while preserving the existing flex-1 behavior.
🤖 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.
Inline comments:
In `@packages/web/app/dashboard/page.tsx`:
- Around line 5-7: Move the shared AppShell wrapper into a persistent Next.js
route-group layout, such as app/(main)/layout.tsx, and place the dashboard,
explore, and start routes under that group. In
packages/web/app/dashboard/page.tsx lines 5-7, packages/web/app/explore/page.tsx
lines 6-8, and packages/web/app/start/page.tsx lines 5-7, remove the AppShell
wrappers and restore each page’s prior return structure so the layout owns the
shell across navigation.
---
Nitpick comments:
In `@packages/web/components/app-shell/app-shell.tsx`:
- Around line 5-12: Update the flex child wrapper next to AppSidenav in AppShell
to include the Tailwind min-w-0 utility, allowing children content to shrink
within the available viewport while preserving the existing flex-1 behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ea2c276-fc65-4586-beb7-5a40b6b47bb6
📒 Files selected for processing (5)
packages/web/app/dashboard/page.tsxpackages/web/app/explore/page.tsxpackages/web/app/start/page.tsxpackages/web/components/app-shell/app-shell.tsxpackages/web/components/app-shell/app-sidenav.tsx
| return ( | ||
| <main className="shell"> | ||
| <DashboardShell /> | ||
| </main> | ||
| <AppShell><main className="shell"><DashboardShell /></main></AppShell> | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Move the shared AppShell wrapper to a Next.js layout to prevent state loss on navigation.
Placing the shared AppShell component inside individual page.tsx files causes it to completely unmount and remount on every route transition between these pages. Because AppSidenav initializes its collapsed state to false and updates it via a client-side useEffect, this unmounting causes the sidebar to "forget" its state during navigation. As a result, the sidebar will revert to its expanded state and visibly animate closed on every single page transition.
To preserve UI state and avoid unnecessary re-renders, shared shells must be implemented in a Next.js layout.tsx file rather than wrapping each page component individually. The idiomatic approach is to create a Route Group (e.g., app/(main)/layout.tsx returning the AppShell) and move these specific routes inside it.
packages/web/app/dashboard/page.tsx#L5-L7: Remove the<AppShell>wrapper and restore the previous return structure.packages/web/app/explore/page.tsx#L6-L8: Remove the<AppShell>wrapper and restore the previous return structure.packages/web/app/start/page.tsx#L5-L7: Remove the<AppShell>wrapper and restore the previous return structure.
📍 Affects 3 files
packages/web/app/dashboard/page.tsx#L5-L7(this comment)packages/web/app/explore/page.tsx#L6-L8packages/web/app/start/page.tsx#L5-L7
🤖 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 `@packages/web/app/dashboard/page.tsx` around lines 5 - 7, Move the shared
AppShell wrapper into a persistent Next.js route-group layout, such as
app/(main)/layout.tsx, and place the dashboard, explore, and start routes under
that group. In packages/web/app/dashboard/page.tsx lines 5-7,
packages/web/app/explore/page.tsx lines 6-8, and packages/web/app/start/page.tsx
lines 5-7, remove the AppShell wrappers and restore each page’s prior return
structure so the layout owns the shell across navigation.
1 participant
Presentational-only pass so the dApp matches the prototype and landing design language.
What changed
What did NOT change
Verified
Merge if you want it for judging; safe to leave for after. Your call.
🤖 Generated with Claude Code
Summary by CodeRabbit