🐛 Block typography creation from missing fonts or multiple texts (#11938)

Creating a typography from a text whose font is no longer installed
baked the broken font-id into a new asset. With several texts selected
there is no single style to capture either. The add-typography event
now does nothing in both cases, and the "Add typography" button in the
local library is disabled with a label that explains why.

The button lives in its own component so selection, shape and editor
changes re-render only the button, not the typography list, and shared
libraries do not subscribe to that state at all. The event and the
button read the editor data through the new `editor-text-options`, so
both see the same font for the wasm, v2 and v1 text editors.

AI-assisted-by: claude-opus-5-5

frontend/playwright/ui/specs/workspace-assets-add-typography.spec.js

@@ -0,0 +1,131 @@
+import { test, expect } from "@playwright/test";
+import { WorkspacePage } from "../pages/WorkspacePage";
+
+// ---------------------------------------------------------------------------
+// The "Add typography" button in the local library typographies section
+// (src/app/main/ui/workspace/sidebar/assets/typographies.cljs) creates a
+// typography from the selected text. It is disabled, and its label explains
+// why, when:
+//   - the selected text uses a font that is not installed
+//   - two or more texts are selected
+// ---------------------------------------------------------------------------
+
+function addTypographyButton(workspace, name) {
+  return workspace.sidebar.getByRole("button", {
+    name,
+    exact: typeof name === "string",
+  });
+}
+
+test.beforeEach(async ({ page }) => {
+  await WorkspacePage.init(page);
+});
+
+test.describe("font of the selected text", () => {
+  // render-wasm/get-file-text-custom-fonts.json has a text shape
+  // ("Penpot & Dragons") using a custom team font-id. The get-font-variants
+  // mock decides whether that font is installed or missing.
+  const FILE = {
+    id: "434b0541-fa2f-802f-8006-59827d964a9b",
+    pageId: "434b0541-fa2f-802f-8006-59827d964a9c",
+  };
+
+  test("Add typography is enabled when the selected text font is installed", async ({
+    page,
+  }) => {
+    const workspace = new WorkspacePage(page);
+    await workspace.setupEmptyFile();
+    await workspace.mockRPC(
+      /get\-file\?/,
+      "render-wasm/get-file-text-custom-fonts.json",
+    );
+    await workspace.mockRPC(
+      "get-font-variants?team-id=*",
+      "render-wasm/get-font-variants-custom-fonts.json",
+    );
+    await workspace.goToWorkspace({ fileId: FILE.id, pageId: FILE.pageId });
+
+    await workspace.clickLeafLayer("Penpot & Dragons");
+    await workspace.clickAssets();
+
+    await expect(
+      addTypographyButton(workspace, "Add typography"),
+    ).toBeEnabled();
+  });
+
+  test("Add typography is disabled when the selected text font is missing", async ({
+    page,
+  }) => {
+    const workspace = new WorkspacePage(page);
+    await workspace.setupEmptyFile();
+    await workspace.mockRPC(
+      /get\-file\?/,
+      "render-wasm/get-file-text-custom-fonts.json",
+    );
+    await workspace.mockRPC(
+      "get-font-variants?team-id=*",
+      "workspace/get-font-variants-empty.json",
+    );
+    await workspace.goToWorkspace({ fileId: FILE.id, pageId: FILE.pageId });
+
+    await workspace.clickLeafLayer("Penpot & Dragons");
+    await workspace.clickAssets();
+
+    await expect(
+      addTypographyButton(workspace, /is no longer available/),
+    ).toBeDisabled();
+  });
+});
+
+test.describe("number of selected texts", () => {
+  // workspace/get-file-text-multiple-selection.json has two text shapes that
+  // use the same built-in font, so only the selection count changes.
+  const FILE = {
+    id: "434b0541-fa2f-802f-8006-6a827d964a9b",
+    pageId: "434b0541-fa2f-802f-8006-6a827d964a9c",
+  };
+
+  test("Add typography is disabled when two texts are selected", async ({
+    page,
+  }) => {
+    const workspace = new WorkspacePage(page);
+    await workspace.setupEmptyFile();
+    await workspace.mockRPC(
+      /get\-file\?/,
+      "workspace/get-file-text-multiple-selection.json",
+    );
+    await workspace.goToWorkspace({ fileId: FILE.id, pageId: FILE.pageId });
+
+    await workspace.clickLeafLayer("Text multiple selection one");
+    await workspace.clickLeafLayer("Text multiple selection two", {
+      modifiers: ["Shift"],
+    });
+    await workspace.clickAssets();
+
+    await expect(
+      addTypographyButton(
+        workspace,
+        "Select one text to create a typography style",
+      ),
+    ).toBeDisabled();
+  });
+
+  test("Add typography is enabled when one text is selected", async ({
+    page,
+  }) => {
+    const workspace = new WorkspacePage(page);
+    await workspace.setupEmptyFile();
+    await workspace.mockRPC(
+      /get\-file\?/,
+      "workspace/get-file-text-multiple-selection.json",
+    );
+    await workspace.goToWorkspace({ fileId: FILE.id, pageId: FILE.pageId });
+
+    await workspace.clickLeafLayer("Text multiple selection one");
+    await workspace.clickAssets();
+
+    await expect(
+      addTypographyButton(workspace, "Add typography"),
+    ).toBeEnabled();
+  });
+});

frontend/src/app/main/data/workspace/texts_events.cljs

@@ -7,7 +7,6 @@
 (ns app.main.data.workspace.texts-events
   (:require
    [app.common.data :as d]
-   [app.common.data.macros :as dm]
    [app.common.files.helpers :as cfh]
    [app.common.math :as mth]
    [app.common.types.text :as txt]
@@ -18,9 +17,24 @@
    [app.main.data.workspace.libraries :as dwl]
    [app.main.data.workspace.pages :as-alias dwpg]
    [app.main.data.workspace.texts :as dwt]
+   [app.main.features :as features]
+   [app.main.fonts :as fonts]
    [beicon.v2.core :as rx]
    [potok.v2.core :as ptk]))
 
+(defn editor-text-options
+  "Editor data that `dwt/current-text-values` needs for `shape-id`,
+  taken from the text editor that is active in `state`."
+  [state shape-id]
+  (let [wasm? (features/active-feature? state "text-editor-wasm/v1")
+        v2?   (features/active-feature? state "text-editor/v2")
+        state-map (if wasm?
+                    (:workspace-wasm-editor-styles state)
+                    (:workspace-editor-state state))]
+    {:editor-styles   (when wasm? (get state-map shape-id))
+     :editor-state    (when-not v2? (get state-map shape-id))
+     :editor-instance (when v2? (:workspace-editor state))}))
+
 ;; This function must be separated from app.main.data.workspace.texts to avoid a circular
 ;; dependency due main.data.workspace.libraries eventually calling app.main.data.workspace.texts.
 
@@ -45,14 +59,20 @@
              shape      (first shapes)
 
              values     (dwt/current-text-values
-                         {:editor-state (dm/get-in state [:workspace-editor-state (:id shape)])
-                          :shape shape
-                          :attrs txt/text-node-attrs})
+                         (assoc (editor-text-options state (:id shape))
+                                :shape shape
+                                :attrs txt/text-node-attrs))
 
              multiple? (or (> 1 (count shapes))
                            (d/seek (partial = :multiple)
                                    (vals values)))
 
