diff --git a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md index 519bac28a..35d40bdfb 100644 --- a/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md +++ b/frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md @@ -18,6 +18,27 @@ 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 +) + +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) diff --git a/frontend/src/app/main/ui/workspace/tokens/form.cljs b/frontend/src/app/main/ui/workspace/tokens/form.cljs index 9dd1930a4..9ac30f39d 100644 --- a/frontend/src/app/main/ui/workspace/tokens/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/form.cljs @@ -8,11 +8,13 @@ (: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] [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] @@ -22,20 +24,35 @@ ;; 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.")}})) + (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] - (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}})] + :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}] - non-existing-token-schema]))) + valid-token-name-schema + path-exists-schema]))) (def token-description-schema (m/schema @@ -44,7 +61,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)))) @@ -81,7 +100,7 @@ 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)] @@ -122,21 +141,24 @@ {::mf/wrap-props false} [{:keys [token token-type] :as _args}] (let [tokens (sd/use-resolved-workspace-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))))) + token-path (mf/use-memo + (mf/deps (:name token)) + #(wtt/token-name->path (:name token))) + tokens-tree (mf/use-memo + (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)) name-errors (mf/use-state nil) validate-name (mf/use-callback - (mf/deps existing-token-names) + (mf/deps tokens-tree) (fn [value] - (let [schema (token-name-schema 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 (debounce (fn [e] @@ -235,9 +257,12 @@ :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 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..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] @@ -82,14 +82,9 @@ (and (set? errors) (get errors :style-dictionary/missing-reference))) -(defn tokens-name-map [tokens] - (->> tokens - (map (fn [[_ x]] [(:name x) x])) - (into {}))) - (defn resolve-tokens+ [tokens & {:keys [debug?] :as config}] - (p/let [sd-tokens (-> (tokens-name-map tokens) + (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..ce288113d --- /dev/null +++ b/frontend/src/app/main/ui/workspace/tokens/token.cljs @@ -0,0 +1,63 @@ +(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-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] + (reduce + (fn [acc [_ {:keys [name] :as token}]] + (when (string? name) + (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 [{: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))) + 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_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")))) diff --git a/frontend/test/token_tests/token_test.cljs b/frontend/test/token_tests/token_test.cljs new file mode 100644 index 000000000..618dc9a21 --- /dev/null +++ b/frontend/test/token_tests/token_test.cljs @@ -0,0 +1,36 @@ +;; 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-test + (:require + [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"] (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-test + (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}"}}}} + (wtt/token-names-tree {:a {:name "foo.bar.baz" + :value "a"} + :b {:name "foo.bar.bam" + :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" {"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"}}))) + (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"}}}}))))