From f9530c5a10d8d31e499abad00a7ed74ef624f4d7 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Wed, 26 Jun 2024 16:01:41 +0200 Subject: [PATCH 01/16] Restrict token naming --- .../app/main/ui/workspace/tokens/form.cljs | 29 ++++++++++++++++++- .../test/token_tests/token_form_test.cljs | 26 +++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 frontend/test/token_tests/token_form_test.cljs diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index 9dd1930a4..47c39b263 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -22,6 +22,30 @@ ;; Schemas --------------------------------------------------------------------- +(def valid-token-name-regexp + "Only allow letters and digits for token names. + Also allow one `.` for a namespace separator. + + Caution: This will allow a trailing dot like `token-name.`, + But we will trim that in the `finalize-name`, + to not throw too many errors while the user is editing." + #"([a-zA-Z0-9]+\.?)*") + +(def valid-token-name-schema + (m/-simple-schema + {:type :token/invalid-token-name + :pred #(re-matches valid-token-name-regexp %) + :type-properties {:error/fn #(str (:value %) " is not a valid token name. +Token names should only contain letters and digits separated by . characters.")}})) + +(comment + (m/validate valid-token-name-schema "Hey [1]") + (m/valid? valid-token-name-schema "Hey") + (m/validate valid-token-name-schema "Hey.foo.") + (m/validate valid-token-name-schema "🤣") + (m/validate valid-token-name-schema ".") + nil) + (defn token-name-schema "Generate a dynamic schema validation to check if a token name already exists. `existing-token-names` should be a set of strings." @@ -35,6 +59,7 @@ (m/schema [:and [:string {:min 1 :max 255}] + valid-token-name-schema non-existing-token-schema]))) (def token-description-schema @@ -44,7 +69,9 @@ ;; Helpers --------------------------------------------------------------------- (defn finalize-name [name] - (str/trim name)) + (-> (str/trim name) + ;; Remove trailing dots + (str/replace #"\.+$" ""))) (defn valid-name? [name] (seq (finalize-name (str name)))) diff --git a/frontend/test/token_tests/token_form_test.cljs b/frontend/test/token_tests/token_form_test.cljs new file mode 100644 index 000000000..2bc14df32 --- /dev/null +++ b/frontend/test/token_tests/token_form_test.cljs @@ -0,0 +1,26 @@ +;; 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 token-tests.token-form-test + (:require + [app.main.ui.workspace.tokens.form :as wtf] + [cljs.test :as t :include-macros true] + [malli.core :as m])) + +(t/deftest test-valid-token-name-schema + ;; Allow regular namespace token names + (t/is (some? (m/validate wtf/valid-token-name-schema "Foo"))) + (t/is (some? (m/validate wtf/valid-token-name-schema "foo"))) + (t/is (some? (m/validate wtf/valid-token-name-schema "FOO"))) + (t/is (some? (m/validate wtf/valid-token-name-schema "Foo.Bar.Baz"))) + ;; Allow trailing tokens + (t/is (nil? (m/validate wtf/valid-token-name-schema "Foo.Bar.Baz...."))) + ;; Disallow multiple separator dots + (t/is (nil? (m/validate wtf/valid-token-name-schema "Foo..Bar.Baz"))) + ;; Disallow any special characters + (t/is (nil? (m/validate wtf/valid-token-name-schema "Hey Foo.Bar"))) + (t/is (nil? (m/validate wtf/valid-token-name-schema "Hey😈Foo.Bar"))) + (t/is (nil? (m/validate wtf/valid-token-name-schema "Hey%Foo.Bar")))) From f24c314d63c85d4d6be0178e6689f817c4ab85c5 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Wed, 26 Jun 2024 16:04:50 +0200 Subject: [PATCH 02/16] Update --- frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md index 519bac28a..e76382425 100644 --- a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md +++ b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md @@ -18,6 +18,11 @@ If possible add video here from PR as well ## Changes +### 2024-06-26 - Disallow special characters in token name + +[Video](https://github.com/tokens-studio/tokens-studio-for-penpot/assets/1898374/7c59c0cc-d6e1-4b0d-9646-9a27eafcccc4 +) + ### 2024-06-26 - Make Tokens JSON Export DTCG compatible ![Screenshot of sample JSON Export in DTCG format](https://private-user-images.githubusercontent.com/9948167/343043570-b4bb39f7-ec53-409a-a053-b284d60848d9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTk0MDMyMzcsIm5iZiI6MTcxOTQwMjkzNywicGF0aCI6Ii85OTQ4MTY3LzM0MzA0MzU3MC1iNGJiMzlmNy1lYzUzLTQwOWEtYTA1My1iMjg0ZDYwODQ4ZDkucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDYyNiUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA2MjZUMTE1NTM3WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MWEzZTU5OWQ0M2JkZWE5MTA5MDc4MTY1OTkyZWE5MmE5YzBlYmQ2NTcwMmEwZTdmMjViNGU5YTFjNWIxYjU5ZCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.qWJxRa_Y7LZ6EDJg5yPdOUIQkURFmZwMNec_BbdH9Co) From 00dabaf38c26f78e5d3542d73a565b3dac0be48c Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Wed, 26 Jun 2024 16:08:21 +0200 Subject: [PATCH 03/16] Remove comment form --- frontend/src/app/main/ui/workspace/tokens/form.cljs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index 47c39b263..ab71333ba 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -38,14 +38,6 @@ :type-properties {:error/fn #(str (:value %) " is not a valid token name. Token names should only contain letters and digits separated by . characters.")}})) -(comment - (m/validate valid-token-name-schema "Hey [1]") - (m/valid? valid-token-name-schema "Hey") - (m/validate valid-token-name-schema "Hey.foo.") - (m/validate valid-token-name-schema "🤣") - (m/validate valid-token-name-schema ".") - nil) - (defn token-name-schema "Generate a dynamic schema validation to check if a token name already exists. `existing-token-names` should be a set of strings." From 3a500fb8a79b0d4dba3d13dcca6d38c39c0d8819 Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Wed, 26 Jun 2024 19:40:06 +0530 Subject: [PATCH 04/16] Update CHANGELOG.md with PR link --- frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md index e76382425..16a7ef809 100644 --- a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md +++ b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md @@ -23,6 +23,8 @@ If possible add video here from PR as well [Video](https://github.com/tokens-studio/tokens-studio-for-penpot/assets/1898374/7c59c0cc-d6e1-4b0d-9646-9a27eafcccc4 ) +https://github.com/tokens-studio/tokens-studio-for-penpot/pull/200 + ### 2024-06-26 - Make Tokens JSON Export DTCG compatible ![Screenshot of sample JSON Export in DTCG format](https://private-user-images.githubusercontent.com/9948167/343043570-b4bb39f7-ec53-409a-a053-b284d60848d9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTk0MDMyMzcsIm5iZiI6MTcxOTQwMjkzNywicGF0aCI6Ii85OTQ4MTY3LzM0MzA0MzU3MC1iNGJiMzlmNy1lYzUzLTQwOWEtYTA1My1iMjg0ZDYwODQ4ZDkucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDYyNiUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA2MjZUMTE1NTM3WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MWEzZTU5OWQ0M2JkZWE5MTA5MDc4MTY1OTkyZWE5MmE5YzBlYmQ2NTcwMmEwZTdmMjViNGU5YTFjNWIxYjU5ZCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.qWJxRa_Y7LZ6EDJg5yPdOUIQkURFmZwMNec_BbdH9Co) From a4bbef991797d6958abee898d27b74399f273cab Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Fri, 28 Jun 2024 14:43:49 +0530 Subject: [PATCH 05/16] Fix token reference issue when name has . --- .../main/ui/workspace/tokens/style_dictionary.cljs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs index d9d3c4a0f..cecd0473d 100644 --- a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs @@ -82,14 +82,18 @@ (and (set? errors) (get errors :style-dictionary/missing-reference))) -(defn tokens-name-map [tokens] - (->> tokens - (map (fn [[_ x]] [(:name x) x])) - (into {}))) +(defn tokens-name-tree [tokens] + (reduce + (fn [acc [_ {:keys [name] :as token}]] + (if (string? name) + (let [name-path (->> (str/split name #"\.") + (remove str/blank?))] + (assoc-in acc name-path token)))) + {} tokens)) (defn resolve-tokens+ [tokens & {:keys [debug?] :as config}] - (p/let [sd-tokens (-> (tokens-name-map tokens) + (p/let [sd-tokens (-> (tokens-name-tree tokens) (resolve-sd-tokens+ config))] (let [resolved-tokens (reduce (fn [acc ^js cur] From ef5f0192002e45183ca0a8c7d1964d5e8cc29b57 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Fri, 28 Jun 2024 13:34:54 +0200 Subject: [PATCH 06/16] Add helper utility to convert name to path --- .../src/app/main/ui/workspace/tokens/core.cljs | 6 ++++++ .../ui/workspace/tokens/style_dictionary.cljs | 3 +++ frontend/test/token_tests/token_core_test.cljs | 15 +++++++++++++++ 3 files changed, 24 insertions(+) create mode 100644 frontend/test/token_tests/token_core_test.cljs diff --git a/frontend/src/app/main/ui/workspace/tokens/core.cljs b/frontend/src/app/main/ui/workspace/tokens/core.cljs index 682cefde4..695e364cb 100644 --- a/frontend/src/app/main/ui/workspace/tokens/core.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/core.cljs @@ -76,6 +76,12 @@ (cond-> (assoc item :label name) (token-applied? item shape (or selected-attributes attributes)) (assoc :selected? true)))))) +(defn name->path + "Splits token-name into a path vector split by `.` characters. + Will concatenate multiple `.` characters into one." + [token-name] + (str/split token-name #"\.+")) + ;; Update functions ------------------------------------------------------------ (defn on-apply-token [{:keys [token token-type-props selected-shapes] :as _props}] diff --git a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs index cecd0473d..f0d36dcef 100644 --- a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs @@ -91,9 +91,12 @@ (assoc-in acc name-path token)))) {} tokens)) +(tokens-name-tree @refs/workspace-tokens) + (defn resolve-tokens+ [tokens & {:keys [debug?] :as config}] (p/let [sd-tokens (-> (tokens-name-tree tokens) + (doto js/console.log) (resolve-sd-tokens+ config))] (let [resolved-tokens (reduce (fn [acc ^js cur] diff --git a/frontend/test/token_tests/token_core_test.cljs b/frontend/test/token_tests/token_core_test.cljs new file mode 100644 index 000000000..96e4321fd --- /dev/null +++ b/frontend/test/token_tests/token_core_test.cljs @@ -0,0 +1,15 @@ +;; 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 token-tests.token-core-test + (:require + [app.main.ui.workspace.tokens.core :as wtc] + [cljs.test :as t :include-macros true])) + +(t/deftest name->path-test + (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo.bar.baz"))) + (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo..bar.baz"))) + (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo..bar.baz....")))) From 504369ec13b18535b2efd5d2213d1ddb032536d3 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Fri, 28 Jun 2024 13:43:41 +0200 Subject: [PATCH 07/16] Move tokens-name-tree to core, add test --- .../app/main/ui/workspace/tokens/core.cljs | 23 ++++++++++++++----- .../ui/workspace/tokens/style_dictionary.cljs | 13 +---------- .../test/token_tests/token_core_test.cljs | 13 +++++++++++ 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/core.cljs b/frontend/src/app/main/ui/workspace/tokens/core.cljs index 695e364cb..8587bab36 100644 --- a/frontend/src/app/main/ui/workspace/tokens/core.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/core.cljs @@ -61,6 +61,23 @@ (->> (map (fn [{:keys [name] :as token}] [name token]) tokens) (into {}))) +(defn name->path + "Splits token-name into a path vector split by `.` characters. + + Will concatenate multiple `.` characters into one." + [token-name] + (str/split token-name #"\.+")) + +(defn tokens-name-tree + "Convert tokens into a nested tree with their `:name` as the path." + [tokens] + (reduce + (fn [acc [_ {:keys [name] :as token}]] + (when (string? name) + (let [path (name->path name)] + (assoc-in acc path token)))) + {} tokens)) + (defn tokens-name-map-for-type "Convert tokens with `token-type` into a map with their `:name` as the key. @@ -76,12 +93,6 @@ (cond-> (assoc item :label name) (token-applied? item shape (or selected-attributes attributes)) (assoc :selected? true)))))) -(defn name->path - "Splits token-name into a path vector split by `.` characters. - Will concatenate multiple `.` characters into one." - [token-name] - (str/split token-name #"\.+")) - ;; Update functions ------------------------------------------------------------ (defn on-apply-token [{:keys [token token-type-props selected-shapes] :as _props}] diff --git a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs index f0d36dcef..892ec724d 100644 --- a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs @@ -82,20 +82,9 @@ (and (set? errors) (get errors :style-dictionary/missing-reference))) -(defn tokens-name-tree [tokens] - (reduce - (fn [acc [_ {:keys [name] :as token}]] - (if (string? name) - (let [name-path (->> (str/split name #"\.") - (remove str/blank?))] - (assoc-in acc name-path token)))) - {} tokens)) - -(tokens-name-tree @refs/workspace-tokens) - (defn resolve-tokens+ [tokens & {:keys [debug?] :as config}] - (p/let [sd-tokens (-> (tokens-name-tree tokens) + (p/let [sd-tokens (-> (wtc/tokens-name-tree tokens) (doto js/console.log) (resolve-sd-tokens+ config))] (let [resolved-tokens (reduce diff --git a/frontend/test/token_tests/token_core_test.cljs b/frontend/test/token_tests/token_core_test.cljs index 96e4321fd..8241640d7 100644 --- a/frontend/test/token_tests/token_core_test.cljs +++ b/frontend/test/token_tests/token_core_test.cljs @@ -13,3 +13,16 @@ (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo.bar.baz"))) (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo..bar.baz"))) (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo..bar.baz....")))) + +(t/deftest tokens-name-tree + (t/is (= {"foo" + {"bar" + {"baz" {:name "foo.bar.baz", :value "a"}, + "bam" {:name "foo.bar.bam", :value "b"}}}, + "baz" {"bar" {"foo" {:name "baz.bar.foo", :value "{foo.bar.baz}"}}}} + (wtc/tokens-name-tree {:a {:name "foo.bar.baz" + :value "a"} + :b {:name "foo.bar.bam" + :value "b"} + :c {:name "baz.bar.foo" + :value "{foo.bar.baz}"}})))) From 2fa152d364b1ec04819397944203303be17dd933 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Fri, 28 Jun 2024 13:51:32 +0200 Subject: [PATCH 08/16] Move to token namespace --- .../app/main/ui/workspace/tokens/core.cljs | 17 ---------------- .../ui/workspace/tokens/style_dictionary.cljs | 5 ++--- .../app/main/ui/workspace/tokens/token.cljs | 20 +++++++++++++++++++ .../{token_core_test.cljs => token_test.cljs} | 12 +++++------ 4 files changed, 28 insertions(+), 26 deletions(-) create mode 100644 frontend/src/app/main/ui/workspace/tokens/token.cljs rename frontend/test/token_tests/{token_core_test.cljs => token_test.cljs} (71%) diff --git a/frontend/src/app/main/ui/workspace/tokens/core.cljs b/frontend/src/app/main/ui/workspace/tokens/core.cljs index 8587bab36..682cefde4 100644 --- a/frontend/src/app/main/ui/workspace/tokens/core.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/core.cljs @@ -61,23 +61,6 @@ (->> (map (fn [{:keys [name] :as token}] [name token]) tokens) (into {}))) -(defn name->path - "Splits token-name into a path vector split by `.` characters. - - Will concatenate multiple `.` characters into one." - [token-name] - (str/split token-name #"\.+")) - -(defn tokens-name-tree - "Convert tokens into a nested tree with their `:name` as the path." - [tokens] - (reduce - (fn [acc [_ {:keys [name] :as token}]] - (when (string? name) - (let [path (name->path name)] - (assoc-in acc path token)))) - {} tokens)) - (defn tokens-name-map-for-type "Convert tokens with `token-type` into a map with their `:name` as the key. diff --git a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs index 892ec724d..73e68ad4c 100644 --- a/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs @@ -4,7 +4,7 @@ ["style-dictionary$default" :as sd] [app.common.data :as d] [app.main.refs :as refs] - [app.util.dom :as dom] + [app.main.ui.workspace.tokens.token :as wtt] [cuerdas.core :as str] [promesa.core :as p] [rumext.v2 :as mf] @@ -84,8 +84,7 @@ (defn resolve-tokens+ [tokens & {:keys [debug?] :as config}] - (p/let [sd-tokens (-> (wtc/tokens-name-tree tokens) - (doto js/console.log) + (p/let [sd-tokens (-> (wtt/token-names-tree tokens) (resolve-sd-tokens+ config))] (let [resolved-tokens (reduce (fn [acc ^js cur] diff --git a/frontend/src/app/main/ui/workspace/tokens/token.cljs b/frontend/src/app/main/ui/workspace/tokens/token.cljs new file mode 100644 index 000000000..a7f2289ac --- /dev/null +++ b/frontend/src/app/main/ui/workspace/tokens/token.cljs @@ -0,0 +1,20 @@ +(ns app.main.ui.workspace.tokens.token + (:require + [cuerdas.core :as str])) + +(defn token-name->path + "Splits token-name into a path vector split by `.` characters. + + Will concatenate multiple `.` characters into one." + [token-name] + (str/split token-name #"\.+")) + +(defn token-names-tree + "Convert tokens into a nested tree with their `:name` as the path." + [tokens] + (reduce + (fn [acc [_ {:keys [name] :as token}]] + (when (string? name) + (let [path (token-name->path name)] + (assoc-in acc path token)))) + {} tokens)) diff --git a/frontend/test/token_tests/token_core_test.cljs b/frontend/test/token_tests/token_test.cljs similarity index 71% rename from frontend/test/token_tests/token_core_test.cljs rename to frontend/test/token_tests/token_test.cljs index 8241640d7..0717130cb 100644 --- a/frontend/test/token_tests/token_core_test.cljs +++ b/frontend/test/token_tests/token_test.cljs @@ -4,15 +4,15 @@ ;; ;; Copyright (c) KALEIDOS INC -(ns token-tests.token-core-test +(ns token-tests.token-test (:require - [app.main.ui.workspace.tokens.core :as wtc] + [app.main.ui.workspace.tokens.token :as wtt] [cljs.test :as t :include-macros true])) (t/deftest name->path-test - (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo.bar.baz"))) - (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo..bar.baz"))) - (t/is (= ["foo" "bar" "baz"] (wtc/name->path "foo..bar.baz....")))) + (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo.bar.baz"))) + (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo..bar.baz"))) + (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo..bar.baz....")))) (t/deftest tokens-name-tree (t/is (= {"foo" @@ -20,7 +20,7 @@ {"baz" {:name "foo.bar.baz", :value "a"}, "bam" {:name "foo.bar.bam", :value "b"}}}, "baz" {"bar" {"foo" {:name "baz.bar.foo", :value "{foo.bar.baz}"}}}} - (wtc/tokens-name-tree {:a {:name "foo.bar.baz" + (wtt/token-names-tree {:a {:name "foo.bar.baz" :value "a"} :b {:name "foo.bar.bam" :value "b"} From 48a7c526647903d97eb4a75ba4a80478a4c31536 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Fri, 28 Jun 2024 14:17:42 +0200 Subject: [PATCH 09/16] Separate errors --- frontend/src/app/main/ui/workspace/tokens/form.cljs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index ab71333ba..34217e1cd 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -254,9 +254,12 @@ Token names should only contain letters and digits separated by . characters.")} :auto-focus true :on-blur on-update-name :on-change on-update-name}}] - (when @name-errors - [:p {:class (stl/css :error)} - (me/humanize @name-errors)])] + (for [error (->> (:errors @name-errors) + (map #(-> (assoc @name-errors :errors [%]) + (me/humanize))))] + [:p {:key error + :class (stl/css :error)} + error])] [:& tokens.common/labeled-input {:label "Value" :input-props {:default-value @value-ref :on-blur on-update-value From 174d91a519dc0ed1dcda73d06bf119d5567efa7a Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Fri, 28 Jun 2024 14:39:36 +0200 Subject: [PATCH 10/16] Add function to check if a token can be placed under a name path --- .../app/main/ui/workspace/tokens/form.cljs | 21 +++++++++++--- .../app/main/ui/workspace/tokens/token.cljs | 28 +++++++++++++++++++ frontend/test/token_tests/token_test.cljs | 9 +++++- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index 34217e1cd..954340249 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -13,6 +13,7 @@ [app.main.store :as st] [app.main.ui.workspace.tokens.common :as tokens.common] [app.main.ui.workspace.tokens.style-dictionary :as sd] + [app.main.ui.workspace.tokens.token :as wtt] [app.util.dom :as dom] [cuerdas.core :as str] [malli.core :as m] @@ -41,17 +42,24 @@ Token names should only contain letters and digits separated by . characters.")} (defn token-name-schema "Generate a dynamic schema validation to check if a token name already exists. `existing-token-names` should be a set of strings." - [existing-token-names] + [{:keys [existing-token-names tokens-tree]}] (let [non-existing-token-schema (m/-simple-schema {:type :token/name-exists :pred #(not (get existing-token-names %)) :type-properties {:error/fn #(str (:value %) " is an already existing token name") + :existing-token-names existing-token-names}}) + path-exists-schema + (m/-simple-schema + {:type :token/name-exists + :pred #(wtt/token-name-path-exists? % tokens-tree) + :type-properties {:error/fn #(str "A token already exists at the path: " (:value %)) :existing-token-names existing-token-names}})] (m/schema [:and [:string {:min 1 :max 255}] valid-token-name-schema + path-exists-schema non-existing-token-schema]))) (def token-description-schema @@ -100,7 +108,7 @@ Token names should only contain letters and digits separated by . characters.")} new-tokens (update tokens token-id merge {:id token-id :value input :name token-name})] - (-> (sd/resolve-tokens+ new-tokens) + (-> (sd/resolve-tokens+ new-tokens {:debug? true}) (p/then (fn [resolved-tokens] (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens token-id)] @@ -141,6 +149,9 @@ Token names should only contain letters and digits separated by . characters.")} {::mf/wrap-props false} [{:keys [token token-type] :as _args}] (let [tokens (sd/use-resolved-workspace-tokens) + tokens-tree (mf/use-memo + (mf/deps tokens) + #(wtt/token-names-tree tokens)) existing-token-names (mf/use-memo (mf/deps tokens) (fn [] @@ -152,10 +163,12 @@ Token names should only contain letters and digits separated by . characters.")} ;; Name name-ref (mf/use-var (:name token)) name-errors (mf/use-state nil) + _ (js/console.log "name-errors" @name-errors) validate-name (mf/use-callback - (mf/deps existing-token-names) + (mf/deps tokens-tree existing-token-names) (fn [value] - (let [schema (token-name-schema existing-token-names)] + (let [schema (token-name-schema {:existing-token-names existing-token-names + :tokens-tree tokens-tree})] (m/explain schema (finalize-name value))))) on-update-name-debounced (mf/use-callback (debounce (fn [e] diff --git a/frontend/src/app/main/ui/workspace/tokens/token.cljs b/frontend/src/app/main/ui/workspace/tokens/token.cljs index a7f2289ac..1762f1977 100644 --- a/frontend/src/app/main/ui/workspace/tokens/token.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/token.cljs @@ -18,3 +18,31 @@ (let [path (token-name->path name)] (assoc-in acc path token)))) {} tokens)) + +(defn token-name-path-exists? + "Traverses the path from `token-name` down a `token-tree` and checks if a token at that path exists. + + It's not allowed to create a token inside a token. E.g.: + Creating a token with + + {:name \"foo.bar\"} + + in the tokens tree: + + {\"foo\" {:name \"other\"}}" + [token-name token-names-tree] + (let [name-path (token-name->path token-name) + result (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))) + token-names-tree name-path)] + (if (map? result) + (some? (:name result)) + result))) diff --git a/frontend/test/token_tests/token_test.cljs b/frontend/test/token_tests/token_test.cljs index 0717130cb..f2e0332c4 100644 --- a/frontend/test/token_tests/token_test.cljs +++ b/frontend/test/token_tests/token_test.cljs @@ -14,7 +14,7 @@ (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo..bar.baz"))) (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo..bar.baz....")))) -(t/deftest tokens-name-tree +(t/deftest tokens-name-tree-test (t/is (= {"foo" {"bar" {"baz" {:name "foo.bar.baz", :value "a"}, @@ -26,3 +26,10 @@ :value "b"} :c {:name "baz.bar.foo" :value "{foo.bar.baz}"}})))) + +(t/deftest token-name-path-exists?-test + (t/is (true? (wtt/token-name-path-exists? "border-radius" {"border-radius" {:name "sm"}}))) + (t/is (true? (wtt/token-name-path-exists? "border-radius.sm" {"border-radius" {:name "sm"}}))) + (t/is (true? (wtt/token-name-path-exists? "border-radius.sm.x" {"border-radius" {:name "sm"}}))) + (t/is (false? (wtt/token-name-path-exists? "other" {"border-radius" {:name "sm"}}))) + (t/is (false? (wtt/token-name-path-exists? "dark.border-radius.md" {"dark" {"border-radius" {"sm" {:name "sm"}}}})))) From a98f59469eb3d319ae18c2717f2e0a6469e1dcbd Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Mon, 1 Jul 2024 09:56:45 +0200 Subject: [PATCH 11/16] Add - to allowed token-name --- frontend/src/app/main/ui/workspace/tokens/form.cljs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index 954340249..fd87d7c1e 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -30,7 +30,7 @@ Caution: This will allow a trailing dot like `token-name.`, But we will trim that in the `finalize-name`, to not throw too many errors while the user is editing." - #"([a-zA-Z0-9]+\.?)*") + #"([a-zA-Z0-9-]+\.?)*") (def valid-token-name-schema (m/-simple-schema From 4a85ef3608dbae86db32eb24bb92709e492cdaed Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Mon, 1 Jul 2024 10:16:15 +0200 Subject: [PATCH 12/16] Split path/selector for disallowing creating tokens at path segments --- .../app/main/ui/workspace/tokens/token.cljs | 46 +++++++++++++------ frontend/test/token_tests/token_test.cljs | 1 + 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/token.cljs b/frontend/src/app/main/ui/workspace/tokens/token.cljs index 1762f1977..0ca965d9c 100644 --- a/frontend/src/app/main/ui/workspace/tokens/token.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/token.cljs @@ -9,6 +9,18 @@ [token-name] (str/split token-name #"\.+")) +(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 (token-name->path token-name) + last-idx (dec (count path-segments)) + [path [selector]] (split-at last-idx path-segments)] + {:path (seq path) + :selector selector})) + (defn token-names-tree "Convert tokens into a nested tree with their `:name` as the path." [tokens] @@ -31,18 +43,22 @@ {\"foo\" {:name \"other\"}}" [token-name token-names-tree] - (let [name-path (token-name->path token-name) - result (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))) - token-names-tree name-path)] - (if (map? result) - (some? (:name result)) - result))) + (let [{:keys [path selector]} (token-name->path-selector token-name) + path-target (reduce + (fn [acc cur] + (let [target (get acc cur)] + (prn target 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))) + token-names-tree path)] + (cond + (boolean? path-target) path-target + (get path-target :name) true + :else (-> (get path-target selector) + (seq) + (boolean))))) diff --git a/frontend/test/token_tests/token_test.cljs b/frontend/test/token_tests/token_test.cljs index f2e0332c4..618dc9a21 100644 --- a/frontend/test/token_tests/token_test.cljs +++ b/frontend/test/token_tests/token_test.cljs @@ -28,6 +28,7 @@ :value "{foo.bar.baz}"}})))) (t/deftest token-name-path-exists?-test + (t/is (true? (wtt/token-name-path-exists? "border-radius" {"border-radius" {"sm" {:name "sm"}}}))) (t/is (true? (wtt/token-name-path-exists? "border-radius" {"border-radius" {:name "sm"}}))) (t/is (true? (wtt/token-name-path-exists? "border-radius.sm" {"border-radius" {:name "sm"}}))) (t/is (true? (wtt/token-name-path-exists? "border-radius.sm.x" {"border-radius" {:name "sm"}}))) From 9d637cbe5eb29883811054efd703ed98f7e9ce5f Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Mon, 1 Jul 2024 10:16:52 +0200 Subject: [PATCH 13/16] Path selector test predicate is enough --- .../app/main/ui/workspace/tokens/form.cljs | 32 +++++-------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index fd87d7c1e..ffce3cf70 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -40,27 +40,18 @@ Token names should only contain letters and digits separated by . characters.")}})) (defn token-name-schema - "Generate a dynamic schema validation to check if a token name already exists. - `existing-token-names` should be a set of strings." - [{:keys [existing-token-names tokens-tree]}] - (let [non-existing-token-schema + "Generate a dynamic schema validation to check if a token path derived from the name already exists at `tokens-tree`." + [{:keys [token tokens-tree]}] + (let [path-exists-schema (m/-simple-schema {:type :token/name-exists - :pred #(not (get existing-token-names %)) - :type-properties {:error/fn #(str (:value %) " is an already existing token name") - :existing-token-names existing-token-names}}) - path-exists-schema - (m/-simple-schema - {:type :token/name-exists - :pred #(wtt/token-name-path-exists? % tokens-tree) - :type-properties {:error/fn #(str "A token already exists at the path: " (:value %)) - :existing-token-names existing-token-names}})] + :pred #(not (wtt/token-name-path-exists? % tokens-tree)) + :type-properties {:error/fn #(str "A token already exists at the path: " (:value %))}})] (m/schema [:and [:string {:min 1 :max 255}] valid-token-name-schema - path-exists-schema - non-existing-token-schema]))) + path-exists-schema]))) (def token-description-schema (m/schema @@ -152,22 +143,15 @@ Token names should only contain letters and digits separated by . characters.")} tokens-tree (mf/use-memo (mf/deps tokens) #(wtt/token-names-tree tokens)) - existing-token-names (mf/use-memo - (mf/deps tokens) - (fn [] - (-> (into #{} (map (fn [[_ {:keys [name]}]] name) tokens)) - ;; Remove the currently editing token name, - ;; as we don't want it to show when checking for duplicate names. - (disj (:name token))))) ;; Name name-ref (mf/use-var (:name token)) name-errors (mf/use-state nil) _ (js/console.log "name-errors" @name-errors) validate-name (mf/use-callback - (mf/deps tokens-tree existing-token-names) + (mf/deps tokens-tree) (fn [value] - (let [schema (token-name-schema {:existing-token-names existing-token-names + (let [schema (token-name-schema {:token token :tokens-tree tokens-tree})] (m/explain schema (finalize-name value))))) on-update-name-debounced (mf/use-callback From ec511cc5896167e2c904e4738e6ae45af1e8db25 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Mon, 1 Jul 2024 10:30:03 +0200 Subject: [PATCH 14/16] Fix setting token to own path --- frontend/src/app/main/ui/workspace/tokens/form.cljs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index ffce3cf70..6b1af948b 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -8,6 +8,7 @@ (:require-macros [app.main.style :as stl]) (:require ["lodash.debounce" :as debounce] + [app.common.data :as d] [app.main.data.modal :as modal] [app.main.data.tokens :as dt] [app.main.store :as st] @@ -140,9 +141,15 @@ Token names should only contain letters and digits separated by . characters.")} {::mf/wrap-props false} [{:keys [token token-type] :as _args}] (let [tokens (sd/use-resolved-workspace-tokens) + token-path (mf/use-memo + (mf/deps (:name token)) + #(wtt/token-name->path (:name token))) tokens-tree (mf/use-memo - (mf/deps tokens) - #(wtt/token-names-tree tokens)) + (mf/deps token-path tokens) + (fn [] + (-> (wtt/token-names-tree tokens) + ;; Allow setting editing token to it's own path + (d/dissoc-in token-path)))) ;; Name name-ref (mf/use-var (:name token)) From 111be97228e34f4842148c531b6bb092457c4ae2 Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Mon, 1 Jul 2024 10:31:16 +0200 Subject: [PATCH 15/16] Remove logs --- frontend/src/app/main/ui/workspace/tokens/form.cljs | 1 - frontend/src/app/main/ui/workspace/tokens/token.cljs | 1 - 2 files changed, 2 deletions(-) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index 6b1af948b..9ac30f39d 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -154,7 +154,6 @@ Token names should only contain letters and digits separated by . characters.")} ;; Name name-ref (mf/use-var (:name token)) name-errors (mf/use-state nil) - _ (js/console.log "name-errors" @name-errors) validate-name (mf/use-callback (mf/deps tokens-tree) (fn [value] diff --git a/frontend/src/app/main/ui/workspace/tokens/token.cljs b/frontend/src/app/main/ui/workspace/tokens/token.cljs index 0ca965d9c..ce288113d 100644 --- a/frontend/src/app/main/ui/workspace/tokens/token.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/token.cljs @@ -47,7 +47,6 @@ path-target (reduce (fn [acc cur] (let [target (get acc cur)] - (prn target cur) (cond ;; Path segment doesn't exist yet (nil? target) (reduced false) From 224b656d57344fd863a8c1ad3e86c6a5b235ee3e Mon Sep 17 00:00:00 2001 From: Florian Schroedl Date: Mon, 1 Jul 2024 10:40:38 +0200 Subject: [PATCH 16/16] Add CHANGELOG --- .../src/app/main/ui/workspace/tokens/CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md index 16a7ef809..35d40bdfb 100644 --- a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md +++ b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md @@ -18,8 +18,22 @@ If possible add video here from PR as well ## Changes +### 2024-07-01 - Disallow creating tokens at existing paths + +Disallow creating tokens at an existing path. + +[Video](https://github.com/tokens-studio/tokens-studio-for-penpot/assets/1898374/557990fe-efe7-445b-8a1d-824396049db7 +) + +Example: +We've got a token with `borderRadius.sm`, so we can't allow to create a token at `borderRadius` or `borderRadius.sm`. +But we can allow creating a token at `borderRadius.md`. + + ### 2024-06-26 - Disallow special characters in token name +- Only allows digits, letters and `-` as a part of a token name + [Video](https://github.com/tokens-studio/tokens-studio-for-penpot/assets/1898374/7c59c0cc-d6e1-4b0d-9646-9a27eafcccc4 )