refactor(theme): single helper owns theme-DOM stamping

state.cljs and theme.cljs both stamped data-theme + body classes onto
the DOM (in apply-theme-to-dom! and the container's mount effect
respectively). The duplicate setAttribute work is idempotent in
practice but the two code paths drift on edge cases (plugin hook
order, custom-theme application) and obscure who owns the stamp.

Promote state/apply-theme-to-dom! to public and have theme.cljs's
effect call it instead of inlining its own copy. Now: set-theme-mode!
calls it synchronously before set-state! (so subscribers see fresh
DOM on next render); the container effect calls it on mount and on
every theme prop change (so the initial render and any external
:ui/theme writes also stamp the DOM). Effect retains its
custom-theme + plugin-hook + re-render-root! responsibilities.

re-render-root! kept on toggle: bg-var cache invalidation (commit
6b4e5fb910) makes recomputed values correct, but components that
don't subscribe to :ui/theme but DO render avatar/contrast colors
would keep stale inline styles without a forced reconciliation.

Verified live: set-theme-mode! "dark" → data-theme stamped to "dark",
`dark` class added to <html>, body classes updated, end state
consistent.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
scheinriese
2026-05-19 13:38:06 +02:00
parent 6b4e5fb910
commit abdfa0ca67
2 changed files with 29 additions and 31 deletions

View File

@@ -39,26 +39,23 @@
[restored-sidebar? set-restored-sidebar?] (rum/use-state false)]
(hooks/use-effect!
#(let [^js doc js/document.documentElement
^js cls (.-classList doc)
^js cls-body (.-classList js/document.body)]
(.setAttribute doc "data-theme" theme)
(if (= theme "dark") ;; for tailwind dark mode
; The white-theme is for backward compatibility. See: https://github.com/logseq/logseq/pull/4652.
(do (.add cls "dark") (doto cls-body (.remove "white-theme" "light-theme") (.add "dark-theme")))
(do (.remove cls "dark") (doto cls-body (.remove "dark-theme") (.add "white-theme" "light-theme"))))
(ui/apply-custom-theme-effect! theme)
(plugin-handler/hook-plugin-app :theme-mode-changed {:mode theme})
;; Force a full reactive re-render after the data-theme attribute
;; is on <html>. A handful of render paths (avatar fallback
;; muted-tint, contrast-adjusted icon colors) compute hex values
;; from CSS variables via getComputedStyle at render time — their
;; inline `style="..."` snapshots would otherwise stay frozen
;; with the previous theme until something unrelated triggered a
;; re-render. Theme toggle is a rare user-initiated action; the
;; one-frame reconciliation cost is imperceptible. Same pattern
;; as language change (settings.cljs:298).
(ui-handler/re-render-root!))
(fn []
;; DOM stamp is owned by state/apply-theme-to-dom! — the same
;; helper set-theme-mode! calls synchronously before set-state!.
;; Calling it here covers the initial mount (where set-theme-mode!
;; hasn't run) and is idempotent on every subsequent toggle.
(state/apply-theme-to-dom! theme)
(ui/apply-custom-theme-effect! theme)
(plugin-handler/hook-plugin-app :theme-mode-changed {:mode theme})
;; Force a full reactive re-render after the data-theme attribute
;; is on <html>. Components that DON'T subscribe to :ui/theme but
;; DO render avatar/contrast colors (computed from CSS vars at
;; render time) would otherwise keep their stale inline styles
;; until something unrelated triggered them to re-render. Theme
;; toggle is a rare user action; the one-frame reconciliation
;; cost is imperceptible. Same pattern as language change
;; (settings.cljs:298).
(ui-handler/re-render-root!))
[theme])
;; theme color

View File

@@ -1261,17 +1261,18 @@ Similar to re-frame subscriptions"
(set-edit-content! edit-input-id content)
(set-editor-last-pos! new-pos)))
(defn- apply-theme-to-dom!
"Synchronously stamp `data-theme` + body classes onto the document
so CSS variables re-resolve to the new theme *before* the state
mutation below triggers React subscribers. Without this, subscribers
that compute hex values from CSS vars at render time (e.g. avatar
fallback colors via `colors/read-bg-var` → `getComputedStyle`)
would re-render with the OLD theme's resolved vars because
`theme.cljs`'s `use-effect!` only sets these attributes *after*
the render commit. That effect remains as a safety net (it's
idempotent — same setAttribute) and continues to handle the
plugin-hook + custom-theme side effects."
(defn apply-theme-to-dom!
"Single source of truth for stamping the theme onto the DOM:
sets `data-theme` on <html>, adds/removes the `dark` class on <html>,
and the legacy `dark-theme`/`light-theme`/`white-theme` classes on
<body>. Idempotent.
Called synchronously from `set-theme-mode!` *before* `set-state!` so
subscribers that read CSS-var values at render time (avatar fallback
muted-tint, contrast-adjusted icon colors via `colors/read-bg-var`)
see the new theme on their next render. Also called from
`components.theme/container`'s mount effect so the initial DOM is
stamped before any subscriber renders."
[mode]
(when (exists? js/document)
(let [^js doc js/document.documentElement