-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: adopt Grok-style layout with shadcn/ui sidebar #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Layout Structure: - Remove (no-right-sidebar) and (with-right-sidebar) route groups - Move pages directly under app/(root)/ - Delete redundant nested layout files Root Layout: - Add main content padding directly to root layout Both nested layouts had identical styling, making the route group separation unnecessary. This simplifies the directory structure and consolidates layout logic in one place.
Components: - Add sidebar.tsx, input.tsx, separator.tsx, skeleton.tsx, tooltip.tsx - Add use-mobile.ts hook for responsive behaviour - Add biome-ignore comments for shadcn/ui lint compatibility Configuration: - Fix components.json CSS path from globals.css to app/globals.css - Install @radix-ui/react-separator and @radix-ui/react-tooltip The CSS path correction resolves Tailwind IntelliSense warnings in VS Code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sidebar: - Replace custom sidebar-provider with shadcn/ui SidebarProvider - Create AppSidebar using SidebarHeader, SidebarContent, SidebarFooter, SidebarRail - Add cookie-based state persistence (sidebar_state) for SSR-correct initial render - Rename sidebar-toggle to sidebar-toggle-button, use shadcn useSidebar hook Navigation: - Simplify nav-link to mobile-only (desktop uses SidebarMenuButton directly) - Extract shared utilities: isRouteActive(), getNavIconClasses(), NAV_LINK_*_CLASSES Configuration: - Adjust sidebar width to 14rem expanded, 3rem collapsed - Change breakpoint from md to sm for sidebar visibility - Set default theme to dark Replaces localStorage-based state with cookie persistence, eliminating hydration mismatches. Adds keyboard shortcut (Ctrl+B), edge-click rail toggle, and tooltip support on collapsed menu items. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Navigation: - Delete `navbar.tsx` in favour of split desktop/mobile headers - Add `content-top-bar.tsx` for desktop (sticky, inside content area) - Add `mobile-header.tsx` for mobile (fixed, viewport-wide) - Make sidebar full viewport height (remove 72px offset) Theming: - Add `HEADER_HEIGHT`, `CONTENT_HORIZONTAL_PADDING`, `MOBILE_HEADER_TOP_OFFSET` to utils - Add tooltip variants (default, sidebar) using class-variance-authority - Reduce logo height from 7 to 6 across components Tests: - Add exact: true to navigation link selectors to avoid logo matches Moves the sidebar flush with viewport top while desktop search/auth controls shift into a content-area top bar. Mobile retains fixed header with hamburger menu. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRoot layout and navigation were refactored: server-side RootLayout now reads cookies for sidebar state; the old Navbar/LeftSidebar and a client-side sidebar provider were removed and replaced by a new cookie-backed Sidebar UI, AppSidebar, MobileHeader, ContentTopBar, several UI primitives, utilities and tests/config updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant RootLayout as RootLayout (server)
participant Cookie as Cookies
participant SidebarProv as SidebarProvider (cookie-backed)
participant AppSidebar as AppSidebar
participant MobileHdr as MobileHeader
participant User as User
RootLayout->>Cookie: read sidebar_state cookie
RootLayout->>SidebarProv: initialise(defaultOpen from cookie)
SidebarProv->>AppSidebar: provide state & handlers
SidebarProv->>MobileHdr: provide state & handlers
User->>AppSidebar: click toggle
AppSidebar->>SidebarProv: toggleOpen()
SidebarProv->>Cookie: write updated sidebar_state (max-age)
SidebarProv-->>AppSidebar: state updated (open/closed)
SidebarProv-->>MobileHdr: state updated (mobile open/closed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js,jsx,md}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
components/clerk-provider.tsx📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-12-10T20:20:54.402ZApplied to files:
📚 Learning: 2025-12-19T05:32:53.729ZApplied to files:
📚 Learning: 2025-12-19T05:32:53.729ZApplied to files:
📚 Learning: 2025-12-10T20:20:46.607ZApplied to files:
📚 Learning: 2025-12-19T05:32:53.729ZApplied to files:
📚 Learning: 2025-12-25T15:46:08.787ZApplied to files:
📚 Learning: 2025-12-19T05:32:53.729ZApplied to files:
📚 Learning: 2025-12-19T05:32:53.729ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonx_docs/figma/grok-screenshot.jpgis excluded by!**/*.jpg
📒 Files selected for processing (33)
CLAUDE.mdapp/(root)/(no-right-sidebar)/layout.tsxapp/(root)/(with-right-sidebar)/layout.tsxapp/(root)/ask-question/page.tsxapp/(root)/collections/page.tsxapp/(root)/community/page.tsxapp/(root)/jobs/page.tsxapp/(root)/layout.tsxapp/(root)/page.tsxapp/(root)/profile/page.tsxapp/(root)/tags/page.tsxapp/layout.tsxcomponents.jsoncomponents/app-sidebar.tsxcomponents/navigation/content-top-bar.tsxcomponents/navigation/full-logo.tsxcomponents/navigation/left-sidebar.tsxcomponents/navigation/mobile-header.tsxcomponents/navigation/mobile-nav.tsxcomponents/navigation/nav-link.tsxcomponents/navigation/navbar.tsxcomponents/navigation/sidebar-toggle-button.tsxcomponents/sidebar-provider.tsxcomponents/ui/input.tsxcomponents/ui/separator.tsxcomponents/ui/sidebar.tsxcomponents/ui/skeleton.tsxcomponents/ui/tooltip.tsxe2e/navigation.spec.tshooks/use-mobile.tslib/utils.tspackage.jsonx_docs/mine/2025-12-25-layout-refactor.md
💤 Files with no reviewable changes (5)
- app/(root)/(no-right-sidebar)/layout.tsx
- components/navigation/navbar.tsx
- app/(root)/(with-right-sidebar)/layout.tsx
- components/sidebar-provider.tsx
- components/navigation/left-sidebar.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English throughout the project
Files:
components/ui/skeleton.tsxcomponents/ui/input.tsxcomponents/app-sidebar.tsxcomponents/navigation/mobile-header.tsxcomponents/navigation/full-logo.tsxe2e/navigation.spec.tscomponents/navigation/content-top-bar.tsxcomponents/ui/separator.tsxcomponents/ui/tooltip.tsxlib/utils.tsapp/(root)/layout.tsxhooks/use-mobile.tscomponents/ui/sidebar.tsxcomponents/navigation/mobile-nav.tsxapp/layout.tsxcomponents/navigation/sidebar-toggle-button.tsxCLAUDE.mdcomponents/navigation/nav-link.tsxx_docs/mine/2025-12-25-layout-refactor.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Only add 'use client' directive when interactivity is needed
Avoid manual useMemo/useCallback unless profiling shows need
Always use @/ import aliases, even for sibling imports (e.g., @/app/fonts not ./fonts)
Files:
components/ui/skeleton.tsxcomponents/ui/input.tsxcomponents/app-sidebar.tsxcomponents/navigation/mobile-header.tsxcomponents/navigation/full-logo.tsxe2e/navigation.spec.tscomponents/navigation/content-top-bar.tsxcomponents/ui/separator.tsxcomponents/ui/tooltip.tsxlib/utils.tsapp/(root)/layout.tsxhooks/use-mobile.tscomponents/ui/sidebar.tsxcomponents/navigation/mobile-nav.tsxapp/layout.tsxcomponents/navigation/sidebar-toggle-button.tsxcomponents/navigation/nav-link.tsx
🧠 Learnings (10)
📚 Learning: 2025-12-10T20:20:46.607Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:46.607Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) from clerk/nextjs can be used inside Server Components without adding 'use client' in the consuming component. They manage client/server boundary internally. When reviewing code, prefer omitting 'use client' in server components that render these Clerk components and avoid introducing client directives solely for these components. This guideline helps maintain server/server boundary and reduce client bundle size.
Applied to files:
components/ui/skeleton.tsxcomponents/ui/input.tsxcomponents/app-sidebar.tsxcomponents/navigation/mobile-header.tsxcomponents/navigation/full-logo.tsxcomponents/navigation/content-top-bar.tsxcomponents/ui/separator.tsxcomponents/ui/tooltip.tsxapp/(root)/layout.tsxcomponents/ui/sidebar.tsxcomponents/navigation/mobile-nav.tsxapp/layout.tsxcomponents/navigation/sidebar-toggle-button.tsxcomponents/navigation/nav-link.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/auth/clerk-signin.tsx : Sign In component (components/auth/clerk-signin.tsx) should be a client component with theme-aware logo
Applied to files:
components/app-sidebar.tsxcomponents/navigation/mobile-header.tsxcomponents/navigation/full-logo.tsxcomponents/navigation/content-top-bar.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Next.js 16 with App Router (not Pages Router)
Applied to files:
components/app-sidebar.tsxapp/(root)/layout.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/auth/clerk-signup.tsx : Sign Up component (components/auth/clerk-signup.tsx) should use static logo
Applied to files:
components/app-sidebar.tsxcomponents/navigation/mobile-header.tsxcomponents/navigation/full-logo.tsxcomponents/navigation/content-top-bar.tsx
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to **/*.{css,postcss} : Use Tailwind v4 import syntax: import "tailwindcss" (not tailwind directives)
Applied to files:
components.json
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Tailwind CSS v4 with PostCSS
Applied to files:
components.json
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to components/clerk-provider.tsx : Implement ClerkProvider in components/clerk-provider.tsx applying shadcn theme and Inter font
Applied to files:
components/navigation/content-top-bar.tsxpackage.jsonapp/layout.tsx
📚 Learning: 2025-12-10T20:20:54.402Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 7
File: components/navigation/navbar/index.tsx:1-12
Timestamp: 2025-12-10T20:20:54.402Z
Learning: Clerk's Next.js components (SignedIn, SignedOut, SignInButton, SignUpButton, UserButton) exported from clerk/nextjs are designed to work in Server Components without requiring a "use client" directive in the consuming component, as they handle the client/server boundary internally with their own directives.
Applied to files:
package.json
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Vitest for unit testing and Playwright for E2E testing
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-19T05:32:53.729Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Lefthook for Git hooks: pre-commit runs lint, typecheck, and unit tests; pre-push runs E2E tests
Applied to files:
CLAUDE.md
🧬 Code graph analysis (8)
components/ui/skeleton.tsx (1)
lib/utils.ts (1)
cn(4-6)
components/ui/input.tsx (1)
lib/utils.ts (1)
cn(4-6)
components/app-sidebar.tsx (5)
components/ui/sidebar.tsx (10)
Sidebar(705-705)SidebarHeader(712-712)SidebarContent(706-706)SidebarGroup(708-708)SidebarGroupContent(710-710)SidebarMenu(715-715)SidebarMenuItem(719-719)SidebarMenuButton(718-718)SidebarFooter(707-707)SidebarRail(725-725)components/navigation/full-logo.tsx (1)
ThemeLogo(15-27)components/navigation/nav-links.constants.ts (1)
NAV_LINKS(9-20)lib/utils.ts (5)
isRouteActive(12-14)cn(4-6)NAV_LINK_ACTIVE_CLASSES(17-18)NAV_LINK_INACTIVE_CLASSES(19-19)getNavIconClasses(29-31)components/navigation/sidebar-toggle-button.tsx (1)
SidebarToggleButton(8-32)
components/ui/separator.tsx (1)
lib/utils.ts (1)
cn(4-6)
components/ui/tooltip.tsx (1)
lib/utils.ts (1)
cn(4-6)
app/(root)/layout.tsx (1)
lib/utils.ts (3)
cn(4-6)MOBILE_HEADER_TOP_OFFSET(24-24)CONTENT_HORIZONTAL_PADDING(22-22)
components/ui/sidebar.tsx (8)
hooks/use-mobile.ts (1)
useIsMobile(5-21)components/ui/tooltip.tsx (4)
TooltipProvider(91-91)TooltipContent(91-91)Tooltip(91-91)TooltipTrigger(91-91)lib/utils.ts (1)
cn(4-6)components/ui/sheet.tsx (5)
Sheet(131-131)SheetContent(134-134)SheetHeader(135-135)SheetTitle(137-137)SheetDescription(138-138)components/ui/button.tsx (1)
Button(65-65)components/ui/input.tsx (1)
Input(21-21)components/ui/separator.tsx (1)
Separator(28-28)components/ui/skeleton.tsx (1)
Skeleton(13-13)
components/navigation/sidebar-toggle-button.tsx (3)
components/ui/sidebar.tsx (1)
useSidebar(728-728)components/ui/button.tsx (1)
Button(65-65)lib/utils.ts (1)
cn(4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run E2E Tests
🔇 Additional comments (25)
components/ui/input.tsx (1)
1-3: LGTM: Imports follow best practices.The type-only import for React and the use of the
@/alias for the utility import are correct per project guidelines.components.json (1)
8-8: LGTM! CSS path correctly updated for app directory structure.The path update from "globals.css" to "app/globals.css" correctly aligns with Next.js app directory conventions and ensures shadcn/ui CLI can locate the global styles file.
components/navigation/mobile-nav.tsx (1)
78-78: LGTM! Padding adjustment aligns with new layout design.The reduction in top padding from
pt-9topt-4is consistent with the Grok-style layout adjustments mentioned in the PR objectives, providing tighter spacing between the logo and navigation links.components/navigation/full-logo.tsx (1)
20-20: LGTM! Logo size adjustment fits the new layout.The height reduction from
h-7toh-6appropriately scales the logo for the redesigned header and sidebar components whilst maintaining the correct aspect ratio.e2e/navigation.spec.ts (1)
12-13: Excellent improvement to test reliability!Adding
exact: trueto all navigation link selectors prevents false matches with logo links and other similarly named elements. This makes the tests more robust and follows Playwright best practices for precise element targeting.Also applies to: 32-33, 50-52, 56-58, 66-68, 75-77
components/ui/separator.tsx (1)
1-28: LGTM! Separator component follows shadcn/ui conventions correctly.The component correctly:
- Includes the "use client" directive (required for Radix UI primitives)
- Uses the @/ import alias
- Applies proper TypeScript typing
- Follows the standard shadcn/ui component pattern with data-slot and className composition
components/ui/skeleton.tsx (1)
1-13: LGTM! Skeleton component correctly omits "use client" directive.The component appropriately remains a Server Component as it contains no interactivity, following the coding guideline to "only add 'use client' directive when interactivity is needed". The implementation is clean and follows shadcn/ui patterns.
hooks/use-mobile.ts (1)
1-21: Critical: Missing "use client" directive for hooks.This file uses React hooks (
useState,useEffect) but lacks the required"use client"directive at the top of the file. All files using hooks must include this directive.🔎 Proposed fix
+"use client"; + import * as React from "react"; const MOBILE_BREAKPOINT = 640; // tailwind sm breakpoint⛔ Skipped due to learnings
Learnt from: CR Repo: michellepace/devflow PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T05:32:53.729Z Learning: Applies to **/*.{ts,tsx} : Only add 'use client' directive when interactivity is neededpackage.json (1)
28-30: The added Radix UI dependencies are appropriate and current. Both@radix-ui/react-separator@^1.1.8and@radix-ui/react-tooltip@^1.2.8are at their latest available versions and have no known security vulnerabilities.lib/utils.ts (1)
12-14: Potential edge case in route matching logic.The current implementation of
isRouteActivewill match nested routes correctly, but could produce false positives in edge cases. For example, if you have both/tagand/tagsroutes, visiting/tagswould incorrectly mark/tagas active because"/tags".startsWith("/tag/")is false, but the function doesn't account for partial path segment matches.However, reviewing the typical route structure, this is unlikely to be an issue if routes are well-named. The current logic is appropriate for most use cases.
components/ui/tooltip.tsx (1)
1-91: LGTM! Well-structured tooltip component with variants.The implementation follows shadcn/ui patterns correctly. Each
Tooltipcomponent wraps itself in aTooltipProvider(lines 56-58), which is the recommended pattern for individual tooltip instances. The variant system cleanly separates default and sidebar styling.components/navigation/nav-link.tsx (1)
1-49: LGTM! Clean mobile navigation implementation.The component correctly uses the new utility functions from
lib/utils.tsand clearly documents its purpose for mobile Sheet navigation. The 'use client' directive is appropriate for theusePathnamehook usage.components/ui/sidebar.tsx (3)
27-28: LGTM! Appropriate cookie configuration for UI state.The cookie name and max-age (7 days) are reasonable for persisting sidebar state. Since this stores only UI preference (open/closed), the security posture is appropriate.
85-89: Document cookie usage is appropriately justified.The biome-ignore comment correctly explains this is the shadcn/ui pattern for sidebar state persistence. The cookie stores only boolean state, not sensitive data, and uses proper path and max-age settings.
117-129: Review the exhaustive dependencies ignore.The
useExhaustiveDependenciesignore on line 117 includessetOpenMobilein the dependencies despite the comment saying it's for "clarity". However,setOpenMobilecomes fromuseStateand is stable, so including it doesn't cause issues. The biome-ignore is acceptable, though the dependency array could technically omitsetOpenMobile.app/layout.tsx (1)
33-33: Verify the intentional default theme change to dark mode.The
defaultThemehas been changed to"dark". This means users will see dark mode by default instead of respecting their system preference. Confirm this aligns with the product requirements.If the goal is to provide a better initial experience whilst still respecting system preferences, consider whether
enableSystemon line 34 should remaintrue, as it may override the dark default.components/navigation/mobile-header.tsx (1)
1-28: LGTM! Appropriate server component usage.The component correctly omits the 'use client' directive, as it contains no interactivity at this level. The child components (
ThemeToggleandMobileNav) manage their own client boundaries. The biome-ignore comments appropriately justify the img tag usage for SVG optimisation and decorative image handling.Based on learnings: Clerk and other client-interactive components manage their own client/server boundaries, so parent server components don't need 'use client'.
components/navigation/content-top-bar.tsx (1)
6-34: LGTM! Correct server component usage with Clerk.The component appropriately omits the 'use client' directive whilst using Clerk's authentication components, which manage the client/server boundary internally. British English is correctly used in comments ("centres" on line 15).
Based on learnings: Clerk's Next.js components can be used in Server Components without adding 'use client'.
components/navigation/sidebar-toggle-button.tsx (1)
23-23: The aria-controls reference is valid and correctly set.The
Sidebarelement inAppSidebar(./components/app-sidebar.tsx) has the matchingid="app-sidebar"attribute, so the ARIA accessibility attribute is properly configured.x_docs/mine/2025-12-25-layout-refactor.md (1)
1-85: LGTM! Well-structured historical documentation.The documentation clearly explains the layout refactor with helpful ASCII diagrams, comparison tables, and implementation summaries. British English spelling is correctly used throughout (e.g., "centred" on lines 36 and 45).
components/app-sidebar.tsx (3)
1-6: Appropriate use of 'use client' directive.The directive is correctly applied here since
usePathname()requires client-side execution. Based on learnings, while Clerk components can work in Server Components, theusePathnamehook necessitates this being a Client Component.
33-52: Clean implementation of collapsible logo switching.The conditional display using
group-data-[collapsible=icon]CSS selectors is an idiomatic pattern for shadcn/ui sidebars. Accessibility is properly handled witharia-labelon the parent link.
94-116: LGTM! Well-structured footer with responsive layout.The footer correctly adapts between horizontal (expanded) and vertical (collapsed) layouts using CSS data-attribute selectors. The
SignedInwrapper aroundUserButtonfollows Clerk patterns, andSidebarRailenables the edge-click toggle mentioned in the PR objectives.app/(root)/layout.tsx (2)
12-14: Correct async cookie access for Next.js 15+.The async pattern with
await cookies()is correct for Next.js 15+ where request APIs are now asynchronous. The default-open logic (!== "false") sensibly defaults to an open sidebar when no cookie is set.
17-38: LGTM! Clean Grok-style layout composition.The layout correctly implements the structure documented in the refactor notes:
MobileHeaderfor mobile-only fixed headerAppSidebarfor full-height navigation- Content area with
ContentTopBar(desktop) and padded main regionThe
MOBILE_HEADER_TOP_OFFSET(pt-14) properly accounts for the fixed mobile header, withsm:pt-10overriding it on larger screens where the mobile header is hidden.
| export const NAV_LINK_ACTIVE_CLASSES = | ||
| "bg-(image:--gradient-primary) font-bold text-primary-foreground"; | ||
| export const NAV_LINK_INACTIVE_CLASSES = "font-medium"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for gradient-primary definition in CSS files
rg -n '\-\-gradient-primary' --type=css -g '!node_modules'
# Also check in Tailwind config
rg -n 'gradient-primary' . --type=ts --type=js -g '!node_modules' | head -20
# Check for global CSS or CSS-in-JS definitions
fd -e css -e scss -e sass | xargs rg -l 'gradient-primary' 2>/dev/null || echo "Checking CSS files..."
# Check if there are any theme configurations
rg -n 'colors\|gradient' tailwind.config.ts tailwind.config.js 2>/dev/null | head -20Repository: michellepace/devflow
Length of output: 738
🏁 Script executed:
#!/bin/bash
# Check the full definition of --gradient-primary in globals.css
sed -n '95,100p' app/globals.css
# Check Tailwind configuration for custom syntax support
if [ -f tailwind.config.ts ] || [ -f tailwind.config.js ]; then
echo "=== Tailwind Config ==="
head -50 tailwind.config.ts tailwind.config.js 2>/dev/null
fiRepository: michellepace/devflow
Length of output: 277
Use correct Tailwind arbitrary value syntax for the gradient.
The --gradient-primary CSS variable is properly defined in app/globals.css. However, the syntax bg-(image:--gradient-primary) uses parentheses, which is not valid Tailwind syntax. Use square brackets with var() instead: bg-[image:var(--gradient-primary)]. This matches the correct syntax used in clerk-provider.tsx and button.tsx.
🤖 Prompt for AI Agents
In lib/utils.ts around lines 17 to 19, the NAV_LINK_ACTIVE_CLASSES uses invalid
Tailwind arbitrary syntax `bg-(image:--gradient-primary)`; replace that token
with the correct Tailwind arbitrary-value form using var():
`bg-[image:var(--gradient-primary)]` so it matches the syntax used in
clerk-provider.tsx and button.tsx and ensures Tailwind interprets the CSS
variable correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai you are incorrect:
Tailwind CSS v4 supports BOTH syntaxes - they are equivalent:
| Syntax | Type | Example |
|---|---|---|
| () parentheses | v4 shorthand | bg-(image:--gradient-primary) |
| [] brackets | Traditional | bg-[image:var(--gradient-primary)] |
I've standardised to v4 shorthand syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michellepace Thank you for the correction! You're absolutely right—I apologize for the incorrect review comment. I wasn't aware of the Tailwind CSS v4 shorthand syntax using parentheses for CSS variable references. The syntax bg-(image:--gradient-primary) is indeed valid in Tailwind v4 and equivalent to the traditional bracket syntax.
I appreciate you clarifying this and noting that the project has standardized on the v4 shorthand. I've noted this for future reviews.
✏️ Learnings added
Learnt from: michellepace
Repo: michellepace/devflow PR: 18
File: lib/utils.ts:17-19
Timestamp: 2025-12-25T15:46:08.787Z
Learning: In Tailwind CSS v4, the shorthand syntax `bg-(image:--gradient-primary)` with parentheses is valid and equivalent to the traditional bracket syntax `bg-[image:var(--gradient-primary)]`. The project michellepace/devflow has standardized on using the v4 shorthand syntax with parentheses for CSS variable references.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Applies to **/*.{css,postcss} : Use Tailwind v4 import syntax: import "tailwindcss" (not tailwind directives)
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T05:32:53.729Z
Learning: Use Tailwind CSS v4 with PostCSS
Learnt from: michellepace
Repo: michellepace/devflow PR: 18
File: components/navigation/content-top-bar.tsx:23-30
Timestamp: 2025-12-25T13:56:02.914Z
Learning: In the Grok-style layout for this project, authenticated user account controls (Clerk's UserButton) are placed in the sidebar footer (components/app-sidebar.tsx), not in the content top bar (components/navigation/content-top-bar.tsx). The content top bar only displays SignedOut authentication options (Sign in/Sign up buttons).
Adds `npm run analyse` using Next.js experimental-analyze feature for inspecting bundle composition, identifying bloat, and verifying code splits.
Components: - Replace `bg-[image:var(--gradient-primary)]` with `bg-(image:--gradient-primary)` - Apply fix to ClerkProvider and button gradient variant Tailwind v4 uses the new `bg-(image:--custom-property)` syntax for CSS custom properties in arbitrary values, replacing the v3 `bg-[image:var(--property)]` format. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
(no-right-sidebar)and(with-right-sidebar)foldersChanges
use-mobilehook and responsive sidebar behaviourleft-sidebarandnavbarwithapp-sidebarcomponentTest plan
🤖 Generated with Claude Code