mirror of
https://github.com/penpot/penpot.git
synced 2026-07-24 23:18:06 +00:00
🐛 Preserve token references when copying and pasting properties (#10665)
Copy/paste of properties resolved tokens to their values, dropping the reference. Carry the token with the value it resolves, at sub-attribute granularity for map-valued attrs. Fixes #9582 Signed-off-by: Akshit Nassa <akshitnassa412@gmail.com> Co-authored-by: Akshit Nassa <akshitnassa412@gmail.com> Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
AK
GitHub
parent
3610b81e4b
commit
8b485b4a8b
@ -698,6 +698,31 @@
|
||||
:r3
|
||||
:r4})
|
||||
|
||||
(def ^:private text-extract-props
|
||||
(into #{} cat [txt/root-attrs txt/paragraph-attrs txt/text-node-attrs]))
|
||||
|
||||
(def ^:private layout-extract-props
|
||||
(set ctsl/layout-attrs))
|
||||
|
||||
;; Token attrs are not shape attrs (:fill token vs :fills attr, :m1..:m4 vs
|
||||
;; :layout-item-margin). A token may only travel with the value it resolves to,
|
||||
;; so its domain is derived from the props that are actually written. The attrs
|
||||
;; holding a map of edges are patched edge by edge, so only the edges present
|
||||
;; in the map are written.
|
||||
(defn- token-attrs
|
||||
[props]
|
||||
(reduce-kv (fn [result attr value]
|
||||
(let [sub-attrs (when (map? value)
|
||||
(not-empty (set (keys value))))]
|
||||
(into result
|
||||
(if (= :layout-gap attr)
|
||||
(if (some? sub-attrs)
|
||||
(filter cto/spacing-gap-keys sub-attrs)
|
||||
cto/spacing-gap-keys)
|
||||
(cto/shape-attr->token-attrs attr sub-attrs)))))
|
||||
#{}
|
||||
props))
|
||||
|
||||
(defn extract-props
|
||||
"Retrieves an object with the 'pasteable' properties for a shape."
|
||||
[shape]
|
||||
@ -729,7 +754,15 @@
|
||||
props)))
|
||||
|
||||
(extract-layout-attrs [props shape]
|
||||
(d/patch-object props (select-keys shape ctsl/layout-attrs)))]
|
||||
(d/patch-object props (select-keys shape ctsl/layout-attrs)))
|
||||
|
||||
(extract-token-props [props shape]
|
||||
(let [tokens (-> (:applied-tokens shape)
|
||||
(select-keys (token-attrs props))
|
||||
(not-empty))]
|
||||
(cond-> props
|
||||
(some? tokens)
|
||||
(assoc :applied-tokens tokens))))]
|
||||
|
||||
(let [;; For texts we don't extract the fill
|
||||
extract-props
|
||||
@ -737,7 +770,8 @@
|
||||
(-> shape
|
||||
(select-keys extract-props)
|
||||
(cond-> (cfh/text-shape? shape) (extract-text-props shape))
|
||||
(cond-> (ctsl/any-layout? shape) (extract-layout-attrs shape))))))
|
||||
(cond-> (ctsl/any-layout? shape) (extract-layout-attrs shape))
|
||||
(extract-token-props shape)))))
|
||||
|
||||
(defn patch-props
|
||||
"Given the object of `extract-props` applies it to a shape. Adapt the shape if necessary"
|
||||
@ -765,12 +799,33 @@
|
||||
(let [shape (d/patch-object shape (select-keys props ctsl/layout-attrs))]
|
||||
(cond-> shape
|
||||
(ctsl/grid-layout? shape)
|
||||
(ctsl/assign-cells objects))))]
|
||||
(ctsl/assign-cells objects))))
|
||||
|
||||
(patched-props [shape props]
|
||||
(let [text? (cfh/text-shape? shape)
|
||||
frame? (cfh/frame-shape? shape)]
|
||||
(select-keys props
|
||||
(filter (fn [attr]
|
||||
(or (contains? basic-extract-props attr)
|
||||
(and text? (contains? text-extract-props attr))
|
||||
(and frame? (contains? layout-extract-props attr))))
|
||||
(keys props)))))
|
||||
|
||||
(patch-token-props [shape props]
|
||||
(let [attrs (token-attrs (patched-props shape props))
|
||||
tokens (-> (:applied-tokens shape)
|
||||
(d/without-keys attrs)
|
||||
(merge (select-keys (:applied-tokens props) attrs))
|
||||
(not-empty))]
|
||||
(if (some? tokens)
|
||||
(assoc shape :applied-tokens tokens)
|
||||
(dissoc shape :applied-tokens))))]
|
||||
|
||||
(-> shape
|
||||
(d/patch-object (select-keys props basic-extract-props))
|
||||
(cond-> (cfh/text-shape? shape) (patch-text-props props))
|
||||
(cond-> (cfh/frame-shape? shape) (patch-layout-props props)))))
|
||||
(cond-> (cfh/frame-shape? shape) (patch-layout-props props))
|
||||
(patch-token-props props))))
|
||||
|
||||
|
||||
|
||||
|
||||
@ -235,6 +235,8 @@
|
||||
[:row-gap {:optional true} schema:token-name]
|
||||
[:column-gap {:optional true} schema:token-name]])
|
||||
|
||||
(def spacing-gap-keys (schema-keys schema:spacing-gap))
|
||||
|
||||
(def ^:private schema:spacing-padding
|
||||
[:map {:title "SpacingPaddingTokenAttrs"}
|
||||
[:p1 {:optional true} schema:token-name]
|
||||
|
||||
@ -48,6 +48,7 @@
|
||||
[frontend-tests.plugins.value-objects-test]
|
||||
[frontend-tests.render-wasm.process-objects-test]
|
||||
[frontend-tests.svg-fills-test]
|
||||
[frontend-tests.tokens.copy-paste-props-test]
|
||||
[frontend-tests.tokens.import-export-test]
|
||||
[frontend-tests.tokens.logic.token-actions-test]
|
||||
[frontend-tests.tokens.logic.token-data-test]
|
||||
@ -127,6 +128,7 @@
|
||||
'frontend-tests.plugins.value-objects-test
|
||||
'frontend-tests.render-wasm.process-objects-test
|
||||
'frontend-tests.svg-fills-test
|
||||
'frontend-tests.tokens.copy-paste-props-test
|
||||
'frontend-tests.tokens.import-export-test
|
||||
'frontend-tests.tokens.logic.token-actions-test
|
||||
'frontend-tests.tokens.logic.token-data-test
|
||||
|
||||
278
frontend/test/frontend_tests/tokens/copy_paste_props_test.cljs
Normal file
278
frontend/test/frontend_tests/tokens/copy_paste_props_test.cljs
Normal file
@ -0,0 +1,278 @@
|
||||
;; This Source Code Form is subject to the terms of the Mozilla Public
|
||||
;; License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
||||
;;
|
||||
;; Copyright (c) KALEIDOS INC Sucursal en España SL
|
||||
|
||||
(ns frontend-tests.tokens.copy-paste-props-test
|
||||
(:require
|
||||
[app.common.types.shape :as cts]
|
||||
[cljs.test :as t :include-macros true]))
|
||||
|
||||
(defn- rect
|
||||
[attrs]
|
||||
(merge {:type :rect
|
||||
:fills [{:fill-color "#111111" :fill-opacity 1}]
|
||||
:r1 4 :r2 4 :r3 4 :r4 4
|
||||
:opacity 1}
|
||||
attrs))
|
||||
|
||||
(defn- content
|
||||
[node]
|
||||
{:type "root"
|
||||
:children [{:type "paragraph-set"
|
||||
:children [{:type "paragraph"
|
||||
:children [node]}]}]})
|
||||
|
||||
(defn- text
|
||||
[attrs]
|
||||
(merge {:type :text
|
||||
:opacity 1
|
||||
:content (content {:text "hello"
|
||||
:font-size "14"
|
||||
:fills [{:fill-color "#111111" :fill-opacity 1}]})}
|
||||
attrs))
|
||||
|
||||
(defn- paste-props
|
||||
[source target]
|
||||
(cts/patch-props target (cts/extract-props source) {}))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; Token references must survive a copy/paste of properties
|
||||
;; ---------------------------------------------------------------------------
|
||||
|
||||
(t/deftest copy-props-keeps-token-references
|
||||
(let [source (rect {:fills [{:fill-color "#ff0000" :fill-opacity 1}]
|
||||
:r1 12
|
||||
:applied-tokens {:fill "brand.primary" :r1 "radius.sm"}})
|
||||
target (rect {})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the fill token reference is pasted, not only its resolved value"
|
||||
(t/is (= "brand.primary" (get-in result [:applied-tokens :fill]))))
|
||||
|
||||
(t/testing "the border radius token reference is pasted"
|
||||
(t/is (= "radius.sm" (get-in result [:applied-tokens :r1]))))
|
||||
|
||||
(t/testing "the resolved values are still pasted"
|
||||
(t/is (= [{:fill-color "#ff0000" :fill-opacity 1}] (:fills result)))
|
||||
(t/is (= 12 (:r1 result))))))
|
||||
|
||||
(t/deftest paste-props-removes-stale-target-tokens
|
||||
(let [source (rect {:fills [{:fill-color "#ff0000" :fill-opacity 1}]})
|
||||
target (rect {:applied-tokens {:fill "brand.secondary"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "a token-less source clears the token of the overwritten property"
|
||||
(t/is (nil? (get-in result [:applied-tokens :fill]))))))
|
||||
|
||||
(t/deftest paste-props-keeps-target-tokens-out-of-the-copied-domain
|
||||
(let [source (rect {:applied-tokens {:fill "brand.primary"}})
|
||||
target (rect {:applied-tokens {:width "size.lg"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "tokens of properties that are not copied are left untouched"
|
||||
(t/is (= "size.lg" (get-in result [:applied-tokens :width]))))
|
||||
|
||||
(t/testing "tokens of copied properties are replaced"
|
||||
(t/is (= "brand.primary" (get-in result [:applied-tokens :fill]))))))
|
||||
|
||||
(t/deftest copy-props-ignores-tokens-of-non-copied-properties
|
||||
(let [source (rect {:applied-tokens {:width "size.lg" :height "size.lg"}})
|
||||
props (cts/extract-props source)
|
||||
result (cts/patch-props (rect {}) props {})]
|
||||
|
||||
(t/testing "sizing tokens are not copied because sizing is not a copied property"
|
||||
(t/is (nil? (get-in result [:applied-tokens :width])))
|
||||
(t/is (nil? (get-in result [:applied-tokens :height]))))))
|
||||
|
||||
(t/deftest copy-props-without-tokens-adds-no-empty-payload
|
||||
(let [source (rect {})
|
||||
props (cts/extract-props source)
|
||||
result (cts/patch-props (rect {}) props {})]
|
||||
|
||||
(t/testing "no empty :applied-tokens is introduced in the copied props"
|
||||
(t/is (not (contains? props :applied-tokens))))
|
||||
|
||||
(t/testing "no empty :applied-tokens is introduced in the pasted shape"
|
||||
(t/is (not (contains? result :applied-tokens))))))
|
||||
|
||||
(t/deftest copy-props-keeps-layout-tokens-between-layout-frames
|
||||
(let [source (rect {:type :frame
|
||||
:layout :flex
|
||||
:layout-padding {:p1 8 :p2 8 :p3 8 :p4 8}
|
||||
:layout-gap {:row-gap 4 :column-gap 4}
|
||||
:applied-tokens {:p1 "spacing.md" :row-gap "spacing.sm"}})
|
||||
target (rect {:type :frame :layout :flex})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "padding and gap tokens travel with the layout properties"
|
||||
(t/is (= "spacing.md" (get-in result [:applied-tokens :p1])))
|
||||
(t/is (= "spacing.sm" (get-in result [:applied-tokens :row-gap])))
|
||||
(t/is (= {:p1 8 :p2 8 :p3 8 :p4 8} (:layout-padding result))))))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; A token may only leave the target when the value it resolves is overwritten
|
||||
;; ---------------------------------------------------------------------------
|
||||
|
||||
(t/deftest paste-props-keeps-tokens-of-margin-edges-the-source-does-not-set
|
||||
(let [source (rect {:layout-item-margin {:m1 8 :m3 8}})
|
||||
target (rect {:layout-item-margin {:m4 16}
|
||||
:applied-tokens {:m4 "spacing.md"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the edges the source does not set keep their value"
|
||||
(t/is (= {:m1 8 :m3 8 :m4 16} (:layout-item-margin result))))
|
||||
|
||||
(t/testing "and so do the tokens that resolve them"
|
||||
(t/is (= "spacing.md" (get-in result [:applied-tokens :m4]))))))
|
||||
|
||||
(t/deftest copy-props-ignores-tokens-of-margin-edges-the-source-does-not-set
|
||||
(let [source (rect {:layout-item-margin {:m1 8}
|
||||
:applied-tokens {:m1 "spacing.sm" :m2 "spacing.lg"}})
|
||||
target (rect {:layout-item-margin {:m2 2}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the edge the source does not set keeps its value"
|
||||
(t/is (= {:m1 8 :m2 2} (:layout-item-margin result))))
|
||||
|
||||
(t/testing "so its token does not travel either"
|
||||
(t/is (nil? (get-in result [:applied-tokens :m2]))))
|
||||
|
||||
(t/testing "the token of the edge the source does set travels"
|
||||
(t/is (= "spacing.sm" (get-in result [:applied-tokens :m1]))))))
|
||||
|
||||
(t/deftest paste-props-keeps-gap-tokens-the-source-does-not-set
|
||||
(let [source (rect {:type :frame
|
||||
:layout :flex
|
||||
:layout-gap {:column-gap 4}})
|
||||
target (rect {:type :frame
|
||||
:layout :flex
|
||||
:layout-gap {:row-gap 2 :column-gap 2}
|
||||
:applied-tokens {:row-gap "gap.row" :column-gap "gap.column"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the gap axis the source does not set keeps its value"
|
||||
(t/is (= {:row-gap 2 :column-gap 4} (:layout-gap result))))
|
||||
|
||||
(t/testing "and so does the token that resolves it"
|
||||
(t/is (= "gap.row" (get-in result [:applied-tokens :row-gap]))))
|
||||
|
||||
(t/testing "the overwritten axis loses its stale token"
|
||||
(t/is (nil? (get-in result [:applied-tokens :column-gap]))))))
|
||||
|
||||
(t/deftest paste-props-keeps-padding-tokens-the-source-does-not-set
|
||||
(let [source (rect {:type :frame
|
||||
:layout :flex
|
||||
:layout-padding {:p1 8}})
|
||||
target (rect {:type :frame
|
||||
:layout :flex
|
||||
:layout-padding {:p1 1 :p3 3}
|
||||
:applied-tokens {:p1 "padding.top" :p3 "padding.bottom"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the padding edge the source does not set keeps its value"
|
||||
(t/is (= {:p1 8 :p3 3} (:layout-padding result))))
|
||||
|
||||
(t/testing "and so does the token that resolves it"
|
||||
(t/is (= "padding.bottom" (get-in result [:applied-tokens :p3]))))
|
||||
|
||||
(t/testing "the overwritten edge loses its stale token"
|
||||
(t/is (nil? (get-in result [:applied-tokens :p1]))))))
|
||||
|
||||
(t/deftest paste-props-keeps-tokens-of-properties-the-source-does-not-carry
|
||||
(let [source {:type :circle
|
||||
:fills [{:fill-color "#ff0000" :fill-opacity 1}]
|
||||
:opacity 1}
|
||||
target (rect {:shadow [{:color {:color "#000000"}}]
|
||||
:layout-item-margin {:m1 5 :m2 5 :m3 5 :m4 5}
|
||||
:applied-tokens {:r1 "radius.sm"
|
||||
:m1 "spacing.xs"
|
||||
:shadow "shadow.md"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the values the source does not carry are untouched"
|
||||
(t/is (= 4 (:r1 result)))
|
||||
(t/is (= {:m1 5 :m2 5 :m3 5 :m4 5} (:layout-item-margin result))))
|
||||
|
||||
(t/testing "and so are the tokens that resolve them"
|
||||
(t/is (= "radius.sm" (get-in result [:applied-tokens :r1])))
|
||||
(t/is (= "spacing.xs" (get-in result [:applied-tokens :m1])))
|
||||
(t/is (= "shadow.md" (get-in result [:applied-tokens :shadow]))))))
|
||||
|
||||
(t/deftest paste-props-keeps-layout-tokens-of-a-frame-when-the-source-has-no-layout
|
||||
(let [source (rect {:applied-tokens {:fill "brand.primary"}})
|
||||
target (rect {:type :frame
|
||||
:layout :flex
|
||||
:layout-padding {:p1 8 :p2 8 :p3 8 :p4 8}
|
||||
:layout-gap {:row-gap 4 :column-gap 4}
|
||||
:applied-tokens {:p1 "spacing.md" :row-gap "spacing.sm"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the layout values are untouched because the source has none"
|
||||
(t/is (= {:p1 8 :p2 8 :p3 8 :p4 8} (:layout-padding result)))
|
||||
(t/is (= {:row-gap 4 :column-gap 4} (:layout-gap result))))
|
||||
|
||||
(t/testing "and so are the padding and gap tokens"
|
||||
(t/is (= "spacing.md" (get-in result [:applied-tokens :p1])))
|
||||
(t/is (= "spacing.sm" (get-in result [:applied-tokens :row-gap]))))
|
||||
|
||||
(t/testing "the copied fill token is still applied"
|
||||
(t/is (= "brand.primary" (get-in result [:applied-tokens :fill]))))))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; Text shapes carry their fill inside the content nodes
|
||||
;; ---------------------------------------------------------------------------
|
||||
|
||||
(t/deftest copy-props-of-a-text-shape-carries-the-fill-token
|
||||
(let [source (text {:content (content {:text "hello"
|
||||
:font-size "14"
|
||||
:fills [{:fill-color "#ff0000" :fill-opacity 1}]})
|
||||
:applied-tokens {:fill "brand.primary"
|
||||
:font-size "font.lg"}})
|
||||
props (cts/extract-props source)
|
||||
result (cts/patch-props (rect {:applied-tokens {:fill "brand.old"}}) props {})]
|
||||
|
||||
(t/testing "the fill of the text content is copied"
|
||||
(t/is (= [{:fill-color "#ff0000" :fill-opacity 1}] (:fills props))))
|
||||
|
||||
(t/testing "so is the fill token that resolves it"
|
||||
(t/is (= "brand.primary" (get-in result [:applied-tokens :fill]))))
|
||||
|
||||
(t/testing "typography tokens are not applied to a shape without text content"
|
||||
(t/is (nil? (get-in result [:applied-tokens :font-size]))))))
|
||||
|
||||
(t/deftest paste-props-on-a-text-target-replaces-the-fill-token
|
||||
(let [source (rect {:fills [{:fill-color "#ff0000" :fill-opacity 1}]
|
||||
:applied-tokens {:fill "brand.primary"}})
|
||||
target (text {:applied-tokens {:fill "brand.old"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the fill of the text content is overwritten"
|
||||
(t/is (= [{:fill-color "#ff0000" :fill-opacity 1}]
|
||||
(get-in result [:content :children 0 :children 0 :children 0 :fills]))))
|
||||
|
||||
(t/testing "so the stale fill token cannot survive"
|
||||
(t/is (= "brand.primary" (get-in result [:applied-tokens :fill]))))))
|
||||
|
||||
(t/deftest paste-props-keeps-typography-tokens-of-a-text-target-when-the-source-is-not-text
|
||||
(let [source (rect {:applied-tokens {:fill "brand.primary"}})
|
||||
target (text {:applied-tokens {:font-size "font.lg" :fill "brand.old"}})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "the font size of the text content is untouched"
|
||||
(t/is (= "14" (get-in result [:content :children 0 :children 0 :children 0 :font-size]))))
|
||||
|
||||
(t/testing "and so is the typography token that resolves it"
|
||||
(t/is (= "font.lg" (get-in result [:applied-tokens :font-size]))))))
|
||||
|
||||
(t/deftest copy-props-of-a-text-shape-does-not-apply-typography-to-a-non-text-target
|
||||
(let [source (text {:applied-tokens {:font-size "font.lg" :opacity "opacity.50"}})
|
||||
target (rect {})
|
||||
result (paste-props source target)]
|
||||
|
||||
(t/testing "typography tokens of a text source are not applied to a non text target"
|
||||
(t/is (nil? (get-in result [:applied-tokens :font-size]))))
|
||||
|
||||
(t/testing "tokens shared by both shapes are applied"
|
||||
(t/is (= "opacity.50" (get-in result [:applied-tokens :opacity]))))))
|
||||
Loading…
x
Reference in New Issue
Block a user