diff --git a/CHANGES.md b/CHANGES.md index 74b309770f..a53289cb87 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -71,6 +71,7 @@ - Fix remove flex button doesn’t work within variant [Taiga #12314](https://tree.taiga.io/project/penpot/issue/12314) - Fix an error translation [Taiga #12402](https://tree.taiga.io/project/penpot/issue/12402) - Fix problem with certain text input in some editable labels (pages, components, tokens...) being in conflict with the drag/drop functionality [Taiga #12316](https://tree.taiga.io/project/penpot/issue/12316) +- Fix not controlled theme renaming [Taiga #12411](https://tree.taiga.io/project/penpot/issue/12411) ## 2.10.1 diff --git a/frontend/src/app/main/ui/workspace/tokens/management/create/form.cljs b/frontend/src/app/main/ui/workspace/tokens/management/create/form.cljs index 934bc89bed..7069cfd292 100644 --- a/frontend/src/app/main/ui/workspace/tokens/management/create/form.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/management/create/form.cljs @@ -29,7 +29,6 @@ [app.main.ui.ds.buttons.button :refer [button*]] [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] [app.main.ui.ds.controls.input :refer [input*]] - [app.main.ui.ds.controls.utilities.hint-message :refer [hint-message*]] [app.main.ui.ds.foundations.assets.icon :as i] [app.main.ui.ds.foundations.typography.heading :refer [heading*]] [app.main.ui.ds.notifications.context-notification :refer [context-notification*]] @@ -51,41 +50,6 @@ [malli.error :as me] [rumext.v2 :as mf])) -;; 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 %) (tr "workspace.tokens.token-name-validation-error"))}})) - -(defn token-name-schema - "Generate a dynamic schema validation to check if a token path derived from the name already exists at `tokens-tree`." - [{:keys [tokens-tree]}] - (let [path-exists-schema - (m/-simple-schema - {:type :token/name-exists - :pred #(not (cft/token-name-path-exists? % tokens-tree)) - :type-properties {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))}})] - (m/schema - [:and - [:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}] - valid-token-name-schema - path-exists-schema]))) - -(def token-description-schema - (m/schema - [:string {:max 2048 :error/fn #(tr "errors.field-max-length" 2048)}])) - ;; Helpers --------------------------------------------------------------------- (defn finalize-name [name] @@ -103,7 +67,53 @@ (defn valid-value? [value] (seq (finalize-value value))) -;; Validation ------------------------------------------------------------------ +;; Schemas --------------------------------------------------------------------- + +(def ^:private well-formed-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 ^:private well-formed-token-name-schema + (m/-simple-schema + {:type :token/invalid-token-name + :pred #(re-matches well-formed-token-name-regexp %) + :type-properties {:error/fn #(str (:value %) (tr "workspace.tokens.token-name-validation-error"))}})) + +(defn- token-name-schema + "Generate a dynamic schema validation to check if a token path derived from the name already exists at `tokens-tree`." + [{:keys [tokens-tree]}] + (let [path-exists-schema + (m/-simple-schema + {:type :token/name-exists + :pred #(not (cft/token-name-path-exists? % tokens-tree)) + :type-properties {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))}})] + (m/schema + [:and + [:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}] + well-formed-token-name-schema + path-exists-schema]))) + +(defn- validate-token-name + [tokens-tree name] + (let [schema (token-name-schema {:tokens-tree tokens-tree}) + validation (m/explain schema (finalize-name name))] + (me/humanize validation))) + +(def ^:private token-description-schema + (m/schema + [:string {:max 2048 :error/fn #(tr "errors.field-max-length" 2048)}])) + +(defn- validate-token-description + [description] + (let [validation (m/explain token-description-schema description)] + (me/humanize validation))) + +;; Value Validation ------------------------------------------------------------- (defn check-empty-value [token-value] (when (empty? (str/trim token-value)) @@ -330,33 +340,26 @@ warning-name-change? (deref warning-name-change*) token-name-ref (mf/use-var (:name token)) name-ref (mf/use-ref nil) - name-errors (mf/use-state nil) - - validate-name - (mf/use-fn - (mf/deps tokens-tree-in-selected-set) - (fn [value] - (let [schema (token-name-schema {:token token - :tokens-tree tokens-tree-in-selected-set})] - (m/explain schema (finalize-name value))))) + name-errors* (mf/use-state nil) + name-errors (deref name-errors*) on-blur-name (mf/use-fn - (mf/deps touched-name? warning-name-change?) + (mf/deps touched-name? warning-name-change? tokens-tree-in-selected-set) (fn [e] (let [value (dom/get-target-val e) - errors (validate-name value)] + errors (validate-token-name tokens-tree-in-selected-set value)] (when touched-name? (reset! warning-name-change* true)) - (reset! name-errors errors)))) + (reset! name-errors* errors)))) on-update-name-debounced (mf/use-fn - (mf/deps touched-name? validate-name) + (mf/deps touched-name? tokens-tree-in-selected-set) (uf/debounce (fn [token-name] - (let [errors (validate-name token-name)] + (let [errors (validate-token-name tokens-tree-in-selected-set token-name)] (when touched-name? - (reset! name-errors errors)))) + (reset! name-errors* errors)))) 300)) on-update-name @@ -370,7 +373,7 @@ (on-update-name-debounced token-name)))) valid-name-field? (and - (not @name-errors) + (not name-errors) (valid-name? @token-name-ref)) ;; Value @@ -430,19 +433,20 @@ description-errors* (mf/use-state nil) description-errors (deref description-errors*) - validate-descripion (mf/use-fn #(m/explain token-description-schema %)) - on-update-description-debounced (mf/use-fn - (uf/debounce (fn [e] - (let [value (dom/get-target-val e) - errors (validate-descripion value)] - (reset! description-errors* errors))))) + on-update-description-debounced + (mf/use-fn + (uf/debounce (fn [e] + (let [value (dom/get-target-val e) + errors (validate-token-description value)] + (reset! description-errors* errors))))) + on-update-description (mf/use-fn (mf/deps on-update-description-debounced) (fn [e] (reset! description-ref (dom/get-target-val e)) (on-update-description-debounced e))) - valid-description-field? (not description-errors) + valid-description-field? (empty? description-errors) ;; Form disabled? (or (not valid-name-field?) @@ -451,7 +455,7 @@ on-submit (mf/use-fn - (mf/deps is-create validate-name validate-descripion token active-theme-tokens validate-token) + (mf/deps is-create tokens-tree-in-selected-set token active-theme-tokens validate-token) (fn [e] (dom/prevent-default e) ;; We have to re-validate the current form values before submitting @@ -460,13 +464,13 @@ ;; and press enter before the next validations could return. (let [final-name (finalize-name @token-name-ref) valid-name? (try - (not (:errors (validate-name final-name))) + (empty? (:errors (validate-token-name tokens-tree-in-selected-set final-name))) (catch js/Error _ nil)) value (mf/ref-val value-ref) final-description @description-ref valid-description? (if final-description (try - (not (:errors (validate-descripion final-description))) + (empty? (:errors (validate-token-description final-description))) (catch js/Error _ nil)) true)] (when (and valid-name? valid-description?) @@ -560,21 +564,12 @@ :variant "comfortable" :auto-focus true :default-value @token-name-ref - :hint-type (when (seq (:errors @name-errors)) "error") + :hint-type (when-not (empty? name-errors) "error") + :hint-message (first name-errors) :ref name-ref :on-blur on-blur-name :on-change on-update-name}]) - (for [error (->> (:errors @name-errors) - (map #(-> (assoc @name-errors :errors [%]) - (me/humanize))) - (map first))] - - [:> hint-message* {:key error - :message error - :type "error" - :id "token-name-hint"}]) - (when (and warning-name-change? (= action "edit")) [:div {:class (stl/css :warning-name-change-notification-wrapper)} [:> context-notification* @@ -611,6 +606,8 @@ :max-length max-input-length :variant "comfortable" :default-value @description-ref + :hint-type (when-not (empty? description-errors) "error") + :hint-message (first description-errors) :on-blur on-update-description :on-change on-update-description}]] @@ -1162,4 +1159,4 @@ :text-case [:> text-case-form* props] :text-decoration [:> text-decoration-form* props] :font-weight [:> font-weight-form* props] - [:> form* props]))) \ No newline at end of file + [:> form* props]))) diff --git a/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.cljs b/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.cljs index dfbacfa370..576c6044c1 100644 --- a/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.cljs +++ b/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.cljs @@ -31,9 +31,32 @@ [app.util.i18n :refer [tr]] [app.util.keyboard :as k] [cuerdas.core :as str] + [malli.core :as m] + [malli.error :as me] [potok.v2.core :as ptk] [rumext.v2 :as mf])) +;; Schemas --------------------------------------------------------------------- + +(defn- theme-name-schema + "Generate a dynamic schema validation to check if a theme path derived from the name already exists at `tokens-tree`." + [{:keys [group theme-id tokens-lib]}] + (m/-simple-schema + {:type :token/name-exists + :pred (fn [name] + (let [theme (ctob/get-theme-by-name tokens-lib group name)] + (or (nil? theme) + (= (ctob/get-id theme) theme-id)))) + :type-properties {:error/fn #(tr "workspace.tokens.theme-name-already-exists")}})) + +(defn- validate-theme-name + [tokens-lib group theme-id name] + (let [schema (theme-name-schema {:tokens-lib tokens-lib :theme-id theme-id :group group}) + validation (m/explain schema (str/trim name))] + (me/humanize validation))) + +;; Form Component -------------------------------------------------------------- + (mf/defc empty-themes [{:keys [change-view]}] (let [create-theme @@ -166,25 +189,36 @@ (mf/defc theme-inputs* [{:keys [theme on-change-field]}] - (let [theme-groups (mf/deref refs/workspace-token-theme-groups) + (let [tokens-lib (mf/deref refs/tokens-lib) + theme-groups (mf/deref refs/workspace-token-theme-groups) theme-name-ref (mf/use-ref (:name theme)) - options (map (fn [group] - {:label group - :id group}) - theme-groups) + options (map (fn [group] + {:label group + :id group}) + theme-groups) + current-group* (mf/use-state (:group theme)) + current-group (deref current-group*) + name-errors* (mf/use-state nil) + name-errors (deref name-errors*) on-update-group (mf/use-fn (mf/deps on-change-field) - #(on-change-field :group %)) + (fn [value] + (reset! current-group* value) + (on-change-field :group value))) on-update-name (mf/use-fn - (mf/deps on-change-field) + (mf/deps on-change-field tokens-lib current-group) (fn [event] - (let [value (-> event dom/get-target dom/get-value)] - (on-change-field :name value) - (mf/set-ref-val! theme-name-ref value))))] + (let [value (-> event dom/get-target dom/get-value) + errors (validate-theme-name tokens-lib current-group (ctob/get-id theme) value)] + (reset! name-errors* errors) + (mf/set-ref-val! theme-name-ref value) + (if (empty? errors) + (on-change-field :name value) + (on-change-field :name "")))))] [:div {:class (stl/css :edit-theme-inputs-wrapper)} [:div {:class (stl/css :group-input-wrapper)} @@ -202,6 +236,8 @@ :variant "comfortable" :default-value (mf/ref-val theme-name-ref) :auto-focus true + :hint-type (when-not (empty? name-errors) "error") + :hint-message (first name-errors) :on-change on-update-name}]]])) (mf/defc theme-modal-buttons* diff --git a/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.scss b/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.scss index 456f0a28cc..c1cc8b59a4 100644 --- a/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.scss +++ b/frontend/src/app/main/ui/workspace/tokens/themes/create_modal.scss @@ -34,7 +34,6 @@ display: flex; flex-direction: column; gap: deprecated.$s-16; - max-height: deprecated.$s-688; } .edit-theme-form { diff --git a/frontend/test/frontend_tests/tokens/token_form_test.cljs b/frontend/test/frontend_tests/tokens/token_form_test.cljs index d42a325b0f..61ac67d934 100644 --- a/frontend/test/frontend_tests/tokens/token_form_test.cljs +++ b/frontend/test/frontend_tests/tokens/token_form_test.cljs @@ -12,15 +12,15 @@ (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"))) + (t/is (some? (m/validate wtf/well-formed-token-name-schema "Foo"))) + (t/is (some? (m/validate wtf/well-formed-token-name-schema "foo"))) + (t/is (some? (m/validate wtf/well-formed-token-name-schema "FOO"))) + (t/is (some? (m/validate wtf/well-formed-token-name-schema "Foo.Bar.Baz"))) ;; Allow trailing tokens - (t/is (nil? (m/validate wtf/valid-token-name-schema "Foo.Bar.Baz...."))) + (t/is (nil? (m/validate wtf/well-formed-token-name-schema "Foo.Bar.Baz...."))) ;; Disallow multiple separator dots - (t/is (nil? (m/validate wtf/valid-token-name-schema "Foo..Bar.Baz"))) + (t/is (nil? (m/validate wtf/well-formed-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")))) + (t/is (nil? (m/validate wtf/well-formed-token-name-schema "Hey Foo.Bar"))) + (t/is (nil? (m/validate wtf/well-formed-token-name-schema "Hey😈Foo.Bar"))) + (t/is (nil? (m/validate wtf/well-formed-token-name-schema "Hey%Foo.Bar")))) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 9ee33bc578..c76c43af56 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -7858,6 +7858,10 @@ msgstr "none | underline | strike-through or {alias}" msgid "workspace.tokens.theme-name" msgstr "Theme %s" +#: src/app/main/ui/workspace/tokens/themes/create_modal.cljs:48 +msgid "workspace.tokens.theme-name-already-exists" +msgstr "A theme with this name already exists" + #: src/app/main/ui/workspace/tokens/themes/create_modal.cljs:96 msgid "workspace.tokens.themes-description" msgstr "" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index c69e6921ae..2211b83947 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -7736,6 +7736,10 @@ msgstr "none | underline | strike-through o {alias}" msgid "workspace.tokens.theme-name" msgstr "Tema %s" +#: src/app/main/ui/workspace/tokens/themes/create_modal.cljs:48 +msgid "workspace.tokens.theme-name-already-exists" +msgstr "Ya existe un tema con este nombre" + #: src/app/main/ui/workspace/tokens/themes/create_modal.cljs:96 msgid "workspace.tokens.themes-description" msgstr ""