+             too-many-texts? (> (count shapes) 1)
+
+             font-missing? (and (not multiple?)
+                                (some? (:font-id values))
+                                (not (fonts/installed? (:font-id values))))
+
              values    (-> (d/without-nils values)
                            (select-keys
                             (d/concat-vec txt/text-font-attrs
@@ -74,12 +94,14 @@
                            (cond-> (string? group-path)
                              (update :name #(str group-path " / " %))))]
 
-         (rx/concat
-          (rx/of (dwl/add-typography typ)
-                 (ev/event {::ev/name "add-asset-to-library"
-                            :asset-type "typography"}))
+         (if (or font-missing? too-many-texts?)
+           (rx/empty)
+           (rx/concat
+            (rx/of (dwl/add-typography typ)
+                   (ev/event {::ev/name "add-asset-to-library"
+                              :asset-type "typography"}))
 
-          (when (not multiple?)
-            (rx/of (dwt/update-attrs (:id shape)
-                                     {:typography-ref-id typ-id
-                                      :typography-ref-file file-id})))))))))
+            (when (not multiple?)
+              (rx/of (dwt/update-attrs (:id shape)
+                                       {:typography-ref-id typ-id
+                                        :typography-ref-file file-id}))))))))))

frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs

@@ -9,7 +9,9 @@
   (:require
    [app.common.data :as d]
    [app.common.data.macros :as dm]
+   [app.common.files.helpers :as cfh]
    [app.common.path-names :as cpn]
+   [app.common.types.text :as txt]
    [app.main.data.event :as ev]
    [app.main.data.modal :as modal]
    [app.main.data.workspace :as dw]
@@ -17,6 +19,7 @@
    [app.main.data.workspace.texts :as dwt]
    [app.main.data.workspace.texts-events :as dwte]
    [app.main.data.workspace.undo :as dwu]
+   [app.main.fonts :as fonts]
    [app.main.refs :as refs]
    [app.main.store :as st]
    [app.main.ui.context :as ctx]
@@ -243,6 +246,66 @@
                                     :selected-full selected-full
                                     :is-read-only is-read-only}]))])]))
 
