From cf78e68787743a6d8265be312ce63e328ee8d0be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Wed, 18 Jun 2025 09:38:50 +0200 Subject: [PATCH] :bug: Fix token unapply for text shapes --- CHANGES.md | 1 + .../src/app/common/files/changes_builder.cljc | 1 - common/src/app/common/logic/shapes.cljc | 17 +++++-- .../app/common/test_helpers/compositions.cljc | 2 - .../src/app/common/test_helpers/shapes.cljc | 16 +++++++ common/src/app/common/types/text.cljc | 46 ++++++++++++++----- .../common_tests/logic/token_apply_test.cljc | 27 +++++++++-- common/test/common_tests/types/text_test.cljc | 23 ++++++++-- 8 files changed, 107 insertions(+), 26 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1361ff568b..79827009da 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -57,6 +57,7 @@ on-premises instances** that want to keep up to date. - Fix button width [Taiga #11394](https://tree.taiga.io/project/penpot/issue/11394) - Fix mixed letter spacing and line height [Taiga #11178](https://tree.taiga.io/project/penpot/issue/11178) - Fix snap nodes shortcut [Taiga #11054](https://tree.taiga.io/project/penpot/issue/11054) +- Fix changing a text property in a text layer does not unapply the previously applied token in the same property [Taiga #11337}(https://tree.taiga.io/project/penpot/issue/11337) ## 2.7.2 diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index 01e7b22215..9dd4da715d 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -161,7 +161,6 @@ (contains? (meta changes) ::file-data) "Call (with-file-data) before using this function")) - (defn- lookup-objects [changes] (let [data (::file-data (meta changes))] diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index 29505b53f4..f26b9f95a8 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -16,19 +16,30 @@ [app.common.types.pages-list :as ctpl] [app.common.types.shape.interactions :as ctsi] [app.common.types.shape.layout :as ctl] + [app.common.types.text :as ctt] [app.common.types.token :as cto] - [app.common.uuid :as uuid])) + [app.common.uuid :as uuid] + [clojure.set :as set])) (defn- generate-unapply-tokens "When updating attributes that have a token applied, we must unapply it, because the value of the attribute now has been given directly, and does not come from the token." [changes objects changed-sub-attr] - (let [mod-obj-changes (->> (:redo-changes changes) + (let [new-objects (pcb/get-objects changes) + mod-obj-changes (->> (:redo-changes changes) (filter #(= (:type %) :mod-obj))) + text-changed-attrs + (fn [shape] + (let [new-shape (get new-objects (:id shape)) + attrs (ctt/get-diff-attrs (:content shape) (:content new-shape))] + (apply set/union (map cto/shape-attr->token-attrs attrs)))) + check-attr (fn [shape changes attr] (let [tokens (get shape :applied-tokens {}) - token-attrs (cto/shape-attr->token-attrs attr changed-sub-attr)] + token-attrs (if (or (not= (:type shape) :text) (not= attr :content)) + (cto/shape-attr->token-attrs attr changed-sub-attr) + (text-changed-attrs shape))] (if (some #(contains? tokens %) token-attrs) (pcb/update-shapes changes [(:id shape)] #(cto/unapply-token-id % token-attrs)) changes))) diff --git a/common/src/app/common/test_helpers/compositions.cljc b/common/src/app/common/test_helpers/compositions.cljc index fb30bfb595..ca6bd064ba 100644 --- a/common/src/app/common/test_helpers/compositions.cljc +++ b/common/src/app/common/test_helpers/compositions.cljc @@ -37,8 +37,6 @@ (merge shape text-params)))) - - (defn add-frame [file frame-label & {:keys [] :as params}] ;; Generated shape tree: diff --git a/common/src/app/common/test_helpers/shapes.cljc b/common/src/app/common/test_helpers/shapes.cljc index 0479efa082..f8fc5aae65 100644 --- a/common/src/app/common/test_helpers/shapes.cljc +++ b/common/src/app/common/test_helpers/shapes.cljc @@ -11,6 +11,7 @@ [app.common.files.helpers :as cfh] [app.common.test-helpers.files :as thf] [app.common.test-helpers.ids-map :as thi] + [app.common.text :as txt] [app.common.types.color :as ctc] [app.common.types.container :as ctn] [app.common.types.pages-list :as ctpl] @@ -81,6 +82,21 @@ (:id page) #(ctst/set-shape % (ctn/set-shape-attr shape attr val))))))) +(defn update-shape-text + [file shape-label attr val & {:keys [page-label]}] + (let [page (if page-label + (thf/get-page file page-label) + (thf/current-page file)) + shape (ctst/get-shape page (thi/id shape-label))] + (update file :data + (fn [file-data] + (ctpl/update-page file-data + (:id page) + #(ctst/set-shape % (txt/update-text-content shape + txt/is-content-node? + d/txt-merge + {attr val}))))))) + (defn sample-library-color [label & {:keys [name path color opacity gradient image]}] (-> {:id (thi/new-id! label) diff --git a/common/src/app/common/types/text.cljc b/common/src/app/common/types/text.cljc index 4d097c48a8..927c2ca36f 100644 --- a/common/src/app/common/types/text.cljc +++ b/common/src/app/common/types/text.cljc @@ -11,9 +11,12 @@ (defn- compare-text-content "Given two content text structures, conformed by maps and vectors, - compare them, and returns a set with the type of differences. - The possibilities are :text-content-text :text-content-attribute and :text-content-structure." - [a b] + compare them, and returns a set with the differences info. + If the structures are equal, it returns an empty set. If the structure + has changed, it returns :text-content-structure. There are two + callbacks to specify what to return when there is a text change with + the same structure, and when attributes change." + [a b {:keys [text-cb attribute-cb] :as callbacks}] (cond ;; If a and b are equal, there is no diff (= a b) @@ -38,18 +41,18 @@ #{:text-content-structure} (into acc (apply set/union - (map #(compare-text-content %1 %2) v1 v2)))) + (map #(compare-text-content %1 %2 callbacks) v1 v2)))) ;; If the key is :text, and they are different, it is a text differece (= k :text) (if (not= v1 v2) - (conj acc :text-content-text) + (text-cb acc) acc) :else ;; If the key is not :text, and they are different, it is an attribute differece (if (not= v1 v2) - (conj acc :text-content-attribute) + (attribute-cb acc k) acc)))) #{} keys)) @@ -57,7 +60,6 @@ :else #{:text-content-structure})) - (defn equal-attrs? "Given a text structure, and a map of attrs, check that all the internal attrs in paragraphs and sentences have the same attrs" @@ -79,10 +81,15 @@ (defn get-diff-type "Given two content text structures, conformed by maps and vectors, compare them, and returns a set with the type of differences. - The possibilities are :text-content-text :text-content-attribute, - :text-content-structure and :text-content-structure-same-attrs." + The possibilities are + :text-content-text + :text-content-attribute, + :text-content-structure + :text-content-structure-same-attrs." [a b] - (let [diff-type (compare-text-content a b)] + (let [diff-type (compare-text-content a b + {:text-cb (fn [acc] (conj acc :text-content-text)) + :attribute-cb (fn [acc _] (conj acc :text-content-attribute))})] (if-not (contains? diff-type :text-content-structure) diff-type (let [;; get attrs of the first paragraph of the first paragraph-set @@ -92,6 +99,24 @@ #{:text-content-structure :text-content-structure-same-attrs} diff-type))))) +(defn get-diff-attrs + "Given two content text structures, conformed by maps and vectors, + compare them, and returns a set with the attributes that have changed. + This is independent of the text structure, so if the structure changes + but the attributes are the same, it will return an empty set." + [a b] + (let [diff-attrs (compare-text-content a b + {:text-cb identity + :attribute-cb (fn [acc attr] (conj acc attr))})] + (if-not (contains? diff-attrs :text-content-structure) + diff-attrs + (let [;; get attrs of the first paragraph of the first paragraph-set + attrs (get-first-paragraph-text-attrs a)] + (if (and (equal-attrs? a attrs) + (equal-attrs? b attrs)) + #{} + (disj diff-attrs :text-content-structure)))))) + ;; TODO We know that there are cases that the blocks of texts are separated ;; differently: ["one" " " "two"], ["one " "two"], ["one" " two"] ;; so this won't work for 100% of the situations. But it's good enough for now, @@ -116,7 +141,6 @@ :else true)) - (defn copy-text-keys "Given two equal content text structures, deep copy all the keys :text from origin to destiny" diff --git a/common/test/common_tests/logic/token_apply_test.cljc b/common/test/common_tests/logic/token_apply_test.cljc index be7be1e5f6..8b028e5ebf 100644 --- a/common/test/common_tests/logic/token_apply_test.cljc +++ b/common/test/common_tests/logic/token_apply_test.cljc @@ -6,6 +6,7 @@ (ns common-tests.logic.token-apply-test (:require + [app.common.data :as d] [app.common.files.changes-builder :as pcb] [app.common.logic.shapes :as cls] [app.common.test-helpers.compositions :as tho] @@ -13,6 +14,7 @@ [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] [app.common.test-helpers.tokens :as tht] + [app.common.text :as txt] [app.common.types.container :as ctn] [app.common.types.token :as cto] [app.common.types.tokens-lib :as ctob] @@ -53,7 +55,8 @@ (ctob/make-token :name "token-dimensions" :type :dimensions :value 100)))) - (tho/add-frame :frame1))) + (tho/add-frame :frame1) + (tho/add-text :text1 "Hello World"))) (defn- apply-all-tokens [file] @@ -64,7 +67,8 @@ (tht/apply-token-to-shape :frame1 "token-stroke-width" [:stroke-width] [:stroke-width] 2) (tht/apply-token-to-shape :frame1 "token-color" [:stroke-color] [:stroke-color] "#00ff00") (tht/apply-token-to-shape :frame1 "token-color" [:fill] [:fill] "#00ff00") - (tht/apply-token-to-shape :frame1 "token-dimensions" [:width :height] [:width :height] 100))) + (tht/apply-token-to-shape :frame1 "token-dimensions" [:width :height] [:width :height] 100) + (tht/apply-token-to-shape :text1 "token-color" [:fill] [:fill] "#00ff00"))) (t/deftest apply-tokens-to-shape (let [;; ==== Setup @@ -171,6 +175,7 @@ (apply-all-tokens)) page (thf/current-page file) frame1 (ths/get-shape file :frame1) + text1 (ths/get-shape file :text1) ;; ==== Action changes (-> (-> (pcb/empty-changes nil) @@ -190,13 +195,25 @@ (ctn/set-shape-attr :width 0) (ctn/set-shape-attr :height 0))) (:objects page) + {}) + (cls/generate-update-shapes [(:id text1)] + (fn [shape] + (txt/update-text-content + shape + txt/is-content-node? + d/txt-merge + {:fills (ths/sample-fills-color :fill-color "#fabada")})) + (:objects page) {})) file' (thf/apply-changes file changes) ;; ==== Get - frame1' (ths/get-shape file' :frame1) - applied-tokens' (:applied-tokens frame1')] + frame1' (ths/get-shape file' :frame1) + text1' (ths/get-shape file' :text1) + applied-tokens-frame' (:applied-tokens frame1') + applied-tokens-text' (:applied-tokens text1')] ;; ==== Check - (t/is (= (count applied-tokens') 0)))) \ No newline at end of file + (t/is (= (count applied-tokens-frame') 0)) + (t/is (= (count applied-tokens-text') 0)))) \ No newline at end of file diff --git a/common/test/common_tests/types/text_test.cljc b/common/test/common_tests/types/text_test.cljc index b72f1387c3..df1cf4c680 100644 --- a/common/test/common_tests/types/text_test.cljc +++ b/common/test/common_tests/types/text_test.cljc @@ -29,11 +29,12 @@ #(conj % line))) (t/deftest test-get-diff-type - (let [diff-text (cttx/get-diff-type content-base content-changed-text) - diff-attr (cttx/get-diff-type content-base content-changed-attr) - diff-both (cttx/get-diff-type content-base content-changed-both) - diff-structure (cttx/get-diff-type content-base content-changed-structure) + (let [diff-text (cttx/get-diff-type content-base content-changed-text) + diff-attr (cttx/get-diff-type content-base content-changed-attr) + diff-both (cttx/get-diff-type content-base content-changed-both) + diff-structure (cttx/get-diff-type content-base content-changed-structure) diff-structure-same-attrs (cttx/get-diff-type content-base content-changed-structure-same-attrs)] + (t/is (= #{:text-content-text} diff-text)) (t/is (= #{:text-content-attribute} diff-attr)) (t/is (= #{:text-content-text :text-content-attribute} diff-both)) @@ -41,6 +42,20 @@ (t/is (= #{:text-content-structure :text-content-structure-same-attrs} diff-structure-same-attrs)))) +(t/deftest test-get-diff-attrs + (let [attrs-text (cttx/get-diff-attrs content-base content-changed-text) + attrs-attr (cttx/get-diff-attrs content-base content-changed-attr) + attrs-both (cttx/get-diff-attrs content-base content-changed-both) + attrs-structure (cttx/get-diff-attrs content-base content-changed-structure) + attrs-structure-same-attrs (cttx/get-diff-attrs content-base content-changed-structure-same-attrs)] + + (t/is (= #{} attrs-text)) + (t/is (= #{:font-size} attrs-attr)) + (t/is (= #{:font-size} attrs-both)) + (t/is (= #{} attrs-structure)) + (t/is (= #{} attrs-structure-same-attrs)))) + + (t/deftest test-equal-structure (t/is (true? (cttx/equal-structure? content-base content-changed-text))) (t/is (true? (cttx/equal-structure? content-base content-changed-attr)))