🐛 Fix token unapply for text shapes

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
 

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

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

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:

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)

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.
+   compare them, and returns a set with the differences info.
-   The possibilities are :text-content-text :text-content-attribute and :text-content-structure."
+   If the structures are equal, it returns an empty set. If the structure
-  [a b]
+   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,
+   The possibilities are
-   :text-content-structure and :text-content-structure-same-attrs."
+     :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"

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)
+        frame1'               (ths/get-shape file' :frame1)
-        applied-tokens' (:applied-tokens 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))))
+    (t/is (= (count applied-tokens-frame') 0))
+    (t/is (= (count applied-tokens-text') 0))))

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)
+  (let [diff-text                 (cttx/get-diff-type content-base content-changed-text)
-        diff-attr (cttx/get-diff-type content-base content-changed-attr)
+        diff-attr                 (cttx/get-diff-type content-base content-changed-attr)
-        diff-both (cttx/get-diff-type content-base content-changed-both)
+        diff-both                 (cttx/get-diff-type content-base content-changed-both)
-        diff-structure (cttx/get-diff-type content-base content-changed-structure)
+        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)))