feat: right sidebar base with three-column layout #23
Conversation
Changes bg-background to bg-sidebar on the desktop header component. Aligns the top bar with the sidebar colour scheme for visual consistency across the navigation areas.
globals.css: - Group variables by origin: shadcn/ui Core, Charts, Sidebar, Custom - Reorder @theme inline variables logically (was reverse alphabetical) - Add section headers throughout :root and .dark blocks - Format @layer declaration across multiple lines No CSS values changed. Purely organisational refactor for maintainability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CSS Variables: - Darken page background lightness from 0.994 to 0.99 - Set sidebar-border to match card colour in light mode, transparent in dark Utilities: - Add shadow-light class for subtle directional shadow (light mode only) Navigation: - Replace border-b on desktop top bar with shadow-light - Replace shadow-sm on mobile top bar with shadow-light Provides consistent visual separation for navigation elements while reducing visual weight in dark mode where borders are now transparent. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Allows tables without space before pipe characters, matching the project's preferred compact markdown style.
Auth Flow Tests:
- Add URL check for "factor-two" as backup when heading text varies
- Replace hamburgerButton with userMenu as auth success indicator (mobile)
- Remove brittle toHaveURL("/") assertion that fails during 2FA redirect
Clerk's OTP/2FA behaviour is non-deterministic; sometimes it prompts, sometimes it
doesn't. Using URL pattern matching alongside heading detection makes tests resilient
to Clerk's varying UI states.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Right Sidebar:
- Add RightSidebar component with Top Questions and Popular Tags sections
- Visible only on xl: breakpoint (≥1280px), hidden on smaller screens
- Sticky positioning below top bar with viewport-aware height calc
┌────────────────────────────────────┐
│ │ DesktopTopBar │
│ ├─────────────────────┬───────│
│ Left │ │ Right │
│ Side │ Page Content │ Side │
│ bar │ {children} │ bar │
│ │ │ │
└──────┴─────────────────────┴───────┘
Layout & Navigation:
- Restructure root layout to three-column flex (left sidebar | main | right)
- Migrate top bar height from TypeScript constant to CSS variable
- Add configurable width prop to Sidebar component
- Style scrollbars with ultra-thin theme-aware appearance (WebKit + Firefox)
Typography:
- Replace element-based heading styles with utility classes (.text-heading-*)
- Decouples visual heading size from semantic HTML level
Establishes foundation for the right sidebar feature. CSS variable approach
centralises layout dimensions for consistent spacing across navigation components.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Navigation components: - Replace `[var(--top-bar-height)]` with `(--top-bar-height)` syntax - Remove outdated comment about var() usage Tailwind v4 supports direct CSS custom property references without explicit var() wrapper. Produces identical CSS output with cleaner source.
|
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. WalkthroughThis PR moves top-bar sizing to CSS variables, consolidates theme tokens and heading utilities in app/globals.css, adds a RightSidebar and two-column layout in app/(root)/layout.tsx, updates sidebar width and API, removes TOP_BAR_HEIGHT, and applies heading styling changes across several pages. Changes
Sequence Diagram(s)(omitted — changes are presentational and layout-focused without new multi-step control flow that benefits from a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-12-10T20:20:46.607ZApplied to files:
📚 Learning: 2025-12-25T13:56:02.914ZApplied to files:
📚 Learning: 2025-12-28T22:41:43.683ZApplied to files:
📚 Learning: 2025-12-28T22:41:43.683ZApplied to files:
📚 Learning: 2025-12-28T22:41:43.683ZApplied to files:
🧬 Code graph analysis (1)components/navigation/mobile-nav.tsx (1)
🔇 Additional comments (4)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/authenticated.mobile.spec.ts (1)
67-83: Use British English spelling.Line 72 uses American spelling "behavior" but should use "behaviour" per the coding guidelines.
🔎 Proposed fix
- // If this fails with "pointer events intercepted", check Sheet modal={false} in mobile-nav.tsx + // If this fails with "pointer events intercepted", check Sheet modal={false} in mobile-nav.tsx const profileHeading = page.getByRole("heading", { name: "Profile details", }); await expect( profileHeading, - "Clerk 'Manage account' modal should open - if blocked, ensure Sheet uses modal={false} to allow Clerk popups", + "Clerk 'Manage account' modal should open - if blocked, ensure Sheet uses modal={false} to allow Clerk pop-ups", ).toBeVisible();Note: Also hyphenating "pop-ups" for British English consistency.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
.markdownlint.yamlapp/(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/globals.csscomponents/navigation/desktop-top-bar.tsxcomponents/navigation/left-sidebar.tsxcomponents/navigation/mobile-top-bar.tsxcomponents/navigation/right-sidebar.tsxcomponents/ui/sidebar.tsxe2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tslib/utils.ts
💤 Files with no reviewable changes (1)
- lib/utils.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English throughout the codebase (comments, strings, documentation)
Files:
app/(root)/jobs/page.tsxapp/(root)/ask-question/page.tsxapp/(root)/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/tags/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/layout.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/profile/page.tsxe2e/authenticated.mobile.spec.tscomponents/navigation/left-sidebar.tsxcomponents/ui/sidebar.tsxe2e/authenticated.desktop.spec.tsapp/(root)/collections/page.tsxapp/(root)/community/page.tsx
**/*.{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
Files:
app/(root)/jobs/page.tsxapp/(root)/ask-question/page.tsxapp/(root)/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/tags/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/layout.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/profile/page.tsxe2e/authenticated.mobile.spec.tscomponents/navigation/left-sidebar.tsxcomponents/ui/sidebar.tsxe2e/authenticated.desktop.spec.tsapp/(root)/collections/page.tsxapp/(root)/community/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use @/ import aliases, even for sibling files (use @/app/fonts instead of ./fonts)
Files:
app/(root)/jobs/page.tsxapp/(root)/ask-question/page.tsxapp/(root)/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/tags/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/layout.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/profile/page.tsxe2e/authenticated.mobile.spec.tscomponents/navigation/left-sidebar.tsxcomponents/ui/sidebar.tsxe2e/authenticated.desktop.spec.tsapp/(root)/collections/page.tsxapp/(root)/community/page.tsx
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
Learning: Applies to components/clerk-provider.tsx : ClerkProvider should be implemented in components/clerk-provider.tsx and apply shadcn theme and Inter font
📚 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:
app/(root)/jobs/page.tsxapp/(root)/ask-question/page.tsxapp/(root)/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/tags/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/layout.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/profile/page.tsxcomponents/navigation/left-sidebar.tsxcomponents/ui/sidebar.tsxapp/(root)/collections/page.tsxapp/(root)/community/page.tsx
📚 Learning: 2025-12-25T13:56:02.914Z
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 a Grok-style layout, move authenticated user controls (Clerk's UserButton) out of the top navigation and into the sidebar footer (e.g., components/app-sidebar.tsx). Ensure components/navigation/content-top-bar.tsx only renders SignedOut options (Sign in/Sign up). Apply this guideline to all TSX files under components/navigation to enforce consistent placement of authentication UI across the navigation layer.
Applied to files:
components/navigation/right-sidebar.tsxcomponents/navigation/mobile-top-bar.tsxcomponents/navigation/desktop-top-bar.tsxcomponents/navigation/left-sidebar.tsx
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
Learning: Applies to **/*.css : Tailwind CSS v4: Use import "tailwindcss" syntax instead of tailwind directives
Applied to files:
app/globals.css
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
Learning: Applies to components/clerk-provider.tsx : ClerkProvider should be implemented in components/clerk-provider.tsx and apply shadcn theme and Inter font
Applied to files:
app/globals.csscomponents/navigation/desktop-top-bar.tsx
📚 Learning: 2025-12-25T15:46:14.812Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 18
File: lib/utils.ts:17-19
Timestamp: 2025-12-25T15:46:14.812Z
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.
Applied to files:
app/globals.css
📚 Learning: 2025-12-25T13:56:09.644Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 18
File: components/navigation/content-top-bar.tsx:23-30
Timestamp: 2025-12-25T13:56:09.644Z
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).
Applied to files:
app/(root)/layout.tsxe2e/authenticated.mobile.spec.ts
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
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/navigation/desktop-top-bar.tsxe2e/authenticated.mobile.spec.ts
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
Learning: Applies to components/auth/clerk-signup.tsx : Sign Up component (components/auth/clerk-signup.tsx) should use a static logo
Applied to files:
components/navigation/desktop-top-bar.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:
e2e/authenticated.mobile.spec.ts
🧬 Code graph analysis (3)
components/navigation/mobile-top-bar.tsx (1)
lib/utils.ts (1)
cn(4-6)
app/(root)/layout.tsx (1)
components/navigation/right-sidebar.tsx (1)
RightSidebar(12-63)
components/navigation/desktop-top-bar.tsx (1)
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 (27)
.markdownlint.yaml (1)
12-12: LGTM!The addition of
MD060: falseis a sensible configuration choice that allows compact table formatting without spaces around pipes, matching the existing pattern of pragmatic rule overrides.components/navigation/mobile-top-bar.tsx (1)
6-24: LGTM!The migration to CSS variable-based height via
h-(--top-bar-height)and theshadow-lightutility aligns well with the PR objectives. The component correctly remains a Server Component without requiring"use client".components/navigation/left-sidebar.tsx (1)
34-41: LGTM!The Sidebar configuration with explicit
width="13rem"andshadow-lightstyling aligns with the new three-column layout design. The CSS variable-based height approach viah-(--top-bar-height)maintains consistency with other navigation components.components/navigation/desktop-top-bar.tsx (1)
6-29: LGTM!The styling updates align the desktop top bar with the sidebar background (
bg-sidebar) and apply consistent shadow styling. The component correctly remains a Server Component whilst using Clerk components, per established patterns.app/(root)/layout.tsx (1)
21-28: LGTM!The three-column layout structure is well-organised with semantic HTML. The flex row arrangement for content and RightSidebar provides a clean separation of concerns, with responsive visibility handled within the RightSidebar component itself.
components/navigation/right-sidebar.tsx (1)
12-62: LGTM!The RightSidebar component is well-structured with appropriate accessibility attributes (
aria-label), semantic headings, and responsive visibility (xl:block). The sticky positioning calculation correctly usesvar()withincalc()as required by CSS syntax, whilst thetop-(--top-bar-height)uses Tailwind v4 shorthand for direct property application.components/ui/sidebar.tsx (2)
156-190: Consider propagating width prop to mobile branch.The
widthprop is applied to thecollapsible="none"and desktop branches but not the mobileSheetbranch (lines 192-215), which always usesSIDEBAR_WIDTH_MOBILE. If custom width should apply on mobile as well, consider adding the style override there. If mobile should always use the fixed width, this is fine as-is.
392-400: LGTM!The ultra-thin scrollbar styling uses appropriate browser-specific approaches: WebKit pseudo-elements for Chromium/Safari and
scrollbar-width: thinwith@supportsfeature detection for Firefox. This provides a consistent visual enhancement across browsers.app/globals.css (4)
1-8: LGTM!The layer ordering declaration before imports is correctly structured, and the addition of the
clerklayer ensures proper cascade ordering with Clerk's shadcn theme. The@import "tailwindcss"syntax follows Tailwind v4 conventions per coding guidelines.
13-67: LGTM!The
@theme inlineblock is well-organised with clear section headers. Theinlinemodifier is correctly applied to embed variable values directly, avoiding CSS scope issues when referencing other variables.
224-245: LGTM!The heading utilities effectively decouple visual styling from semantic HTML elements, allowing flexibility in choosing the appropriate heading level for accessibility whilst maintaining consistent visual hierarchy. The placement in
@layer componentsis appropriate.
258-265: Verify dark mode selector compatibility.The dark mode override uses
.dark .shadow-lightwhich expects.shadow-lightto be a descendant of.dark. This should work with the@custom-variant dark (&:where(.dark, .dark *))defined on line 11, but verify that elements withshadow-lightare always within a.darkancestor when dark mode is active. If using a class-based dark mode toggle on<html>or<body>, this pattern is correct.app/(root)/tags/page.tsx (1)
2-2: LGTM! Consistent heading styling applied.The
text-heading-2xlutility correctly standardises the heading presentation across root pages, aligning with the PR's typography system updates.app/(root)/ask-question/page.tsx (1)
2-2: LGTM! Consistent heading styling applied.The heading utility correctly decouples semantic HTML from visual presentation, consistent with the typography updates across all root pages.
app/(root)/jobs/page.tsx (1)
2-2: LGTM! Consistent heading styling applied.The typography utility is correctly applied, maintaining consistency with the site-wide heading system.
app/(root)/community/page.tsx (1)
2-2: LGTM! Consistent heading styling applied.The change correctly adopts the standardised heading utilities introduced in this PR.
app/(root)/profile/page.tsx (1)
2-2: LGTM! Consistent heading styling applied.The typography utility maintains consistency with the site-wide heading updates.
app/(root)/page.tsx (2)
3-3: LGTM! Primary heading styling updated.The
text-heading-2xlutility correctly standardises the primary heading presentation.
45-45: LGTM! Secondary heading styling refactored.Replacing inline styles (
text-xl font-semibold) with thetext-heading-smutility improves maintainability and consistency across the application.app/(root)/collections/page.tsx (1)
2-2: LGTM! Consistent heading styling applied.The typography utility correctly completes the standardisation of heading styles across all root pages.
e2e/authenticated.desktop.spec.ts (2)
40-41: LGTM! Clarifying comment added.The comment correctly documents the expected UserButton visibility behaviour during authentication.
46-54: LGTM! Improved OTP detection resilience.The URL-based fallback (
page.url().includes("factor-two")) effectively handles cases where Clerk's heading text may vary, and the consolidatedneedsOtpboolean improves code clarity. This change aligns with the PR objective to make E2E tests more resilient.e2e/authenticated.mobile.spec.ts (5)
1-13: LGTM! Proper environment setup and validation.The imports and environment variable validation follow best practices for E2E testing.
18-38: LGTM! Clear sign-in flow with reusable user menu handle.The introduction of the
userMenuhandle improves test readability and maintainability. Playwright locators are lazily evaluated, so defining the handle before authentication completes is correct.
39-60: Excellent resilience improvement with URL-based OTP fallback.The enhanced OTP detection logic now handles multiple scenarios:
- Email OTP via heading text
- 2FA/TOTP via heading text
- URL-based detection as fallback when Clerk heading text varies
This directly addresses the PR objective to "improve E2E test resilience by adding a URL-based Clerk OTP detection fallback" and makes the test more robust against Clerk UI variations.
62-66: LGTM! Proper verification of authenticated state.Reopening the menu and verifying the
userMenubutton is visible provides a clear confirmation that authentication succeeded.
85-99: LGTM! Clear sign-out flow with proper verification.The sign-out flow correctly handles menu state and verifies the user is signed out by checking for the "Sign in" button. The assumption that the menu closes after modal interaction (line 86) is reasonable for this UI behaviour.
|
@coderabbitai I see that you have no actionable comments for me, that is quite impossible - my code is not perfect. Can you please do a full code review of this PR? |
|
@michellepace: You're absolutely right! I'll conduct a thorough review of all the changes in this PR. Let me examine the code in detail and provide you with actionable feedback on the implementation, potential issues, and areas for improvement. ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/authenticated.desktop.spec.ts (1)
51-57: Missing explicit wait after OTP entry causes webkit failure.After typing the OTP (line 53), the test immediately checks for the authenticated user menu (line 57) without waiting for the authentication to complete. The OTP must be submitted, verified server-side, and the authenticated UI must render—this can take time, especially in webkit.
The pipeline failure at line 57 ("Locator 'getByRole("button", { name: "Open user menu" })' was not visible within 5000ms") directly indicates this timing issue.
Add an explicit wait or assertion after OTP entry to ensure authentication completes before proceeding:
🔎 Proposed fix to add explicit wait
if (needsOtp) { await page.getByRole("textbox").first().click(); await page.keyboard.type(TEST_OTP); + // Wait for authentication to complete after OTP submission + await page.waitForURL((url) => !url.toString().includes("factor-two"), { + timeout: 10000, + }); } // Verify authenticated state - UserButton visible in left sidebar await expect(userMenu).toBeVisible();Alternatively, wait for a known authenticated-state element:
if (needsOtp) { await page.getByRole("textbox").first().click(); await page.keyboard.type(TEST_OTP); + // Wait for authentication to complete (URL navigates away from sign-in) + await expect(userMenu).toBeVisible({ timeout: 10000 }); } -// Verify authenticated state - UserButton visible in left sidebar -await expect(userMenu).toBeVisible();
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
.markdownlint.yamlapp/(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/globals.csscomponents/navigation/desktop-top-bar.tsxcomponents/navigation/left-sidebar.tsxcomponents/navigation/mobile-top-bar.tsxcomponents/navigation/right-sidebar.tsxcomponents/ui/sidebar.tsxe2e/authenticated.desktop.spec.tse2e/authenticated.mobile.spec.tslib/utils.ts
💤 Files with no reviewable changes (1)
- lib/utils.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British English throughout the codebase (comments, strings, documentation)
Files:
app/(root)/jobs/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/profile/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/layout.tsxapp/(root)/collections/page.tsxapp/(root)/page.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/community/page.tsxcomponents/ui/sidebar.tsxcomponents/navigation/left-sidebar.tsxapp/(root)/tags/page.tsxe2e/authenticated.desktop.spec.tsapp/(root)/ask-question/page.tsxe2e/authenticated.mobile.spec.ts
**/*.{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
Files:
app/(root)/jobs/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/profile/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/layout.tsxapp/(root)/collections/page.tsxapp/(root)/page.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/community/page.tsxcomponents/ui/sidebar.tsxcomponents/navigation/left-sidebar.tsxapp/(root)/tags/page.tsxe2e/authenticated.desktop.spec.tsapp/(root)/ask-question/page.tsxe2e/authenticated.mobile.spec.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use @/ import aliases, even for sibling files (use @/app/fonts instead of ./fonts)
Files:
app/(root)/jobs/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/profile/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/layout.tsxapp/(root)/collections/page.tsxapp/(root)/page.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/community/page.tsxcomponents/ui/sidebar.tsxcomponents/navigation/left-sidebar.tsxapp/(root)/tags/page.tsxe2e/authenticated.desktop.spec.tsapp/(root)/ask-question/page.tsxe2e/authenticated.mobile.spec.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: michellepace
Repo: michellepace/devflow PR: 18
File: components/navigation/content-top-bar.tsx:23-30
Timestamp: 2025-12-25T13:56:09.644Z
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).
📚 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:
app/(root)/jobs/page.tsxcomponents/navigation/mobile-top-bar.tsxapp/(root)/profile/page.tsxcomponents/navigation/right-sidebar.tsxapp/(root)/layout.tsxapp/(root)/collections/page.tsxapp/(root)/page.tsxcomponents/navigation/desktop-top-bar.tsxapp/(root)/community/page.tsxcomponents/ui/sidebar.tsxcomponents/navigation/left-sidebar.tsxapp/(root)/tags/page.tsxapp/(root)/ask-question/page.tsx
📚 Learning: 2025-12-25T13:56:02.914Z
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 a Grok-style layout, move authenticated user controls (Clerk's UserButton) out of the top navigation and into the sidebar footer (e.g., components/app-sidebar.tsx). Ensure components/navigation/content-top-bar.tsx only renders SignedOut options (Sign in/Sign up). Apply this guideline to all TSX files under components/navigation to enforce consistent placement of authentication UI across the navigation layer.
Applied to files:
components/navigation/mobile-top-bar.tsxcomponents/navigation/right-sidebar.tsxcomponents/navigation/desktop-top-bar.tsxcomponents/navigation/left-sidebar.tsx
📚 Learning: 2025-12-25T13:56:09.644Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 18
File: components/navigation/content-top-bar.tsx:23-30
Timestamp: 2025-12-25T13:56:09.644Z
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).
Applied to files:
app/(root)/layout.tsxe2e/authenticated.mobile.spec.ts
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
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/navigation/desktop-top-bar.tsxe2e/authenticated.mobile.spec.ts
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
Learning: Applies to components/auth/clerk-signup.tsx : Sign Up component (components/auth/clerk-signup.tsx) should use a static logo
Applied to files:
components/navigation/desktop-top-bar.tsx
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
Learning: Applies to components/clerk-provider.tsx : ClerkProvider should be implemented in components/clerk-provider.tsx and apply shadcn theme and Inter font
Applied to files:
components/navigation/desktop-top-bar.tsxapp/globals.css
📚 Learning: 2025-12-25T15:46:14.812Z
Learnt from: michellepace
Repo: michellepace/devflow PR: 18
File: lib/utils.ts:17-19
Timestamp: 2025-12-25T15:46:14.812Z
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.
Applied to files:
app/globals.css
📚 Learning: 2025-12-28T22:41:43.683Z
Learnt from: CR
Repo: michellepace/devflow PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-28T22:41:43.683Z
Learning: Applies to **/*.css : Tailwind CSS v4: Use import "tailwindcss" syntax instead of tailwind directives
Applied to files:
app/globals.css
📚 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:
e2e/authenticated.mobile.spec.ts
🧬 Code graph analysis (2)
components/navigation/mobile-top-bar.tsx (1)
lib/utils.ts (1)
cn(4-6)
components/navigation/desktop-top-bar.tsx (1)
lib/utils.ts (1)
cn(4-6)
🪛 GitHub Actions: E2E Tests (playwright)
e2e/authenticated.desktop.spec.ts
[error] 18-18: Playwright test failed. Locator 'getByRole("button", { name: "Open user menu" })' was not visible within 5000ms; element not found during test '[webkit] Authenticated User Flow - Desktop'.
🔇 Additional comments (22)
e2e/authenticated.desktop.spec.ts (1)
40-41: Clear comment explaining authentication-dependent visibility.The comment accurately explains when the UserButton appears. Optionally, you could make it more specific by noting it appears "in the left sidebar" to align with the verification at line 56.
.markdownlint.yaml (1)
12-12: LGTM!The addition of
MD060: falseis well-documented and aligns with the existing configuration style. This allows compact table formatting without enforced spacing.app/(root)/ask-question/page.tsx (1)
1-3: LGTM!Consistent application of the
text-heading-2xlutility class. The component correctly remains a server component as no interactivity is required.app/(root)/community/page.tsx (1)
1-3: LGTM!Heading styling is consistent with other root pages in this PR.
app/(root)/jobs/page.tsx (1)
1-3: LGTM!Heading styling is consistent with other root pages in this PR.
app/(root)/profile/page.tsx (1)
1-3: LGTM!Heading styling is consistent with other root pages in this PR.
components/navigation/mobile-top-bar.tsx (2)
4-4: LGTM!Clean import update following the removal of
TOP_BAR_HEIGHTfromlib/utils.ts. The@/alias is correctly used per coding guidelines.
8-11: Good migration to CSS variable-based height.The change from a TypeScript constant to
h-(--top-bar-height)using Tailwind v4's CSS variable syntax is a solid approach — it centralises the height definition in CSS and enables easier theming. Theshadow-lightutility provides mode-aware shadow behaviour.app/(root)/page.tsx (2)
3-3: LGTM!Consistent application of
text-heading-2xlutility on the primary heading.
45-47: Good use of heading utility abstraction.Replacing individual classes (
text-xl font-semibold) withtext-heading-smimproves maintainability by centralising typography styles in the utility definition.app/(root)/collections/page.tsx (1)
1-3: LGTM!Heading styling is consistent with other root pages in this PR.
app/(root)/tags/page.tsx (1)
1-3: LGTM!The page correctly applies the new
text-heading-2xlutility class, maintaining consistency with other root pages in this PR.components/navigation/left-sidebar.tsx (1)
34-41: LGTM!The sidebar configuration with
width="13rem"andclassName="shadow-light"aligns well with the three-column layout objectives. The CSS variable-based heighth-(--top-bar-height)is correctly using Tailwind v4 shorthand syntax.Minor nit: Line 41 wraps a single static string in
cn(), which is unnecessary but harmless.app/(root)/layout.tsx (1)
21-28: LGTM!The two-column inner layout (main content + RightSidebar) is well-structured. The
flex-1on main ensures it takes available space while RightSidebar maintains its fixed width. This cleanly implements the three-column layout objective.components/navigation/right-sidebar.tsx (1)
12-61: LGTM!The RightSidebar implementation is well-structured with proper accessibility (
aria-label), responsive visibility (xl:block), and correct Tailwind v4 CSS variable syntax. The sticky positioning calculationh-[calc(100svh-var(--top-bar-height))]correctly accounts for the top bar.components/ui/sidebar.tsx (2)
156-169: LGTM!The
widthprop addition to the Sidebar component is well-designed, allowing flexible width customisation while maintaining the default--sidebar-widthvariable behaviour. The TypeScript type annotation is correct.
392-400: Good progressive enhancement for scrollbar styling.The WebKit scrollbar customisation with a
@supportsfallback for Firefox is a solid approach. The ultra-thin scrollbar styling enhances the sidebar's visual polish without breaking functionality in unsupported browsers.app/globals.css (4)
1-6: LGTM!The layer order declaration before imports is the correct pattern for Tailwind v4, and adding the "clerk" layer ensures proper cascade ordering with Clerk's styles.
69-73: Good consolidation of layout tokens.Moving
--top-bar-heightto CSS variables is a cleaner approach than the previous TypeScript constant. This allows both CSS-based height calculations and Tailwind utilities to reference the same source of truth.
224-245: Well-designed heading utility system.The heading utilities (
text-heading-2xlthroughtext-heading-xs) provide a clean separation between semantic HTML (h1–h6) and visual styling. This approach aligns with accessibility best practices where heading levels should reflect document structure, not visual appearance.
259-265: The descendant selector.dark .shadow-lightworks correctly for the current usage patterns.The
shadow-lightclass is applied to navigation components (sidebars and headers) that are descendants of the element with the.darkclass (the root or body). The.dark .shadow-lightselector will apply the override as intended. No changes required.e2e/authenticated.mobile.spec.ts (1)
18-99: Excellent test structure and improved resilience.The updated mobile authentication flow test is well-structured and aligns perfectly with the PR objectives:
✓ Clear separation of test phases with descriptive comments
✓ Proper use ofuserMenuhandle for authenticated state verification
✓ URL-based OTP detection fallback improves test resilience
✓ Comprehensive coverage: sign in → manage account → sign out
✓ Helpful troubleshooting comments (e.g., line 72 regarding pointer events)
✓ Correct use of @/ import aliases
✓ British English throughoutThe test properly handles the mobile menu-based authentication flow and verifies state at each critical step.
…ssNames Navigation: - Remove cn() from desktop-top-bar, mobile-top-bar, and left-sidebar headers - Remove unused cn import from top bar components The cn() utility adds no value when wrapping single static strings with no conditionals or class merging. This cleanup reduces unnecessary function calls while preserving identical className output. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Slightly larger logo in mobile navigation drawer for better visual balance.
Summary
text-heading-2xltotext-heading-xs) decoupling semantic HTML from visual styling--top-bar-heightCSS custom propertywidthprop to Sidebar component with ultra-thin scrollbar stylingTest plan
🤖 Generated with Claude Code