diff --git a/frontend/playwright/ui/specs/workspace-assets-add-typography.spec.js b/frontend/playwright/ui/specs/workspace-assets-add-typography.spec.js new file mode 100644 index 0000000000..b9639cb7a0 --- /dev/null +++ b/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(); + }); +}); diff --git a/frontend/src/app/main/data/workspace/texts_events.cljs b/frontend/src/app/main/data/workspace/texts_events.cljs index 4dc4591cf9..b9ef657f76 100644 --- a/frontend/src/app/main/data/workspace/texts_events.cljs +++ b/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})))))))))) diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs index 5ad9cae78e..f9ec09486f 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/typographies.cljs +++ b/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 diff --git a/frontend/test/frontend_tests/data/workspace_texts_test.cljs b/frontend/test/frontend_tests/data/workspace_texts_test.cljs index 252a86cc09..6994f1a219 100644 --- a/frontend/test/frontend_tests/data/workspace_texts_test.cljs +++ b/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 ;; diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 0ccebacee0..815de4addb 100644 --- a/frontend/translations/en.po +++ b/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" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 0726cbb4d3..2d22dedfdf 100644 --- a/frontend/translations/es.po +++ b/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"