From 43bbf756b0a89b2ff76b7e15cb40134549fc1e91 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Aug 2026 18:30:33 +0200 Subject: [PATCH] :bug: Fix font change regression and activate font preview by default (#11341) * :sparkles: Materialize font-preview flag to make it permanent * :bug: Fix font change regression when WebGL is disabled * :paperclip: Add fixes for e2e tests * :bug: Fix font change from legacy render --------- Co-authored-by: Luis de Dios --- .../playwright/ui/specs/tokens/crud.spec.js | 8 +-- .../src/app/main/data/workspace/texts.cljs | 17 +++-- frontend/src/app/main/fonts.cljs | 6 +- frontend/src/app/main/ui/workspace.cljs | 8 +-- .../main/ui/workspace/shapes/text/editor.cljs | 38 ++++++----- .../workspace/sidebar/options/menus/text.cljs | 2 +- .../sidebar/options/menus/typography.cljs | 64 ++++++++----------- .../sidebar/options/menus/typography.scss | 21 +++--- .../src/app/main/ui/workspace/viewport.cljs | 4 +- .../app/main/ui/workspace/viewport_wasm.cljs | 4 +- frontend/src/app/util/dom.cljs | 10 +++ .../data/workspace_texts_test.cljs | 10 +-- 12 files changed, 95 insertions(+), 97 deletions(-) diff --git a/frontend/playwright/ui/specs/tokens/crud.spec.js b/frontend/playwright/ui/specs/tokens/crud.spec.js index 4ba8d7fd49..35922e0af1 100644 --- a/frontend/playwright/ui/specs/tokens/crud.spec.js +++ b/frontend/playwright/ui/specs/tokens/crud.spec.js @@ -569,7 +569,7 @@ test.describe("Tokens - creation", () => { }); await selectDropdown.click(); - const fontOption = tokensUpdateCreateModal.getByText("ABeeZee"); + const fontOption = tokensUpdateCreateModal.getByRole('img', { name: 'ABeeZee' }) await expect(fontOption).toBeVisible(); await fontOption.click(); @@ -583,7 +583,7 @@ test.describe("Tokens - creation", () => { name: "Search font", }); await searchField.fill("alme"); - const fontOption2 = tokensUpdateCreateModal.getByText("Almendra Display"); + const fontOption2 = tokensUpdateCreateModal.getByRole('img', {name: "Almendra Display"}); await expect(fontOption2).toBeVisible(); await fontOption2.click(); @@ -1521,7 +1521,7 @@ test.describe("Tokens - creation", () => { }); await selectDropdown.click(); - const fontOption = tokensUpdateCreateModal.getByText("ABeeZee"); + const fontOption = tokensUpdateCreateModal.getByRole("img", {name: "ABeeZee"}); await expect(fontOption).toBeVisible(); await fontOption.click(); @@ -1536,7 +1536,7 @@ test.describe("Tokens - creation", () => { name: "Search font", }); await searchField.fill("alme"); - const fontOption2 = tokensUpdateCreateModal.getByText("Almendra Display"); + const fontOption2 = tokensUpdateCreateModal.getByRole("img", {name: "Almendra Display"}); await expect(fontOption2).toBeVisible(); await fontOption2.click(); await expect( diff --git a/frontend/src/app/main/data/workspace/texts.cljs b/frontend/src/app/main/data/workspace/texts.cljs index 3db75ccb8e..dd8741218d 100644 --- a/frontend/src/app/main/data/workspace/texts.cljs +++ b/frontend/src/app/main/data/workspace/texts.cljs @@ -174,7 +174,12 @@ (rx/mapcat (fn [_] (rx/from (fonts/ensure-loaded! font-id font-variant-id)))) (rx/take-until (text-work-stopper stream)) - (rx/ignore) + (rx/mapcat (fn [_] + (st/emit! (dwsh/update-shapes + ids + #(dissoc % :position-data) + {:save-undo? false})) + (rx/empty))) (wrf/with-pending :font ids)))) ;; -- Content helpers @@ -850,16 +855,16 @@ ::resize-text-debounce-event))))) (rx/empty)))))) -(defn save-font +(defn save-default-font [data] - (ptk/reify ::save-font + (ptk/reify ::save-default-font ptk/UpdateEvent (update [_ state] - (let [multiple? (->> data vals (d/seek #(= % :multiple)))] + (let [multiple? (->> data vals (d/seek #(= % :multiple))) + font (dissoc data :typography-ref-id :typography-ref-file)] (cond-> state (not multiple?) - (assoc-in [:workspace-global :default-font] - (dissoc data :typography-ref-id :typography-ref-file))))))) + (update :workspace-global assoc :default-font font)))))) (defn apply-text-modifier [shape text-modifier] diff --git a/frontend/src/app/main/fonts.cljs b/frontend/src/app/main/fonts.cljs index 644169d3de..6db6fd3817 100644 --- a/frontend/src/app/main/fonts.cljs +++ b/frontend/src/app/main/fonts.cljs @@ -242,7 +242,7 @@ (when (zero? (:refs new-state)) (dom/remove! node)))) -(defn- add-font-css! +(defn- add-font-css "Creates a style element and attaches it to the dom." [id css] (let [node (dom/create-element "style")] @@ -306,7 +306,7 @@ (->> (request-gfont-css url) (rx/map process-gfont-css) (rx/tap #(on-loaded id)) - (rx/subs! (partial add-font-css! id) + (rx/subs! (partial add-font-css id) #(when (fn? on-failed) (on-failed %)))) nil))) @@ -346,7 +346,7 @@ (when (globals/browser?) (log/dbg :hint "load-font" :font-id id :backend "custom") (let [css (generate-custom-font-css font)] - (add-font-css! id css) + (add-font-css id css) (when (fn? on-loaded) (on-loaded))))) diff --git a/frontend/src/app/main/ui/workspace.cljs b/frontend/src/app/main/ui/workspace.cljs index 278663bbcb..a5920570a6 100644 --- a/frontend/src/app/main/ui/workspace.cljs +++ b/frontend/src/app/main/ui/workspace.cljs @@ -8,7 +8,6 @@ (:require-macros [app.main.style :as stl]) (:require [app.common.data.macros :as dm] - [app.config :as cf] [app.main.data.common :as dcm] [app.main.data.helpers :as dsh] [app.main.data.persistence :as dps] @@ -229,12 +228,8 @@ (st/emit! (dps/initialize-persistence) (dpl/update-plugins-permissions-peek))) - ;; FLAG :font-preview — prefetch the preview sprite markup on workspace mount - ;; (kept in memory, not the DOM) so the typography selector renders previews on - ;; open with no network wait. Remove the flag check to drop the feature. (mf/with-effect [] - (when (contains? cf/flags :font-preview) - (fonts/prefetch-preview-sprite!))) + (fonts/prefetch-preview-sprite!)) ;; Setting the layout preset by its name (mf/with-effect [layout-name] @@ -302,4 +297,3 @@ (when (uuid? file-id) [:> workspace* props]))) - diff --git a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs index 536febb61b..6f949e5e72 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/editor.cljs @@ -30,18 +30,24 @@ ;; --- Text Editor Rendering -(mf/defc block-component* - [{:keys [block-props] :as props}] - (let [data (.-data ^js block-props) - style (sts/generate-paragraph-styles (.-shape ^js block-props) data) - dir (:text-direction data "auto")] +(mf/defc block-component + {::mf/wrap-props false} + [props] + (let [bprops (obj/get props "blockProps") + data (obj/get bprops "data") + style (sts/generate-paragraph-styles (obj/get bprops "shape") + (obj/get bprops "data")) + dir (:text-direction data "auto")] + [:div {:style style :dir dir} [:> draft/EditorBlock props]])) -(mf/defc selection-component* - [{:keys [children]}] - [:span {:style {:background "#ccc" :display "inline-block"}} children]) +(mf/defc selection-component + {::mf/wrap-props false} + [props] + (let [children (obj/get props "children")] + [:span {:style {:background "#ccc" :display "inline-block"}} children])) (defn- render-block [block shape] @@ -49,7 +55,7 @@ (case type "unstyled" #js {:editable true - :component block-component* + :component block-component :props #js {:data (ted/get-editor-block-data block) :shape shape}} nil))) @@ -63,7 +69,7 @@ (sts/generate-text-styles shape data {:show-text? false}))) (def default-decorator - (ted/create-decorator "PENPOT_SELECTION" selection-component*)) + (ted/create-decorator "PENPOT_SELECTION" selection-component)) (def empty-editor-state (ted/create-editor-state nil default-decorator)) @@ -89,11 +95,12 @@ "bottom" "flex-end" nil)) -(mf/defc text-shape-edit-html* +(mf/defc text-shape-edit-html {::mf/wrap [mf/memo] + ::mf/wrap-props false ::mf/forward-ref true} - [{:keys [shape]} _] - (let [{:keys [id content]} shape + [props _] + (let [{:keys [id content] :as shape} (obj/get props "shape") state-map (mf/deref refs/workspace-editor-state) state (get state-map id empty-editor-state) @@ -268,7 +275,8 @@ (-> (gpt/subtract pt box) (gpt/multiply zoom))))) -(mf/defc text-editor-svg* +(mf/defc text-editor-svg + {::mf/wrap-props false} [{:keys [shape modifiers]}] (let [shape-id (dm/get-prop shape :id) modifiers (dm/get-in modifiers [shape-id :modifiers]) @@ -341,6 +349,6 @@ [:foreignObject {:x x :y y :width width :height height} [:div {:style style} - [:> text-shape-edit-html* + [:& text-shape-edit-html {:shape shape :key (dm/str shape-id)}]]]])) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs index 60b0c15c02..2054cc763c 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/text.cljs @@ -412,7 +412,7 @@ (select-keys txt/text-node-attrs))] (when (features/active-feature? @st/state "text-editor-wasm/v1") (st/emit! (dwt-v3/v3-update-text-editor-styles (first ids) attrs))) - (st/emit! (dwt/save-font updated-attrs) + (st/emit! (dwt/save-default-font updated-attrs) (dwt/update-all-attrs ids attrs))))) on-change diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs index 1a085ef1c4..53a5a26b01 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs @@ -12,7 +12,6 @@ [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.types.text :as txt] - [app.config :as cf] [app.main.constants :refer [max-input-length]] [app.main.data.common :as dcm] [app.main.data.fonts :as fts] @@ -91,19 +90,13 @@ (constantly nil))))) @loaded?)) -;; --- FEATURE: font preview (flag :font-preview) ------------------------------ -;; font-item-preview* and use-font-lazy-load are the whole feature. They are only -;; rendered/called behind the `:font-preview` flag check in font-item* below, so -;; their hooks never run when the flag is off. To remove the flag, inline -;; font-item-preview* into font-item* and drop the plain-name branch. - (mf/defc font-item-preview* "Row content with previews: a vector preview from the shared sprite for catalog fonts, or the font's own name lazily loaded for custom fonts the sprite doesn't cover." {::mf/wrap [mf/memo]} [{:keys [font]}] - (let [font-id (:id font) + (let [font-id (get font :id) sprite (mf/deref fonts/preview-sprite) ;; The sprite is only referenceable once it's been attached to the DOM, @@ -133,29 +126,20 @@ {::mf/wrap [mf/memo]} [{:keys [font is-current on-click style]}] (let [item-ref (mf/use-ref) - on-click (mf/use-fn (mf/deps font) #(on-click font)) - ;; FLAG :font-preview — gates the feature markup AND its row styling - ;; (.font-item-preview-on in the scss). Remove this and its two uses below. - preview? (contains? cf/flags :font-preview)] + on-click (mf/use-fn (mf/deps font) #(on-click font))] - (mf/use-effect - (mf/deps is-current) - (fn [] - (when is-current - (let [element (mf/ref-val item-ref)] - (when-not (dom/is-in-viewport? element) - (dom/scroll-into-view! element)))))) + (mf/with-effect [is-current] + (when is-current + (let [element (mf/ref-val item-ref)] + (when-not (dom/is-in-viewport? element) + (dom/scroll-into-view! element))))) [:div {:class (stl/css :font-wrapper) :style style :ref item-ref :on-click on-click} - [:div {:class (stl/css-case :font-item true - :font-item-preview-on preview? - :selected is-current)} - (if preview? - [:> font-item-preview* {:font font}] - [:span {:class (stl/css :font-item-label)} (:name font)]) + [:div {:class (stl/css-case :font-item true :selected is-current)} + [:> font-item-preview* {:font font}] (when is-current [:> icon* {:icon-id i/tick :size "s"}])]])) @@ -260,14 +244,13 @@ (let [key (events/listen js/document "keydown" on-key-down)] #(events/unlistenByKey key))) - ;; FLAG :font-preview — materialize the preview sprite into the DOM only while - ;; the picker is open (markup is prefetched on workspace load), removing it on - ;; close so its ~2000 nodes aren't kept around idle. The attachment is deferred - ;; so the dropdown can paint first with plain names, then the sprite swaps in - ;; on the next tick. Remove the flag clause to drop the feature. + ;; Materialize the preview sprite into the DOM only while the picker is open + ;; (markup is prefetched on workspace load), removing it on close so its + ;; ~2000 nodes aren't kept around idle. The attachment is deferred so the + ;; dropdown can paint first with plain names, then the sprite swaps in on the + ;; next tick. (mf/with-effect [sprite-status] - (when (and (contains? cf/flags :font-preview) - (= :ready sprite-status)) + (when (= :ready sprite-status) (let [node* (volatile! nil) task (tm/schedule (fn [] @@ -278,9 +261,11 @@ (fonts/detach-preview-sprite! n)))))) (mf/with-effect [@selected] - (when-let [inst (mf/ref-val flist)] - (when-let [index (:index @selected)] - (.scrollToRow ^js inst index)))) + (let [node (mf/ref-val flist) + index (:index @selected)] + ;; This is nil safe operation, do nothing if node or index are + ;; invalid. + (dom/scroll-to-row node index))) (mf/with-effect [@selected] (on-select @selected)) @@ -291,11 +276,12 @@ (st/emit! (dsc/pop-shortcuts :typography)))) (mf/with-effect [] - (let [index (d/index-of-pred fonts #(= (:id %) (:id current-font))) - inst (mf/ref-val flist)] + (let [index (d/index-of-pred fonts #(= (:id %) (:id current-font))) + node (mf/ref-val flist)] (tm/schedule - #(let [offset (.getOffsetForRow ^js inst #js {:alignment "center" :index index})] - (.scrollToPosition ^js inst offset))))) + #(let [offset (.getOffsetForRow ^js node #js {:alignment "center" :index index})] + ;; Safe operaton, do nothing if node or offset has invalid values + (dom/scroll-to-position node offset))))) [:div {:class [(stl/css-case :font-selector true :fonts-on-modal (not full-size?))]} diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss index c5b1a59d2f..967e9d7c0f 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.scss @@ -388,20 +388,7 @@ $font-preview-box-height: 28px; &.selected { color: var(--color-foreground-primary); } -} -.font-item-label { - @include t.use-typography("body-small"); - @include text-ellipsis; - - flex-grow: 1; - min-inline-size: 0; -} - -// --- FLAG :font-preview row styling. Only applied when font-item* adds the -// .font-item-preview-on modifier; remove this whole block with the flag so rows -// render exactly as before. -.font-item-preview-on { // Center & clip so a previewed font's own metrics never grow/overflow the row. align-items: center; overflow: hidden; @@ -414,6 +401,14 @@ $font-preview-box-height: 28px; } } +.font-item-label { + @include t.use-typography("body-small"); + @include text-ellipsis; + + flex-grow: 1; + min-inline-size: 0; +} + // `currentColor` makes the glyph fill follow the row text color (theme + selected). .font-item-preview { flex-grow: 1; diff --git a/frontend/src/app/main/ui/workspace/viewport.cljs b/frontend/src/app/main/ui/workspace/viewport.cljs index 4814be5042..0f5fbfeeac 100644 --- a/frontend/src/app/main/ui/workspace/viewport.cljs +++ b/frontend/src/app/main/ui/workspace/viewport.cljs @@ -455,8 +455,8 @@ [:& editor-v2/text-editor {:shape editing-shape :canvas-ref canvas-ref :modifiers modifiers}] - [:> editor-v1/text-editor-svg* {:shape editing-shape - :modifiers modifiers}])) + [:& editor-v1/text-editor-svg {:shape editing-shape + :modifiers modifiers}])) (when show-frame-outline? (let [outlined-frame-id diff --git a/frontend/src/app/main/ui/workspace/viewport_wasm.cljs b/frontend/src/app/main/ui/workspace/viewport_wasm.cljs index 5a9601502d..1967662aa1 100644 --- a/frontend/src/app/main/ui/workspace/viewport_wasm.cljs +++ b/frontend/src/app/main/ui/workspace/viewport_wasm.cljs @@ -747,8 +747,8 @@ :canvas-ref canvas-ref :ref text-editor-ref}] - :else [:> editor-v1/text-editor-svg* {:shape editing-shape - :ref text-editor-ref}])) + :else [:& editor-v1/text-editor-svg {:shape editing-shape + :ref text-editor-ref}])) (when show-frame-outline? (let [outlined-frame-id (->> @hover-ids diff --git a/frontend/src/app/util/dom.cljs b/frontend/src/app/util/dom.cljs index 6a0f514018..5995a51264 100644 --- a/frontend/src/app/util/dom.cljs +++ b/frontend/src/app/util/dom.cljs @@ -244,6 +244,16 @@ height (.-clientHeight scroll-node)] (/ distance height))) +(defn scroll-to-row + [node index] + (when (and (some? node) (number? index)) + (.scrollToRow ^js node index))) + +(defn scroll-to-position + [node offset] + (when (and (some? node) (number? offset)) + (.scrollToPosition ^js node offset))) + (def get-target-val (comp get-value get-target)) (def get-target-scroll (comp get-scroll-position get-target)) diff --git a/frontend/test/frontend_tests/data/workspace_texts_test.cljs b/frontend/test/frontend_tests/data/workspace_texts_test.cljs index 5822b571cf..ebb315b3f6 100644 --- a/frontend/test/frontend_tests/data/workspace_texts_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_texts_test.cljs @@ -379,14 +379,14 @@ "float letter-spacing is normalised to 2-decimal string"))))))) ;; --------------------------------------------------------------------------- -;; Tests: save-font must not persist typography refs into the global default font +;; Tests: save-default-font must not persist typography refs into the global default font ;; ;; Root cause of #10925: typography assets are file-specific references, but -;; save-font used to write :typography-ref-id / :typography-ref-file into the +;; save-default-font used to write :typography-ref-id / :typography-ref-file into the ;; session-global [:workspace-global :default-font]. That state survives a file ;; switch, and v2-default-text-content bakes it into brand-new text shapes in ;; the other file, so they got a non-existent typography asset instead of the -;; default Penpot font. save-font now strips those two keys. +;; default Penpot font. save-default-font now strips those two keys. ;; --------------------------------------------------------------------------- (t/deftest save-font-strips-typography-refs-from-default-font @@ -405,7 +405,7 @@ :typography-ref-id (uuid/next) :typography-ref-file (:id file)}] (ths/run-store - store done [(dwt/save-font attrs)] + store done [(dwt/save-default-font attrs)] (fn [new-state] (let [default-font (get-in new-state [:workspace-global :default-font])] (t/is (some? default-font)) @@ -425,7 +425,7 @@ :letter-spacing "0" :typography-ref-id (uuid/next) :typography-ref-file (uuid/next)}] - (ths/run-store store done [(dwt/save-font attrs)] + (ths/run-store store done [(dwt/save-default-font attrs)] (fn [new-state] (let [default-font (get-in new-state [:workspace-global :default-font])] (t/is (= "Open Sans" (:font-family default-font)))