Skip to content

Conversation

@srijanravisankar
Copy link

@srijanravisankar srijanravisankar commented Nov 3, 2025

Description

This pull request fixes the unused Moon icon and resolves the hydration mismatch error in the ThemeToggle component.
The toggle now correctly switches between the Sun (dark → light) and Moon (light → dark) icons based on the current theme without any errors.

Fixes #648

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Verified that the Moon icon now appears correctly when the app is in light mode, and the Sun icon appears in dark mode.
  • Checked that no hydration errors appear in the console on reload or navigation.

Test Configuration:

  • Node version: v24.11.0
  • Browser (if applicable): Brave
  • Operating System: Windows 11

Screenshots (if applicable)

Before:

  • Screenshot 2025-11-02 210337
  • Screenshot 2025-11-02 210349
  • Screenshot 2025-11-02 210541

Fix:

image

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have added screenshots if ui has been changed
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional context

Test:

opencut-moon-toggle-hydration-error-fix-testing.mp4

Summary by CodeRabbit

  • New Features

    • Theme toggle now available on the Projects page for convenient light/dark mode switching.
  • Bug Fixes

    • Fixed rendering issue ensuring smooth theme toggle experience across all pages.
  • Accessibility

    • Enhanced screen reader support with descriptive labels for improved theme controls.

@netlify
Copy link

netlify bot commented Nov 3, 2025

👷 Deploy request for appcut pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit a7b234e

@vercel
Copy link

vercel bot commented Nov 3, 2025

@srijanravisankar is attempting to deploy a commit to the OpenCut OSS Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Integrates a ThemeToggle component into the Projects page and fixes a bug in ThemeToggle where it always displayed the Sun icon and triggered hydration errors. The fix adds a client-side mounting guard and implements conditional icon rendering based on the current theme.

Changes

Cohort / File(s) Summary
ThemeToggle component fix
apps/web/src/components/theme-toggle.tsx
Adds mounted state with useEffect to prevent hydration mismatch; conditionally renders Sun icon in dark mode and Moon icon in light mode; updates aria-label and screen-reader text to describe the action being performed
Projects page integration
apps/web/src/app/projects/page.tsx
Imports and renders ThemeToggle component in the header area when not in selection mode

Sequence Diagram

sequenceDiagram
    participant Client
    participant Server
    participant Theme as Theme Component
    
    rect rgb(200, 220, 240)
    Note over Client,Server: Before: Hydration Mismatch
    Server->>Theme: Render (theme undefined)
    Server->>Client: Send HTML (e.g., Sun icon)
    Client->>Theme: Hydrate (theme determined client-side)
    Client->>Client: Theme value differs from server output ✗
    end
    
    rect rgb(220, 240, 200)
    Note over Client,Server: After: Hydration-Safe
    Server->>Theme: Render nothing (mounted = false)
    Server->>Client: Send empty placeholder
    Client->>Theme: useEffect mounts component
    Client->>Client: useTheme() hook initialized
    alt theme is dark
        Client->>Theme: Render Moon icon
    else theme is light
        Client->>Theme: Render Sun icon
    end
    Client->>Client: Server and client match ✓
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Area of attention: The hydration guard pattern using mounted state is a standard solution; verify the useEffect dependency array is correct and useTheme() hook behavior is well-understood
  • Area of attention: Confirm conditional icon logic (Sun in dark, Moon in light) aligns with expected UX

Poem

🐰 The Sun icon hopped with glee,
Always bright, never let to be—
But now it bows when darkness calls,
Moon rises up through shadowed halls,
No hydration hiccups, smooth as silk! 🌙✨

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Feature/fix unused moon and hydration error in theme toggle" clearly and specifically summarizes the main changes. It directly addresses the two key issues: the unused Moon icon and the hydration error in the ThemeToggle component. The title is concise, avoids noise, and provides enough specificity that a reviewer can immediately understand the primary focus of the changeset.
Linked Issues Check ✅ Passed The code changes directly address the two requirements from linked issue #648. The ThemeToggle component now conditionally renders the Sun icon when theme is dark and Moon icon when light, fulfilling the requirement to display correct icons based on the current theme. The introduction of a client-side mounting guard with mounted state and useEffect eliminates the hydration mismatch error by preventing server-side rendering of dynamic theme-dependent content. Both primary coding objectives from the linked issue are implemented in the changes to apps/web/src/components/theme-toggle.tsx.
Out of Scope Changes Check ✅ Passed The pull request changes are focused on the core requirements from issue #648. The primary changes to apps/web/src/components/theme-toggle.tsx directly fix the identified bugs (icon display and hydration error). The modification to apps/web/src/app/projects/page.tsx adds the ThemeToggle component to the Projects page, which appears to be part of integrating or demonstrating the fixed component rather than an unrelated change. All modifications directly support the stated objectives of fixing the ThemeToggle component issues.
Description Check ✅ Passed The pull request description comprehensively follows the required template. It includes a clear summary of changes with a reference to issue #648, marks the change type as a bug fix, provides testing details with configuration information (Node v24.11.0, Brave browser, Windows 11), includes before and after screenshots demonstrating the fix, and completes most checklist items. While documentation updates and local unit test verification are unchecked, the description is largely complete and informative, with additional context provided via a test video link.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bee7aba and a7b234e.

