Conversation
…onfiguration for theme switching.
…d button color transitions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new client-side ThemeToggle component, wires it into the layout configuration as Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant ThemeToggle as ThemeToggle UI
participant RootProvider as Theme Provider (next-themes)
participant OS as System Preference
User->>ThemeToggle: Clicks theme button (light/system/dark)
alt option != "system"
ThemeToggle->>RootProvider: setTheme("light"|"dark")
RootProvider-->>User: Apply selected theme classes/styles
else option == "system"
ThemeToggle->>RootProvider: setTheme("system")
RootProvider->>OS: query prefers-color-scheme
OS-->>RootProvider: returns light/dark
RootProvider-->>User: Apply system theme classes/styles
end
note right of ThemeToggle: Mounted-state guard prevents SSR mismatch\nARIA attributes updated for active button
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/www/components/ui/theme-toggle.tsx (1)
42-43: Simplify redundant ternary expression.The current logic
(theme === "system" ? "system" : theme) === option.valueis redundant and can be simplified to justtheme === option.value, as the ternary always returnstheme.🔎 Simplified logic
- const isActive = - (theme === "system" ? "system" : theme) === option.value; + const isActive = theme === option.value;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/www/app/layout.config.tsxapps/www/app/layout.tsxapps/www/components/ui/theme-toggle.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Prefertypeoverinterfacefor type definitions in TypeScript
Use TypeScript 5.1+ features when appropriate
Leverageconsttype parameters for better inference in TypeScript
Use JSDoc comments for public APIs
Use tabs for indentation (configured in Biome)
Use double quotes for strings (configured in Biome)
Organize imports automatically (Biome handles this)
Avoid explicit types when TypeScript can infer them (noInferrableTypeserror)
Useas constwhere appropriate for immutable values (useAsConstAssertionerror)
Don't reassign function parameters (noParameterAssignerror)
Place default parameters last in function signatures (useDefaultParameterLasterror)
Always initialize enum values (useEnumInitializerserror)
Declare one variable per statement (useSingleVarDeclaratorerror)
Avoid unnecessary template literals (noUnusedTemplateLiteralerror)
PreferNumber.parseIntover globalparseInt(useNumberNamespaceerror)
Use kebab-case for TypeScript filenames (e.g.,create-env.ts)
Use camelCase for function names (e.g.,createEnv)
Use PascalCase for type names (e.g.,ArkEnvError)
Use UPPER_SNAKE_CASE for environment variables and constants
Include examples in JSDoc comments when helpful for public APIs
Document complex type logic with JSDoc comments
UseArkEnvErrorfor environment variable validation errors
Provide clear, actionable error messages that include the variable name and expected type
**/*.{ts,tsx}: UsecreateEnv(schema)function (or default import asarkenv) to create validated environment objects in TypeScript
Use built-in validators (host, port, url, email) from ArkEnv when available instead of custom ArkType schemas
Provide default values for optional environment variables using ArkType syntax (e.g., 'boolean = false')
Use ArkEnvError for environment variable errors instead of generic Error types
For environment schema definition, use ArkType string literal syntax for enumerated values (e.g., "'deve...
Files:
apps/www/components/ui/theme-toggle.tsxapps/www/app/layout.tsxapps/www/app/layout.config.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
Use self-closing JSX elements (
useSelfClosingElementserror)
Files:
apps/www/components/ui/theme-toggle.tsxapps/www/app/layout.tsxapps/www/app/layout.config.tsx
apps/www/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/www/.cursor/rules/posthog-integration.mdc)
apps/www/**/*.{ts,tsx,js,jsx}: If using TypeScript, use an enum to store feature flag names. If using JavaScript, store feature flag names as strings to an object declared as a constant to simulate an enum. Use UPPERCASE_WITH_UNDERSCORE naming convention for enum/const object members.
If a custom property for a person or event is referenced in two or more files or two or more callsites in the same file, use an enum or const object with UPPERCASE_WITH_UNDERSCORE naming convention, similar to feature flags.
Files:
apps/www/components/ui/theme-toggle.tsxapps/www/app/layout.tsxapps/www/app/layout.config.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-25T21:14:25.628Z
Learnt from: yamcodes
Repo: yamcodes/arkenv PR: 616
File: apps/www/app/(home)/page.tsx:18-18
Timestamp: 2025-12-25T21:14:25.628Z
Learning: In projects using fumadocs-ui's HomeLayout component, the HomeLayout itself renders a top-level <main> element with id="nd-home-layout", so page components should not include their own <main> wrapper to avoid duplicate main landmarks.
Applied to files:
apps/www/app/layout.config.tsx
🧬 Code graph analysis (3)
apps/www/components/ui/theme-toggle.tsx (1)
apps/www/lib/utils/cn.ts (1)
cn(4-6)
apps/www/app/layout.tsx (1)
tooling/playwright-www/tests/theme-switching.test.ts (3)
test(3-87)page(4-64)window(39-44)
apps/www/app/layout.config.tsx (1)
apps/www/components/ui/theme-toggle.tsx (1)
ThemeToggle(14-64)
⏰ 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). (2)
- GitHub Check: test-e2e (a11y)
- GitHub Check: test-e2e (e2e)
🔇 Additional comments (6)
apps/www/app/layout.tsx (1)
48-48: LGTM! System theme support successfully added.The explicit
themesarray properly enables the three-option theme switcher (system, light, dark) that resolves the sync issue described in #608. This works correctly with the existingenableSystem: trueconfiguration.apps/www/app/layout.config.tsx (1)
3-3: LGTM! Theme toggle properly integrated.The ThemeToggle component is correctly imported and configured as the layout's theme switch control, completing the integration with fumadocs-ui's BaseLayoutProps.
Also applies to: 16-18
apps/www/components/ui/theme-toggle.tsx (4)
1-6: LGTM! Proper client component setup.The "use client" directive is correctly placed for this interactive component, and all imports are appropriate for the theme toggle functionality.
8-12: LGTM! Type definition follows coding guidelines.The
Optiontype correctly usestypeinstead ofinterfaceas per the project's coding guidelines, and the structure is well-suited for the theme options.
26-30: LGTM! Well-typed options array.The
as const satisfies Option[]syntax leverages TypeScript 5.1+ features for strong type inference while maintaining const assertions, and the icon-to-theme mappings are semantically clear.
32-63: LGTM! Excellent accessibility implementation.The component render demonstrates strong accessibility practices:
- Semantic
<fieldset>grouping- Proper ARIA attributes (
aria-label,aria-pressed)- Clear visual state indicators with keyboard-friendly styling
- Explicit
type="button"to prevent unintended form submissionsThe integration with next-themes via
setThemeis correct and will properly update the theme across the application.
…, update dependencies, and refine Bun React env type.
yamcodes
deleted the
608-darklight-mode-switcher-doesnt-sync-reliably-with-system-setting
branch
1 participant
Closes #608
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.