From abdfa0ca67fe8db0aa334ebf4e0aa1e7db42a543 Mon Sep 17 00:00:00 2001 From: scheinriese Date: Tue, 19 May 2026 13:38:06 +0200 Subject: [PATCH] refactor(theme): single helper owns theme-DOM stamping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 , body classes updated, end state consistent. Co-Authored-By: Claude Opus 4.7 --- src/main/frontend/components/theme.cljs | 37 ++++++++++++------------- src/main/frontend/state.cljs | 23 +++++++-------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/main/frontend/components/theme.cljs b/src/main/frontend/components/theme.cljs index be7c043899..049ae5e1f7 100644 --- a/src/main/frontend/components/theme.cljs +++ b/src/main/frontend/components/theme.cljs @@ -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 . 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 . 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 diff --git a/src/main/frontend/state.cljs b/src/main/frontend/state.cljs index 20cfc178ff..ee441d79cf 100644 --- a/src/main/frontend/state.cljs +++ b/src/main/frontend/state.cljs @@ -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 , adds/removes the `dark` class on , + and the legacy `dark-theme`/`light-theme`/`white-theme` classes on + . 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