From 3df3e22ae59d1db72d836cd8ec79cbee4d23da24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 14 Apr 2026 14:38:25 +0200 Subject: [PATCH] :bug: Detect duplicated token names in the whole library --- common/src/app/common/files/tokens.cljc | 36 +-- common/src/app/common/time.cljc | 18 ++ common/src/app/common/types/tokens_lib.cljc | 188 ++++++++++---- .../common_tests/types/tokens_lib_test.cljc | 235 +++++++++++++++++- .../types/tokens_migrations_test.cljc | 214 ++++++++++++++++ .../playwright/ui/specs/tokens/crud.spec.js | 61 +++-- .../main/data/workspace/tokens/errors.cljs | 5 + .../data/workspace/tokens/import_export.cljs | 9 +- .../main/ui/workspace/tokens/management.cljs | 3 +- .../tokens/management/forms/color.cljs | 9 +- .../tokens/management/forms/font_family.cljs | 9 +- .../management/forms/form_container.cljs | 15 -- .../tokens/management/forms/generic_form.cljs | 13 +- .../management/forms/rename_node_modal.cljs | 16 +- .../tokens/management/forms/shadow.cljs | 13 +- .../tokens/management/forms/typography.cljs | 12 +- .../ui/workspace/tokens/management/group.cljs | 5 +- frontend/src/app/plugins/tokens.cljs | 28 +-- frontend/translations/en.po | 8 + frontend/translations/es.po | 8 + 20 files changed, 760 insertions(+), 145 deletions(-) create mode 100644 common/test/common_tests/types/tokens_migrations_test.cljc diff --git a/common/src/app/common/files/tokens.cljc b/common/src/app/common/files/tokens.cljc index cd6dc44cb2..03c9b21297 100644 --- a/common/src/app/common/files/tokens.cljc +++ b/common/src/app/common/files/tokens.cljc @@ -134,26 +134,26 @@ (defn make-token-name-schema "Dynamically generates a schema to check a token name, adding translated error messages - and two additional validations: + and additional validations: - Min and max length. - - Checks if other token with a path derived from the name already exists at `tokens-tree`. - e.g. it's not allowed to create a token `foo.bar` if a token `foo` already exists." - [tokens-tree] + - Checks if other token with a path derived from the name already exists in the library. + e.g. it's not allowed to create a token `foo.bar` if a token `foo` already exists. + - Also checks if there is a token with the exact same name in the current set, but different + from the current token." + [tokens-lib set-id token-id] [:and [:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}] (-> cto/schema:token-name (sm/update-properties assoc :error/fn #(str (:value %) (tr "workspace.tokens.token-name-validation-error")))) [:fn {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))} - #(and (some? tokens-tree) - (not (ctob/token-name-path-exists? % tokens-tree)))]]) + #(or (nil? tokens-lib) + (not (ctob/token-name-path-exists? % tokens-lib set-id token-id)))]]) (defn make-node-token-name-schema - "Dynamically generates a schema to check a token node name, adding translated error messages - and two additional validations: - - Min and max length. - - Checks if other token with a path derived from the name already exists at `tokens-tree`. - e.g. it's not allowed to create a token `foo.bar` if a token `foo` already exists." - [active-tokens tokens-tree node] + "Dynamically generates a schema to check the name of a token node, that may be a final token or a group. + This runs same checks as make-token-name-schema, but for all tokens that will be renamed by this change, + if the group already contains tokens." + [active-tokens tokens-lib node set-id] [:and [:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}] (-> cto/schema:token-node-name @@ -164,20 +164,20 @@ current-name (:name node) new-tokens (ctob/update-tokens-group active-tokens current-path current-name name)] (and (some? new-tokens) - (some (fn [[token-name _]] - (not (ctob/token-name-path-exists? token-name tokens-tree))) + (some (fn [[token-name token]] + (not (ctob/token-name-path-exists? token-name tokens-lib set-id (ctob/get-id token)))) new-tokens))))]]) (def schema:token-description [:string {:max 2048 :error/fn #(tr "errors.field-max-length" 2048)}]) (defn make-token-schema - [tokens-tree token-type] + [tokens-lib token-type set-id token-id] [:and (sm/merge cto/schema:token-attrs [:map - [:name (make-token-name-schema tokens-tree)] + [:name (make-token-name-schema tokens-lib set-id token-id)] [:value (make-token-value-schema token-type)] [:description {:optional true} schema:token-description]]) [:fn {:error/field :value @@ -187,9 +187,9 @@ (not (cto/token-value-self-reference? name value))))]]) (defn make-node-token-schema - [active-tokens tokens-tree node] + [active-tokens tokens-lib node set-id] [:map - [:name (make-node-token-name-schema active-tokens tokens-tree node)]]) + [:name (make-node-token-name-schema active-tokens tokens-lib node set-id)]]) (defn convert-dtcg-token "Convert token attributes as they come from a decoded json, with DTCG types, to internal types. diff --git a/common/src/app/common/time.cljc b/common/src/app/common/time.cljc index d73225008c..9549695161 100644 --- a/common/src/app/common/time.cljc +++ b/common/src/app/common/time.cljc @@ -202,6 +202,24 @@ (zero? result) false :else false))) +(defn is-after-or-equal? + "Analgous to: da >= db" + [da db] + (let [result (compare da db)] + (cond + (neg? result) false + (zero? result) true + :else true))) + +(defn is-before-or-equal? + "Analgous to: da <= db" + [da db] + (let [result (compare da db)] + (cond + (neg? result) true + (zero? result) true + :else false))) + (defn inst? [o] #?(:clj (instance? Instant o) diff --git a/common/src/app/common/types/tokens_lib.cljc b/common/src/app/common/types/tokens_lib.cljc index a54a243bab..3a85391623 100644 --- a/common/src/app/common/types/tokens_lib.cljc +++ b/common/src/app/common/types/tokens_lib.cljc @@ -74,13 +74,19 @@ modified-at) (rename [this new-name] - (assoc this :name new-name)) + (assoc this + :name new-name + :modified-at (ct/now))) (reid [this new-id] - (assoc this :id new-id)) + (assoc this + :id new-id + :modified-at (ct/now))) (set-description [this new-description] - (assoc this :description new-description))) + (assoc this + :description new-description + :modified-at (ct/now)))) (defmethod pp/simple-dispatch Token [^Token obj] @@ -1477,49 +1483,63 @@ Will return a value that matches this schema: (rename copy-name) (reid (uuid/next)))))) -(defn- token-name->path-selector - "Splits token-name into map with `:path` and `:selector` using `token-name->path`. - - `:selector` is the last item of the names path - `:path` is everything leading up the the `:selector`." - [token-name] - (let [path-segments (get-token-path {:name token-name}) - last-idx (dec (count path-segments)) - [path [selector]] (split-at last-idx path-segments)] - {:path (seq path) - :selector selector})) - (defn token-name-path-exists? - "Traverses the path from `token-name` down a `tokens-tree` and checks if a token at that path exists. + "Check if a token name or fragment exists in any part of the library, to prevent creating + duplicated names that may clash when merging sets and resolving tokens. - It's not allowed to create a token inside a token. E.g.: - Creating a token with + Matches any combination of of names completely included inside group or token names. + For example: + - Matches the name \"foo.bar\" with an existing token named \"foo.bar.baz\" or \"foo\". + - Does not match the name \"foo.bar\" with an existing token named \"foo.baz\". - {:name \"foo.bar\"} + You can give a current set id, and it will check if there is a token with the exact same + name in this set (there may be tokens with same name in different sets for overriding + values, but not in the same set). You can also give a token id to ignore, to search for + a token that is a different one. - in the tokens tree: + If the function finds a match, it returns the part of the name that is duplicated; + if not, it returns null." + [token-name tokens-lib current-set-id token-id-to-ignore] + (letfn [(exists-in-set? + [set] + (let [tokens-tree (-> set (get-tokens-) (tokens-tree)) + token-name-path (get-token-path {:name token-name})] + (loop [path-segment token-name-path + subtree tokens-tree] + (if (empty? path-segment) + ;; All path segments found -> return full name + token-name + (let [node (get subtree (first path-segment))] + (cond + ;; Path segment doesn't exist + (nil? node) nil + ;; A token exists at this path + (token? node) + (if (and (some? token-id-to-ignore) + (= (get-id node) token-id-to-ignore)) + ;; This is the token to ignore + nil + (if (and (not= (get-id set) current-set-id) + (= (get-name node) token-name)) + ;; A token with the same name in a different set is allowed + nil + ;; If we are in the same set or the name of the token is a subpath of the + ;; current name: this is a conflict + ;; -> return the part of the name until this point + (str/join "." (take (- (count token-name-path) (count (rest path-segment))) + token-name-path)))) + ;; Continue traversing the tree + :else (recur (rest path-segment) node)))))))] - {\"foo\" {:name \"other\"}}" - [token-name tokens-tree] - (let [{:keys [path selector]} (token-name->path-selector token-name) - path-target (reduce - (fn [acc cur] - (let [target (get acc cur)] - (cond - ;; Path segment doesn't exist yet - (nil? target) (reduced false) - ;; A token exists at this path - (:name target) (reduced true) - ;; Continue traversing the true - :else target))) - tokens-tree - path)] - (cond - (boolean? path-target) path-target - (get path-target :name) true - :else (-> (get path-target selector) - (seq) - (boolean))))) + (if (or (nil? tokens-lib) (empty? (get-sets tokens-lib)) + (nil? token-name) (str/empty? token-name)) + nil + (do + (assert (or (nil? current-set-id) + (some? (get-set tokens-lib current-set-id))) + (str "Set '" current-set-id "' does not exist in the library")) + (assert (or (nil? token-id-to-ignore) (uuid? token-id-to-ignore))) + (some exists-in-set? (get-sets tokens-lib)))))) (defn update-tokens-group "Updates the active tokens path when renaming a group node. @@ -1530,7 +1550,9 @@ Will return a value that matches this schema: active-tokens: map of token-name to token-object for all active tokens in the set current-path: the path of the group being renamed, e.g. \"foo.bar\" current-name: the current name of the group being renamed, e.g. \"bar\" - new-name: the new name for the group being renamed, e.g. \"baz\"" + new-name: the new name for the group being renamed, e.g. \"baz\" + + Returns a sequence of [name token] for each renamed token." [active-tokens current-path current-name new-name] (let [path-prefix (str/replace current-path current-name "")] @@ -1879,7 +1901,11 @@ Will return a value that matches this schema: library (reduce (fn [library name] (if-let [tokens (get sets name)] - (add-set library (make-token-set :name name :tokens tokens)) + (do (doseq [token (vals tokens)] + (when (token-name-path-exists? (get-name token) library nil (get-id token)) + (throw (ex-info (get-name token) + {:error/code :error.import/duplicated-token-name})))) + (add-set library (make-token-set :name name :tokens tokens))) library)) library ordered-set-names) @@ -2136,6 +2162,27 @@ Will return a value that matches this schema: ;; MIGRATIONS HELPERS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(defn update-all-tokens + "Walk through all tokens in the library and apply the given function to them. + The function receives the library, the set and the token as arguments, + and should return the updated token." + [tokens-lib update-fn] + (let [update-one-set + (fn [lib set] + (reduce (fn [lib' token] + (update-token lib' + (get-id set) + (get-id token) + #(update-fn lib' + (get-set lib' (get-id set)) + %))) + lib + (vals (get-tokens lib (get-id set)))))] + (reduce (fn [lib set] + (update-one-set lib set)) + tokens-lib + (get-sets tokens-lib)))) + (defn fix-duplicate-token-set-ids "Given an instance of TokensLib fixes it internal sets data sturcture for ensure each set has unique id; @@ -2163,6 +2210,29 @@ Will return a value that matches this schema: (map->tokens-lib) (check))))) +(defn fix-conflicting-token-names + [tokens-lib] + (let [counter (atom 0) + match-suffixes (atom {}) + + generate-name + (fn [name match] + (let [matches (if (contains? @match-suffixes match) + @match-suffixes + (swap! match-suffixes assoc match (swap! counter inc))) + suffix (get matches match)] + (str (str/slice name 0 (count match)) + "-" suffix + (str/slice name (count match)))))] + + (update-all-tokens + tokens-lib + (fn [lib set token] + (let [name (get-name token)] + (if-let [match (token-name-path-exists? name lib (:id set) (get-id token))] + (rename token (generate-name name match)) + token)))))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; SERIALIZATION (FRESIAN) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -2200,7 +2270,7 @@ Will return a value that matches this schema: #?(:clj (defn- migrate-to-v1-3 "Migrate the TokensLib data structure internals to v1.3 version; it - expects input from v1.2 version" + expects input from v1.2 version" [{:keys [sets themes] :as params}] (let [migrate-token (fn [token] @@ -2248,7 +2318,7 @@ Will return a value that matches this schema: #?(:clj (defn- migrate-to-v1-4 "Migrate the TokensLib data structure internals to v1.4 version; it - expects input from v1.3 version" + expects input from v1.3 version" [params] (let [migrate-set-node (fn recurse [node] @@ -2279,7 +2349,8 @@ Will return a value that matches this schema: (migrate-to-v1-2) (migrate-to-v1-3) (migrate-to-v1-4) - (map->tokens-lib))))) + (map->tokens-lib) + (fix-conflicting-token-names))))) #?(:clj (defn- read-tokens-lib-v1-2 @@ -2294,7 +2365,8 @@ Will return a value that matches this schema: :active-themes active-themes} (migrate-to-v1-3) (migrate-to-v1-4) - (map->tokens-lib))))) + (map->tokens-lib) + (fix-conflicting-token-names))))) #?(:clj (defn- read-tokens-lib-v1-3 @@ -2309,7 +2381,22 @@ Will return a value that matches this schema: :themes themes :active-themes active-themes} (migrate-to-v1-4) - (map->tokens-lib))))) + (map->tokens-lib) + (fix-conflicting-token-names))))) + +#?(:clj + (defn- read-tokens-lib-v1-4 + "Reads the tokens lib data structure and fix conflicting token names." + [r] + (let [sets (fres/read-object! r) + themes (fres/read-object! r) + active-themes (fres/read-object! r)] + + (-> {:sets sets + :themes themes + :active-themes active-themes} + (map->tokens-lib) + (fix-conflicting-token-names))))) #?(:clj (defn- write-tokens-lib @@ -2370,8 +2457,11 @@ Will return a value that matches this schema: {:name "penpot/tokens-lib/v1.3" :rfn read-tokens-lib-v1-3} - ;; CURRENT TOKENS LIB READER & WRITTER {:name "penpot/tokens-lib/v1.4" + :rfn read-tokens-lib-v1-4} + + ;; CURRENT TOKENS LIB READER & WRITTER + {:name "penpot/tokens-lib/v1.5" :class TokensLib :wfn write-tokens-lib :rfn read-tokens-lib})) diff --git a/common/test/common_tests/types/tokens_lib_test.cljc b/common/test/common_tests/types/tokens_lib_test.cljc index de18be42de..abba5d9454 100644 --- a/common/test/common_tests/types/tokens_lib_test.cljc +++ b/common/test/common_tests/types/tokens_lib_test.cljc @@ -2027,12 +2027,235 @@ (t/is (= (:value imported-ref) (:value original-ref)))))))) (t/deftest token-name-path-exists?-test - (t/is (true? (ctob/token-name-path-exists? "border-radius" {"border-radius" {"sm" {:name "sm"}}}))) - (t/is (true? (ctob/token-name-path-exists? "border-radius" {"border-radius" {:name "sm"}}))) - (t/is (true? (ctob/token-name-path-exists? "border-radius.sm" {"border-radius" {:name "sm"}}))) - (t/is (true? (ctob/token-name-path-exists? "border-radius.sm.x" {"border-radius" {:name "sm"}}))) - (t/is (false? (ctob/token-name-path-exists? "other" {"border-radius" {:name "sm"}}))) - (t/is (false? (ctob/token-name-path-exists? "dark.border-radius.md" {"dark" {"border-radius" {"sm" {:name "sm"}}}})))) + (let [tokens-lib (ctob/make-tokens-lib) + add-set (fn [lib set-label set-name token-names] + (ctob/add-set lib (ctob/make-token-set + :id (thi/new-id! set-label) + :name set-name + :tokens (into {} + (map (fn [token-name] + [token-name (ctob/make-token + {:name token-name + :type :border-radius + :value "1"})])) + token-names))))] + + ;; Empty cases + + (t/testing "returns match for no library or empty library or empty name" + (t/is (not (ctob/token-name-path-exists? nil nil nil nil))) + (t/is (not (ctob/token-name-path-exists? nil tokens-lib nil nil))) + (t/is (not (ctob/token-name-path-exists? "" tokens-lib nil nil))) + (t/is (not (ctob/token-name-path-exists? "bad-name" tokens-lib nil nil))) + (t/is (not (ctob/token-name-path-exists? "bad-name" + (ctob/add-theme tokens-lib + (ctob/make-token-theme {:name "theme1"})) + nil + nil)))) + + (t/testing "throws error when giving a bad set id" + (t/is (thrown-with-msg? #?(:clj AssertionError :cljs js/Error) + #"Set '[0-9a-f-]+' does not exist in the library" + (ctob/token-name-path-exists? "some-name" + (-> tokens-lib + (add-set :empty-set "empty-set" [])) + (thi/new-id! :non-existent-set) nil)))) + + (t/testing "does not throw error when giving a nil set id" + (t/is (not (ctob/token-name-path-exists? "some-name" + (-> tokens-lib + (add-set :empty-set "empty-set" [])) + nil nil)))) + + (t/testing "returns not match for empty set" + (t/is (not (ctob/token-name-path-exists? "some-name" + (-> tokens-lib + (add-set :empty-set "empty-set" [])) + (thi/id :empty-set) nil)))) + + ;; Search in the current set + + (t/testing "returns match when name matches exactly a token in the set without groups" + (t/is (= "token1" + (ctob/token-name-path-exists? "token1" + (-> tokens-lib + (add-set :set1 "set1" ["token1" "token2" "token3"])) + (thi/id :set1) nil)))) + + (t/testing "returns match when name matches exactly a token in the set with groups" + (t/is (= "group1.subgroup1.token2" + (ctob/token-name-path-exists? "group1.subgroup1.token2" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"])) + (thi/id :set1) nil)))) + + (t/testing "returns match when name is a subpath of a token in the set" + (t/is (= "group1" + (ctob/token-name-path-exists? "group1" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"])) + (thi/id :set1) nil))) + (t/is (= "group1.subgroup1" + (ctob/token-name-path-exists? "group1.subgroup1" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"])) + (thi/id :set1) nil)))) + + (t/testing "returns match when one of the token names in the set is a subpath of the name" + (t/is (= "group2.subgroup2.token3" + (ctob/token-name-path-exists? "group2.subgroup2.token3.subtoken" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"])) + (thi/id :set1) nil)))) + + (t/testing "returns not match when name matches part of the path but not the full token name" + (t/is (not (ctob/token-name-path-exists? "group1.subgroup1.token4" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"])) + (thi/id :set1) nil)))) + + (t/testing "returns not match when name does not match any part of the token names in the set" + (t/is (not (ctob/token-name-path-exists? "token4" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"])) + (thi/id :set1) nil)))) + + ;; Search in other set + + (t/testing "returns not match when name matches exactly a token in other set without groups" + (t/is (not (ctob/token-name-path-exists? "token1" + (-> tokens-lib + (add-set :set1 "set1" ["token1" "token2" "token3"]) + (add-set :set2 "set2" [])) + (thi/id :set2) nil)))) + + (t/testing "returns not match when name matches exactly a token in other set with groups" + (t/is (not (ctob/token-name-path-exists? "group1.subgroup1.token2" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" [])) + (thi/id :set2) nil)))) + + (t/testing "returns match when name is a subpath of a token in other set" + (t/is (= "group1" + (ctob/token-name-path-exists? "group1" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" [])) + (thi/id :set2) nil))) + (t/is (= "group1.subgroup1" + (ctob/token-name-path-exists? "group1.subgroup1" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" [])) + (thi/id :set2) nil)))) + + (t/testing "returns match when one of the token names in other set is a subpath of the name" + (t/is (= "group2.subgroup2.token3" + (ctob/token-name-path-exists? "group2.subgroup2.token3.subtoken" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" [])) + (thi/id :set2) nil)))) + + (t/testing "returns not match when name matches part of the path but not the full token name" + (t/is (not (ctob/token-name-path-exists? "group1.subgroup1.token4" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" [])) + (thi/id :set2) nil)))) + + (t/testing "returns not match when name does not match any part of the token names in the set" + (t/is (not (ctob/token-name-path-exists? "token4" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" [])) + (thi/id :set2) nil)))) + + ;; Additional cases + + (t/testing "returns match when matches an exact token with several sets" + (t/is (= "group3.subgroup3.token4" + (ctob/token-name-path-exists? "group3.subgroup3.token4" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" ["group3.subgroup3.token4"])) + (thi/id :set2) nil)))) + + (t/testing "returns match when matches in one of the sets, even if the set is disabled" + (let [tokens-lib (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" ["group3.subgroup3.token4"])) + hidden-theme (ctob/get-hidden-theme tokens-lib) + tokens-lib (ctob/toggle-set-in-theme tokens-lib (:id hidden-theme) "set2")] + (t/is (= "group3.subgroup3.token4" + (ctob/token-name-path-exists? "group3.subgroup3.token4" + tokens-lib + (thi/id :set2) + nil))))) + + (t/testing "returns not match when does not match in any of the sets" + (t/is (not (ctob/token-name-path-exists? "group3.subgroup3.token5" + (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" ["group3.subgroup3.token4"])) + (thi/id :set1) + nil)))) + + (t/testing "returns not match when the token exists but is the one we have told it to ignore" + (let [tokens-lib (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" ["group3.subgroup3.token4"])) + token4 (ctob/get-token-by-name tokens-lib "set2" "group3.subgroup3.token4")] + (t/is (not (ctob/token-name-path-exists? "group3.subgroup3.token4" + tokens-lib + (thi/id :set2) + (:id token4)))))) + + (t/testing "returns match when we give an id to ignore but is not the token that matches" + (let [tokens-lib (-> tokens-lib + (add-set :set1 "set1" ["group1.subgroup1.token1" + "group1.subgroup1.token2" + "group2.subgroup2.token3"]) + (add-set :set2 "set2" ["group3.subgroup3.token4"])) + token4 (ctob/get-token-by-name tokens-lib "set2" "group3.subgroup3.token4")] + (t/is (= "group1.subgroup1.token1" + (ctob/token-name-path-exists? "group1.subgroup1.token1" + tokens-lib + (thi/id :set1) + (:id token4)))))))) #?(:clj (t/deftest token-set-encode-decode-roundtrip-with-invalid-set-name diff --git a/common/test/common_tests/types/tokens_migrations_test.cljc b/common/test/common_tests/types/tokens_migrations_test.cljc new file mode 100644 index 0000000000..e59a2ea026 --- /dev/null +++ b/common/test/common_tests/types/tokens_migrations_test.cljc @@ -0,0 +1,214 @@ +;; 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 + +(ns common-tests.types.tokens-migrations-test + #?(:clj + (:require + [app.common.data :as d] + [app.common.test-helpers.ids-map :as thi] + [app.common.time :as ct] + [app.common.types.tokens-lib :as ctob] + [clojure.datafy :refer [datafy]] + [clojure.test :as t]))) + +#?(:clj + (t/deftest test-v1-5-fix-token-names + + (t/testing "empty tokens-lib should not need any action" + (let [tokens-lib (ctob/make-tokens-lib) + tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)] + (t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib')))))) + + (t/testing "tokens with valid names should not need any action" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set1) + :name "set1" + :tokens {"name1" (ctob/make-token + {:id (thi/new-id! :token1) + :name "name1" + :type :border-radius + :value "1"})})) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set2) + :name "set2" + :tokens {"name1" (ctob/make-token ;; Same name in different + {:id (thi/new-id! :token2) ;; sets is ok + :name "name1" + :type :border-radius + :value "2"})}))) + + tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)] + + (t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib')))))) + + (t/testing "tokens with conflicting names should be renamed, and the rest of the library should be unchanged" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set1) + :name "set1" + :tokens {"name1" (ctob/make-token + {:id (thi/new-id! :token1) + :name "name1" + :type :border-radius + :value "1"})})) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set2) + :name "set2" + :tokens {"name1.name2" (ctob/make-token + {:id (thi/new-id! :token2) + :name "name1.name2" + :type :border-radius + :value "2"})}))) + + tokens-lib' (ctob/fix-conflicting-token-names tokens-lib) + + token-sets (ctob/get-set-tree tokens-lib) + set1 (ctob/get-set tokens-lib (thi/id :set1)) + set2 (ctob/get-set tokens-lib (thi/id :set2)) + tokens1 (ctob/get-tokens tokens-lib (thi/id :set1)) + tokens2 (ctob/get-tokens tokens-lib (thi/id :set2)) + token1 (ctob/get-token tokens-lib (thi/id :set1) (thi/id :token1)) + token2 (ctob/get-token tokens-lib (thi/id :set2) (thi/id :token2)) + + token-sets' (ctob/get-set-tree tokens-lib') + set1' (ctob/get-set tokens-lib' (thi/id :set1)) + set2' (ctob/get-set tokens-lib' (thi/id :set2)) + tokens1' (ctob/get-tokens tokens-lib' (thi/id :set1)) + tokens2' (ctob/get-tokens tokens-lib' (thi/id :set2)) + token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1)) + token2' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token2))] + + (t/is (= (count token-sets') (count token-sets))) + + (t/is (= (ctob/get-id set1') (ctob/get-id set1))) + (t/is (= (ctob/get-name set1') (ctob/get-name set1))) + (t/is (= (ctob/get-description set1') (ctob/get-description set1))) + (t/is (ct/is-after-or-equal? (ctob/get-modified-at set1') (ctob/get-modified-at set1))) ;; <-- MODIFIED + + (t/is (= (ctob/get-id set2') (ctob/get-id set2))) + (t/is (= (ctob/get-name set2') (ctob/get-name set2))) + (t/is (= (ctob/get-description set2') (ctob/get-description set2))) + (t/is (= (ctob/get-modified-at set2') (ctob/get-modified-at set2))) + + (t/is (= (count tokens1') (count tokens1))) + (t/is (= (count tokens2') (count tokens2))) + + (t/is (= (ctob/get-id token1') (ctob/get-id token1))) + (t/is (= (ctob/get-name token1') "name1-1")) ;; <-- RENAMED + (t/is (= (ctob/get-description token1') (ctob/get-description token1))) + (t/is (ct/is-after-or-equal? (ctob/get-modified-at set1') (ctob/get-modified-at set1))) ;; <-- MODIFIED + (t/is (= (:type token1') (:type token1))) + (t/is (= (:value token1') (:value token1))) + + (t/is (= (ctob/get-id token2') (ctob/get-id token2))) + (t/is (= (ctob/get-name token2') (ctob/get-name token2))) + (t/is (= (ctob/get-description token2') (ctob/get-description token2))) + (t/is (= (ctob/get-modified-at token2') (ctob/get-modified-at token2))) + (t/is (= (:type token2') (:type token2))) + (t/is (= (:value token2') (:value token2))))) + + (t/testing "the renamed token is always the first one found with a conflicting name" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set1) + :name "set1" + :tokens {"name1.name2" (ctob/make-token + {:id (thi/new-id! :token1) + :name "name1.name2" + :type :border-radius + :value "1"})})) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set2) + :name "set2" + :tokens {"name1" (ctob/make-token + {:id (thi/new-id! :token2) + :name "name1" + :type :border-radius + :value "2"})}))) + + tokens-lib' (ctob/fix-conflicting-token-names tokens-lib) + token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1)) + token2' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token2))] + + (t/is (= "name1-1.name2" (ctob/get-name token1'))) + (t/is (= "name1" (ctob/get-name token2'))))) + + (t/testing "several tokens with the same conflicting prefix should be assigned the same number as suffixes" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set1) + :name "set1" + :tokens {"name1.name2" (ctob/make-token + {:id (thi/new-id! :token1) + :name "name1.name2" + :type :border-radius + :value "1"}) + "name1.name3" (ctob/make-token + {:id (thi/new-id! :token2) + :name "name1.name3" + :type :border-radius + :value "2"})})) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set2) + :name "set2" + :tokens {"name1" (ctob/make-token + {:id (thi/new-id! :token3) + :name "name1" + :type :border-radius + :value "3"})}))) + + tokens-lib' (ctob/fix-conflicting-token-names tokens-lib) + token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1)) + token2' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token2)) + token3' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token3))] + + (t/is (= "name1-1.name2" (ctob/get-name token1'))) + (t/is (= "name1-1.name3" (ctob/get-name token2'))) + (t/is (= "name1" (ctob/get-name token3'))))) + + (t/testing "tokens with diferent conflicting prefixes should be assigned consecutive numbers as suffixes" + (let [tokens-lib (-> (ctob/make-tokens-lib) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set1) + :name "set1" + :tokens {"name1" (ctob/make-token + {:id (thi/new-id! :token1) + :name "name1" + :type :border-radius + :value "1"}) + "name2" (ctob/make-token + {:id (thi/new-id! :token2) + :name "name2" + :type :border-radius + :value "2"})})) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set2) + :name "set2" + :tokens {"name1.subname1" (ctob/make-token + {:id (thi/new-id! :token3) + :name "name1.subname1" + :type :border-radius + :value "3"})})) + (ctob/add-set (ctob/make-token-set + :id (thi/new-id! :set3) + :name "set3" + :tokens {"name2.subname2" (ctob/make-token + {:id (thi/new-id! :token4) + :name "name2.subname2" + :type :border-radius + :value "3"})}))) + + tokens-lib' (ctob/fix-conflicting-token-names tokens-lib) + token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1)) + token2' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token2)) + token3' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token3)) + token4' (ctob/get-token tokens-lib' (thi/id :set3) (thi/id :token4))] + + (t/is (= "name1-1" (ctob/get-name token1'))) + (t/is (= "name2-2" (ctob/get-name token2'))) + (t/is (= "name1.subname1" (ctob/get-name token3'))) + (t/is (= "name2.subname2" (ctob/get-name token4'))))))) \ No newline at end of file diff --git a/frontend/playwright/ui/specs/tokens/crud.spec.js b/frontend/playwright/ui/specs/tokens/crud.spec.js index 4bfd1c6a4b..a579d87877 100644 --- a/frontend/playwright/ui/specs/tokens/crud.spec.js +++ b/frontend/playwright/ui/specs/tokens/crud.spec.js @@ -1687,29 +1687,58 @@ test.describe("Tokens - creation", () => { // Submit button should remain disabled when value is empty await expect(submitButton).toBeDisabled(); }); +}); - test("User duplicate color token", async ({ page }) => { - const { tokensSidebar, tokenContextMenuForToken } = - await setupTokensFileRender(page); +test("User cannot create token with a conflicting name in other set", async ({ page }) => { + const { tokensUpdateCreateModal, tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = + await setupTokensFileRender(page); - await expect(tokensSidebar).toBeVisible(); + await expect(tokensSidebar).toBeVisible(); - await unfoldTokenType(tokensSidebar, "color"); + await tokenThemesSetsSidebar.getByRole('button', { name: 'light', exact: true }).click(); - const colorToken = tokensSidebar.getByRole("button", { - name: "colors.blue.100", - }); + const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" }); + await tokensTabPanel + .getByRole("button", { name: "Add Token: Color" }) + .click(); - await colorToken.click({ button: "right" }); - await expect(tokenContextMenuForToken).toBeVisible(); + await expect(tokensUpdateCreateModal).toBeVisible(); - await tokenContextMenuForToken.getByText("Duplicate token").click(); - await expect(tokenContextMenuForToken).not.toBeVisible(); - - await expect( - tokensSidebar.getByRole("button", { name: "colors.blue.100-copy" }), - ).toBeVisible(); + const nameField = tokensUpdateCreateModal.getByLabel("Name"); + const valueField = tokensUpdateCreateModal.getByLabel("Value"); + const submitButton = tokensUpdateCreateModal.getByRole("button", { + name: "Save", }); + + // Initially submit button should be disabled + await expect(submitButton).toBeDisabled(); + + await nameField.click(); + + // Fill in the name of an existing token in the current set + await nameField.fill("accent.default"); + + // An error message should appear and submit button should be disabled + await expect(tokensUpdateCreateModal.getByText('A token already exists at the path: accent.default')) + .toBeVisible() + + await expect(submitButton).toBeDisabled(); + + // Fill in a name that clashes with tokens like colors.red.600 in set core + await nameField.fill("colors.red"); + + // An error message should appear and submit button should be disabled + await expect(tokensUpdateCreateModal.getByText('A token already exists at the path: colors.red')) + .toBeVisible() + + await expect(submitButton).toBeDisabled(); + + // Fill in a name that matches exactly a token in another set + await nameField.fill("colors.red.600"); + await valueField.fill("#6000000"); + + // Submit button should be enabled now + await expect(submitButton).toBeEnabled(); }); test("User creates grouped color token", async ({ page }) => { diff --git a/frontend/src/app/main/data/workspace/tokens/errors.cljs b/frontend/src/app/main/data/workspace/tokens/errors.cljs index 30ab2e30b9..98d0ef9f3a 100644 --- a/frontend/src/app/main/data/workspace/tokens/errors.cljs +++ b/frontend/src/app/main/data/workspace/tokens/errors.cljs @@ -27,6 +27,11 @@ :error/fn #(tr "workspace.tokens.invalid-json-token-name") :error/detail #(tr "workspace.tokens.invalid-json-token-name-detail" %)} + :error.import/duplicated-token-name + {:error/code :error.import/duplicated-token-name + :error/fn #(tr "workspace.tokens.duplicated-json-token-name") + :error/detail #(tr "workspace.tokens.duplicated-json-token-name-detail" %)} + :error.import/style-dictionary-reference-errors {:error/code :error.import/style-dictionary-reference-errors :error/fn #(str (tr "workspace.tokens.import-error") "\n\n" (first %)) diff --git a/frontend/src/app/main/data/workspace/tokens/import_export.cljs b/frontend/src/app/main/data/workspace/tokens/import_export.cljs index f9793d38c8..87a6ee6067 100644 --- a/frontend/src/app/main/data/workspace/tokens/import_export.cljs +++ b/frontend/src/app/main/data/workspace/tokens/import_export.cljs @@ -20,6 +20,12 @@ [beicon.v2.core :as rx] [cuerdas.core :as str])) +(defn- extract-error-with-code + "Return the error if it has an error code generated from Penpot code" + [err] + (when (contains? (ex-data err) :error/code) + (wte/error-ex-info (:error/code (ex-data err)) (ex-message err) err))) + (defn- extract-reference-errors "Extracts reference errors from errors produced by StyleDictionary." [err] @@ -75,7 +81,8 @@ {:tokens-lib (ctob/parse-decoded-json decoded-json file-name) :unknown-tokens (ctob/get-tokens-of-unknown-type decoded-json {})} (catch js/Error e - (let [err (or (extract-name-error e) + (let [err (or (extract-error-with-code e) + (extract-name-error e) (wte/error-ex-info :error.import/invalid-json-data decoded-json e))] (throw err))))) diff --git a/frontend/src/app/main/ui/workspace/tokens/management.cljs b/frontend/src/app/main/ui/workspace/tokens/management.cljs index e3a0dafed1..15f780914a 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management.cljs @@ -327,4 +327,5 @@ :type type :selected-shapes selected-shapes :is-selected-inside-layout is-selected-inside-layout - :active-theme-tokens resolved-active-tokens}])])) + :active-theme-tokens resolved-active-tokens + :selected-token-set-id selected-token-set-id}])])) diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/color.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/color.cljs index 6c295990e7..b6d41273ca 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/color.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/color.cljs @@ -8,12 +8,13 @@ (:require [app.common.files.tokens :as cfo] [app.common.schema :as sm] + [app.common.types.tokens-lib :as ctob] [app.main.ui.workspace.tokens.management.forms.controls :as token.controls] [app.main.ui.workspace.tokens.management.forms.generic-form :as generic] [rumext.v2 :as mf])) (mf/defc form* - [{:keys [token token-type] :as props}] + [{:keys [token token-type selected-token-set-id] :as props}] (let [initial (mf/with-memo [token-type token] {:type token-type @@ -22,7 +23,11 @@ :description (:description token "") :color-result ""}) - props (mf/spread-props props {:make-schema #(-> (cfo/make-token-schema %1 token-type) + props (mf/spread-props props {:make-schema #(-> (cfo/make-token-schema %1 + token-type + selected-token-set-id + (when (ctob/token? token) + (ctob/get-id token))) (sm/dissoc-key :id) (sm/assoc-key :color-result :string)) :initial initial diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/font_family.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/font_family.cljs index b7bba0ea45..4815952948 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/font_family.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/font_family.cljs @@ -9,6 +9,7 @@ [app.common.files.tokens :as cfo] [app.common.schema :as sm] [app.common.types.token :as cto] + [app.common.types.tokens-lib :as ctob] [app.main.data.workspace.tokens.errors :as wte] [app.main.ui.workspace.tokens.management.forms.controls :as token.controls] [app.main.ui.workspace.tokens.management.forms.generic-form :as generic] @@ -29,7 +30,7 @@ (default-validate-token))) (mf/defc form* - [{:keys [token token-type] :rest props}] + [{:keys [token token-type selected-token-set-id] :rest props}] (let [token (mf/with-memo [token] (if token @@ -37,7 +38,11 @@ {:type token-type})) props (mf/spread-props props {:token token :token-type token-type - :make-schema #(-> (cfo/make-token-schema %1 token-type) + :make-schema #(-> (cfo/make-token-schema %1 + token-type + selected-token-set-id + (when (ctob/token? token) + (ctob/get-id token))) (sm/dissoc-key :id) ;; The value as edited in the form is a simple stirng. ;; It's converted to vector in the validator. diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/form_container.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/form_container.cljs index 0e6d55df03..81b3b7f991 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/form_container.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/form_container.cljs @@ -6,10 +6,7 @@ (ns app.main.ui.workspace.tokens.management.forms.form-container (:require - [app.common.data :as d] - [app.common.types.tokens-lib :as ctob] [app.config :as cf] - [app.main.refs :as refs] [app.main.ui.workspace.tokens.management.forms.color :as color] [app.main.ui.workspace.tokens.management.forms.controls :as token.controls] [app.main.ui.workspace.tokens.management.forms.font-family :as font-family] @@ -24,20 +21,8 @@ (let [token-type (or (:type token) token-type) - tokens-in-selected-set - (mf/deref refs/workspace-all-tokens-in-selected-set) - - token-path - (mf/with-memo [token] - (ctob/get-token-path token)) - - tokens-tree-in-selected-set - (mf/with-memo [token-path tokens-in-selected-set] - (-> (ctob/tokens-tree tokens-in-selected-set) - (d/dissoc-in token-path))) props (mf/spread-props props {:token-type token-type - :tokens-tree-in-selected-set tokens-tree-in-selected-set :token token}) text-case-props (mf/spread-props props {:input-value-placeholder (tr "workspace.tokens.text-case-value-enter")}) text-decoration-props (mf/spread-props props {:input-value-placeholder (tr "workspace.tokens.text-decoration-value-enter")}) diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/generic_form.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/generic_form.cljs index a4cc813bbc..ffa652b372 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/generic_form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/generic_form.cljs @@ -57,7 +57,6 @@ action is-create selected-token-set-id - tokens-tree-in-selected-set token-type make-schema input-component @@ -66,7 +65,11 @@ value-subfield input-value-placeholder] :as props}] - (let [make-schema (or make-schema #(-> (cfo/make-token-schema % token-type) + (let [make-schema (or make-schema #(-> (cfo/make-token-schema % + token-type + selected-token-set-id + (when (ctob/token? token) + (ctob/get-id token))) (sm/dissoc-key :id))) input-component (or input-component token.controls/input*) validate-token (or validator default-validate-token) @@ -85,6 +88,8 @@ tokens (mf/deref refs/workspace-all-tokens-map) + tokens-lib (mf/deref refs/tokens-lib) + tokens-in-selected-set (mf/deref refs/workspace-all-tokens-in-selected-set) @@ -102,8 +107,8 @@ (delay (ctob/group-by-type tokens))) schema - (mf/with-memo [tokens-tree-in-selected-set active-tab] - (make-schema tokens-tree-in-selected-set active-tab)) + (mf/with-memo [tokens-lib active-tab] + (make-schema tokens-lib active-tab)) initial (mf/with-memo [token initial] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/rename_node_modal.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/rename_node_modal.cljs index 194b731f03..601a77f13a 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/rename_node_modal.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/rename_node_modal.cljs @@ -3,8 +3,8 @@ (:require [app.common.data :as d] [app.common.files.tokens :as cfo] - [app.common.types.tokens-lib :as ctob] [app.main.data.modal :as modal] + [app.main.refs :as refs] [app.main.store :as st] [app.main.ui.ds.buttons.button :refer [button*]] [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] @@ -18,8 +18,8 @@ [rumext.v2 :as mf])) (mf/defc rename-node-form* - [{:keys [new-node-name node active-tokens tokens-tree variant on-close on-submit]}] - (let [make-schema #(cfo/make-node-token-schema active-tokens tokens-tree node) + [{:keys [new-node-name node active-tokens tokens-lib selected-token-set-id variant on-close on-submit]}] + (let [make-schema #(cfo/make-node-token-schema active-tokens tokens-lib node selected-token-set-id) schema (mf/with-memo [active-tokens] @@ -82,10 +82,9 @@ (let [variant (d/nilv variant "rename") ;; "rename" or "duplicate" - tokens-tree-in-selected-set - (mf/with-memo [tokens-in-active-set node] - (-> (ctob/tokens-tree tokens-in-active-set) - (d/dissoc-in (:name node)))) + selected-token-set-id (mf/deref refs/selected-token-set-id) + + tokens-lib (mf/deref refs/tokens-lib) close-modal (mf/use-fn @@ -118,6 +117,7 @@ :node node :variant variant :active-tokens tokens-in-active-set - :tokens-tree tokens-tree-in-selected-set + :tokens-lib tokens-lib + :selected-token-set-id selected-token-set-id :on-close close-modal :on-submit rename}]]])) diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/shadow.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/shadow.cljs index 722683d54a..d2446662b8 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/shadow.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/shadow.cljs @@ -255,7 +255,7 @@ ;; TODO: use cfo/make-schema:token-value and extend it with shadow and reference fields (defn- make-schema - [tokens-tree active-tab] + [set-id token-id tokens-lib active-tab] (sm/schema [:and [:map @@ -266,7 +266,7 @@ (sm/update-properties cto/schema:token-name assoc :error/fn #(str (:value %) (tr "workspace.tokens.token-name-validation-error"))) [:fn {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))} - #(not (ctob/token-name-path-exists? % tokens-tree))]]] + #(not (ctob/token-name-path-exists? % tokens-lib set-id token-id))]]] [:value [:map @@ -340,8 +340,7 @@ :shadow [default-token-shadow]})) (mf/defc form* - [{:keys [token - token-type] :as props}] + [{:keys [token token-type selected-token-set-id] :as props}] (let [token (mf/with-memo [token] (or token @@ -352,6 +351,12 @@ {:type token-type :value {:reference nil :shadow [default-token-shadow]}}))) + + make-schema + (mf/with-memo [selected-token-set-id token] + (partial make-schema selected-token-set-id (when (ctob/token? token) + (ctob/get-id token)))) + initial (mf/with-memo [token] (let [raw-value (:value token) diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/typography.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/typography.cljs index cb926f72fd..0ccecd81a8 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/typography.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/typography.cljs @@ -209,7 +209,7 @@ ;; TODO: use cfo/make-schema:token-value and extend it with typography and reference fields (defn- make-schema - [tokens-tree active-tab] + [set-id token-id tokens-lib active-tab] (sm/schema [:and [:map @@ -220,7 +220,7 @@ (sm/update-properties cto/schema:token-name assoc :error/fn #(str (:value %) (tr "workspace.tokens.token-name-validation-error"))) [:fn {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))} - #(not (ctob/token-name-path-exists? % tokens-tree))]]] + #(not (ctob/token-name-path-exists? % tokens-lib set-id token-id))]]] [:value [:map @@ -269,7 +269,7 @@ result))]])) (mf/defc form* - [{:keys [token] :as props}] + [{:keys [token selected-token-set-id] :as props}] (let [initial (mf/with-memo [token] (let [value (:value token) @@ -296,6 +296,12 @@ {:name (:name token "") :value processed-value :description (:description token "")})) + + make-schema + (mf/with-memo [selected-token-set-id token] + (partial make-schema selected-token-set-id (when (ctob/token? token) + (ctob/get-id token)))) + props (mf/spread-props props {:initial initial :make-schema make-schema :token token diff --git a/frontend/src/app/main/ui/workspace/tokens/management/group.cljs b/frontend/src/app/main/ui/workspace/tokens/management/group.cljs index a9b6914b9f..e73018223a 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/group.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/group.cljs @@ -131,7 +131,7 @@ on-popover-open-click (mf/use-fn - (mf/deps type title modal) + (mf/deps type title modal selected-token-set-id) (fn [event] (dom/stop-propagation event) (st/emit! @@ -143,7 +143,8 @@ :fields (:fields modal) :title title :action "create" - :token-type type}))))) + :token-type type + :selected-token-set-id selected-token-set-id}))))) on-token-pill-click (mf/use-fn diff --git a/frontend/src/app/plugins/tokens.cljs b/frontend/src/app/plugins/tokens.cljs index ad338ca32b..b279d1cb9c 100644 --- a/frontend/src/app/plugins/tokens.cljs +++ b/frontend/src/app/plugins/tokens.cljs @@ -109,7 +109,9 @@ (ctob/get-name token))) :schema (cfo/make-token-name-schema (some-> (u/locate-tokens-lib file-id) - (ctob/get-tokens set-id))) + (ctob/get-tokens set-id)) + set-id + id) :set (fn [_ value] (st/emit! (dwtl/update-token set-id id {:name value})))} @@ -294,19 +296,17 @@ :addToken {:enumerable false :schema (fn [args] - (let [tokens-tree (-> (u/locate-tokens-lib file-id) - (ctob/get-tokens id) - ;; Convert to the adecuate format for schema - (ctob/tokens-tree))] - [:tuple (-> (cfo/make-token-schema - tokens-tree - (cto/dtcg-token-type->token-type (-> args (first) (get "type")))) - ;; Don't allow plugins to set the id - (sm/dissoc-key :id) - ;; Instruct the json decoder in obj/reify not to process map keys (:key-fn below) - ;; and set a converter that changes DTCG types to internal types (:decode/json). - ;; E.g. "FontFamilies" -> :font-family or "BorderWidth" -> :stroke-width - (sm/update-properties assoc :decode/json cfo/convert-dtcg-token))])) + [:tuple (-> (cfo/make-token-schema + (u/locate-tokens-lib file-id) + (cto/dtcg-token-type->token-type (-> args (first) (get "type"))) + id + (-> args (first) (get "id"))) + ;; Don't allow plugins to set the id + (sm/dissoc-key :id) + ;; Instruct the json decoder in obj/reify not to process map keys (:key-fn below) + ;; and set a converter that changes DTCG types to internal types (:decode/json). + ;; E.g. "FontFamilies" -> :font-family or "BorderWidth" -> :stroke-width + (sm/update-properties assoc :decode/json cfo/convert-dtcg-token))]) :decode/options {:key-fn identity} :fn (fn [attrs] (let [tokens-lib (u/locate-tokens-lib file-id) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 54a33820ec..fecdc0e52a 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -8366,6 +8366,10 @@ msgstr "Import Error: Invalid token data in JSON." msgid "workspace.tokens.invalid-json-token-name" msgstr "Import Error: Invalid token name in JSON." +#: src/app/main/data/workspace/tokens/errors.cljs:32 +msgid "workspace.tokens.duplicated-json-token-name" +msgstr "Import Error: Duplicated token name in JSON." + #: src/app/main/data/workspace/tokens/errors.cljs:28 msgid "workspace.tokens.invalid-json-token-name-detail" msgstr "" @@ -8373,6 +8377,10 @@ msgstr "" "Token names should only contain letters and digits separated by . " "characters and must not start with a $ sign." +#: src/app/main/data/workspace/tokens/errors.cljs:33 +msgid "workspace.tokens.duplicated-json-token-name-detail" +msgstr "A token already exists at the path '%s' or at a prefix thereof, in another set." + #: src/app/main/data/workspace/tokens/errors.cljs:105 msgid "workspace.tokens.invalid-shadow-type-token-value" msgstr "Invalid shadow type: only 'innerShadow' or 'dropShadow' are accepted" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 6855d5fa84..d878e965d5 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -8198,6 +8198,10 @@ msgstr "Error al importar: Datos de token no válidos en JSON." msgid "workspace.tokens.invalid-json-token-name" msgstr "Error al importar: Nombre de token no válido en JSON." +#: src/app/main/data/workspace/tokens/errors.cljs:32 +msgid "workspace.tokens.duplicated-json-token-name" +msgstr "Error al importar: Nombre de token duplicado en JSON." + #: src/app/main/data/workspace/tokens/errors.cljs:28 msgid "workspace.tokens.invalid-json-token-name-detail" msgstr "" @@ -8205,6 +8209,10 @@ msgstr "" "Los nombres de token solo pueden contener letras y dígitos separados por " "caracteres . y no pueden empezar con un signo $." +#: src/app/main/data/workspace/tokens/errors.cljs:33 +msgid "workspace.tokens.duplicated-json-token-name-detail" +msgstr "Existe un token en la ruta '%s' o en un prefijo del mismo, en otro set." + #: src/app/main/data/workspace/tokens/errors.cljs:105 msgid "workspace.tokens.invalid-shadow-type-token-value" msgstr "Tipo de sombra no válida: solo se aceptan 'innerShadow' o 'dropShadow'"