📒 Files selected for processing (2)
  • apps/web/src/app/projects/page.tsx (2 hunks)
  • apps/web/src/components/theme-toggle.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{jsx,tsx}: Don't use accessKey attribute on any HTML element.
Don't set aria-hidden="true" on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like <marquee> or <blink>.
Only use the scope prop on <th> elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assign tabIndex to non-interactive HTML elements.
Don't use positive integers for tabIndex property.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include a title element for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
Assign tabIndex to non-interactive HTML elements with aria-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include a type attribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden).
Always include a lang attribute on the html element.
Always include a title attribute for iframe elements.
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress.
Accompany onMouseOver/onMouseOut with onFocus/onBlur.
Include caption tracks for audio and video elements.
Use semantic elements instead of role attributes in JSX.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with...

Files:

  • apps/web/src/app/projects/page.tsx
  • apps/web/src/components/theme-toggle.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the ! postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use either T[] or Array<T> consistently.
Initialize each enum member value explicitly.
Use export type for types.
Use import type for types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use the TypeScript directive @ts-ignore.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Use either T[] or Array consistently
Don't use the any type

Files:

  • apps/web/src/app/projects/page.tsx
  • apps/web/src/components/theme-toggle.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,ts,tsx}: Don't use the return value of React.render.
Don't use consecutive spaces in regular expression literals.
Don't use the arguments object.
Don't use primitive type aliases or misleading types.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of...

Files:

  • apps/web/src/app/projects/page.tsx
  • apps/web/src/components/theme-toggle.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{tsx,jsx}: Always include a title element for icons unless there's text beside the icon
Always include a type attribute for button elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Don't import React itself
Don't define React components inside other components
Don't use both children and dangerouslySetInnerHTML on the same element
Don't insert comments as text nodes
Use <>...</> instead of ...

Files:

  • apps/web/src/app/projects/page.tsx
  • apps/web/src/components/theme-toggle.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{ts,tsx,js,jsx}: Don't use the comma operator
Use for...of statements instead of Array.forEach
Don't initialize variables to undefined
Use .flatMap() instead of map().flat() when possible
Don't assign a value to itself
Avoid unused imports and variables
Don't use await inside loops
Don't hardcode sensitive data like API keys and tokens
Don't use the delete operator
Don't use global eval()
Use String.slice() instead of String.substr() and String.substring()
Don't use else blocks when the if block breaks early
Put default function parameters and optional function parameters last
Use new when throwing an error
Use String.trimStart() and String.trimEnd() over String.trimLeft() and String.trimRight()

Files:

  • apps/web/src/app/projects/page.tsx
  • apps/web/src/components/theme-toggle.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{jsx,tsx} : Make sure all dependencies are correctly specified in React hooks.

Applied to files:

  • apps/web/src/app/projects/page.tsx
📚 Learning: 2025-08-09T09:03:49.797Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-09T09:03:49.797Z
Learning: Applies to **/*.{jsx,tsx} : Use semantic elements instead of role attributes in JSX.

Applied to files:

  • apps/web/src/components/theme-toggle.tsx
🧬 Code graph analysis (2)
apps/web/src/app/projects/page.tsx (1)
apps/web/src/components/theme-toggle.tsx (1)
  • ThemeToggle (12-37)
apps/web/src/components/theme-toggle.tsx (1)
apps/web/src/app/layout.tsx (1)
  • RootLayout (22-55)
⏰ 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: Vercel Agent Review
🔇 Additional comments (4)
apps/web/src/app/projects/page.tsx (1)

21-21: ThemeToggle integration looks good!

The import and placement of the ThemeToggle component are appropriate. Rendering it in the header alongside other action buttons when not in selection mode provides a clean user experience.

Also applies to: 219-219

apps/web/src/components/theme-toggle.tsx (3)

14-19: Excellent fix for the hydration mismatch issue!

The client-side mounting guard correctly prevents server-side rendering until the theme is resolved on the client. This is the standard pattern for next-themes and eliminates the hydration error described in issue #648.


29-33: Correct icon logic implementation!

The conditional rendering now properly displays Sun in dark mode and Moon in light mode, fixing the unused Moon icon issue. The logic is semantically correct—the icon represents the target state (where clicking will take you), not the current state.


26-26: Strong accessibility implementation!

The combination of aria-label on the button and dynamic sr-only text provides excellent screen reader support. While slightly redundant, having both ensures the button is properly labeled and the action is clearly communicated.

Also applies to: 34-34


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] ThemeToggle always shows Sun icon and causes hydration error during SSR

1 participant