Conversation
Dokploy Preview Deployment
|
WalkthroughRemoved the root layout and footer, refactored conditional layout and navigation into a bottom icon bar, replaced dropdown theme toggle with an inline cycling button, tightened profile image sizing, added auth config flags, and introduced CI scripts/workflows to auto-increment and build versions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/auth/src/index.ts (1)
24-24: Simplify the boolean expression.The ternary operator is redundant since the comparison already returns a boolean.
Apply this diff:
- disableCSRFCheck: process.env.NODE_ENV === "development" ? true : false, + disableCSRFCheck: process.env.NODE_ENV === "development",apps/web/src/app/user/[userid]/page.tsx (1)
46-50: Remove redundant Tailwind size utilities.The Image component already has
width="100"andheight="100"props that control sizing. The additional Tailwind classesw-[100px]andh-[100px]are redundant and could potentially conflict with Next.js Image optimization.Apply this diff:
- className="rounded-full border border-black dark:border-white select-none w-[100px] h-[100px]" + className="rounded-full border border-black dark:border-white select-none"apps/web/src/components/mode-toggle.tsx (1)
5-5: Remove unused import.
ThemeProvideris imported but never used in this file.Apply this diff:
-import { LaptopMinimalIcon, Moon, Sun } from "lucide-react"; -import { ThemeProvider, useTheme } from "next-themes"; +import { LaptopMinimalIcon, Moon, Sun } from "lucide-react"; +import { useTheme } from "next-themes";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/src/app/dashboard/sidebar.tsx(2 hunks)apps/web/src/app/user/[userid]/page.tsx(1 hunks)apps/web/src/components/conditional-layout.tsx(1 hunks)apps/web/src/components/footer.tsx(0 hunks)apps/web/src/components/layout.tsx(0 hunks)apps/web/src/components/mode-toggle.tsx(1 hunks)apps/web/src/components/navigation.tsx(2 hunks)apps/web/src/components/theme-provider.tsx(1 hunks)packages/auth/src/index.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/src/components/footer.tsx
- apps/web/src/components/layout.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
apps/web/src/components/conditional-layout.tsx (1)
apps/web/src/components/navigation.tsx (1)
Navigation(30-52)
apps/web/src/components/navigation.tsx (5)
apps/web/src/components/ui/button.tsx (1)
Button(60-60)apps/web/src/components/mode-toggle.tsx (1)
ModeToggle(14-38)apps/web/src/components/user-menu.tsx (1)
UserMenu(15-59)apps/web/src/components/ui/spinner.tsx (1)
Spinner(5-14)apps/web/src/lib/auth-client.ts (1)
authClient(4-6)
apps/web/src/components/mode-toggle.tsx (2)
apps/web/src/components/ui/button.tsx (1)
Button(60-60)apps/web/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(242-242)DropdownMenuTrigger(244-244)DropdownMenuContent(245-245)DropdownMenuItem(248-248)
apps/web/src/app/dashboard/sidebar.tsx (1)
apps/web/src/components/mode-toggle.tsx (1)
ModeToggleDropdown(40-65)
🔇 Additional comments (6)
packages/auth/src/index.ts (1)
24-25: Verify proxy and CSRF configuration aligns with deployment environment.Disabling CSRF checks in development is acceptable, but ensure
trustedProxyHeaders: trueis appropriate for your production proxy setup (e.g., when behind nginx, Cloudflare, or AWS ALB). Incorrect proxy header trust can lead to security issues like IP spoofing.apps/web/src/components/theme-provider.tsx (1)
6-11: LGTM!The formatting adjustments maintain the same functionality while improving code readability.
apps/web/src/app/dashboard/sidebar.tsx (1)
17-17: LGTM!The import and usage of
ModeToggleDropdowncorrectly replaces the previous inline theme toggle implementation, centralizing the theme control UI.Also applies to: 143-143
apps/web/src/components/conditional-layout.tsx (1)
19-26: LGTM!The spacing adjustments correctly accommodate the new bottom navigation: reduced top margin (mt-2), added bottom margin (mb-12) for the fixed bottom bar, and removed the header wrapper. The Footer removal aligns with the broader layout simplification.
apps/web/src/components/navigation.tsx (2)
30-52: LGTM!The bottom navigation layout is well-structured with appropriate fixed positioning, backdrop blur effects, and icon-based controls. The addition of a copyright notice is a nice touch.
54-112: LGTM!The UserMenu implementation correctly handles all three states (loading, unauthenticated, authenticated) with appropriate icon buttons and navigation actions. The sign-out flow properly integrates with authClient.
| export function ModeToggle() { | ||
| const { setTheme } = useTheme(); | ||
| const { setTheme, theme } = useTheme(); | ||
|
|
||
| return ( | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button variant="outline" size="icon"> | ||
| <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all dark:-rotate-90 dark:scale-0" /> | ||
| <Moon className="absolute h-[1.2rem] w-[1.2rem] rotate-90 scale-0 transition-all dark:rotate-0 dark:scale-100" /> | ||
| <span className="sr-only">Toggle theme</span> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="end"> | ||
| <DropdownMenuItem onClick={() => setTheme("light")}> | ||
| Light | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem onClick={() => setTheme("dark")}> | ||
| Dark | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem onClick={() => setTheme("system")}> | ||
| System | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| ); | ||
| return ( | ||
| <Button | ||
| variant="outline" | ||
| size="icon" | ||
| onClick={() => | ||
| theme === "white" | ||
| ? setTheme("dark") | ||
| : theme === "dark" | ||
| ? setTheme("system") | ||
| : setTheme("white") | ||
| } | ||
| > | ||
| {theme === "white" ? ( | ||
| <Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all" /> | ||
| ) : theme === "dark" ? ( | ||
| <Moon className="absolute h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" /> | ||
| ) : ( | ||
| <LaptopMinimalIcon className="absolute h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" /> | ||
| )} | ||
| </Button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Critical: Theme value inconsistency between components.
ModeToggle cycles between "white", "dark", and "system" (lines 22-26), while ModeToggleDropdown uses "light", "dark", and "system" (lines 53-59). This mismatch will cause unexpected behavior:
- If a user selects "Light" from the dropdown, the theme is set to
"light" - Clicking the
ModeTogglebutton will match the else clause (line 26) and set the theme to"white" - This creates two different light theme states that don't align
Standardize on a single theme value. Apply this diff to use "light" consistently:
<Button
variant="outline"
size="icon"
onClick={() =>
- theme === "white"
+ theme === "light"
? setTheme("dark")
: theme === "dark"
? setTheme("system")
- : setTheme("white")
+ : setTheme("light")
}
>
- {theme === "white" ? (
+ {theme === "light" ? (
<Sun className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all" />
) : theme === "dark" ? (Also applies to: 40-65
🤖 Prompt for AI Agents
In apps/web/src/components/mode-toggle.tsx around lines 14 to 38 (also applies
to lines 40–65), the ModeToggle component uses the theme value "white" while
other components use "light", causing inconsistent behavior; update ModeToggle
to use "light" everywhere it currently references "white" (both in the onClick
toggle branches and the JSX conditional rendering) so the cycle becomes "light"
↔ "dark" ↔ "system" and all components use the same theme string.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/navigation.tsx (2)
4-11: Remove unused imports.The following imports are no longer used after refactoring from dropdown to inline icon buttons:
- DropdownMenu components (lines 4-11)
- Skeleton component (line 14)
- Moon and Sun icons (lines 24, 26) — these are likely used inside the ModeToggle component, not here
Apply this diff to remove unused imports:
-import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuLabel, - DropdownMenuSeparator, - DropdownMenuTrigger, -} from "@/components/ui/dropdown-menu"; import { authClient } from "@/lib/auth-client"; import { Button } from "./ui/button"; -import { Skeleton } from "./ui/skeleton"; import { Spinner } from "./ui/spinner"; import { useRouter } from "next/navigation"; import * as React from "react"; import { ModeToggle } from "./mode-toggle"; import { HouseIcon, LayoutDashboardIcon, LogInIcon, LogOutIcon, - Moon, SearchIcon, - Sun, UserIcon, } from "lucide-react"; -import { useTheme } from "next-themes";Also applies to: 14-14, 24-24, 26-26
3-3: Remove unused React imports.Line 3 imports
useEffectanduseStatewhich are no longer used after removing scroll-based state logic.Apply this diff:
-import { useEffect, useState } from "react"; +import * as React from "react";Note: Line 17 already imports React, so you can consolidate or remove line 3 entirely.
🧹 Nitpick comments (5)
apps/web/src/components/navigation.tsx (1)
48-48: Remove trailing space in copyright text.Minor formatting: there's an unnecessary trailing space after the year.
Apply this diff:
- <span className="text-sm mt-1">© {new Date().getFullYear()} </span> + <span className="text-sm mt-1">© {new Date().getFullYear()}</span>.github/workflows/version-and-build.yml (2)
63-63: Add[skip ci]marker for broader CI compatibility.While the emoji marker
🤖 Auto-increment versionworks with your workflow guard, adding[skip ci]is a widely recognized convention that prevents other CI systems and tools from triggering on these automated commits.Apply this diff:
- git commit -m "🤖 Auto-increment version to ${{ steps.version_update.outputs.new }}" + git commit -m "🤖 Auto-increment version to ${{ steps.version_update.outputs.new }} [skip ci]"
80-80: Verify version output is set before using in Docker tags.If the increment script fails to set the
newoutput, this would attempt to create a Docker tag with an empty value, potentially causing the build to fail or create an incorrectly tagged image.Consider adding a verification step after version update:
- name: Verify version outputs run: | if [ -z "${{ steps.version_update.outputs.new }}" ]; then echo "Error: Version output not set" exit 1 fi.github/scripts/increment-version.js (2)
61-74: Preserve original quote style in version string.The regex matches both single and double quotes but the replacement always uses double quotes, changing the code style when the original used single quotes.
Apply this diff to preserve the quote character:
// Extract current version using regex - const versionPattern = /version:\s*["']([^"']+)["']/; + const versionPattern = /version:\s*(["'])([^"']+)\1/; const match = content.match(versionPattern); if (!match) { throw new Error("Could not find version property in projectData.ts"); } - const currentVersion = match[1]; + const quoteChar = match[1]; + const currentVersion = match[2]; const newVersion = incrementVersion(currentVersion); // Replace the version in the content const newContent = content.replace( versionPattern, - `version: "${newVersion}"`, + `version: ${quoteChar}${newVersion}${quoteChar}`, );
78-78: Consider atomic file writes for robustness.Using
writeFileSyncdirectly could leave the file corrupted if the write operation is interrupted. Consider writing to a temporary file first, then renaming for atomic updates.Example atomic write pattern:
import { writeFileSync, renameSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; const tempPath = join(tmpdir(), `projectData.ts.tmp.${Date.now()}`); writeFileSync(tempPath, newContent, "utf8"); renameSync(tempPath, PROJECT_DATA_PATH);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/scripts/README.md(1 hunks).github/scripts/increment-version.js(1 hunks).github/workflows/auto-update-version.yml.disabled(1 hunks).github/workflows/build_docker_image.yml.disabled(1 hunks).github/workflows/version-and-build.yml(1 hunks)apps/web/projectData.ts(1 hunks)apps/web/src/components/navigation.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/scripts/README.md
- apps/web/projectData.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/navigation.tsx (5)
apps/web/src/components/ui/button.tsx (1)
Button(60-60)apps/web/src/components/mode-toggle.tsx (1)
ModeToggle(14-38)apps/web/src/components/user-menu.tsx (1)
UserMenu(15-59)apps/web/src/components/ui/spinner.tsx (1)
Spinner(5-14)apps/web/src/lib/auth-client.ts (1)
authClient(4-6)
🔇 Additional comments (4)
apps/web/src/components/navigation.tsx (1)
90-90: No action required —session.user.idis properly typed and guaranteed to exist in better-auth's Session type. The property is used consistently throughout the codebase for database queries and navigation without issues..github/workflows/build_docker_image.yml.disabled (1)
20-21: LGTM! Guard correctly prevents CI loops.The conditional properly skips workflow execution when triggered by the auto-increment bot, preventing infinite loops. This is consistent with the pattern used in the other workflow files.
.github/workflows/auto-update-version.yml.disabled (1)
1-64: LGTM! Well-structured auto-versioning workflow.The workflow correctly:
- Guards against infinite loops (line 16)
- Executes the increment script and captures outputs
- Commits only when changes are detected
- Includes both emoji marker and
[skip ci]for broad CI system compatibility (line 58).github/scripts/increment-version.js (1)
12-53: LGTM! Flexible version increment logic.The multi-pattern approach correctly handles:
- Dash-suffixed versions (e.g., "0.1.10-canery-1")
- Semantic versions (e.g., "0.1.10")
- Fallback for unusual formats
The pattern matching order ensures more specific patterns match first, preventing conflicts.
| <Link href="/"> | ||
| <Button variant="outline" size="icon" className="cursor-pointer"> | ||
| <HouseIcon /> | ||
| </Button> | ||
| </Link> | ||
| <nav className="md:flex md:gap-4"> | ||
| {links.map(({ to, label }) => { | ||
| return ( | ||
| <Link key={to} href={to} className=""> | ||
| {label} | ||
| </Link> | ||
| ); | ||
| })} | ||
| </nav> | ||
| </div> | ||
| <div className="flex items-center gap-2 my-0 py-0"> | ||
| <Link href="/search"> | ||
| <Button variant="outline"> | ||
| <SearchIcon className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all" /> | ||
| <Button variant="outline" size="icon" className="cursor-pointer"> | ||
| <SearchIcon /> | ||
| </Button> | ||
| </Link> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use consistent Link/Button pattern with asChild.
Lines 35-44 nest Link around Button, while lines 68-72 correctly use the asChild pattern. The asChild approach is the recommended pattern for shadcn/ui Button components with Next.js Link.
Apply this diff for consistency:
- <Link href="/">
- <Button variant="outline" size="icon" className="cursor-pointer">
+ <Button variant="outline" size="icon" asChild aria-label="Home">
+ <Link href="/">
<HouseIcon />
- </Button>
- </Link>
- <Link href="/search">
- <Button variant="outline" size="icon" className="cursor-pointer">
+ </Link>
+ </Button>
+ <Button variant="outline" size="icon" asChild aria-label="Search">
+ <Link href="/search">
<SearchIcon />
- </Button>
- </Link>
+ </Link>
+ </Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link href="/"> | |
| <Button variant="outline" size="icon" className="cursor-pointer"> | |
| <HouseIcon /> | |
| </Button> | |
| </Link> | |
| <nav className="md:flex md:gap-4"> | |
| {links.map(({ to, label }) => { | |
| return ( | |
| <Link key={to} href={to} className=""> | |
| {label} | |
| </Link> | |
| ); | |
| })} | |
| </nav> | |
| </div> | |
| <div className="flex items-center gap-2 my-0 py-0"> | |
| <Link href="/search"> | |
| <Button variant="outline"> | |
| <SearchIcon className="h-[1.2rem] w-[1.2rem] rotate-0 scale-100 transition-all" /> | |
| <Button variant="outline" size="icon" className="cursor-pointer"> | |
| <SearchIcon /> | |
| </Button> | |
| </Link> | |
| <Button variant="outline" size="icon" asChild aria-label="Home"> | |
| <Link href="/"> | |
| <HouseIcon /> | |
| </Link> | |
| </Button> | |
| <Button variant="outline" size="icon" asChild aria-label="Search"> | |
| <Link href="/search"> | |
| <SearchIcon /> | |
| </Link> | |
| </Button> |
🤖 Prompt for AI Agents
In apps/web/src/components/navigation.tsx around lines 35 to 44, the Link
components currently wrap Button elements; change to the consistent shadcn/ui
pattern by using Button with the asChild prop and placing Next.js Link as the
Button's child (i.e., <Button asChild ...><Link href="/">...</Link></Button>),
keep the same props (variant, size, className) on Button and preserve the icon
children, and ensure accessibility attributes and imports remain unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/components/mode-toggle.tsx (1)
22-30: Theme value "white" should be "light" for consistency with next-themes.The standard theme values in
next-themesare"light","dark", and"system". Using"white"will likely not work correctly unless custom theme configuration exists.This issue was already flagged in a previous review. Please apply the suggested fix to use
"light"instead of"white".
🧹 Nitpick comments (3)
apps/web/src/components/mode-toggle.tsx (1)
4-12: Remove unused imports.
ThemeProviderand theDropdownMenucomponents are imported but no longer used after the refactor from dropdown to button-based toggle.import * as React from "react"; -import { LaptopMinimalIcon, Moon, Sun } from "lucide-react"; -import { ThemeProvider, useTheme } from "next-themes"; +import { LaptopMinimalIcon, Moon, Sun } from "lucide-react"; +import { useTheme } from "next-themes"; import { Button } from "@/components/ui/button"; -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from "@/components/ui/dropdown-menu";apps/web/src/components/navigation.tsx (2)
4-11: Remove unused imports.Several imports are no longer used after the refactoring:
DropdownMenuand related components (lines 4-11)Skeleton(line 14)Moon,Sunicons (lines 24, 26)useThemehook (line 29)"use client"; import Link from "next/link"; -import { useEffect, useState } from "react"; -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuLabel, - DropdownMenuSeparator, - DropdownMenuTrigger, -} from "@/components/ui/dropdown-menu"; +import { useState } from "react"; import { authClient } from "@/lib/auth-client"; import { Button } from "./ui/button"; -import { Skeleton } from "./ui/skeleton"; import { Spinner } from "./ui/spinner"; import { useRouter } from "next/navigation"; import * as React from "react"; import { ModeToggle } from "./mode-toggle"; import { HouseIcon, LayoutDashboardIcon, LogInIcon, LogOutIcon, - Moon, SearchIcon, - Sun, UserIcon, } from "lucide-react"; -import { useTheme } from "next-themes";Also applies to: 14-14, 24-27, 29-29
35-54: Consider using theasChildpattern for consistency.Lines 83-93 correctly use the
asChildpattern for the Login button, but the Home and Search buttons (lines 35-44, 45-54) still wrapButtoninsideLink. While functional, usingasChildconsistently is the recommended approach for shadcn/ui.- <Link href="/"> - <Button - variant="outline" - size="icon" - className="cursor-pointer" - aria-label="Home" - > + <Button + variant="outline" + size="icon" + className="cursor-pointer" + aria-label="Home" + asChild + > + <Link href="/"> <HouseIcon /> - </Button> - </Link> - <Link href="/search"> - <Button - variant="outline" - size="icon" - className="cursor-pointer" - aria-label="Search" - > + </Link> + </Button> + <Button + variant="outline" + size="icon" + className="cursor-pointer" + aria-label="Search" + asChild + > + <Link href="/search"> <SearchIcon /> - </Button> - </Link> + </Link> + </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/increment-version.js(1 hunks)apps/web/src/app/dashboard/sidebar.tsx(1 hunks)apps/web/src/components/mode-toggle.tsx(1 hunks)apps/web/src/components/navigation.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/scripts/increment-version.js
🔇 Additional comments (2)
apps/web/src/app/dashboard/sidebar.tsx (1)
15-15: LGTM! Clean extraction of theme toggle to shared component.The refactoring to use an external
ModeTogglecomponent improves code reuse across the dashboard sidebar and navigation bar. The integration is straightforward and correctly placed in the sidebar header.Also applies to: 131-131
apps/web/src/components/navigation.tsx (1)
35-56: Good: Accessibility labels added to icon buttons.All icon-only buttons now have appropriate
aria-labelattributes, addressing the accessibility concerns from the previous review. This ensures screen reader users can understand the purpose of each button.Also applies to: 68-134
|
yay |
1 participant
Summary by CodeRabbit
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.