+;; Kept apart from `typographies-section*` so that selection, shape and
+;; editor changes only re-render this button, not the typography list.
+(mf/defc add-typography-button*
+  {::mf/private true}
+  [{:keys [file-id]}]
+  (let [selected      (mf/deref refs/selected-shapes)
+        objects       (mf/deref refs/workspace-page-objects)
+        text-shapes   (mf/with-memo [selected objects]
+                        (into []
+                              (comp (keep (d/getf objects))
+                                    (filter cfh/text-shape?))
+                              selected))
+        many-texts?   (> (count text-shapes) 1)
+        shape         (when-not many-texts? (first text-shapes))
+        shape-id      (:id shape)
+
+        ;; The v2 editor instance is mutable, so its per-shape state is
+        ;; included only to re-render when the editor styles change.
+        editor-ref    (mf/with-memo [shape-id]
+                        (l/derived (fn [state]
+                                     (when shape-id
+                                       [(dwte/editor-text-options state shape-id)
+                                        (dm/get-in state [:workspace-v2-editor-state shape-id])]))
+                                   st/state =))
+        editor-data   (mf/deref editor-ref)
+        loaded-fonts  (mf/deref fonts/fontsdb)
+
+        values        (mf/with-memo [shape editor-data]
+                        (when shape
+                          (dwt/current-text-values
+                           (assoc (first editor-data)
+                                  :shape shape
+                                  :attrs txt/text-node-attrs))))
+        font-id       (:font-id values)
+        font-missing? (and (some? font-id)
+                           (not (d/seek (partial = :multiple) (vals values)))
+                           (not (contains? loaded-fonts font-id)))
+
+        on-click
+        (mf/use-fn
+         (mf/deps file-id)
+         (fn [_]
+           (st/emit! (dw/set-assets-section-open file-id :typographies true))
+           (st/emit! (dwte/add-typography file-id))))]
+
+    [:> icon-button* {:variant "ghost"
+                      :aria-label (cond
+                                    many-texts?
+                                    (tr "workspace.assets.typography.multiple-texts-selected")
+
+                                    font-missing?
+                                    (tr "workspace.options.font-not-available"
+                                        (:font-family values))
+
+                                    :else
+                                    (tr "workspace.assets.typography.add-typography"))
+                      :on-click on-click
+                      :disabled (or many-texts? font-missing?)
+                      :icon i/add}]))
+
 (mf/defc typographies-section*
   [{:keys [file file-id typographies open-status-ref selected
            is-local is-open is-force-open is-reverse-sort
@@ -274,13 +337,6 @@
 
         open-groups          (mf/deref open-groups-ref)
 
-        add-typography
-        (mf/use-fn
-         (mf/deps file-id)
-         (fn [_]
-           (st/emit! (dw/set-assets-section-open file-id :typographies true))
-           (st/emit! (dwte/add-typography file-id))))
-
         handle-change
         (mf/use-fn
          (mf/deps file-id)
@@ -427,10 +483,7 @@
       (when is-local
         [:> cmm/asset-section-block* {:role :title-button}
          (when-not read-only?
-           [:> icon-button* {:variant "ghost"
-                             :aria-label (tr "workspace.assets.typography.add-typography")
-                             :on-click add-typography
-                             :icon i/add}])])
+           [:> add-typography-button* {:file-id file-id}])])
 
       [:> cmm/asset-section-block* {:role :content}
        [:& typographies-group {:file-id file-id

frontend/test/frontend_tests/data/workspace_texts_test.cljs

@@ -379,6 +379,74 @@
            (t/is (= "0.1" (:letter-spacing (first typographies)))
                  "float letter-spacing is normalised to 2-decimal string")))))))
 
