From 8b485b4a8b7bebc8566bea92c974a35eb7867889 Mon Sep 17 00:00:00 2001 From: AK <144495202+AKnassa@users.noreply.github.com> Date: Fri, 24 Jul 2026 07:08:01 -0400 Subject: [PATCH] :bug: 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 Co-authored-by: Akshit Nassa Co-authored-by: Andrey Antukh --- common/src/app/common/types/shape.cljc | 63 +++- common/src/app/common/types/token.cljc | 2 + frontend/test/frontend_tests/runner.cljs | 2 + .../tokens/copy_paste_props_test.cljs | 278 ++++++++++++++++++ 4 files changed, 341 insertions(+), 4 deletions(-) create mode 100644 frontend/test/frontend_tests/tokens/copy_paste_props_test.cljs diff --git a/common/src/app/common/types/shape.cljc b/common/src/app/common/types/shape.cljc index cec7844fab..d645e72ae1 100644 --- a/common/src/app/common/types/shape.cljc +++ b/common/src/app/common/types/shape.cljc @@ -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)))) diff --git a/common/src/app/common/types/token.cljc b/common/src/app/common/types/token.cljc index 619bc5e2b8..10cedd5c19 100644 --- a/common/src/app/common/types/token.cljc +++ b/common/src/app/common/types/token.cljc @@ -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] diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 8882abbfa7..ee116cf11f 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -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 diff --git a/frontend/test/frontend_tests/tokens/copy_paste_props_test.cljs b/frontend/test/frontend_tests/tokens/copy_paste_props_test.cljs new file mode 100644 index 0000000000..47435af432 --- /dev/null +++ b/frontend/test/frontend_tests/tokens/copy_paste_props_test.cljs @@ -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]))))))