diff --git a/common/src/app/common/files/tokens.cljc b/common/src/app/common/files/tokens.cljc index e263d5401f..95ff5da4df 100644 --- a/common/src/app/common/files/tokens.cljc +++ b/common/src/app/common/files/tokens.cljc @@ -165,19 +165,62 @@ (some (fn [[token-name _]] (not (ctob/token-name-path-exists? token-name tokens-tree))) new-tokens))))]]) +(defn find-refs [value] + (prn value) + (cond + (string? value) + (cto/find-token-value-references value) + + (map? value) + (->> (vals value) + (keep :reference) + (mapcat cto/find-token-value-references)) + + :else + nil)) + +(defn token-circular-reference? + "Checks if the given `tokens` map contains a circular reference reachable from + `token-name`. Uses DFS with 3-color marking (:in-progress / :done) to detect + cycles without false positives on diamond dependencies (A->B, A->C, B->C). + Returns the token name that closes the cycle, or nil." + [tokens token-name] + (let [state (atom {})] + (letfn [(visit [name] + (let [mark (get @state name)] + (if (= mark :in-progress) + name + (when-not (= mark :done) + (swap! state assoc name :in-progress) + (let [token (get tokens name) + result (when token + (let [refs (find-refs (:value token))] + (some visit refs)))] + (swap! state assoc name :done) + result)))))] + (let [token (get tokens token-name)] + (when token + (let [refs (find-refs (:value token))] + (some visit refs))))))) (def schema:token-description [:string {:max 2048 :error/fn #(tr "errors.field-max-length" 2048)}]) (defn make-token-schema - [tokens-tree token-type] + [tokens-tree token-type current-token-path] [:and (sm/merge cto/schema:token-attrs [:map - [:name (make-token-name-schema tokens-tree)] + [:name (make-token-name-schema (-> tokens-tree + (d/dissoc-in current-token-path)))] [:value (make-token-value-schema token-type)] [:description {:optional true} schema:token-description]]) + [:fn {:error/field :value + :error/fn #(tr "errors.tokens.circular-reference")} + (fn [{:keys [name]}] + (when name + (not (token-circular-reference? tokens-tree name))))] [:fn {:error/field :value :error/fn #(tr "errors.tokens.self-reference")} (fn [{:keys [name value]}] diff --git a/frontend/playwright/ui/specs/tokens/crud.spec.js b/frontend/playwright/ui/specs/tokens/crud.spec.js index 3ce9047f3c..cb83a5b720 100644 --- a/frontend/playwright/ui/specs/tokens/crud.spec.js +++ b/frontend/playwright/ui/specs/tokens/crud.spec.js @@ -95,7 +95,7 @@ test.describe("Tokens - creation", () => { await toggleDropdownButton.click(); const option = page.getByRole("option", { name: "my-token" }); await expect(option).toBeVisible(); - const resolvedValue = option.getByText('3'); + const resolvedValue = option.getByText("3"); await expect(resolvedValue).toBeVisible(); await option.click(); await expect( @@ -156,7 +156,7 @@ test.describe("Tokens - creation", () => { await nameField.fill("my-token-2"); await valueField.fill("4 + 4"); - await expect( + await expect( tokensUpdateCreateModal.getByText("Resolved value: 8"), ).toBeVisible(); @@ -1956,8 +1956,9 @@ test.describe("User can't create groups that clash with token names", () => { await valueField.fill(value); // Check that the value has an error - const errorNode = - tokensUpdateCreateModal.getByText(`Group name of ${name} conflicts with a token of the same name in another active set.`); + const errorNode = tokensUpdateCreateModal.getByText( + `Group name of ${name} conflicts with a token of the same name in another active set.`, + ); await expect(errorNode).toBeVisible(); @@ -1968,10 +1969,12 @@ test.describe("User can't create groups that clash with token names", () => { await expect(submitButton).toBeDisabled(); }; - test("User can't create Border Radius token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Border Radius token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -1981,10 +1984,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Border Radius", "rad1.bad", "Value", "10"); }); - test("User can't create Color token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Color token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -1994,10 +1999,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Color", "col1.bad", "Value", "red"); }); - test("User can't create Dimensions token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Dimensions token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2007,10 +2014,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Dimensions", "dim1.bad", "Value", "100"); }); - test("User can't create Font Size token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Font Size token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2020,10 +2029,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Font Size", "fsiz1.bad", "Value", "16"); }); - test("User can't create Font Weight token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Font Weight token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2033,10 +2044,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Font Weight", "wei1.bad", "Value", "400"); }); - test("User can't create Letter Spacing token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Letter Spacing token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2046,10 +2059,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Letter Spacing", "lspa1.bad", "Value", "1"); }); - test("User can't create Number token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Number token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2059,10 +2074,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Number", "num1.bad", "Value", "10"); }); - test("User can't create Rotation token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Rotation token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2072,10 +2089,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Rotation", "rot1.bad", "Value", "90"); }); - test("User can't create Sizing token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Sizing token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2085,10 +2104,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Sizing", "siz1.bad", "Value", "100"); }); - test("User can't create Spacing token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Spacing token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2098,10 +2119,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Spacing", "spa1.bad", "Value", "10"); }); - test("User can't create Stroke Width token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Stroke Width token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2111,10 +2134,12 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Stroke Width", "str1.bad", "Value", "2"); }); - test("User can't create Text Case token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Text Case token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2124,23 +2149,33 @@ test.describe("User can't create groups that clash with token names", () => { await createBadToken(page, "Text Case", "cas1.bad", "Value", "uppercase"); }); -test("User can't create Text Decoration token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Text Decoration token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar, tokenContextMenuForToken } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); await createSet(tokenThemesSetsSidebar, "Second set"); - await createBadToken(page, "Text Decoration", "dec1.bad", "Value", "strike-through"); + await createBadToken( + page, + "Text Decoration", + "dec1.bad", + "Value", + "strike-through", + ); }); - test("User can't create Typography token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Typography token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2200,15 +2235,19 @@ test("User can't create Text Decoration token with group name that clashes with name: "Save", }); - const errorNode = tokensUpdateCreateModal.getByText("Group name of typ1.bad conflicts with a token of the same name in another active set."); + const errorNode = tokensUpdateCreateModal.getByText( + "Group name of typ1.bad conflicts with a token of the same name in another active set.", + ); await expect(errorNode).toHaveCount(1); await expect(submitButton).toBeDisabled(); }); - test("User can't create Shadow token with group name that clashes with existing token", async ({ page }) => { + test("User can't create Shadow token with group name that clashes with existing token", async ({ + page, + }) => { const { tokenThemesSetsSidebar, tokensSidebar } = await setupTokensFileRender(page, { - file: "workspace/get-file-tokens-all-types.json" + file: "workspace/get-file-tokens-all-types.json", }); await expect(tokensSidebar).toBeVisible(); @@ -2258,7 +2297,9 @@ test("User can't create Text Decoration token with group name that clashes with name: "Save", }); - const errorNode = tokensUpdateCreateModal.getByText("Group name of sha1.bad conflicts with a token of the same name in another active set."); + const errorNode = tokensUpdateCreateModal.getByText( + "Group name of sha1.bad conflicts with a token of the same name in another active set.", + ); await expect(errorNode).toHaveCount(1); await expect(submitButton).toBeDisabled(); }); @@ -2306,10 +2347,19 @@ test.describe("Tokens tab - edition", () => { // Show error with line-height depending on invalid font-size await fontSizeField.fill(""); + await page.keyboard.press("Enter"); await expect(saveButton).toBeDisabled(); + await expect( + tokensUpdateCreateModal.getByText(/Invalid token value:/), + ).not.toBeVisible(); + + await expect( + tokensUpdateCreateModal.getByText(/Line Height depends on Font Size/), + ).toBeVisible(); // Fill in values for all fields and verify they persist when switching tabs - await fontSizeField.fill("16"); + await fontSizeField.fill("16 + 3"); + await expect(saveButton).toBeEnabled(); const fontWeightField = tokensUpdateCreateModal.getByRole("textbox", { @@ -2462,6 +2512,139 @@ test.describe("Tokens tab - edition", () => { await valueSaturationSelector.click({ position: { x: 0, y: 0 } }); await expect(valueField).toHaveValue(/^rgba(.*)$/); }); + + test("User sees self-reference error when editing a token to create a circular reference", async ({ + page, + }) => { + const { tokensUpdateCreateModal, tokensSidebar, tokenContextMenuForToken } = + await setupEmptyTokensFileRender(page, { + flags: ["enable-token-combobox", "enable-feature-token-input"], + }); + + const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" }); + + // Create first token "base" with value "10" + const addTokenButton = tokensTabPanel.getByRole("button", { + name: "Add Token: Border Radius", + }); + await addTokenButton.click(); + await expect(tokensUpdateCreateModal).toBeVisible(); + await createToken(page, "Border radius", "base", "Value", "combobox", "10"); + + await unfoldTokenType(tokensTabPanel, "border radius"); + await expect( + tokensTabPanel.getByRole("button", { name: "base" }), + ).toBeEnabled(); + + // Create second token "linear" referencing "base" + await createToken( + page, + "Border radius", + "linear", + "Value", + "combobox", + "{base} * 0.25", + ); + + await expect( + tokensTabPanel.getByRole("button", { name: "linear" }), + ).toBeEnabled(); + + // Open the edit modal for "base" + const baseToken = tokensTabPanel.getByRole("button", { name: "base" }); + await expect(baseToken).toBeVisible(); + await baseToken.click({ button: "right" }); + await expect(tokenContextMenuForToken).toBeVisible(); + const editOption = tokenContextMenuForToken + .getByRole("listitem") + .filter({ hasText: "Edit token" }); + await expect(editOption).toBeVisible(); + await editOption.click(); + await expect(tokensUpdateCreateModal).toBeVisible(); + + // Change value to reference "linear.0.25", creating a circular reference + const editValueField = tokensUpdateCreateModal.getByRole("combobox", { + name: "Value", + }); + await editValueField.fill("{linear}"); + + // The circular reference error should appear gracefully + const circularError = tokensUpdateCreateModal.getByText( + "Token has circular reference", + ); + await expect(circularError).toBeVisible({ timeout: 5000 }); + + // Save button should be disabled + const editSubmitButton = tokensUpdateCreateModal.getByRole("button", { + name: "Save", + }); + await expect(editSubmitButton).toBeDisabled(); + }); + + test("User sees self-reference error for a three-step circular reference (A → B → C → A)", async ({ + page, + }) => { + const { tokensUpdateCreateModal, tokensSidebar, tokenContextMenuForToken } = + await setupEmptyTokensFileRender(page, { + flags: ["enable-token-combobox", "enable-feature-token-input"], + }); + + const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" }); + + // Create first token "a" with value "10" + const addTokenButton = tokensTabPanel.getByRole("button", { + name: "Add Token: Border Radius", + }); + + await createToken( + page, + "Border radius", + "First", + "Value", + "combobox", + "10", + ); + await createToken( + page, + "Border radius", + "Second", + "Value", + "combobox", + "{First}", + ); + await createToken( + page, + "Border radius", + "Third", + "Value", + "combobox", + "{Second}", + ); + + // Edit "First" to reference "Third", completing the cycle A → B → C → A + const tokenA = tokensTabPanel.getByRole("button", { name: "First" }); + await tokenA.click({ button: "right" }); + await expect(tokenContextMenuForToken).toBeVisible(); + await tokenContextMenuForToken.getByText("Edit token").click(); + await expect(tokensUpdateCreateModal).toBeVisible(); + + const editValueField = tokensUpdateCreateModal.getByRole("combobox", { + name: "Value", + }); + await editValueField.fill("{Third}"); + + // The circular reference error should appear gracefully + const circularError = tokensUpdateCreateModal.getByText( + "Token has circular reference", + ); + await expect(circularError).toBeVisible({ timeout: 5000 }); + + // Save button should be disabled + const editSubmitButton = tokensUpdateCreateModal.getByRole("button", { + name: "Save", + }); + await expect(editSubmitButton).toBeDisabled(); + }); }); test.describe("Tokens tab - delete", () => { @@ -2496,7 +2679,14 @@ test("BUG: 14262 Token pill must be highlighted when value references a token in await expect(tokensSidebar).toBeVisible(); await unfoldTokenType(tokensSidebar, "Border radius"); - await createToken(page, "Border radius", "base-radius", "Value", "textbox", "20"); + await createToken( + page, + "Border radius", + "base-radius", + "Value", + "textbox", + "20", + ); await createToken( page, "Border radius", @@ -2529,7 +2719,14 @@ test("BUG: 14262 Token pill must be highlighted when value references a token in .getByRole("button", { name: "New set" }) .getByRole("checkbox") .click(); - await createToken(page, "Border radius", "new-ref", "Value", "textbox", "{base-radius}"); + await createToken( + page, + "Border radius", + "new-ref", + "Value", + "textbox", + "{base-radius}", + ); // Pill is highlighted if the referenced token is in a different disabled set than the token with the reference const newBrokenTokenPill = tokensSidebar.getByRole("button", { diff --git a/frontend/playwright/ui/specs/tokens/helpers.js b/frontend/playwright/ui/specs/tokens/helpers.js index 4280c574e1..8418c2b6a2 100644 --- a/frontend/playwright/ui/specs/tokens/helpers.js +++ b/frontend/playwright/ui/specs/tokens/helpers.js @@ -76,6 +76,7 @@ const setupEmptyTokensFileRender = async (page, options = {}) => { tokenSetItems: workspacePage.tokenSetItems, tokensSidebar: workspacePage.tokensSidebar, tokenSetGroupItems: workspacePage.tokenSetGroupItems, + tokenContextMenuForToken: workspacePage.tokenContextMenuForToken, tokenContextMenuForSet: workspacePage.tokenContextMenuForSet, }; }; diff --git a/frontend/src/app/main/data/style_dictionary.cljs b/frontend/src/app/main/data/style_dictionary.cljs index ebf7aedc6e..446e130bdb 100644 --- a/frontend/src/app/main/data/style_dictionary.cljs +++ b/frontend/src/app/main/data/style_dictionary.cljs @@ -108,7 +108,7 @@ {:errors [(wte/error-with-value :error.style-dictionary/invalid-token-value value)]}))) (defn- parse-sd-token-general-value - "Parses `value` of a number `sd-token` into a map like `{:value 1 :unit \"px\"}`. + "Parses `value` of a `sd-token` into a map like `{:value 1 :unit \"px\"}`. If the `value` is not parseable and/or has missing references returns a map with `:errors`." [value] (let [parsed-value (cfo/parse-token-value value) diff --git a/frontend/src/app/main/data/workspace/tokens/errors.cljs b/frontend/src/app/main/data/workspace/tokens/errors.cljs index 966de5cca0..e5716f07be 100644 --- a/frontend/src/app/main/data/workspace/tokens/errors.cljs +++ b/frontend/src/app/main/data/workspace/tokens/errors.cljs @@ -44,6 +44,10 @@ {:error/code :error.token/direct-self-reference :error/fn #(tr "errors.tokens.self-reference")} + :error.token/circular-reference + {:error/code :error.token/circular-reference + :error/fn #(tr "errors.tokens.circular-reference")} + :error.token/invalid-color {:error/code :error.token/invalid-color :error/fn #(str (tr "errors.tokens.invalid-color" %))} diff --git a/frontend/src/app/main/ui/forms.cljs b/frontend/src/app/main/ui/forms.cljs index 67b56174ed..82e072aab9 100644 --- a/frontend/src/app/main/ui/forms.cljs +++ b/frontend/src/app/main/ui/forms.cljs @@ -79,7 +79,6 @@ (or (nil? form) (true? disabled) (not (:valid form-state)) - (seq (:async-errors form-state)) (seq (:extra-errors form-state)))))) handle-key-down-save 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 1c9d3cb10f..b208852c12 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 @@ -13,7 +13,7 @@ [rumext.v2 :as mf])) (mf/defc form* - [{:keys [token token-type] :as props}] + [{:keys [token token-type current-token-path] :as props}] (let [initial (mf/with-memo [token-type token] {:type token-type @@ -22,7 +22,7 @@ :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 current-token-path) (sm/dissoc-key :id) (sm/assoc-key :color-result :string)) :initial initial diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs index 4399040099..a655d0756a 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/color_input.cljs @@ -9,6 +9,7 @@ (:require [app.common.data :as d] [app.common.data.macros :as dm] + [app.common.files.tokens :as cfo] [app.common.types.color :as cl] [app.common.types.token :as cto] [app.common.types.tokens-lib :as ctob] @@ -72,19 +73,21 @@ ;; Remove previous token when renaming a token (dissoc (:name prev-token)) (update (:name token) #(ctob/make-token (merge % prev-token token))))] - - (->> (if (contains? cf/flags :tokenscript) - (rx/of (ts/resolve-tokens tokens)) - (sd/resolve-tokens-interactive tokens)) - (rx/mapcat - (fn [resolved-tokens] - (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) - resolved-value (if (contains? cf/flags :tokenscript) - (ts/tokenscript-symbols->penpot-unit resolved-value) - resolved-value)] - (if resolved-value - (rx/of {:value resolved-value}) - (rx/of {:error (first errors)})))))))) + ;; TODO: Review this when tokenscript is fully integrated. + (if (cfo/token-circular-reference? tokens (:name token)) + (rx/of {:error (wte/error-with-value :error.token/circular-reference nil)}) + (->> (if (contains? cf/flags :tokenscript) + (rx/of (ts/resolve-tokens tokens)) + (sd/resolve-tokens-interactive tokens)) + (rx/mapcat + (fn [resolved-tokens] + (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) + resolved-value (if (contains? cf/flags :tokenscript) + (ts/tokenscript-symbols->penpot-unit resolved-value) + resolved-value)] + (if resolved-value + (rx/of {:value resolved-value}) + (rx/of {:error (first errors)}))))))))) (defn- hex->color-obj [hex] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs index c69d877aaf..7c2e6b65dd 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/combobox.cljs @@ -8,6 +8,7 @@ (:require-macros [app.main.style :as stl]) (:require [app.common.data :as d] + [app.common.files.tokens :as cfo] [app.common.types.token :as cto] [app.common.types.tokens-lib :as ctob] [app.config :as cf] @@ -49,21 +50,23 @@ ;; Remove previous token when renaming a token (dissoc (:name prev-token)) (update (:name token) #(ctob/make-token (merge % prev-token token))))] - - (->> (if (contains? cf/flags :tokenscript) - (rx/of (ts/resolve-tokens tokens)) - (sd/resolve-tokens-interactive tokens)) - (rx/mapcat - (fn [resolved-tokens] - (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) - resolved-value (if (contains? cf/flags :tokenscript) - (ts/tokenscript-symbols->penpot-unit resolved-value) - resolved-value)] - (if resolved-value - (rx/of {:value resolved-value}) - (rx/of {:error (if errors - (first errors) - (wte/error-with-value :error/unknown value))})))))))) + ;; TODO: Review this when tokenscript is fully integrated. + (if (cfo/token-circular-reference? tokens (:name token)) + (rx/of {:error (wte/error-with-value :error.token/circular-reference nil)}) + (->> (if (contains? cf/flags :tokenscript) + (rx/of (ts/resolve-tokens tokens)) + (sd/resolve-tokens-interactive tokens)) + (rx/mapcat + (fn [resolved-tokens] + (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) + resolved-value (if (contains? cf/flags :tokenscript) + (ts/tokenscript-symbols->penpot-unit resolved-value) + resolved-value)] + (if resolved-value + (rx/of {:value resolved-value}) + (rx/of {:error (if errors + (first errors) + (wte/error-with-value :error/unknown value))}))))))))) (mf/defc value-combobox* [{:keys [name tokens token token-type empty-to-end ref] :rest props}] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs index cde4c74f42..a0eabbdeba 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/fonts_combobox.cljs @@ -8,6 +8,7 @@ (:require-macros [app.main.style :as stl]) (:require [app.common.data :as d] + [app.common.files.tokens :as cfo] [app.common.types.token :as cto] [app.common.types.tokens-lib :as ctob] [app.config :as cf] @@ -67,21 +68,23 @@ tokens (update tokens (:name token) #(ctob/make-token (merge % prev-token token)))] - - (->> (if (contains? cf/flags :tokenscript) - (rx/of (ts/resolve-tokens tokens)) - (sd/resolve-tokens-interactive tokens)) - (rx/mapcat - (fn [resolved-tokens] - (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) - resolved-value (if (contains? cf/flags :tokenscript) - (ts/tokenscript-symbols->penpot-unit resolved-value) - resolved-value)] - (if resolved-value - (rx/of {:value resolved-value}) - (rx/of {:error (if errors - (first errors) - (wte/error-with-value :error/unknown value))})))))))) + ;; TODO: Review this when tokenscript is fully integrated. + (if (cfo/token-circular-reference? tokens (:name token)) + (rx/of {:error (wte/error-with-value :error.token/circular-reference nil)}) + (->> (if (contains? cf/flags :tokenscript) + (rx/of (ts/resolve-tokens tokens)) + (sd/resolve-tokens-interactive tokens)) + (rx/mapcat + (fn [resolved-tokens] + (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) + resolved-value (if (contains? cf/flags :tokenscript) + (ts/tokenscript-symbols->penpot-unit resolved-value) + resolved-value)] + (if resolved-value + (rx/of {:value resolved-value}) + (rx/of {:error (if errors + (first errors) + (wte/error-with-value :error/unknown value))}))))))))) (mf/defc fonts-combobox* [{:keys [token tokens name] :rest props}] diff --git a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs index a1411ca73d..aed66af134 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/forms/controls/input.cljs @@ -171,20 +171,22 @@ ;; Remove previous token when renaming a token (dissoc (:name prev-token)) (update (:name token) #(ctob/make-token (merge % prev-token token))))] - - (->> tokens - (sd/resolve-tokens-interactive) - (rx/mapcat - (fn [resolved-tokens] - (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) - resolved-value (if (contains? cf/flags :tokenscript) - (ts/tokenscript-symbols->penpot-unit resolved-value) - resolved-value)] - (if resolved-value - (rx/of {:value resolved-value}) - (rx/of {:error (if errors - (first errors) - (wte/error-with-value :error/unknown value))})))))))) + ;; TODO: Review this when tokenscript is fully integrated. + (if (cft/token-circular-reference? tokens (:name token)) + (rx/of {:error (wte/error-with-value :error.token/circular-reference nil)}) + (->> tokens + (sd/resolve-tokens-interactive) + (rx/mapcat + (fn [resolved-tokens] + (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens (:name token)) + resolved-value (if (contains? cf/flags :tokenscript) + (ts/tokenscript-symbols->penpot-unit resolved-value) + resolved-value)] + (if resolved-value + (rx/of {:value resolved-value}) + (rx/of {:error (if errors + (first errors) + (wte/error-with-value :error/unknown value))}))))))))) (mf/defc input* [{:keys [name tokens token] :rest props}] @@ -325,8 +327,12 @@ :variant "comfortable" :hint-message (:message hint) :hint-type (:type hint)}) + + ;; On Typography composite tokens, line-height depends on font-size. If a typography + ;; token contains a line-height but no font-size, validation should fail and surface + ;; the corresponding error so the user understands why submission is blocked. props - (if (or extra-error (and touched? error)) + (if (or extra-error (and touched? error) (and (= :line-height input-name) error)) (mf/spread-props props {:hint-type "error" :hint-message (:message (or error extra-error))}) props) @@ -375,7 +381,6 @@ message (tr "workspace.tokens.resolved-value" (or resolved-value value))] (swap! form update :errors dissoc :value) (swap! form update :extra-errors dissoc :value) - (swap! form update :async-errors dissoc :reference) (if (= input-value (str resolved-value)) (reset! hint* {}) (reset! hint* {:message message :type "hint"})))))))] 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 d30e72964d..0168121331 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 @@ -29,7 +29,7 @@ (default-validate-token))) (mf/defc form* - [{:keys [token token-type] :rest props}] + [{:keys [token token-type current-token-path] :rest props}] (let [token (mf/with-memo [token] (if token @@ -37,7 +37,7 @@ {: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 current-token-path) (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 a54ed8142a..31fc0b3c35 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,7 +6,6 @@ (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] @@ -35,15 +34,15 @@ (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))) + (mf/with-memo [tokens-in-selected-set] + (ctob/tokens-tree tokens-in-selected-set)) props (mf/spread-props props {:token-type token-type :initial-errors initial-errors :tokens-tree-in-selected-set tokens-tree-in-selected-set :selected-token-set-id selected-token-set-id + :current-token-path token-path :token token}) props @@ -68,5 +67,4 @@ :text-case [:> generic/form* text-case-props] :text-decoration [:> generic/form* text-decoration-props] :font-weight [:> generic/form* font-weight-props] - :border-radius [:> generic/form* props] [:> generic/form* props]))) 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 9e60baf431..efc0048f82 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 @@ -77,9 +77,10 @@ initial-errors value-type value-subfield - input-value-placeholder] :as props}] + input-value-placeholder + current-token-path] :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 current-token-path) (sm/dissoc-key :id))) input-component (or input-component token.controls/input*) validate-token (or validator default-validate-token) @@ -165,9 +166,9 @@ (fn [new-tab] (let [new-tab (keyword new-tab)] (if (= new-tab :reference) - (swap! form assoc-in [:async-errors :reference] + (swap! form assoc-in [:errors :reference] {:message "Need valid reference"}) - (swap! form update :async-errors dissoc :reference)) + (swap! form update :errors dissoc :reference)) (reset! active-tab* new-tab)))) on-cancel 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 c69ec04a76..41e0c2ce88 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 @@ -260,7 +260,7 @@ ;; TODO: use cfo/make-schema:token-value and extend it with shadow and reference fields (defn- make-schema - [tokens-tree active-tab] + [current-token-path tokens-tree active-tab] (sm/schema [:and [:map @@ -271,7 +271,8 @@ (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-tree + (d/dissoc-in current-token-path))))]]] [:value [:map @@ -349,7 +350,8 @@ (mf/defc form* [{:keys [token - token-type] :as props}] + token-type + current-token-path] :as props}] (let [token (mf/with-memo [token] (or token @@ -372,7 +374,7 @@ props (mf/spread-props props {:token token :token-type token-type :initial initial - :make-schema make-schema + :make-schema (partial make-schema current-token-path) :value-type :indexed :value-subfield :shadow :input-component tabs-wrapper* 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 72b43f1685..62b9a63410 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 @@ -8,9 +8,9 @@ (:require-macros [app.main.style :as stl]) (:require [app.common.data :as d] + [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.components.radio-buttons :refer [radio-button radio-buttons]] [app.main.ui.ds.foundations.assets.icon :as i] @@ -209,19 +209,12 @@ ;; TODO: use cfo/make-schema:token-value and extend it with typography and reference fields (defn- make-schema - [tokens-tree active-tab] + [current-token-path tokens-tree active-tab] (sm/schema [:and [:map - [:name - [:and - [:string {:min 1 :max 255 - :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}] - (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))]]] - + [:name (cfo/make-token-name-schema (-> tokens-tree + (d/dissoc-in current-token-path)))] [:value [:map [:font-family {:optional true} [:maybe :string]] @@ -269,7 +262,7 @@ result))]])) (mf/defc form* - [{:keys [token] :as props}] + [{:keys [token current-token-path] :as props}] (let [initial (mf/with-memo [token] (let [value (:value token) @@ -297,7 +290,7 @@ :value processed-value :description (:description token "")})) props (mf/spread-props props {:initial initial - :make-schema make-schema + :make-schema (partial make-schema current-token-path) :token token :validator validate-typography-token :value-type :composite diff --git a/frontend/src/app/plugins/tokens.cljs b/frontend/src/app/plugins/tokens.cljs index a895d06e56..13ea39e6d0 100644 --- a/frontend/src/app/plugins/tokens.cljs +++ b/frontend/src/app/plugins/tokens.cljs @@ -317,7 +317,8 @@ (ctob/tokens-tree))] [:tuple (-> (cfo/make-token-schema tokens-tree - (cto/dtcg-token-type->token-type (-> args (first) (get "type")))) + (cto/dtcg-token-type->token-type (-> args (first) (get "type"))) + nil) ;; 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) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index a1a6e33197..12aa2313b1 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1896,6 +1896,10 @@ msgstr "Opacity must be between 0 and 100% or 0 and 1 (e.g. 50% or 0.5)." msgid "errors.tokens.self-reference" msgstr "Token has self reference" +#: src/app/common/files/tokens.cljc +msgid "errors.tokens.circular-reference" +msgstr "Token has circular reference" + #: src/app/main/data/workspace/tokens/errors.cljs:124 msgid "errors.tokens.shadow-blur-range" msgstr "Shadow blur must be greater than or equal to 0." diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 90864167e8..a2d4b750a7 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -1854,6 +1854,10 @@ msgstr "La opacidad debe estar entre 0 y 100% o 0 y 1 (p.e. 50% o 0.5)." msgid "errors.tokens.self-reference" msgstr "El token tiene una autoreferencia" +#: src/app/common/files/tokens.cljc +msgid "errors.tokens.circular-reference" +msgstr "El token tiene una referencia circular" + #: src/app/main/data/workspace/tokens/errors.cljs:124 msgid "errors.tokens.shadow-blur-range" msgstr "El desenfoque (blur) de la sombra debe ser mayor o igual a 0."