Skip to content

The theme toggle should show the monitor icon when the system theme is selected #45

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.

Sign up for GitHub

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

Open
fulopkovacs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

The theme toggle should show the monitor icon when the system theme is selected #45

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 40 additions & 18 deletions src/components/mode-toggle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,60 @@
import { MonitorIcon, MoonIcon, SunIcon } from "lucide-react";
import { useTheme } from "@/components/theme-provider";
import { Button } from "@/components/ui/button";
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuRadioGroup,
DropdownMenuRadioItem,
DropdownMenuTrigger,
} from "@/components/ui/dropdown-menu";
import { type UserTheme, useTheme } from "./theme-provider";

export function ModeToggle() {
const { setTheme } = useTheme();
type ThemeToggleProps = {
getTheme?: () => UserTheme;
};
Comment on lines +12 to +14
Copy link

@coderabbitai coderabbitai bot Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the mode-toggle.tsx file to understand the context
cd src/components && wc -l mode-toggle.tsx

Repository: fulopkovacs/trytanstackdb.com

Length of output: 91

🏁 Script executed:

# Read the mode-toggle.tsx file to understand the implementation
cat -n src/components/mode-toggle.tsx

Repository: fulopkovacs/trytanstackdb.com

Length of output: 2259

🏁 Script executed:

# Find all ModeToggle usages
rg -n -C3 '<ModeToggle'

Repository: fulopkovacs/trytanstackdb.com

Length of output: 450

🏁 Script executed:

# Find getTheme prop assignments
rg -n -C3 'getTheme\s*='

Repository: fulopkovacs/trytanstackdb.com

Length of output: 55

🏁 Script executed:

# Find UserTheme type definition in theme-provider
rg -n "type UserTheme|interface UserTheme" src/components/

Repository: fulopkovacs/trytanstackdb.com

Length of output: 261

🏁 Script executed:

# Get more context around UserTheme definition
cat -n src/components/theme-provider.tsx | head -50

Repository: fulopkovacs/trytanstackdb.com

Length of output: 2144

Document or remove the unused getTheme prop, and clarify its contract.

The optional getTheme parameter is never used in the codebase but could easily be misused if provided. If a caller passes a function returning a resolved theme (light/dark) instead of the user's selection (which could be "system"), the icon and radio group will display incorrect state. Either add inline documentation specifying that getTheme() must return the user-selected theme including "system", or remove the parameter if it's not needed.

🤖 Prompt for AI Agents 
In `@src/components/mode-toggle.tsx` around lines 12 - 14, Remove the unused
getTheme prop from ThemeToggleProps and the component's props list: delete the
getTheme?: () => UserTheme declaration and any references to getTheme in the
mode-toggle component so the component relies solely on the
internal/user-selection state; update the ThemeToggleProps type and the
component signature (e.g., ThemeToggleProps, ThemeToggle/ModeToggle) to reflect
the removed prop and run type checks to ensure no callers pass getTheme anymore.


function ThemeIcon({ theme }: { theme: UserTheme }) {
const iconClass = "h-[1.2rem] w-[1.2rem]";

if (theme === "system") {
return <MonitorIcon className={iconClass} />;
}
if (theme === "dark") {
return <MoonIcon className={iconClass} />;
}
return <SunIcon className={iconClass} />;
}

export function ModeToggle({ getTheme }: ThemeToggleProps) {
const { setTheme, userTheme } = useTheme();
const theme = getTheme ? getTheme() : userTheme;

return (
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button variant="outline" size="icon">
<SunIcon className="h-[1.2rem] w-[1.2rem] scale-100 rotate-0 transition-all dark:scale-0 dark:-rotate-90" />
<MoonIcon className="absolute h-[1.2rem] w-[1.2rem] scale-0 rotate-90 transition-all dark:scale-100 dark:rotate-0" />
<ThemeIcon theme={theme} />
<span className="sr-only">Toggle theme</span>
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="center">
<DropdownMenuItem onClick={() => setTheme("light")}>
<SunIcon />
Light
</DropdownMenuItem>
<DropdownMenuItem onClick={() => setTheme("dark")}>
<MoonIcon />
Dark
</DropdownMenuItem>
<DropdownMenuItem onClick={() => setTheme("system")}>
<MonitorIcon />
System
</DropdownMenuItem>
<DropdownMenuRadioGroup
value={theme}
onValueChange={(value) => setTheme(value as UserTheme)}
>
<DropdownMenuRadioItem value="light">
<SunIcon />
Light
</DropdownMenuRadioItem>
<DropdownMenuRadioItem value="dark">
<MoonIcon />
Dark
</DropdownMenuRadioItem>
<DropdownMenuRadioItem value="system">
<MonitorIcon />
System
</DropdownMenuRadioItem>
</DropdownMenuRadioGroup>
</DropdownMenuContent>
</DropdownMenu>
);
Expand Down