Skip to content

fix(theme): update code view on system theme change#2484

Open
janburzinski wants to merge 1 commit into
mainfrom
jan/eng-1562-code-view-stays-in-light-mode-after-system-switches-to-dark
Open

fix(theme): update code view on system theme change#2484
janburzinski wants to merge 1 commit into
mainfrom
jan/eng-1562-code-view-stays-in-light-mode-after-system-switches-to-dark

Conversation

@janburzinski

Copy link
Copy Markdown
Collaborator

Description

  • when updating your theme while emdash is open, the code editor updates it now too

Screenshot/Recording (if applicable)

https://streamable.com/i13f4z

Checklist
  • I kept this PR small and focused
  • I ran a self-review before opening this PR
  • I ran the relevant local checks or explained why not
  • I updated docs when behavior or setup changed
  • I added or updated tests when behavior changed, or explained why not
  • I only added comments where the logic is not obvious
  • I used Conventional Commits for commit
    messages and, when possible, the PR title

@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Replaces a manual useEffect-based matchMedia subscription with useSyncExternalStore so React drives system theme reactivity, ensuring the code editor (xterm) updates when the OS appearance changes while the app is open.

  • Adds subscribeToSystemTheme and getSystemTheme as stable module-level functions for useSyncExternalStore, deriving systemTheme and feeding it into effectiveTheme when no explicit user theme is set.
  • Removes the old useEffect that imperatively called applyTheme + applyThemeToAll() in a matchMedia change handler, consolidating both manual and system theme changes through the existing useLayoutEffect/useEffect pair.

Confidence Score: 5/5

Safe to merge — the change is a clean, focused refactor of a single reactive subscription with no behavioral regressions.

Both the subscribe and snapshot functions are defined at module level, giving useSyncExternalStore the stable references it requires. The useLayoutEffect/useEffect ordering that applies CSS classes before updating xterm themes is preserved. The only minor note is that the subscription is always active (even when an explicit theme is set) rather than conditionally, but effectiveTheme correctly falls back to theme in that case, so the extra listener is harmless. No correctness issues were found.

No files require special attention.

Important Files Changed

Filename Overview
apps/emdash-desktop/src/renderer/lib/providers/theme-provider.tsx Refactors system-theme subscription from a manual useEffect to useSyncExternalStore; stable module-level subscribe/snapshot functions are used correctly, and the layout-effect/effect ordering for applyTheme and applyThemeToAll is preserved.

Sequence Diagram

sequenceDiagram
    participant OS as OS Theme Change
    participant MQ as matchMedia listener
    participant React as React (useSyncExternalStore)
    participant LE as useLayoutEffect
    participant UE as useEffect

    OS->>MQ: fires 'change' event
    MQ->>React: calls onChange callback
    React->>React: re-renders, calls getSystemTheme()
    Note over React: systemTheme updated → effectiveTheme updated (when theme === null)
    React->>LE: effectiveTheme changed
    LE->>LE: applyTheme(effectiveTheme) — updates CSS class
    React->>UE: effectiveTheme changed
    UE->>UE: applyThemeToAll() — updates xterm / code editor
Loading

Reviews (1): Last reviewed commit: "fix(theme): update code view on system t..." | Re-trigger Greptile

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.

1 participant