+;; ---------------------------------------------------------------------------
+;; Tests: add-typography must not create an asset from a deleted font
+;;
+;; A text shape can keep referencing a font-id that is no longer installed
+;; (the font was removed, or belongs to an unavailable library). Creating a
+;; typography from it would bake that broken font-id into a brand-new asset.
+;; ---------------------------------------------------------------------------
+
+(t/deftest add-typography-skips-shape-with-deleted-font
+  (t/async
+    done
+    (let [content   (txt/change-text nil "hello" :font-id "deleted-font-id")
+          file      (-> (cthf/sample-file :file1)
+                        (cths/add-sample-shape :text1
+                                               :type    :text
+                                               :x 0 :y 0
+                                               :content content))
+          shape-id  (:id (cths/get-shape file :text1))
+          file-id   (:id file)
+          store     (ths/setup-store file)
+          events    [(fn [state] (assoc-in state [:workspace-local :selected] #{shape-id}))
+                     (dwte/add-typography file-id)]]
+
+      (ths/run-store
+       store done events
+       (fn [new-state]
+         (let [file'        (ths/get-file-from-state new-state)
+               typographies (vals (get-in file' [:data :typographies]))
+               shape'       (cths/get-shape file' :text1)]
+           (t/is (= 0 (count typographies))
+                 "no typography was added for a shape with a deleted font")
+           (t/is (nil? (:typography-ref-id shape'))
+                 "the shape was not linked to a typography")))))))
+
+;; ---------------------------------------------------------------------------
+;; Tests: add-typography must not create an asset from two or more selected texts
+;;
+;; Deriving a typography from a specific text only makes sense for a single
+;; shape; with several texts selected there is no one style to capture.
+;; ---------------------------------------------------------------------------
+
+(t/deftest add-typography-skips-multiple-selected-texts
+  (t/async
+    done
+    (let [file      (-> (cthf/sample-file :file1)
+                        (cths/add-sample-shape :text1
+                                               :type    :text
+                                               :x 0 :y 0
+                                               :content (txt/change-text nil "hello"))
+                        (cths/add-sample-shape :text2
+                                               :type    :text
+                                               :x 0 :y 100
+                                               :content (txt/change-text nil "world")))
+          shape-id-1 (:id (cths/get-shape file :text1))
+          shape-id-2 (:id (cths/get-shape file :text2))
+          file-id   (:id file)
+          store     (ths/setup-store file)
+          events    [(fn [state] (assoc-in state [:workspace-local :selected] #{shape-id-1 shape-id-2}))
+                     (dwte/add-typography file-id)]]
+
+      (ths/run-store
+       store done events
+       (fn [new-state]
+         (let [file'        (ths/get-file-from-state new-state)
+               typographies (vals (get-in file' [:data :typographies]))]
+           (t/is (= 0 (count typographies))
+                 "no typography was added when two texts are selected")))))))
+
 ;; ---------------------------------------------------------------------------
 ;; Tests: save-default-font must not persist typography refs into the global default font
 ;;

frontend/translations/en.po

@@ -6245,6 +6245,10 @@ msgstr "Typographies"
 msgid "workspace.assets.typography.add-typography"
 msgstr "Add typography"
 
+#: src/app/main/ui/workspace/sidebar/assets/typographies.cljs:461
+msgid "workspace.assets.typography.multiple-texts-selected"
+msgstr "Select one text to create a typography style"
+
 #: src/app/main/ui/workspace/sidebar/options/menus/typography.cljs:855
 msgid "workspace.assets.typography.font-size"
 msgstr "Size"

frontend/translations/es.po

@@ -6314,6 +6314,10 @@ msgstr "Tipografías"
 msgid "workspace.assets.typography.add-typography"
 msgstr "Añadir tipografía"
 
+#: src/app/main/ui/workspace/sidebar/assets/typographies.cljs:461
+msgid "workspace.assets.typography.multiple-texts-selected"
+msgstr "Selecciona un solo texto para crear un estilo de tipografía"
+
 #: src/app/main/ui/workspace/sidebar/options/menus/typography.cljs:855
 msgid "workspace.assets.typography.font-size"
 msgstr "Tamaño"