Skip to content

Fix theme source not resetting when using system theme#3756

Open
fmartingr wants to merge 1 commit into
masterfrom
fix/reset-theme-source-on-system-theme
Open

Fix theme source not resetting when using system theme#3756
fmartingr wants to merge 1 commit into
masterfrom
fix/reset-theme-source-on-system-theme

Conversation

@fmartingr
Copy link
Copy Markdown

@fmartingr fmartingr commented Mar 27, 2026

Summary

  • Fixed an inverted condition in ThemeManager where isUsingSystemTheme was negated, preventing resetThemeSource() from being called when the user switches to the system theme
  • Now correctly resets the theme source when the system theme is active and applies the custom theme source only when the system theme is not in use

Test plan

  • Set a server theme, then switch to "Use system theme" — verify the desktop app follows the OS dark/light mode
  • Switch from system theme to a custom theme — verify the desktop app applies the correct light/dark mode based on the center channel background color
  • Toggle between system and custom themes multiple times to ensure no stale state

Change Impact: 🟡 Medium

Regression Risk: The change corrects an inverted conditional in updateMainViews(), flipping when resetThemeSource() is called based on the isUsingSystemTheme flag. While the code modification is minimal and isolated to a single method, it addresses a logic bug that affects user-facing theme behavior. Potential risks include: (1) if existing workarounds or dependencies exist around the broken behavior, this fix could surface unexpected issues; (2) theme state transitions between system and custom themes need to work correctly across both main and popout views; (3) the fix changes the path through which nativeTheme.themeSource gets updated, which could have side effects with Electron's native theme handling if there are edge cases not covered by tests.

QA Recommendation: Manual QA is essential. Verify both code paths thoroughly following the test plan provided in the PR: confirm system theme correctly follows OS dark/light mode changes, verify custom theme application uses proper light/dark modes based on server colors, and stress-test repeated toggling between system and custom themes. A test file exists (src/main/themeManager.test.js), but coverage verification is recommended to ensure both conditional branches are tested. Pay particular attention to theme persistence and visual consistency when switching between themes.

The condition checking isUsingSystemTheme was inverted, preventing
the theme source from being reset when the user switches to the
system theme. This ensures resetThemeSource() is called correctly.
@mm-cloud-bot mm-cloud-bot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 27, 2026
@mm-cloud-bot
Copy link
Copy Markdown

@fmartingr: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

I understand the commands that are listed here

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: dd2cf129-d559-4414-93d4-01d64927cd77

📥 Commits

Reviewing files that changed from the base of the PR and between 85e7929 and a37d0b0.

📒 Files selected for processing (1)
  • src/main/themeManager.ts

📝 Walkthrough

Walkthrough

The ThemeManager.updateMainViews method's theme source update logic is modified to conditionally reset the native theme source to 'system' when the server uses system theme, versus deriving the theme from server colors when not using system theme.

Changes

Cohort / File(s) Summary
Theme Source Update Logic
src/main/themeManager.ts
Modified updateMainViews method to flip the conditional: when isUsingSystemTheme is true, calls resetThemeSource() to set nativeTheme.themeSource to 'system'; when false, derives theme from centerChannelBg and updates nativeTheme.themeSource if changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix in the changeset: correcting the inverted condition so that resetThemeSource() is called when using system theme, which was the primary bug being addressed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/reset-theme-source-on-system-theme

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

@github-actions github-actions Bot removed the E2E/Run Run Desktop E2E Tests label Mar 27, 2026
@fmartingr fmartingr self-assigned this Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/release-note-label-needed kind/bug Categorizes issue or PR as related to a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants