🐛 Fix font change regression and activate font preview by default (#11341)

* ✨ Materialize font-preview flag to make it permanent

* 🐛 Fix font change regression when WebGL is disabled

* 📎 Add fixes for e2e tests

* 🐛 Fix font change from legacy render

---------

Co-authored-by: Luis de Dios <luis.dedios@kaleidos.net>

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(

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]

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)))))
 

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])))
-

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)}]]]]))

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

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?))]}

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;

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

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

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))

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)))