clone/logseq
1
0
Fork 0
mirror of https://github.com/logseq/logseq.git synced 2026-05-18 09:52:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: theme-toggle re-render + text icon contrast

Browse Source 
Two related fixes converging on the same root issue: render-time JS
reads of CSS variables (via `colors/read-bg-var` / `getComputedStyle`)
produce hex values that go stale or out of contrast across themes.

1) Global theme-toggle reactivity (theme.cljs)

   The earlier pointwise `(state/sub :ui/theme)` subscriptions in
   `avatar-image-cp`, `get-node-icon-cp`, and `icon-picker-trigger-icon`
   covered the high-traffic surfaces but left picker-internal tiles
   and future icon-rendering components vulnerable to the same
   staleness. Pivot to one `(ui-handler/re-render-root!)` call inside
   theme.cljs's existing data-theme `use-effect!`. Fires after the
   DOM attribute is stamped on <html>, marks every reactive component
   dirty, and lets the next reconciliation re-sample fresh CSS
   variables. Pattern-consistent with the language-change handler at
   settings.cljs:298. Theme toggle is a rare user-initiated action;
   the one-frame reconciliation cost is imperceptible. Removes the
   three pointwise subs as redundant.

2) Text icon contrast (icon.cljs text branch)

   The text-icon SVG previously baked the raw user-picked hex into
   `:fill`, which won the cascade over the surrounding `color?`
   wrapper's contrast-adjusted `:style {:color …}`. Result: a
   `#000000` text-icon pick rendered as invisible black on a dark
   theme even though the wrapper had computed a readable shade right
   above it in the tree. Same `colors/adjust-for-contrast` path
   tabler icons have always used — they inherit via
   `stroke="currentColor"`; text icons just need to opt into the
   same inheritance via `:fill "currentColor"`. One-line behavioral
   change; the `text-color` let-binding is no longer needed.

Net diff: theme.cljs +12 / icon.cljs -20.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
scheinriese
2026-05-12 23:29:29 +02:00
parent 290ebf6d55
commit 942a87fae6
2 changed files with 21 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
src/main/frontend/components/icon.cljs
View File

@@ -478,15 +478,6 @@
;; the design rationale (subscribing to the per-tx atom is
;; what catches retractions; `model/sub-block` doesn't).
_latest-tx (state/sub :db/latest-transacted-entity-uuids)
;; Re-render on theme toggle so `avatar-fallback-style` re-
;; samples `--ls-primary-background-color` from the new
;; cascade. The avatar's inline `style` is computed in JS
;; from a `getComputedStyle` snapshot — without a Rum
;; subscription on `:ui/theme`, the snapshot stays frozen
;; after a theme toggle (state.cljs:1283) and the avatar
;; renders with the previous theme's surface tone until
;; some unrelated re-render fires.
_theme (state/sub :ui/theme)
asset-entity (when (and asset-uuid (string? asset-uuid))
(try (db/entity [:block/uuid (uuid asset-uuid)])
(catch :default _ nil)))
@@ -654,7 +645,6 @@
(and (map? normalized) (= :text (:type normalized)) (get-in normalized [:data :value]))
(let [text-value (get-in normalized [:data :value])
text-color (get-in normalized [:data :color])
text-align (get-in normalized [:data :alignment])
display-text (if (> (count text-value) 8)
(subs text-value 0 8)
@@ -678,11 +668,17 @@
y2 (+ 50 (/ line-spacing 2))
;; SVG text-anchor from alignment
anchor (case text-align "left" "start" "right" "end" "middle")
x (case text-align "left" 8 "right" 92 50)
fill (or text-color "currentColor")]
x (case text-align "left" 8 "right" 92 50)]
;; Fill inherits via `currentColor` so the outer `color?`
;; wrapper's contrast-adjusted color (icon.cljs:772-789,
;; same `colors/adjust-for-contrast` path tabler icons
;; use) actually reaches the text. Previously this branch
;; baked the raw user-picked hex directly into `:fill`,
;; which bypassed the wrapper's adjustment — a `#000000`
;; pick rendered as invisible black on dark themes.
[:svg {:viewBox "0 0 100 100"
:width size :height size
:style {:fill fill
:style {:fill "currentColor"
:flex-shrink 0}}
(if multi-line?
(list
@@ -902,16 +898,7 @@
(rum/defc get-node-icon-cp < rum/reactive db-mixins/query
[node-entity opts]
(let [;; Re-render on theme toggle so the bare-avatar branch inside
;; `(icon ...)` (text/letters/emoji avatars) re-samples the
;; live `--ls-primary-background-color` via `read-bg-var`
;; when computing `avatar-fallback-style`. Without this sub,
;; the inline `style` hexes baked in at the previous render
;; stay frozen across theme toggles — none of the other
;; subscriptions in this component (`:ui/icon-hover-preview`,
;; `model/sub-block`) tick on `:ui/theme` changes.
_theme (state/sub :ui/theme)
;; Get fresh entity using db/sub-block to make it reactive to property changes
(let [;; Get fresh entity using db/sub-block to make it reactive to property changes
fresh-entity (when-let [db-id (:db/id node-entity)]
(or (model/sub-block db-id) node-entity))
entity (or fresh-entity node-entity)
@@ -6970,14 +6957,6 @@
trigger only reflects previews from a Default-Icon-scoped picker."
[icon-value preview-target-db-id icon-props & [property]]
(let [property (or property :logseq.property/icon)
;; Re-render on theme toggle so the inline `style="background-
;; color:#...;color:#..."` produced by `avatar-fallback-style`
;; recomputes against the new theme's CSS variables. The
;; page-title (`block.cljs:3223`) routes through `icon-picker`
;; → this trigger and bypasses `get-node-icon-cp` entirely, so
;; the `:ui/theme` sub on get-node-icon-cp (icon.cljs:913)
;; doesn't cover the page-title path.
_theme (state/sub :ui/theme)
preview (when preview-target-db-id (state/sub :ui/icon-hover-preview))
preview-active? (and preview
(icon-preview-matches? preview preview-target-db-id property))

12
src/main/frontend/components/theme.cljs
View File

@@ -49,7 +49,17 @@
(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}))
(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!))
[theme])
;; theme color