From cbb9e5d971cf1394b1e85cb63b06e8b68e61d36e Mon Sep 17 00:00:00 2001 From: Alonso Torres Date: Wed, 23 Sep 2026 19:59:22 +0200 Subject: [PATCH] :sparkles: Add end-to-end tests for plugins validation (#11587) * :sparkles: Add missing plugin data validations * :sparkles: Add migration to fix the new schema validations * :sparkles: Add end-to-end tests for plugins validation * :bug: Fix unit tests after merge * :bug: Change normalize behavior --- common/src/app/common/files/migrations.cljc | 106 ++++++++- common/src/app/common/files/tokens.cljc | 27 ++- common/src/app/common/schema.cljc | 14 ++ common/src/app/common/types/grid.cljc | 16 +- common/src/app/common/types/page.cljc | 21 +- common/src/app/common/types/shape.cljc | 18 +- .../common/types/shape/background_blur.cljc | 4 +- common/src/app/common/types/shape/blur.cljc | 2 +- common/src/app/common/types/shape/export.cljc | 2 +- .../app/common/types/shape/interactions.cljc | 53 ++++- common/src/app/common/types/shape/layout.cljc | 24 +- common/src/app/common/types/shape/shadow.cljc | 3 +- common/src/app/common/types/text.cljc | 34 +++ common/src/app/common/types/token.cljc | 10 +- common/src/app/common/types/variant.cljc | 32 ++- .../common_tests/files_migrations_test.cljc | 133 +++++++++++ .../types/shape_interactions_test.cljc | 18 ++ frontend/src/app/main/data/comments.cljs | 9 +- .../app/main/data/workspace/libraries.cljs | 11 + .../data/workspace/tokens/application.cljs | 30 +-- .../src/app/main/data/workspace/variants.cljs | 40 +++- frontend/src/app/main/ui/comments.cljs | 6 +- .../sidebar/options/menus/component.cljs | 31 +-- .../sidebar/options/menus/interactions.cljs | 14 +- .../sidebar/options/menus/typography.cljs | 12 +- .../main/ui/workspace/sidebar/sitemap.cljs | 4 +- frontend/src/app/plugins/api.cljs | 25 +-- frontend/src/app/plugins/comments.cljs | 4 +- frontend/src/app/plugins/flex.cljs | 24 +- frontend/src/app/plugins/fonts.cljs | 17 +- frontend/src/app/plugins/grid.cljs | 30 +-- frontend/src/app/plugins/library.cljs | 202 ++++++++++------- frontend/src/app/plugins/page.cljs | 73 +++--- frontend/src/app/plugins/parser.cljs | 3 +- frontend/src/app/plugins/shape.cljs | 122 ++++++---- frontend/src/app/plugins/text.cljs | 33 ++- frontend/src/app/plugins/tokens.cljs | 175 +++++++++++---- frontend/src/app/plugins/tracks.cljs | 2 +- .../plugins/interactions_test.cljs | 20 ++ .../frontend_tests/plugins/page_test.cljs | 37 ++-- .../plugins/shape_bugfixes_test.cljs | 2 + .../frontend_tests/plugins/text_test.cljs | 51 +++-- .../frontend_tests/plugins/tokens_test.cljs | 33 ++- .../tokens/logic/token_actions_test.cljs | 31 +++ .../src/tests/comments.test.ts | 19 ++ .../src/tests/components.test.ts | 31 ++- .../src/tests/file.test.ts | 4 +- .../src/tests/fills-strokes.test.ts | 16 +- .../src/tests/fonts.test.ts | 13 ++ .../src/tests/interactions.test.ts | 196 ++++++++++++++-- .../src/tests/layout.test.ts | 112 +++++++++- .../src/tests/library.test.ts | 79 ++++++- .../src/tests/pages.test.ts | 75 +++++++ .../src/tests/shadows-blur.test.ts | 40 ++++ .../src/tests/shapes-geometry.test.ts | 34 ++- .../src/tests/text.test.ts | 57 +++++ .../src/tests/tokens.test.ts | 209 +++++++++++++++++- .../src/tests/value-objects.test.ts | 6 +- .../src/tests/variants.test.ts | 111 +++++++++- .../src/tests/viewport-guides.test.ts | 108 +++++++++ 60 files changed, 2190 insertions(+), 478 deletions(-) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index cdc990a746..8542a559c4 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -36,6 +36,7 @@ [app.common.types.shape.text :as ctst] [app.common.types.text :as types.text] [app.common.types.tokens-lib :as ctob] + [app.common.types.variant :as ctv] [app.common.uuid :as uuid] [clojure.set :as set] [cuerdas.core :as str])) @@ -2006,6 +2007,108 @@ (cfo/make-tokens-status-from-lib tokens-lib)) data)) +(defmethod migrate-data "0028-normalize-constrained-values" + ;; Existing files can contain values outside the limits now shared by the UI + ;; and file schemas. Normalize them before checking the migrated file. + [data _] + (letfn [(clamp-minimum [value minimum] + (if (number? value) + (max value minimum) + value)) + + (positive-or-default [value default] + (if (and (number? value) (not (pos? value))) + default + value)) + + (clamp-attrs [value attrs] + (reduce #(d/update-when %1 %2 clamp-minimum 0) value attrs)) + + (repair-vector [value repair-item] + (if (vector? value) + (mapv repair-item value) + value)) + + (repair-grid-params [params type] + (cond + (= type :square) + (d/update-when params :size clamp-minimum 0.01) + + (#{:row :column} type) + (d/update-when params :size clamp-minimum 1) + + :else + params)) + + (repair-grid [grid] + (d/update-when grid :params repair-grid-params (:type grid))) + + (repair-default-grids [grids] + (-> grids + (d/update-when :square repair-grid-params :square) + (d/update-when :row repair-grid-params :row) + (d/update-when :column repair-grid-params :column))) + + (repair-grid-track [track] + (d/update-when track :value clamp-minimum 0)) + + (repair-export [export] + (d/update-when export :scale positive-or-default 1)) + + (repair-stroke [stroke] + (clamp-attrs stroke [:stroke-width + :stroke-width-top + :stroke-width-right + :stroke-width-bottom + :stroke-width-left])) + + (repair-shadow [shadow] + (d/update-when shadow :blur clamp-minimum 0)) + + (repair-blur [blur] + (d/update-when blur :value clamp-minimum 0)) + + (repair-shape [shape] + (-> shape + (clamp-attrs [:r1 :r2 :r3 :r4 + :layout-item-min-w :layout-item-max-w + :layout-item-min-h :layout-item-max-h]) + (d/update-when :layout-gap clamp-attrs [:row-gap :column-gap]) + (d/update-when :layout-padding clamp-attrs [:p1 :p2 :p3 :p4]) + (d/update-when :layout-grid-rows repair-vector repair-grid-track) + (d/update-when :layout-grid-columns repair-vector repair-grid-track) + (d/update-when :strokes repair-vector repair-stroke) + (d/update-when :shadow repair-vector repair-shadow) + (d/update-when :blur repair-blur) + (d/update-when :background-blur repair-blur) + (d/update-when :exports repair-vector repair-export) + (d/update-when :grids repair-vector repair-grid))) + + (truncate-property-text [value] + (if (and (string? value) + (> (count value) ctv/property-max-length)) + (subs value 0 ctv/property-max-length) + value)) + + (repair-variant-property [property] + (-> property + (d/update-when :name truncate-property-text) + (d/update-when :value truncate-property-text))) + + (repair-container [container] + (-> container + (d/update-when :objects d/update-vals repair-shape) + (d/update-when :variant-properties repair-vector repair-variant-property))) + + (repair-page [page] + (-> page + (repair-container) + (d/update-when :default-grids repair-default-grids)))] + + (-> data + (update :pages-index d/update-vals repair-page) + (d/update-when :components d/update-vals repair-container)))) + (def available-migrations (into (d/ordered-set) ["legacy-2" @@ -2090,4 +2193,5 @@ "0024b-fix-stroke-cap-placement" "0025-repair-empty-text-content" "0026-fix-svg-raw-shapes-uuids" - "0027-separate-tokens-status"])) + "0027-separate-tokens-status" + "0028-normalize-constrained-values"])) diff --git a/common/src/app/common/files/tokens.cljc b/common/src/app/common/files/tokens.cljc index bdffdb686f..a9506f938e 100644 --- a/common/src/app/common/files/tokens.cljc +++ b/common/src/app/common/files/tokens.cljc @@ -81,15 +81,20 @@ ::sm/text]) ;; Leave references or formulas to be checked by the resolver (def schema:token-value-typography-map - [:map - [:font-family {:optional true} schema:token-value-font-family] - [:font-size {:optional true} schema:token-value-numeric] - [:font-weight {:optional true} schema:token-value-font-weight] - [:line-height {:optional true} schema:token-value-percent] - [:letter-spacing {:optional true} schema:token-value-generic] - [:paragraph-spacing {:optional true} schema:token-value-generic] - [:text-decoration {:optional true} schema:token-value-generic] - [:text-case {:optional true} schema:token-value-generic]]) + [:and + [:map + [:font-family {:optional true} schema:token-value-font-family] + [:font-size {:optional true} schema:token-value-numeric] + [:font-weight {:optional true} schema:token-value-font-weight] + [:line-height {:optional true} schema:token-value-percent] + [:letter-spacing {:optional true} schema:token-value-generic] + [:paragraph-spacing {:optional true} schema:token-value-generic] + [:text-decoration {:optional true} schema:token-value-generic] + [:text-case {:optional true} schema:token-value-generic]] + [:fn (fn [value] + (and (seq value) + (or (not (contains? value :line-height)) + (contains? value :font-size))))]]) (def schema:token-value-typography [:or @@ -97,7 +102,7 @@ schema:token-value-composite-ref]) (def schema:token-value-shadow-vector - [:vector + [:vector {:min 1} [:map [:offset-x :string] [:offset-y :string] @@ -282,6 +287,7 @@ [tokens-lib set-id] [:and [:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}] + [:fn #(not (str/blank? (ctob/normalize-set-name %)))] [:fn {:error/fn #(tr "errors.token-set-already-exists")} (fn [name] (or (nil? tokens-lib) @@ -322,6 +328,7 @@ [tokens-lib group theme-id] [:and [:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}] + [:fn #(not (str/blank? %))] [:fn {:error/fn #(tr "errors.token-theme-already-exists" (str group "/" (:value %)))} (fn [name] (or (nil? tokens-lib) diff --git a/common/src/app/common/schema.cljc b/common/src/app/common/schema.cljc index b8cb121b5e..27bbee14bb 100644 --- a/common/src/app/common/schema.cljc +++ b/common/src/app/common/schema.cljc @@ -883,6 +883,14 @@ (register! ::safe-number [::number {:gen/gen (sg/small-double) :max max-safe-int :min min-safe-int}]) +(register! ::non-negative-safe-number + [:and {:gen/gen (sg/small-double :min 0)} + ::safe-number + [:fn #(not (neg? %))]]) +(register! ::positive-safe-number + [:and {:gen/gen (sg/small-double :min 0.01)} + ::safe-number + [:fn pos?]]) (defn parse-boolean [v] @@ -1108,6 +1116,12 @@ (def valid-safe-number? (lazy-validator ::safe-number)) +(def valid-non-negative-safe-number? + (lazy-validator ::non-negative-safe-number)) + +(def valid-positive-safe-number? + (lazy-validator ::positive-safe-number)) + (def valid-safe-int? (lazy-validator ::safe-int)) diff --git a/common/src/app/common/types/grid.cljc b/common/src/app/common/types/grid.cljc index 4fa1e8bf4e..2846e3d076 100644 --- a/common/src/app/common/types/grid.cljc +++ b/common/src/app/common/types/grid.cljc @@ -7,6 +7,7 @@ (ns app.common.types.grid (:require [app.common.schema :as sm] + [app.common.schema.generators :as sg] [app.common.types.color :as clr])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -18,18 +19,28 @@ [:color clr/schema:hex-color] [:opacity ::sm/safe-number]]) +(def schema:grid-count + [:and {:gen/gen (sg/small-double :min 1)} + ::sm/safe-number + [:fn #(<= 1 %)]]) + +(def schema:square-size + [:and {:gen/gen (sg/small-double :min 0.01)} + ::sm/safe-number + [:fn #(<= 0.01 %)]]) + (def schema:column-params [:map {:title "ColumnGridParams"} [:color schema:grid-color] [:type {:optional true} [::sm/one-of #{:stretch :left :center :right}]] - [:size {:optional true} [:maybe ::sm/safe-number]] + [:size {:optional true} [:maybe schema:grid-count]] [:margin {:optional true} [:maybe ::sm/safe-number]] [:item-length {:optional true} [:maybe ::sm/safe-number]] [:gutter {:optional true} [:maybe ::sm/safe-number]]]) (def schema:square-params [:map {:title "SquareGridParams"} - [:size {:optional true} [:maybe ::sm/safe-number]] + [:size {:optional true} [:maybe schema:square-size]] [:color schema:grid-color]]) (def schema:grid @@ -78,4 +89,3 @@ {:square default-square-params :column default-layout-params :row default-layout-params}) - diff --git a/common/src/app/common/types/page.cljc b/common/src/app/common/types/page.cljc index b4631a3698..e7c4e12efd 100644 --- a/common/src/app/common/types/page.cljc +++ b/common/src/app/common/types/page.cljc @@ -14,7 +14,8 @@ [app.common.types.grid :as ctg] [app.common.types.plugins :as ctpg] [app.common.types.shape :as cts] - [app.common.uuid :as uuid])) + [app.common.uuid :as uuid] + [cuerdas.core :as str])) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; SCHEMAS @@ -73,6 +74,24 @@ (def check-page (sm/check-fn schema:page)) +(defn normalize-page-name + [name] + (some-> name str/trim)) + +(defn valid-page-name? + [name] + (let [name (normalize-page-name name)] + (and (string? name) (not (str/blank? name))))) + +(defn valid-flow-starting-frame? + [page frame-id flow-id] + (let [frame (get-in page [:objects frame-id])] + (and (= :frame (:type frame)) + (not-any? (fn [[id flow]] + (and (not= id flow-id) + (= frame-id (:starting-frame flow)))) + (:flows page))))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; INIT & HELPERS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/common/src/app/common/types/shape.cljc b/common/src/app/common/types/shape.cljc index 6a617c21cc..166c041f3b 100644 --- a/common/src/app/common/types/shape.cljc +++ b/common/src/app/common/types/shape.cljc @@ -138,13 +138,13 @@ [:stroke-opacity {:optional true} ::sm/safe-number] [:stroke-style {:optional true} [::sm/one-of #{:solid :dotted :dashed :mixed}]] - [:stroke-width {:optional true} ::sm/safe-number] + [:stroke-width {:optional true} ::sm/non-negative-safe-number] ;; wasm-render only, backwards compatible [:stroke-per-side {:optional true} :boolean] - [:stroke-width-top {:optional true} ::sm/safe-number] - [:stroke-width-right {:optional true} ::sm/safe-number] - [:stroke-width-bottom {:optional true} ::sm/safe-number] - [:stroke-width-left {:optional true} ::sm/safe-number] + [:stroke-width-top {:optional true} ::sm/non-negative-safe-number] + [:stroke-width-right {:optional true} ::sm/non-negative-safe-number] + [:stroke-width-bottom {:optional true} ::sm/non-negative-safe-number] + [:stroke-width-left {:optional true} ::sm/non-negative-safe-number] [:stroke-dash {:optional true} ::sm/safe-number] [:stroke-gap {:optional true} ::sm/safe-number] [:stroke-alignment {:optional true} @@ -211,10 +211,10 @@ [:constraints-v {:optional true} [::sm/one-of vertical-constraint-types]] [:fixed-scroll {:optional true} :boolean] - [:r1 {:optional true} ::sm/safe-number] - [:r2 {:optional true} ::sm/safe-number] - [:r3 {:optional true} ::sm/safe-number] - [:r4 {:optional true} ::sm/safe-number] + [:r1 {:optional true} ::sm/non-negative-safe-number] + [:r2 {:optional true} ::sm/non-negative-safe-number] + [:r3 {:optional true} ::sm/non-negative-safe-number] + [:r4 {:optional true} ::sm/non-negative-safe-number] [:opacity {:optional true} ::sm/safe-number] [:grids {:optional true} [:vector {:gen/max 2} ctg/schema:grid]] diff --git a/common/src/app/common/types/shape/background_blur.cljc b/common/src/app/common/types/shape/background_blur.cljc index 6eb6b7ad38..b25f9f07c5 100644 --- a/common/src/app/common/types/shape/background_blur.cljc +++ b/common/src/app/common/types/shape/background_blur.cljc @@ -12,5 +12,5 @@ [:map {:title "BackgroundBlur"} [:id ::sm/uuid] [:type [:enum :background-blur]] - [:value ::sm/safe-number] - [:hidden :boolean]]) \ No newline at end of file + [:value ::sm/non-negative-safe-number] + [:hidden :boolean]]) diff --git a/common/src/app/common/types/shape/blur.cljc b/common/src/app/common/types/shape/blur.cljc index 59decef09e..671edb41d3 100644 --- a/common/src/app/common/types/shape/blur.cljc +++ b/common/src/app/common/types/shape/blur.cljc @@ -12,5 +12,5 @@ [:map {:title "Blur"} [:id ::sm/uuid] [:type [:enum :layer-blur]] - [:value ::sm/safe-number] + [:value ::sm/non-negative-safe-number] [:hidden :boolean]]) diff --git a/common/src/app/common/types/shape/export.cljc b/common/src/app/common/types/shape/export.cljc index 5c54f0455b..5a7bf9566a 100644 --- a/common/src/app/common/types/shape/export.cljc +++ b/common/src/app/common/types/shape/export.cljc @@ -13,5 +13,5 @@ (def schema:export [:map {:title "ShapeExport"} [:type [::sm/one-of types]] - [:scale ::sm/safe-number] + [:scale ::sm/positive-safe-number] [:suffix :string]]) diff --git a/common/src/app/common/types/shape/interactions.cljc b/common/src/app/common/types/shape/interactions.cljc index 38db81323d..953ec1f8d8 100644 --- a/common/src/app/common/types/shape/interactions.cljc +++ b/common/src/app/common/types/shape/interactions.cljc @@ -10,7 +10,9 @@ [app.common.files.helpers :as cfh] [app.common.geom.point :as gpt] [app.common.schema :as sm] - [app.common.schema.generators :as sg])) + [app.common.schema.generators :as sg] + [app.common.uri :as uri] + [cuerdas.core :as str])) ;; WARNING: options are not deleted when changing event or action ;; type, so it can be restored if the user changes it back later. @@ -216,15 +218,17 @@ (declare calc-overlay-pos-initial) (declare allowed-animation?) +(defn valid-event-type-for-shape? + [shape event-type] + (and (contains? event-types event-type) + (or (not= event-type :after-delay) + (cfh/frame-shape? shape)))) + (defn set-event-type [interaction event-type shape] (assert (check-interaction interaction)) - (assert (contains? event-types event-type) - "should be a valid event type") - - (assert (or (not= event-type :after-delay) - (cfh/frame-shape? shape)) - "the `:after-delay` event type incompatible with not frame shapes") + (assert (valid-event-type-for-shape? shape event-type) + "event type incompatible with shape") (if (= (:event-type interaction) event-type) interaction @@ -290,12 +294,19 @@ (defn set-delay [interaction delay] (assert (check-interaction interaction)) - (assert (sm/check-safe-int delay)) + (assert (and (sm/check-safe-int delay) (not (neg? delay)))) (assert (has-delay interaction) "expected compatible interaction event type") (assoc interaction :delay delay)) +(defn valid-delay? + [interaction] + (or (not (has-delay interaction)) + (let [delay (:delay interaction)] + (and (sm/valid-safe-int? delay) + (not (neg? delay)))))) + ;; FIXME: rename to proper name, very confusing one because it does ;; not checks if interaction has distination, it checks if it can have ;; one. @@ -325,6 +336,32 @@ (assoc :overlay-pos-type :center :overlay-position (gpt/point 0 0)))) +(defn valid-destination? + [objects shape destination] + (or (nil? destination) + (let [target (get objects destination)] + (and (cfh/frame-shape? target) + (not= destination (:id shape)) + (not= destination (:frame-id shape)))))) + +(defn normalize-url + [value] + (when (string? value) + (let [value (str/trim value) + ;; A colon followed by a digit is a port (`localhost:3000`), not a scheme + explicit-scheme? (re-find #"(?i)^[a-z][a-z0-9+.-]*:(?!\d)" value)] + (when (or (not explicit-scheme?) + (re-find #"(?i)^https?://" value)) + (let [value (if explicit-scheme? value (str "http://" value))] + (try + (let [parsed (uri/uri value)] + (when (and (not (re-find #"\s" value)) + (contains? #{"http" "https"} (:scheme parsed)) + (seq (:host parsed))) + value)) + (catch #?(:clj Exception :cljs :default) _ + nil))))))) + (defn has-preserve-scroll [interaction] (= (:action-type interaction) :navigate)) diff --git a/common/src/app/common/types/shape/layout.cljc b/common/src/app/common/types/shape/layout.cljc index 1f3c215ab8..e6e776d8a8 100644 --- a/common/src/app/common/types/shape/layout.cljc +++ b/common/src/app/common/types/shape/layout.cljc @@ -25,7 +25,7 @@ ;; :layout-justify-content ;; :start :center :end :space-between :space-around :space-evenly ;; :layout-wrap-type ;; :wrap, :nowrap ;; :layout-padding-type ;; :simple, :multiple -;; :layout-padding ;; {:p1 num :p2 num :p3 num :p4 num} number could be negative +;; :layout-padding ;; {:p1 num :p2 num :p3 num :p4 num} ;; layout-grid-rows ;; vector of grid-track ;; layout-grid-columns ;; vector of grid-track @@ -103,7 +103,7 @@ (def ^:private schema:grid-track [:map {:title "GridTrack"} [:type [::sm/one-of grid-track-types]] - [:value {:optional true} [:maybe ::sm/safe-number]]]) + [:value {:optional true} [:maybe ::sm/non-negative-safe-number]]]) (def schema:layout-attrs [:map {:title "LayoutAttrs"} @@ -111,17 +111,17 @@ [:layout-flex-dir {:optional true} [::sm/one-of flex-direction-types]] [:layout-gap {:optional true} [:map - [:row-gap {:optional true} ::sm/safe-number] - [:column-gap {:optional true} ::sm/safe-number]]] + [:row-gap {:optional true} ::sm/non-negative-safe-number] + [:column-gap {:optional true} ::sm/non-negative-safe-number]]] [:layout-gap-type {:optional true} [::sm/one-of gap-types]] [:layout-wrap-type {:optional true} [::sm/one-of wrap-types]] [:layout-padding-type {:optional true} [::sm/one-of padding-type]] [:layout-padding {:optional true} [:map - [:p1 ::sm/safe-number] - [:p2 ::sm/safe-number] - [:p3 ::sm/safe-number] - [:p4 ::sm/safe-number]]] + [:p1 ::sm/non-negative-safe-number] + [:p2 ::sm/non-negative-safe-number] + [:p3 ::sm/non-negative-safe-number] + [:p4 ::sm/non-negative-safe-number]]] [:layout-justify-content {:optional true} [::sm/one-of justify-content-types]] [:layout-justify-items {:optional true} [::sm/one-of justify-items-types]] [:layout-align-content {:optional true} [::sm/one-of align-content-types]] @@ -163,10 +163,10 @@ [:m2 {:optional true} ::sm/safe-number] [:m3 {:optional true} ::sm/safe-number] [:m4 {:optional true} ::sm/safe-number]]] - [:layout-item-max-h {:optional true} ::sm/safe-number] - [:layout-item-min-h {:optional true} ::sm/safe-number] - [:layout-item-max-w {:optional true} ::sm/safe-number] - [:layout-item-min-w {:optional true} ::sm/safe-number] + [:layout-item-max-h {:optional true} ::sm/non-negative-safe-number] + [:layout-item-min-h {:optional true} ::sm/non-negative-safe-number] + [:layout-item-max-w {:optional true} ::sm/non-negative-safe-number] + [:layout-item-min-w {:optional true} ::sm/non-negative-safe-number] [:layout-item-h-sizing {:optional true} [::sm/one-of item-h-sizing-types]] [:layout-item-v-sizing {:optional true} [::sm/one-of item-v-sizing-types]] [:layout-item-align-self {:optional true} [::sm/one-of item-align-self-types]] diff --git a/common/src/app/common/types/shape/shadow.cljc b/common/src/app/common/types/shape/shadow.cljc index 1398a7b5eb..49b9f1c80d 100644 --- a/common/src/app/common/types/shape/shadow.cljc +++ b/common/src/app/common/types/shape/shadow.cljc @@ -25,7 +25,7 @@ [:style [::sm/one-of styles]] [:offset-x ::sm/safe-number] [:offset-y ::sm/safe-number] - [:blur ::sm/safe-number] + [:blur ::sm/non-negative-safe-number] [:spread ::sm/safe-number] [:hidden :boolean] [:color schema:color]]) @@ -35,4 +35,3 @@ (def valid-shadow? (sm/validator schema:shadow)) - diff --git a/common/src/app/common/types/text.cljc b/common/src/app/common/types/text.cljc index 9a63ab11e5..82c4a20f82 100644 --- a/common/src/app/common/types/text.cljc +++ b/common/src/app/common/types/text.cljc @@ -56,6 +56,40 @@ (def text-transform-attrs [:text-transform]) +(def font-size-min 3) +(def font-size-max 1000) +(def spacing-min -200) +(def spacing-max 200) +(def text-transform-values + #{"uppercase" "capitalize" "lowercase" "none" "unset"}) + +(def ^:private numeric-text-re + #"^-?(?:[0-9]+(?:\.[0-9]*)?|\.[0-9]+)$") + +(defn- valid-numeric-text-in-range? + [value min-value max-value] + (and (string? value) + (re-matches numeric-text-re value) + (let [value (d/parse-double value)] + (and (some? value) + (<= min-value value max-value))))) + +(defn valid-font-size? + [value] + (valid-numeric-text-in-range? value font-size-min font-size-max)) + +(defn valid-line-height? + [value] + (valid-numeric-text-in-range? value spacing-min spacing-max)) + +(defn valid-letter-spacing? + [value] + (valid-numeric-text-in-range? value spacing-min spacing-max)) + +(defn valid-text-transform? + [value] + (contains? text-transform-values value)) + (def text-fills [:fills]) diff --git a/common/src/app/common/types/token.cljc b/common/src/app/common/types/token.cljc index 880149afbb..b7f641709e 100644 --- a/common/src/app/common/types/token.cljc +++ b/common/src/app/common/types/token.cljc @@ -122,9 +122,13 @@ (def composite-dtcg-token-type->token-type "Same as above, in the opposite direction." - (assoc dtcg-token-type->token-type - "lineHeights" :line-height - "lineHeight" :line-height)) + (let [mapping (assoc dtcg-token-type->token-type + "lineHeights" :line-height + "lineHeight" :line-height)] + (into mapping + (map (fn [[key value]] + [(keyword (str/kebab key)) value])) + mapping))) (def token-types (into #{} (keys token-type->dtcg-token-type))) diff --git a/common/src/app/common/types/variant.cljc b/common/src/app/common/types/variant.cljc index 8a37939756..660fec5304 100644 --- a/common/src/app/common/types/variant.cljc +++ b/common/src/app/common/types/variant.cljc @@ -16,10 +16,12 @@ ;; SCHEMA ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(def property-max-length 60) + (def schema:variant-property [:map - [:name :string] - [:value :string]]) + [:name [:string {:max property-max-length}]] + [:value [:string {:max property-max-length}]]]) (def schema:variant-component "A component that is part of a variant set" @@ -47,9 +49,29 @@ (def property-prefix "Property ") (def property-regex (re-pattern (str property-prefix "(\\d+)"))) -(def property-max-length 60) (def value-prefix "Value ") +(defn normalize-property-text + [value] + (some-> value str/trim)) + +(defn valid-property-name? + [value] + (let [value (normalize-property-text value)] + (and (string? value) + (not (str/blank? value)) + (<= (count value) property-max-length)))) + +(defn valid-property-value? + [value] + (let [value (normalize-property-text value)] + (and (string? value) + (<= (count value) property-max-length)))) + +(defn can-remove-property? + [properties] + (> (count properties) 1)) + (defn properties-to-name "Transform the properties into a name, with the values separated by comma" [properties] @@ -124,8 +146,8 @@ (mapv #(str/split % "=" 2)) (every? #(and (= 2 (count %)) (not (str/blank? (first %))) - (< (count (first %)) property-max-length) - (< (count (second %)) property-max-length))))) + (<= (count (first %)) property-max-length) + (<= (count (second %)) property-max-length))))) (defn find-properties-to-remove "Compares two property maps to find which properties should be removed" diff --git a/common/test/common_tests/files_migrations_test.cljc b/common/test/common_tests/files_migrations_test.cljc index 7a8f757c45..2e8e423c07 100644 --- a/common/test/common_tests/files_migrations_test.cljc +++ b/common/test/common_tests/files_migrations_test.cljc @@ -9,6 +9,7 @@ [app.common.data :as d] [app.common.files.migrations :as cfm] [app.common.types.file :as ctf] + [app.common.types.shape :as cts] [app.common.uuid :as uuid] [clojure.test :as t])) @@ -73,3 +74,135 @@ (let [shape (get-in data' [:pages-index page-id :objects shape-id])] (t/is (nil? (:stroke-cap-start shape)) "top-level cap removed even with no strokes") (t/is (nil? (:stroke-cap-end shape)) "top-level cap removed even with no strokes")))) + +(t/deftest migration-0028-normalizes-constrained-shape-values + (let [file-id (uuid/next) + page-id (uuid/next) + shape-id (uuid/next) + shape (-> (cts/setup-shape {:id shape-id :type :frame}) + (assoc :r1 -1 + :r2 -2 + :r3 -3 + :r4 -4 + :layout-gap {:row-gap -5 :column-gap -6} + :layout-padding {:p1 -7 :p2 -8 :p3 -9 :p4 -10} + :layout-grid-rows [{:type :fixed :value -11}] + :layout-grid-columns [{:type :percent :value -12}] + :layout-item-min-w -13 + :layout-item-max-w -14 + :layout-item-min-h -15 + :layout-item-max-h -16 + :strokes [{:stroke-color "#000000" + :stroke-width -17 + :stroke-width-top -18 + :stroke-width-right -19 + :stroke-width-bottom -20 + :stroke-width-left -21}] + :shadow [{:id nil + :style :drop-shadow + :offset-x 0 + :offset-y 0 + :blur -22 + :spread 0 + :hidden false + :color {:color "#000000" :opacity 1}}] + :blur {:id (uuid/next) + :type :layer-blur + :value -23 + :hidden false} + :background-blur {:id (uuid/next) + :type :background-blur + :value -24 + :hidden false} + :exports [{:type :png :scale 0 :suffix ""}] + :grids [{:type :square + :display true + :params {:size 0 + :color {:color "#000000" :opacity 1}}} + {:type :column + :display true + :params {:size -25 + :color {:color "#000000" :opacity 1}}}])) + data (-> (ctf/make-file-data file-id page-id) + (assoc-in [:pages-index page-id :objects shape-id] shape))] + + (t/is (thrown? #?(:clj Exception :cljs js/Error) + (ctf/check-file-data data)) + "new schemas reject legacy negative values") + + (let [data' (cfm/migrate-data data "0028-normalize-constrained-values") + shape' (get-in data' [:pages-index page-id :objects shape-id])] + (t/is (= data' (ctf/check-file-data data')) "migrated file data passes the schema") + (t/is (every? zero? (map #(get shape' %) [:r1 :r2 :r3 :r4])) "corner radii clamped") + (t/is (= {:row-gap 0 :column-gap 0} (:layout-gap shape')) "layout gaps clamped") + (t/is (= {:p1 0 :p2 0 :p3 0 :p4 0} (:layout-padding shape')) "layout padding clamped") + (t/is (= [0] (mapv :value (:layout-grid-rows shape'))) "row tracks clamped") + (t/is (= [0] (mapv :value (:layout-grid-columns shape'))) "column tracks clamped") + (t/is (every? zero? + (map #(get shape' %) + [:layout-item-min-w :layout-item-max-w + :layout-item-min-h :layout-item-max-h])) + "layout item bounds clamped") + (t/is (every? zero? + (map (first (:strokes shape')) + [:stroke-width :stroke-width-top :stroke-width-right + :stroke-width-bottom :stroke-width-left])) + "stroke widths clamped") + (t/is (zero? (get-in shape' [:shadow 0 :blur])) "shadow blur clamped") + (t/is (zero? (get-in shape' [:blur :value])) "layer blur clamped") + (t/is (zero? (get-in shape' [:background-blur :value])) "background blur clamped") + (t/is (= 1 (get-in shape' [:exports 0 :scale])) "export scale reset to default") + (t/is (= 0.01 (get-in shape' [:grids 0 :params :size])) "square grid size clamped") + (t/is (= 1 (get-in shape' [:grids 1 :params :size])) "column grid count clamped")))) + +(t/deftest migration-0028-normalizes-page-grids-and-variant-properties + (let [file-id (uuid/next) + page-id (uuid/next) + component-id (uuid/next) + long-name (apply str (repeat 61 "n")) + long-value (apply str (repeat 61 "v")) + data (-> (ctf/make-file-data file-id page-id) + (assoc-in [:pages-index page-id :default-grids] + {:square {:size -1 + :color {:color "#000000" :opacity 1}} + :row {:size 0.5 + :color {:color "#000000" :opacity 1}} + :column {:size -2 + :color {:color "#000000" :opacity 1}}}) + (assoc-in [:components component-id] + {:id component-id + :name "Variant" + :variant-properties [{:name long-name + :value long-value}]})) + data' (cfm/migrate-data data "0028-normalize-constrained-values")] + + (t/is (= 0.01 (get-in data' [:pages-index page-id :default-grids :square :size])) + "default square grid size clamped") + (t/is (= 1 (get-in data' [:pages-index page-id :default-grids :row :size])) + "default row grid count reset") + (t/is (= 1 (get-in data' [:pages-index page-id :default-grids :column :size])) + "default column grid count reset") + (t/is (= 60 (count (get-in data' [:components component-id :variant-properties 0 :name]))) + "variant property name truncated") + (t/is (= 60 (count (get-in data' [:components component-id :variant-properties 0 :value]))) + "variant property value truncated") + (t/is (= data' (cfm/migrate-data data' "0028-normalize-constrained-values")) + "migration is idempotent"))) + +(t/deftest migration-0028-runs-through-file-migration + (let [migration-id "0028-normalize-constrained-values" + shape-id (uuid/next) + file (ctf/make-file {:name "Legacy constrained values"}) + page-id (first (get-in file [:data :pages])) + shape (-> (cts/setup-shape {:id shape-id :type :rect}) + (assoc :r1 -1)) + file (-> file + (assoc :migrations (disj cfm/available-migrations migration-id)) + (assoc-in [:data :pages-index page-id :objects shape-id] shape)) + file' (cfm/migrate-file file {})] + + (t/is (cfm/need-migration? file) "new migration detected") + (t/is (not (cfm/need-migration? file')) "new migration recorded") + (t/is (contains? (:migrations file') migration-id) "migration id persisted") + (t/is (zero? (get-in file' [:data :pages-index page-id :objects shape-id :r1])) + "migration repaired file data before schema validation"))) diff --git a/common/test/common_tests/types/shape_interactions_test.cljc b/common/test/common_tests/types/shape_interactions_test.cljc index a310c6bb5a..122fb6c9aa 100644 --- a/common/test/common_tests/types/shape_interactions_test.cljc +++ b/common/test/common_tests/types/shape_interactions_test.cljc @@ -1138,3 +1138,21 @@ (t/testing (str "overlay position ignores filter bounds for " pos-type) (t/is (= pos-plain pos-shadow)) (t/is (= snap-plain snap-shadow))))))) + +(t/deftest normalize-url + (t/testing "adds http to urls without a scheme, including host:port" + (t/is (= "http://example.com" (ctsi/normalize-url "example.com"))) + (t/is (= "http://localhost:3000" (ctsi/normalize-url "localhost:3000"))) + (t/is (= "http://example.com:8080/x" (ctsi/normalize-url " example.com:8080/x ")))) + + (t/testing "keeps http and https urls" + (t/is (= "https://a.example" (ctsi/normalize-url "https://a.example"))) + (t/is (= "http://a.example/p?q=1" (ctsi/normalize-url "http://a.example/p?q=1")))) + + (t/testing "rejects other schemes and malformed urls" + (t/is (nil? (ctsi/normalize-url "javascript:alert(1)"))) + (t/is (nil? (ctsi/normalize-url "mailto:someone@example.com"))) + (t/is (nil? (ctsi/normalize-url "ftp://example.com"))) + (t/is (nil? (ctsi/normalize-url "www.example.com/a b"))) + (t/is (nil? (ctsi/normalize-url ""))) + (t/is (nil? (ctsi/normalize-url nil))))) diff --git a/frontend/src/app/main/data/comments.cljs b/frontend/src/app/main/data/comments.cljs index 90e83ff4de..3371dcd592 100644 --- a/frontend/src/app/main/data/comments.cljs +++ b/frontend/src/app/main/data/comments.cljs @@ -21,6 +21,7 @@ [app.util.i18n :as i18n :refer [tr]] [app.util.storage :as storage] [beicon.v2.core :as rx] + [cuerdas.core :as str] [potok.v2.core :as ptk])) (def ^:private schema:comment-thread @@ -67,6 +68,13 @@ (def r-mentions #"@\[([^\]]*)\]\(([^\)]*)\)") +(defn valid-comment-content? + [content] + (when (string? content) + (let [content (str/trim content)] + (and (not (str/blank? content)) + (not= content "\u200b"))))) + (defn extract-mentions "Retrieves the mentions in the content as an array of uuids" [content] @@ -729,4 +737,3 @@ (rx/map (fn [profiles] #(update % :profiles merge (d/index-by :id profiles))))))))) - diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index ae4928cfc4..a37912ede0 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1049,6 +1049,17 @@ second) 0))))) +(defn component-swap-nesting-loop? + [objects shape library-data component-id] + (let [component (ctkl/get-component library-data component-id true) + page (ctf/get-component-page library-data component) + root (ctf/get-component-root library-data component)] + (and page + root + (cfh/components-nesting-loop? + (cfh/get-children-with-self (:objects page) (:id root)) + (cfh/get-parents-with-self objects (:parent-id shape)))))) + (defn component-swap "Swaps a component with another one" [shape file-id id-new-component keep-touched?] diff --git a/frontend/src/app/main/data/workspace/tokens/application.cljs b/frontend/src/app/main/data/workspace/tokens/application.cljs index b2d79480a4..8708fad699 100644 --- a/frontend/src/app/main/data/workspace/tokens/application.cljs +++ b/frontend/src/app/main/data/workspace/tokens/application.cljs @@ -104,16 +104,17 @@ ([value shape-ids attributes] (update-stroke-width value shape-ids attributes nil)) ([value shape-ids _attributes page-id] ; The attributes param is needed to have the same arity that other update functions (when (number? value) - (dwsh/update-shapes shape-ids - (fn [shape] - (if (seq (:strokes shape)) - (assoc-in shape [:strokes 0 :stroke-width] value) - (let [stroke (assoc cts/default-stroke :stroke-width value)] - (assoc shape :strokes [stroke])))) - {:reg-objects? true - :ignore-touched true - :page-id page-id - :attrs [:strokes]})))) + (let [value (max 0 value)] + (dwsh/update-shapes shape-ids + (fn [shape] + (if (seq (:strokes shape)) + (assoc-in shape [:strokes 0 :stroke-width] value) + (let [stroke (assoc cts/default-stroke :stroke-width value)] + (assoc shape :strokes [stroke])))) + {:reg-objects? true + :ignore-touched true + :page-id page-id + :attrs [:strokes]}))))) (defn update-color [f value shape-ids page-id] (when-let [tc (tinycolor/valid-color value)] @@ -163,7 +164,7 @@ :hidden false :offset-x offset-x :offset-y offset-y - :blur blur + :blur (cond-> blur (number? blur) (max 0)) :color (value->color color) :spread spread :style @@ -239,7 +240,7 @@ (let [ids-with-layout (shape-ids-with-layout state (or page-id (:current-page-id state)) shape-ids)] (rx/of (dwsl/update-layout ids-with-layout - {:layout-padding (zipmap attrs (repeat value))} + {:layout-padding (zipmap attrs (repeat (max 0 value)))} {:ignore-touched true :page-id page-id})))))))) @@ -264,7 +265,7 @@ (watch [_ state _] (when (number? value) (let [ids-with-layout (shape-ids-with-layout state (or page-id (:current-page-id state)) shape-ids) - layout-attributes (attributes->layout-gap attributes value)] + layout-attributes (attributes->layout-gap attributes (max 0 value))] (rx/of (dwsl/update-layout ids-with-layout layout-attributes @@ -278,7 +279,8 @@ ptk/WatchEvent (watch [_ _ _] (when (number? value) - (let [props (-> {:layout-item-min-w value + (let [value (max 0 value) + props (-> {:layout-item-min-w value :layout-item-min-h value :layout-item-max-w value :layout-item-max-h value} diff --git a/frontend/src/app/main/data/workspace/variants.cljs b/frontend/src/app/main/data/workspace/variants.cljs index 9a3ee11095..09f6003a9d 100644 --- a/frontend/src/app/main/data/workspace/variants.cljs +++ b/frontend/src/app/main/data/workspace/variants.cljs @@ -733,6 +733,45 @@ (redirect-to-page page-id) (combine current-page)))))) +(defn valid-components-for-variants? + [state page-id ids] + (let [ids (distinct ids) + objects (dsh/lookup-page-objects state page-id) + data (dsh/lookup-file-data state)] + (and (= page-id (:current-page-id state)) + (> (count ids) 1) + (every? + (fn [id] + (let [shape (get objects id) + component (ctkl/get-component data (:component-id shape) false)] + (and (ctc/main-instance? shape) + component + (not (ctc/is-variant? component))))) + ids)))) + +(defn valid-variant-switch? + [state shape pos val] + (let [libraries (dsh/lookup-libraries state) + component (ctf/get-component libraries + (:component-file shape) + (:component-id shape) + :include-deleted? false) + component-file-data (dm/get-in libraries [(:component-file shape) :data]) + component-page (dsh/get-page component-file-data (:main-instance-page component)) + component-page-objects (:objects component-page) + variant-components (when component + (cfv/find-variant-components component-file-data + component-page-objects + (:variant-id component)))] + (and (ctc/instance-head? shape) + (ctc/in-component-copy? shape) + (ctc/is-variant? component) + (nat-int? pos) + (< pos (count (:variant-properties component))) + (string? val) + (some #(= val (dm/get-in % [:variant-properties pos :value])) + variant-components)))) + (defn combine-selected-as-variants [options] (ptk/reify ::combine-selected-as-variants @@ -799,4 +838,3 @@ (with-meta (meta it)))))) (rx/of (dwu/commit-undo-transaction undo-id) (dws/select-shapes ids))))))) - diff --git a/frontend/src/app/main/ui/comments.cljs b/frontend/src/app/main/ui/comments.cljs index da0510654e..706e41c72e 100644 --- a/frontend/src/app/main/ui/comments.cljs +++ b/frontend/src/app/main/ui/comments.cljs @@ -146,11 +146,7 @@ (defn- blank-content? [content] - (let [content (str/trim content)] - (or (str/blank? content) - (str/empty? content) - (and (= (count content) 1) - (= (first content) zero-width-space))))) + (not (dcm/valid-comment-content? content))) (defn- composing-event? "True when a keydown belongs to an active IME composition. keyCode 229 diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs index 5036ad87a7..8d46f46a3f 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/component.cljs @@ -9,7 +9,6 @@ (:require [app.common.data :as d] [app.common.data.macros :as dm] - [app.common.files.helpers :as cfh] [app.common.files.variant :as cfv] [app.common.path-names :as cpn] [app.common.types.component :as ctk] @@ -390,7 +389,7 @@ (mf/use-fn (mf/deps component-ids) (fn [pos value] - (let [value (d/nilv (str/trim value) "")] + (let [value (ctv/normalize-property-text (d/nilv value ""))] (doseq [id component-ids] (st/emit! (ev/event {::ev/name "variant-edit-property-value" ::ev/origin "workspace:combo-design-tab"}) @@ -401,11 +400,11 @@ (mf/use-fn (mf/deps variant-id) (fn [event] - (let [value (str/trim (dom/get-target-val event)) + (let [value (ctv/normalize-property-text (dom/get-target-val event)) pos (-> (dom/get-current-target event) (dom/get-data "position") int)] - (when (seq value) + (when (ctv/valid-property-name? value) (st/emit! (dwv/update-property-name variant-id pos value {:trigger "workspace:design-tab-variant"})))))) @@ -743,17 +742,6 @@ (->> (concat groups components) (sort-by :name))) - find-parent-components - (mf/use-fn - (mf/deps objects) - (fn [shape] - (->> (cfh/get-parents objects (:id shape)) - (map :component-id) - (remove nil?)))) - - ;; Get the ids of the components that are parents of the shapes, to avoid loops - parent-components (mapcat find-parent-components shapes) - libraries-options (map (fn [library] {:value (:id library) :label (:name library)}) (vals libraries)) @@ -843,10 +831,9 @@ (let [data (dm/get-in libraries [current-library-id :data]) container (ctf/get-component-page data item) root-shape (ctf/get-component-root data item) - components (->> (cfh/get-children-with-self (:objects container) (:id root-shape)) - (keep :component-id) - set) - loop? (some #(contains? components %) parent-components)] + loop? (some #(dwl/component-swap-nesting-loop? + objects % data (:id item)) + shapes)] [:> component-swap-item* {:key (dm/str (:id item)) :item item :loop loop? @@ -1243,11 +1230,11 @@ (mf/use-fn (mf/deps variant-id) (fn [event] - (let [value (dom/get-target-val event) + (let [value (ctv/normalize-property-text (dom/get-target-val event)) pos (-> (dom/get-current-target event) (dom/get-data "position") int)] - (when (seq value) + (when (ctv/valid-property-name? value) (st/emit! (dwv/update-property-name variant-id pos value {:trigger "workspace:design-tab-component"})))))) @@ -1258,7 +1245,7 @@ (let [pos (-> (dom/get-current-target event) (dom/get-data "position") int)] - (when (> (count properties) 1) + (when (ctv/can-remove-property? properties) (st/emit! (ev/event {::ev/name "variant-remove-property" ::ev/origin "workspace:button-design-tab"}) (dwv/remove-property variant-id pos)))))) diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs index f6bebb8d8f..4f2eb3918f 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs @@ -245,17 +245,13 @@ (fn [event] (let [target (dom/get-target event) value (dom/get-value target) - has-prefix? (or (str/starts-with? value "http://") - (str/starts-with? value "https://")) - value (if has-prefix? - value - (str "http://" value))] - (when-not has-prefix? - (dom/set-value! target value)) - (if (dom/valid? target) + normalized (ctsi/normalize-url value)] + (when (and normalized (not= normalized value)) + (dom/set-value! target normalized)) + (if normalized (do (dom/remove-class! target "error") - (update-interaction index #(ctsi/set-url % value))) + (update-interaction index #(ctsi/set-url % normalized))) (dom/add-class! target "error"))))) change-overlay-pos-type diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs index 06ece36af4..f4974abc7b 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/typography.cljs @@ -620,8 +620,8 @@ :options size-options :type "number" :placeholder (tr "settings.multiple") - :min 3 - :max 1000 + :min txt/font-size-min + :max txt/font-size-max :on-change on-font-size-change :on-blur on-blur}])] @@ -669,8 +669,8 @@ :alt (tr "workspace.options.text-options.line-height")} deprecated-icon/text-lineheight] [:> deprecated-input/numeric-input* - {:min -200 - :max 200 + {:min txt/spacing-min + :max txt/spacing-max :step 0.1 :default-value "1.2" :class (stl/css :line-height-input) @@ -688,8 +688,8 @@ :alt (tr "workspace.options.text-options.letter-spacing")} deprecated-icon/text-letterspacing] [:> deprecated-input/numeric-input* - {:min -200 - :max 200 + {:min txt/spacing-min + :max txt/spacing-max :step 0.1 :default-value "0" :class (stl/css :letter-spacing-input) diff --git a/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs b/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs index ac18b484d0..3f1ec3ab8d 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs @@ -138,8 +138,8 @@ (mf/use-fn (mf/deps id is-separator?) (fn [event] - (let [new-name (str/trim (dom/get-target-val event))] - (if (str/empty? new-name) + (let [new-name (ctp/normalize-page-name (dom/get-target-val event))] + (if (not (ctp/valid-page-name? new-name)) (when is-separator? (st/emit! (dw/delete-page id))) (st/emit! (dw/rename-page id new-name)))) diff --git a/frontend/src/app/plugins/api.cljs b/frontend/src/app/plugins/api.cljs index 0b265a0148..226b324c1f 100644 --- a/frontend/src/app/plugins/api.cljs +++ b/frontend/src/app/plugins/api.cljs @@ -14,7 +14,6 @@ [app.common.geom.point :as gpt] [app.common.schema :as sm] [app.common.types.color :as ctc] - [app.common.types.component :as ctk] [app.common.types.shape :as cts] [app.common.types.text :as txt] [app.common.uuid :as uuid] @@ -705,13 +704,15 @@ :createVariantFromComponents (fn [shapes] (cond - (or (not (seq shapes)) + (or (not (array? shapes)) + (not (seq shapes)) (not (every? u/is-main-component-proxy? shapes))) (u/not-valid plugin-id :shapes shapes) :else - (let [file-id (obj/get (first shapes) "$file") - page-id (obj/get (first shapes) "$page") + (let [state @st/state + file-id (:current-file-id state) + page-id (:current-page-id state) ;; Keep the input order: it determines the order of the ;; resulting variant components (see combine-as-variants) ids (->> shapes @@ -719,23 +720,17 @@ (distinct) (vec)) - ;; Check that every component is: - ;; - in the same page - ;; - not already a variant - valid? - (every? - (fn [id] - (let [shape (u/locate-shape file-id page-id id) - component (u/locate-library-component file-id (:component-id shape))] - (not (ctk/is-variant? component)))) - ids)] + valid? (and (every? #(and (= file-id (obj/get % "$file")) + (= page-id (obj/get % "$page"))) + shapes) + (dwv/valid-components-for-variants? state page-id ids))] (if valid? (let [variant-id (uuid/next)] (st/emit! (-> (dwv/combine-as-variants ids {:trigger "plugin:combine-as-variants" :variant-id variant-id}) (se/add-event plugin-id))) - (shape/shape-proxy plugin-id variant-id)) + (shape/shape-proxy plugin-id file-id page-id variant-id)) (u/not-valid plugin-id :shapes "One of the components is not on the same page or is already a variant"))))) diff --git a/frontend/src/app/plugins/comments.cljs b/frontend/src/app/plugins/comments.cljs index 6c9a7cbcf5..ee07ff96ac 100644 --- a/frontend/src/app/plugins/comments.cljs +++ b/frontend/src/app/plugins/comments.cljs @@ -61,7 +61,7 @@ (fn [content] (let [profile (:profile @st/state)] (cond - (or (not (string? content)) (empty? content)) + (not (dc/valid-comment-content? content)) (u/not-valid plugin-id :content "Not valid") (not= (:id profile) (:owner-id data)) @@ -188,7 +188,7 @@ (not (r/check-permission plugin-id "comment:write")) (u/not-valid plugin-id :reply "Plugin doesn't have 'comment:write' permission") - (or (not (string? content)) (empty? content)) + (not (dc/valid-comment-content? content)) (u/not-valid plugin-id :reply "Not valid") :else diff --git a/frontend/src/app/plugins/flex.cljs b/frontend/src/app/plugins/flex.cljs index 229cd24f99..57bf9f1953 100644 --- a/frontend/src/app/plugins/flex.cljs +++ b/frontend/src/app/plugins/flex.cljs @@ -165,7 +165,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :rowGap value) (not (r/check-permission plugin-id "content:write")) @@ -183,7 +183,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :columnGap value) (not (r/check-permission plugin-id "content:write")) @@ -201,7 +201,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :verticalPadding value) (not (r/check-permission plugin-id "content:write")) @@ -219,7 +219,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :horizontalPadding value) (not (r/check-permission plugin-id "content:write")) @@ -237,7 +237,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :topPadding value) (not (r/check-permission plugin-id "content:write")) @@ -255,7 +255,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :rightPadding value) (not (r/check-permission plugin-id "content:write")) @@ -273,7 +273,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :bottomPadding value) (not (r/check-permission plugin-id "content:write")) @@ -291,7 +291,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :leftPadding value) (not (r/check-permission plugin-id "content:write")) @@ -641,7 +641,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :maxWidth value) (not (r/check-permission plugin-id "content:write")) @@ -659,7 +659,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :minWidth value) (not (r/check-permission plugin-id "content:write")) @@ -677,7 +677,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :maxHeight value) (not (r/check-permission plugin-id "content:write")) @@ -695,7 +695,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :minHeight value) (not (r/check-permission plugin-id "content:write")) diff --git a/frontend/src/app/plugins/fonts.cljs b/frontend/src/app/plugins/fonts.cljs index 3f460a83d3..062b761c17 100644 --- a/frontend/src/app/plugins/fonts.cljs +++ b/frontend/src/app/plugins/fonts.cljs @@ -21,8 +21,9 @@ (defn font-variant-proxy? [p] (obj/type-of? p "FontVariantProxy")) -(defn font-variant-proxy [name id weight style] +(defn font-variant-proxy [font-id name id weight style] (obj/reify {:name "FontVariantProxy"} + :$font-id {:enumerable false :get (constantly font-id)} :name {:get (fn [] name)} :fontVariantId {:get (fn [] id)} :fontWeight {:get (fn [] weight)} @@ -47,8 +48,8 @@ {:get (fn [] (format/format-array - (fn [{:keys [id name style weight]}] - (font-variant-proxy name id weight style)) + (fn [{variant-id :id :keys [name style weight]}] + (font-variant-proxy id name variant-id weight style)) variants))} :applyToText @@ -63,6 +64,11 @@ (not (u/page-active? (obj/get text "$page"))) (u/not-valid plugin-id :applyToText "Cannot modify a page that is not currently active") + (and (some? variant) + (or (not (font-variant-proxy? variant)) + (not= id (obj/get variant "$font-id")))) + (u/not-valid plugin-id :applyToText variant) + :else (let [text-id (obj/get text "$id") values {:font-id id @@ -84,6 +90,11 @@ (not (u/page-active? (obj/get range "$page"))) (u/not-valid plugin-id :applyToRange "Cannot modify a page that is not currently active") + (and (some? variant) + (or (not (font-variant-proxy? variant)) + (not= id (obj/get variant "$font-id")))) + (u/not-valid plugin-id :applyToRange variant) + :else (let [range-id (obj/get range "$id") start (obj/get range "$start") diff --git a/frontend/src/app/plugins/grid.cljs b/frontend/src/app/plugins/grid.cljs index 32cc2bf9ea..f15980bf2a 100644 --- a/frontend/src/app/plugins/grid.cljs +++ b/frontend/src/app/plugins/grid.cljs @@ -190,7 +190,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :rowGap value) (not (r/check-permission plugin-id "content:write")) @@ -208,7 +208,7 @@ :set (fn [_ value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :columnGap value) (not (r/check-permission plugin-id "content:write")) @@ -226,7 +226,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :verticalPadding value) (not (r/check-permission plugin-id "content:write")) @@ -244,7 +244,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :horizontalPadding value) (not (r/check-permission plugin-id "content:write")) @@ -262,7 +262,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :topPadding value) (not (r/check-permission plugin-id "content:write")) @@ -280,7 +280,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :rightPadding value) (not (r/check-permission plugin-id "content:write")) @@ -298,7 +298,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :bottomPadding value) (not (r/check-permission plugin-id "content:write")) @@ -316,7 +316,7 @@ :set (fn [this value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :leftPadding value) (not (r/check-permission plugin-id "content:write")) @@ -356,7 +356,7 @@ (u/not-valid plugin-id :addRow-type type) (and (or (= :percent type) (= :flex type) (= :fixed type)) - (not (sm/valid-safe-number? value))) + (not (sm/valid-non-negative-safe-number? value))) (u/not-valid plugin-id :addRow-value value) (not (r/check-permission plugin-id "content:write")) @@ -383,7 +383,7 @@ (u/not-valid plugin-id :addRowAtIndex-type type) (and (or (= :percent type) (= :flex type) (= :fixed type)) - (not (sm/valid-safe-number? value))) + (not (sm/valid-non-negative-safe-number? value))) (u/not-valid plugin-id :addRowAtIndex-value value) (not (r/check-permission plugin-id "content:write")) @@ -402,8 +402,8 @@ (not (contains? ctl/grid-track-types type)) (u/not-valid plugin-id :addColumn-type type) - (and (or (= :percent type) (= :flex type) (= :lex type)) - (not (sm/valid-safe-number? value))) + (and (or (= :percent type) (= :flex type) (= :fixed type)) + (not (sm/valid-non-negative-safe-number? value))) (u/not-valid plugin-id :addColumn-value value) (not (r/check-permission plugin-id "content:write")) @@ -430,7 +430,7 @@ (u/not-valid plugin-id :addColumnAtIndex-type type) (and (or (= :percent type) (= :flex type) (= :fixed type)) - (not (sm/valid-safe-number? value))) + (not (sm/valid-non-negative-safe-number? value))) (u/not-valid plugin-id :addColumnAtIndex-value value) (not (r/check-permission plugin-id "content:write")) @@ -495,7 +495,7 @@ (u/not-valid plugin-id :setColumn-type type) (and (or (= :percent type) (= :flex type) (= :fixed type)) - (not (sm/valid-safe-number? value))) + (not (sm/valid-non-negative-safe-number? value))) (u/not-valid plugin-id :setColumn-value value) (not (r/check-permission plugin-id "content:write")) @@ -522,7 +522,7 @@ (u/not-valid plugin-id :setRow-type type) (and (or (= :percent type) (= :flex type) (= :fixed type)) - (not (sm/valid-safe-number? value))) + (not (sm/valid-non-negative-safe-number? value))) (u/not-valid plugin-id :setRow-value value) (not (r/check-permission plugin-id "content:write")) diff --git a/frontend/src/app/plugins/library.cljs b/frontend/src/app/plugins/library.cljs index 234a470b8c..998f8b0b5c 100644 --- a/frontend/src/app/plugins/library.cljs +++ b/frontend/src/app/plugins/library.cljs @@ -14,12 +14,15 @@ [app.common.types.color :as clr] [app.common.types.component :as ctk] [app.common.types.file :as ctf] + [app.common.types.text :as txt] [app.common.types.typography :as ctt] + [app.common.types.variant :as ctv] [app.common.uuid :as uuid] [app.main.data.plugins :as dp] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.texts :as dwt] [app.main.data.workspace.variants :as dwv] + [app.main.fonts :as fonts] [app.main.repo :as rp] [app.main.store :as st] [app.plugins.format :as format] @@ -32,6 +35,7 @@ [app.plugins.utils :as u] [app.util.object :as obj] [beicon.v2.core :as rx] + [cuerdas.core :as str] [potok.v2.core :as ptk])) (declare lib-color-proxy) @@ -289,6 +293,20 @@ (defn lib-typography-proxy? [p] (obj/type-of? p "LibraryTypographyProxy")) +(defn- font-data + [font variant] + {:font-id (:id font) + :font-family (:family font) + :font-variant-id (:id variant) + :font-style (:style variant) + :font-weight (:weight variant)}) + +(defn- variant-data + [variant] + {:font-variant-id (:id variant) + :font-style (:style variant) + :font-weight (:weight variant)}) + (defn lib-typography-proxy [plugin-id file-id id] (assert (uuid? file-id)) @@ -341,136 +359,150 @@ :get #(-> % u/proxy->library-typography :font-id) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :fontId value) + (let [font (when (string? value) (fonts/get-font-data value)) + variant (fonts/get-default-variant font)] + (cond + (nil? font) + (u/not-valid plugin-id :fontId value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :fontId "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :fontId "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :font-id value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (let [typo (-> (u/proxy->library-typography self) + (merge (font-data font variant)))] + (st/emit! (dwl/update-typography typo file-id))))))} :fontFamily {:this true :get #(-> % u/proxy->library-typography :font-family) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :fontFamily value) + (let [font (when (string? value) (fonts/find-font-data {:family value})) + variant (fonts/get-default-variant font)] + (cond + (nil? font) + (u/not-valid plugin-id :fontFamily value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :fontFamily "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :fontFamily "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :font-family value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (let [typo (-> (u/proxy->library-typography self) + (merge (font-data font variant)))] + (st/emit! (dwl/update-typography typo file-id))))))} :fontVariantId {:this true :get #(-> % u/proxy->library-typography :font-variant-id) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :fontVariantId value) + (let [typo (u/proxy->library-typography self) + font (fonts/get-font-data (:font-id typo)) + variant (when (string? value) (fonts/find-variant font {:id value}))] + (cond + (nil? variant) + (u/not-valid plugin-id :fontVariantId value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :fontVariantId "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :fontVariantId "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :font-variant-id value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (st/emit! (dwl/update-typography (merge typo (variant-data variant)) file-id)))))} :fontSize {:this true :get #(-> % u/proxy->library-typography :font-size) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :fontSize value) + (let [value (some-> value str/trim)] + (cond + (not (txt/valid-font-size? value)) + (u/not-valid plugin-id :fontSize value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :fontSize "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :fontSize "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :font-size value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (let [typo (-> (u/proxy->library-typography self) + (assoc :font-size value))] + (st/emit! (dwl/update-typography typo file-id))))))} :fontWeight {:this true :get #(-> % u/proxy->library-typography :font-weight) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :fontWeight value) + (let [typo (u/proxy->library-typography self) + font (fonts/get-font-data (:font-id typo)) + variant (when (string? value) + (or (fonts/find-variant font {:style (:font-style typo) :weight value}) + (fonts/find-variant font {:weight value})))] + (cond + (nil? variant) + (u/not-valid plugin-id :fontWeight value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :fontWeight "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :fontWeight "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :font-weight value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (st/emit! (dwl/update-typography (merge typo (variant-data variant)) file-id)))))} :fontStyle {:this true :get #(-> % u/proxy->library-typography :font-style) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :fontStyle value) + (let [typo (u/proxy->library-typography self) + font (fonts/get-font-data (:font-id typo)) + variant (when (string? value) + (or (fonts/find-variant font {:weight (:font-weight typo) :style value}) + (fonts/find-variant font {:style value})))] + (cond + (nil? variant) + (u/not-valid plugin-id :fontStyle value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :fontStyle "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :fontStyle "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :font-style value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (st/emit! (dwl/update-typography (merge typo (variant-data variant)) file-id)))))} :lineHeight {:this true - :get #(-> % u/proxy->library-typography :font-height) + :get #(-> % u/proxy->library-typography :line-height) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :lineHeight value) + (let [value (some-> value str/trim)] + (cond + (not (txt/valid-line-height? value)) + (u/not-valid plugin-id :lineHeight value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :lineHeight "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :lineHeight "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :font-height value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (let [typo (-> (u/proxy->library-typography self) + (assoc :line-height value))] + (st/emit! (dwl/update-typography typo file-id))))))} :letterSpacing {:this true :get #(-> % u/proxy->library-typography :letter-spacing) :set (fn [self value] - (cond - (not (string? value)) - (u/not-valid plugin-id :letterSpacing value) + (let [value (some-> value str/trim)] + (cond + (not (txt/valid-letter-spacing? value)) + (u/not-valid plugin-id :letterSpacing value) - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :letterSpacing "Plugin doesn't have 'library:write' permission") + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :letterSpacing "Plugin doesn't have 'library:write' permission") - :else - (let [typo (-> (u/proxy->library-typography self) - (assoc :letter-spacing value))] - (st/emit! (dwl/update-typography typo file-id)))))} + :else + (let [typo (-> (u/proxy->library-typography self) + (assoc :letter-spacing value))] + (st/emit! (dwl/update-typography typo file-id))))))} :textTransform {:this true @@ -478,7 +510,7 @@ :set (fn [self value] (cond - (not (string? value)) + (not (txt/valid-text-transform? value)) (u/not-valid plugin-id :textTransform value) (not (r/check-permission plugin-id "library:write")) @@ -495,6 +527,11 @@ (not (obj/type-of? font "FontProxy")) (u/not-valid plugin-id :setFont font) + (and (some? variant) + (or (not (obj/type-of? variant "FontVariantProxy")) + (not= (obj/get font "fontId") (obj/get variant "$font-id")))) + (u/not-valid plugin-id :setFont variant) + (not (r/check-permission plugin-id "library:write")) (u/not-valid plugin-id :setFont "Plugin doesn't have 'library:write' permission") @@ -720,7 +757,8 @@ :removeProperty (fn [pos] - (let [nprops (->> (get-variant-components file-id id) first :variant-properties count)] + (let [properties (->> (get-variant-components file-id id) first :variant-properties) + nprops (count properties)] (cond (or (not (nat-int? pos)) (>= pos nprops)) (u/not-valid plugin-id :pos pos) @@ -728,6 +766,9 @@ (not (r/check-permission plugin-id "library:write")) (u/not-valid plugin-id :removeProperty "Plugin doesn't have 'library:write' permission") + (not (ctv/can-remove-property? properties)) + (u/not-valid plugin-id :removeProperty "A variant must keep at least one property") + :else (st/emit! (se/event plugin-id "remove-property") @@ -740,7 +781,7 @@ (or (not (nat-int? pos)) (>= pos nprops)) (u/not-valid plugin-id :pos pos) - (not (string? name)) + (not (ctv/valid-property-name? name)) (u/not-valid plugin-id :name name) (not (r/check-permission plugin-id "library:write")) @@ -748,7 +789,8 @@ :else (st/emit! - (dwv/update-property-name id pos name {:trigger "plugin:rename-property"}))))))) + (dwv/update-property-name id pos (ctv/normalize-property-text name) + {:trigger "plugin:rename-property"}))))))) (set! shape/variant-proxy variant-proxy) @@ -978,8 +1020,8 @@ (or (not (nat-int? pos)) (>= pos nprops)) (u/not-valid plugin-id :pos (str pos)) - (not (string? value)) - (u/not-valid plugin-id :name value) + (not (ctv/valid-property-value? value)) + (u/not-valid plugin-id :value value) (not (r/check-permission plugin-id "library:write")) (u/not-valid plugin-id :setVariantProperty "Plugin doesn't have 'library:write' permission") @@ -987,7 +1029,7 @@ :else (st/emit! (se/event plugin-id "variant-edit-property-value") - (dwv/update-property-value id pos value))))))) + (dwv/update-property-value id pos (ctv/normalize-property-text value)))))))) (defn library-proxy? [p] (obj/type-of? p "LibraryProxy")) diff --git a/frontend/src/app/plugins/page.cljs b/frontend/src/app/plugins/page.cljs index 2be488813f..c75133a692 100644 --- a/frontend/src/app/plugins/page.cljs +++ b/frontend/src/app/plugins/page.cljs @@ -12,6 +12,7 @@ [app.common.geom.point :as gpt] [app.common.schema :as sm] [app.common.types.color :as cc] + [app.common.types.page :as ctp] [app.common.uuid :as uuid] [app.main.data.comments :as dc] [app.main.data.common :as dcm] @@ -78,15 +79,19 @@ (shape/shape-proxy plugin-id file-id page-id frame))) :set (fn [_ value] - (cond - (not (shape/shape-proxy? value)) - (u/not-valid plugin-id :startingBoard value) + (let [page (u/locate-page file-id page-id)] + (cond + (or (not (shape/shape-proxy? value)) + (not= file-id (obj/get value "$file")) + (not= page-id (obj/get value "$page")) + (not (ctp/valid-flow-starting-frame? page (obj/get value "$id") id))) + (u/not-valid plugin-id :startingBoard value) - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :startingBoard "Plugin doesn't have 'content:write' permission") + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :startingBoard "Plugin doesn't have 'content:write' permission") - :else - (st/emit! (dwi/update-flow page-id id #(assoc % :starting-frame (obj/get value "$id"))))))} + :else + (st/emit! (dwi/update-flow page-id id #(assoc % :starting-frame (obj/get value "$id")))))))} :remove (fn [] @@ -115,15 +120,16 @@ :get #(-> % u/proxy->page :name) :set (fn [_ value] - (cond - (not (string? value)) - (u/not-valid plugin-id :name value) + (let [value (ctp/normalize-page-name value)] + (cond + (not (ctp/valid-page-name? value)) + (u/not-valid plugin-id :name value) - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :name "Plugin doesn't have 'content:write' permission") + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :name "Plugin doesn't have 'content:write' permission") - :else - (st/emit! (dw/rename-page id value))))} + :else + (st/emit! (dw/rename-page id value)))))} :getRoot (fn [] @@ -319,22 +325,26 @@ :createFlow (fn [name frame] - (cond - (or (not (string? name)) (empty? name)) - (u/not-valid plugin-id :createFlow-name name) + (let [page (u/locate-page file-id id)] + (cond + (or (not (string? name)) (empty? name)) + (u/not-valid plugin-id :createFlow-name name) - (not (shape/shape-proxy? frame)) - (u/not-valid plugin-id :createFlow-frame frame) + (or (not (shape/shape-proxy? frame)) + (not= file-id (obj/get frame "$file")) + (not= id (obj/get frame "$page")) + (not (ctp/valid-flow-starting-frame? page (obj/get frame "$id") nil))) + (u/not-valid plugin-id :createFlow-frame frame) - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :createFlow "Plugin doesn't have 'content:write' permission") + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :createFlow "Plugin doesn't have 'content:write' permission") - :else - (let [flow-id (uuid/next)] - (st/emit! - (dwi/add-flow flow-id id name (obj/get frame "$id")) - (se/event plugin-id "add-flow")) - (flow-proxy plugin-id file-id id flow-id)))) + :else + (let [flow-id (uuid/next)] + (st/emit! + (dwi/add-flow flow-id id name (obj/get frame "$id")) + (se/event plugin-id "add-flow")) + (flow-proxy plugin-id file-id id flow-id))))) :removeFlow (fn [flow] @@ -352,7 +362,8 @@ :addRulerGuide (fn [orientation value board] - (let [shape (u/proxy->shape board)] + (let [shape (when (shape/shape-proxy? board) + (u/locate-shape file-id id (obj/get board "$id")))] (cond (not (sm/valid-safe-number? value)) (u/not-valid plugin-id :addRulerGuide "Value not a safe number") @@ -360,8 +371,10 @@ (not (contains? #{"vertical" "horizontal"} orientation)) (u/not-valid plugin-id :addRulerGuide "Orientation should be either 'vertical' or 'horizontal'") - (and (some? shape) + (and (some? board) (or (not (shape/shape-proxy? board)) + (not= file-id (obj/get board "$file")) + (not= id (obj/get board "$page")) (not (cfh/frame-shape? shape)))) (u/not-valid plugin-id :addRulerGuide "The shape is not a board") @@ -405,7 +418,7 @@ (let [shape (when board (u/proxy->shape board)) position (parser/parse-point position)] (cond - (or (not (string? content)) (empty? content)) + (not (dc/valid-comment-content? content)) (u/not-valid plugin-id :addCommentThread "Content not valid") (or (not (sm/valid-safe-number? (:x position))) diff --git a/frontend/src/app/plugins/parser.cljs b/frontend/src/app/plugins/parser.cljs index a13dd5e69d..70d0f4d5a8 100644 --- a/frontend/src/app/plugins/parser.cljs +++ b/frontend/src/app/plugins/parser.cljs @@ -10,6 +10,7 @@ [app.common.geom.point :as gpt] [app.common.json :as json] [app.common.types.path :as path] + [app.common.types.shape.interactions :as ctsi] [app.common.uuid :as uuid] [app.util.object :as obj] [cuerdas.core :as str])) @@ -509,7 +510,7 @@ :open-url {:action-type action-type - :url (obj/get action "url")} + :url (ctsi/normalize-url (obj/get action "url"))} nil))))) diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index bbf3de8e1e..522788620a 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -64,7 +64,7 @@ [app.plugins.strokes :as strokes] [app.plugins.system-events :as se] [app.plugins.text :as text] - [app.plugins.tokens :refer [applied-tokens-plugin->applied-tokens token-attr-plugin->token-attr token-attr?]] + [app.plugins.tokens :refer [applied-tokens-plugin->applied-tokens token-attr-plugin->token-attr token-attr? valid-token-resolution?]] [app.plugins.utils :as u] [app.util.http :as http] [app.util.object :as obj] @@ -79,6 +79,24 @@ (defn interaction-proxy? [p] (obj/type-of? p "InteractionProxy")) +(defn- valid-interaction-action? + [file-id page-id source raw-action interaction] + (let [page (u/locate-page file-id page-id) + destination-proxy (obj/get raw-action "destination") + destination-id (:destination interaction) + animation-type (get-in interaction [:animation :animation-type])] + (and (sm/validate ctsi/schema:interaction interaction) + (ctsi/valid-delay? interaction) + ;; Interactions keep a stale destination after switching to an action + ;; without one, so only check it for actions that use it + (or (not (ctsi/has-destination interaction)) + (and (or (nil? destination-proxy) + (and (shape-proxy? destination-proxy) + (= file-id (obj/get destination-proxy "$file")) + (= page-id (obj/get destination-proxy "$page")))) + (ctsi/valid-destination? (:objects page) source destination-id))) + (ctsi/allowed-animation? (:action-type interaction) animation-type)))) + (defn interaction-proxy "Proxy over one interaction of a shape. @@ -106,9 +124,10 @@ :set (fn [_ value] (let [value (parser/parse-keyword value) + shape (u/locate-shape file-id page-id shape-id) index (locate-index)] (cond - (not (contains? ctsi/event-types value)) + (not (ctsi/valid-event-type-for-shape? shape value)) (u/not-valid plugin-id :trigger value) (not (r/check-permission plugin-id "content:write")) @@ -120,11 +139,11 @@ :else (do (st/emit! (dwi/update-interaction - (u/locate-shape file-id page-id shape-id) + shape index - #(assoc % :event-type value) + #(ctsi/set-event-type % value shape) {:page-id page-id})) - (swap! current assoc :event-type value)))))} + (swap! current ctsi/set-event-type value shape)))))} :delay {:this true @@ -156,11 +175,11 @@ :get #(-> % u/proxy->interaction (format/format-action plugin-id file-id page-id)) :set (fn [self value] - (let [params (parser/parse-action value) - index (locate-index) - interaction - (-> (u/proxy->interaction self) - (d/patch-object params))] + (let [shape (u/locate-shape file-id page-id shape-id) + params (parser/parse-action value) + index (locate-index) + interaction (-> (u/proxy->interaction self) + (d/patch-object params))] (cond (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :action "Plugin doesn't have 'content:write' permission") @@ -170,13 +189,13 @@ (nil? index) (u/not-valid plugin-id :action "The interaction is not part of the shape anymore") - (not (sm/validate ctsi/schema:interaction interaction)) + (not (valid-interaction-action? file-id page-id shape value interaction)) (u/not-valid plugin-id :action interaction) :else (do (st/emit! (dwi/update-interaction - (u/locate-shape file-id page-id shape-id) + shape index #(d/patch-object % params) {:page-id page-id})) @@ -534,7 +553,7 @@ (fn [self value] (let [id (obj/get self "$id")] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :borderRadiusTopLeft value) (not (r/check-permission plugin-id "content:write")) @@ -553,7 +572,7 @@ (fn [self value] (let [id (obj/get self "$id")] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :borderRadiusTopRight value) (not (r/check-permission plugin-id "content:write")) @@ -572,7 +591,7 @@ (fn [self value] (let [id (obj/get self "$id")] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :borderRadiusBottomRight value) (not (r/check-permission plugin-id "content:write")) @@ -591,7 +610,7 @@ (fn [self value] (let [id (obj/get self "$id")] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :borderRadiusBottomLeft value) (not (r/check-permission plugin-id "content:write")) @@ -1528,7 +1547,12 @@ :swapComponent (fn [component] - (let [shape (u/locate-shape file-id page-id id)] + (let [shape (u/locate-shape file-id page-id id) + objects (u/locate-objects file-id page-id) + valid-component? (obj/type-of? component "LibraryComponentProxy") + target-file (when valid-component? (obj/get component "$file")) + target-id (when valid-component? (obj/get component "$id")) + target-data (some-> (u/locate-file target-file) :data)] (cond (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :swapComponent "Plugin doesn't have 'content:write' permission") @@ -1536,16 +1560,19 @@ (not (u/page-active? page-id)) (u/not-valid plugin-id :swapComponent "Cannot modify a page that is not currently active") - (not (obj/type-of? component "LibraryComponentProxy")) + (not valid-component?) (u/not-valid plugin-id :swapComponent "Component not valid") (not (ctk/in-component-copy? shape)) (u/not-valid plugin-id :swapComponent "The shape is not a component copy instance") + (dwl/component-swap-nesting-loop? objects shape target-data target-id) + (u/not-valid plugin-id :swapComponent "The swap would create a component nesting loop") + :else (st/emit! (dwl/component-swap shape - (obj/get component "$file") - (obj/get component "$id") + target-file + target-id true))))) :resetOverrides @@ -1639,11 +1666,15 @@ ;; Interactions :addInteraction (fn [trigger action delay] - (let [interaction - (-> ctsi/default-interaction - (d/patch-object (parser/parse-interaction trigger action delay)))] + (let [shape (u/locate-shape file-id page-id id) + event-type (parser/parse-keyword trigger) + interaction (when (ctsi/valid-event-type-for-shape? shape event-type) + (-> ctsi/default-interaction + (ctsi/set-event-type event-type shape) + (d/patch-object (parser/parse-action action)) + (cond-> (some? delay) (assoc :delay delay))))] (cond - (not (sm/validate ctsi/schema:interaction interaction)) + (not (valid-interaction-action? file-id page-id shape action interaction)) (u/not-valid plugin-id :addInteraction interaction) (not (r/check-permission plugin-id "content:write")) @@ -1746,15 +1777,23 @@ [:fn token-proxy?] [:maybe [::sm/set [:and ::sm/keyword [:fn token-attr?]]]]] :fn (fn [token attrs] - (let [token (u/locate-token file-id (obj/get token "$set-id") (obj/get token "$id")) + (let [set-id (obj/get token "$set-id") + token-id (obj/get token "$id") + token (u/locate-token file-id set-id token-id) kw-attrs (into #{} (map token-attr-plugin->token-attr attrs))] (cond - (some #(not (token-attr? %)) kw-attrs) - (u/not-valid plugin-id :applyToken attrs) - (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :applyToken "Plugin doesn't have 'content:write' permission") + (nil? token) + (u/not-valid plugin-id :applyToken token-id) + + (not (valid-token-resolution? file-id set-id token-id)) + (u/not-valid plugin-id :applyToken (:value token)) + + (some #(not (token-attr? %)) kw-attrs) + (u/not-valid plugin-id :applyToken attrs) + :else (st/emit! (-> (dwta/toggle-token {:token token @@ -1777,21 +1816,24 @@ :switchVariant (fn [pos value] (cond + (not (u/page-active? page-id)) + (u/not-valid plugin-id :switchVariant "Cannot modify a page that is not currently active") + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :switchVariant "Plugin doesn't have 'content:write' permission") + (not (nat-int? pos)) (u/not-valid plugin-id :pos pos) (not (string? value)) (u/not-valid plugin-id :value value) - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :switchVariant "Plugin doesn't have 'content:write' permission") - :else - (let [shape (u/locate-shape file-id page-id id) - component (u/locate-library-component file-id (:component-id shape))] - (when (and component (ctk/is-variant? component)) + (let [shape (u/locate-shape file-id page-id id)] + (if (dwv/valid-variant-switch? @st/state shape pos value) (st/emit! (-> (dwv/variants-switch {:shapes [shape] :pos pos :val value}) - (se/add-event plugin-id))))))) + (se/add-event plugin-id))) + (u/not-valid plugin-id :switchVariant "Shape, property, or value is not valid"))))) :combineAsVariants (fn [ids] @@ -1813,13 +1855,7 @@ (distinct)) ids) - valid? - (every? - (fn [id] - (let [shape (u/locate-shape file-id page-id id) - component (u/locate-library-component file-id (:component-id shape))] - (not (ctk/is-variant? component)))) - ids)] + valid? (dwv/valid-components-for-variants? @st/state page-id ids)] (if valid? (let [variant-id (uuid/next)] @@ -1827,7 +1863,7 @@ ids {:trigger "plugin:combine-as-variants" :variant-id variant-id}) (se/add-event plugin-id))) - (shape-proxy plugin-id variant-id)) + (shape-proxy plugin-id file-id page-id variant-id)) (u/not-valid plugin-id :ids "One of the components is not on the same page or is already a variant")))))) diff --git a/frontend/src/app/plugins/text.cljs b/frontend/src/app/plugins/text.cljs index 901deb9f5b..66b2f2fb88 100644 --- a/frontend/src/app/plugins/text.cljs +++ b/frontend/src/app/plugins/text.cljs @@ -28,13 +28,6 @@ [app.util.text-editor :as ted] [cuerdas.core :as str])) -;; This regex seems duplicated but probably in the future when we support diferent units -;; this will need to reflect changes for each property - -(def ^:private font-size-re #"^\d*\.?\d*$") -(def ^:private line-height-re #"^\d*\.?\d*$") -(def ^:private letter-spacing-re #"^-?\d*\.?\d*$") -(def ^:private text-transform-re #"uppercase|capitalize|lowercase|none") (def ^:private text-decoration-re #"underline|line-through|none") (def ^:private text-direction-re #"ltr|rtl") (def ^:private text-align-re #"left|center|right|justify") @@ -160,7 +153,7 @@ (let [font (fonts/find-font-data {:family value}) variant (fonts/get-default-variant font)] (cond - (not (string? value)) + (nil? font) (u/not-valid plugin-id :fontFamily value) (not (r/check-permission plugin-id "content:write")) @@ -182,9 +175,10 @@ :set (fn [self value] (let [font (fonts/get-font-data (obj/get self "fontId")) - variant (fonts/get-variant font value)] + variant (when (string? value) + (fonts/find-variant font {:id value}))] (cond - (not (string? value)) + (nil? variant) (u/not-valid plugin-id :fontVariantId value) (not (r/check-permission plugin-id "content:write")) @@ -207,7 +201,7 @@ (fn [_ value] (let [value (str/trim (dm/str value))] (cond - (or (empty? value) (not (re-matches font-size-re value))) + (not (txt/valid-font-size? value)) (u/not-valid plugin-id :fontSize value) (not (r/check-permission plugin-id "content:write")) @@ -289,7 +283,7 @@ (fn [_ value] (let [value (str/trim (dm/str value))] (cond - (or (empty? value) (not (re-matches line-height-re value))) + (not (txt/valid-line-height? value)) (u/not-valid plugin-id :lineHeight value) (not (r/check-permission plugin-id "content:write")) @@ -312,7 +306,7 @@ (fn [_ value] (let [value (str/trim (dm/str value))] (cond - (or (not (string? value)) (not (re-matches letter-spacing-re value))) + (not (txt/valid-letter-spacing? value)) (u/not-valid plugin-id :letterSpacing value) (not (r/check-permission plugin-id "content:write")) @@ -334,7 +328,7 @@ :set (fn [_ value] (cond - (and (string? value) (not (re-matches text-transform-re value))) + (not (txt/valid-text-transform? value)) (u/not-valid plugin-id :textTransform value) (not (r/check-permission plugin-id "content:write")) @@ -550,7 +544,8 @@ (fn [self value] (let [id (obj/get self "$id") font (fonts/get-font-data (obj/get self "fontId")) - variant (fonts/get-variant font value)] + variant (when (string? value) + (fonts/find-variant font {:id value}))] (cond (not variant) (u/not-valid plugin-id :fontVariantId value) @@ -571,7 +566,7 @@ (let [id (obj/get self "$id") value (str/trim (dm/str value))] (cond - (or (empty? value) (not (re-matches font-size-re value))) + (not (txt/valid-font-size? value)) (u/not-valid plugin-id :fontSize value) (not (r/check-permission plugin-id "content:write")) @@ -640,7 +635,7 @@ (let [id (obj/get self "$id") value (str/trim (dm/str value))] (cond - (or (empty? value) (not (re-matches line-height-re value))) + (not (txt/valid-line-height? value)) (u/not-valid plugin-id :lineHeight value) (not (r/check-permission plugin-id "content:write")) @@ -659,7 +654,7 @@ (let [id (obj/get self "$id") value (str/trim (dm/str value))] (cond - (or (not (string? value)) (not (re-matches letter-spacing-re value))) + (not (txt/valid-letter-spacing? value)) (u/not-valid plugin-id :letterSpacing value) (not (r/check-permission plugin-id "content:write")) @@ -677,7 +672,7 @@ (fn [self value] (let [id (obj/get self "$id")] (cond - (or (not (string? value)) (not (re-matches text-transform-re value))) + (not (txt/valid-text-transform? value)) (u/not-valid plugin-id :textTransform value) (not (r/check-permission plugin-id "content:write")) diff --git a/frontend/src/app/plugins/tokens.cljs b/frontend/src/app/plugins/tokens.cljs index 922ae739d4..ba1fd09fee 100644 --- a/frontend/src/app/plugins/tokens.cljs +++ b/frontend/src/app/plugins/tokens.cljs @@ -7,6 +7,7 @@ (ns app.plugins.tokens (:require [app.common.data.macros :as dm] + [app.common.files.helpers :as cfh] [app.common.files.tokens :as cfo] [app.common.json :as json] [app.common.schema :as sm] @@ -88,6 +89,76 @@ [attr] (cto/token-attr? (token-attr-plugin->token-attr attr))) +(defn- token-name-schema + [file-id set-id token] + (let [tokens-lib (u/locate-tokens-lib file-id) + tokens (-> (ctob/get-tokens tokens-lib set-id) + (dissoc (:name token)) + (ctob/tokens-tree))] + (cfo/make-token-name-schema tokens))) + +(defn- token-value-schema + [token] + (let [base (cfo/make-token-value-schema (:type token))] + (if (= :font-family (:type token)) + [:or :string base] + base))) + +(defn- normalize-token-value + [token value] + (case (:type token) + :font-family (ctob/convert-dtcg-font-family value) + :typography (ctob/convert-dtcg-typography-composite value) + :shadow (ctob/convert-dtcg-shadow-composite value) + value)) + +(defn- resolution-tokens + "Tokens used to resolve references for a token in `set-id`: every token in + the library, with the tokens of `set-id` taking precedence." + [tokens-lib set-id] + (merge (ctob/get-all-tokens-map tokens-lib) + (ctob/get-tokens tokens-lib set-id))) + +(defn- resolved-without-errors? + [resolved] + (and (contains? resolved :resolved-value) + (empty? (:errors resolved)))) + +(defn- valid-token-candidate? + [file-id set-id token attrs] + (let [tokens-lib (u/locate-tokens-lib file-id) + candidate (merge (datafy token) attrs) + tokens (-> (resolution-tokens tokens-lib set-id) + (dissoc (:name token)) + (assoc (:name candidate) candidate)) + resolved (get (ts/resolve-tokens tokens) (:name candidate))] + (and (sm/validate (token-name-schema file-id set-id token) (:name candidate)) + (sm/validate (cfo/make-token-value-schema (:type candidate)) (:value candidate)) + (or (nil? (:description candidate)) + (sm/validate cfo/schema:token-description (:description candidate))) + (resolved-without-errors? resolved)))) + +;; Last resolution result, reused while the tokens library and set are unchanged +(def ^:private resolution-cache (atom nil)) + +(defn- resolve-set-tokens + [tokens-lib set-id] + (let [{:keys [lib set resolved]} @resolution-cache] + (if (and (identical? lib tokens-lib) (= set set-id)) + resolved + (let [resolved (ts/resolve-tokens (resolution-tokens tokens-lib set-id))] + (reset! resolution-cache {:lib tokens-lib :set set-id :resolved resolved}) + resolved)))) + +(defn valid-token-resolution? + "Checks that an existing token resolves without errors, which is the only + requirement to apply it." + [file-id set-id id] + (when-let [token (u/locate-token file-id set-id id)] + (-> (resolve-set-tokens (u/locate-tokens-lib file-id) set-id) + (get (:name token)) + (resolved-without-errors?)))) + (defn- apply-token-to-shapes [plugin-id file-id set-id id shape-ids attrs] (cond @@ -96,8 +167,17 @@ :else (let [token (u/locate-token file-id set-id id)] - (if (some #(not (token-attr? %)) attrs) + (cond + (nil? token) + (u/not-valid plugin-id :applyToSelected id) + + (not (valid-token-resolution? file-id set-id id)) + (u/not-valid plugin-id :applyToSelected (:value token)) + + (some #(not (token-attr? %)) attrs) (u/not-valid plugin-id :applyToSelected attrs) + + :else (st/emit! (-> (dwta/toggle-token {:token token :attrs (into #{} (map token-attr-plugin->token-attr) attrs) @@ -208,9 +288,8 @@ (fn [_] (let [token (u/locate-token file-id set-id id)] (ctob/get-name token))) - :schema (cfo/make-token-name-schema - (some-> (u/locate-tokens-lib file-id) - (ctob/get-tokens set-id))) + :schema (fn [_] + (token-name-schema file-id set-id (u/locate-token file-id set-id id))) :set (fn [_ value] (cond @@ -220,8 +299,11 @@ :else (do (u/check-editable-tokens file-id) - (st/emit! (-> (dwtl/update-token set-id id {:name value}) - (se/add-event plugin-id))))))} + (let [token (u/locate-token file-id set-id id)] + (if (valid-token-candidate? file-id set-id token {:name value}) + (st/emit! (-> (dwtl/update-token set-id id {:name value}) + (se/add-event plugin-id))) + (u/not-valid plugin-id :name value))))))} :type {:this true @@ -236,14 +318,11 @@ (fn [_] (let [token (u/locate-token file-id set-id id)] (json/->js (:value token)))) - :schema (let [token (u/locate-token file-id set-id id) - base (cfo/make-token-value-schema (:type token))] - ;; plugin-types declares the fontFamilies value as - ;; `string | string[]`, but the core schema only accepts a - ;; vector/ref; also accept a plain string (normalized in :set). - (if (= :font-family (:type token)) - [:or :string base] - base)) + :decode/fn (fn [value] + (let [token (u/locate-token file-id set-id id)] + (normalize-token-value token (json/->clj value)))) + :schema (fn [_] + (token-value-schema (u/locate-token file-id set-id id))) :set (fn [_ value] (cond @@ -254,10 +333,10 @@ (do (u/check-editable-tokens file-id) (let [token (u/locate-token file-id set-id id) - value (cond-> value - (= :font-family (:type token)) - (ctob/convert-dtcg-font-family))] - (st/emit! (dwtl/update-token set-id id {:value value}))))))} + value (normalize-token-value token value)] + (if (valid-token-candidate? file-id set-id token {:value value}) + (st/emit! (dwtl/update-token set-id id {:value value})) + (u/not-valid plugin-id :value value))))))} :resolvedValue {:this true @@ -315,9 +394,11 @@ (do (u/check-editable-tokens file-id) (let [token (u/locate-token file-id set-id id) + names (map :name (vals (ctob/get-tokens (u/locate-tokens-lib file-id) set-id))) + name (cfh/generate-unique-name (:name token) names :suffix "copy") token' (ctob/make-token (-> (datafy token) - (dissoc :id - :modified-at)))] + (assoc :name name) + (dissoc :id :modified-at)))] (st/emit! (-> (dwtl/create-token set-id token') (se/add-event plugin-id))) (token-proxy plugin-id file-id set-id (:id token')))))) @@ -378,9 +459,10 @@ (if (some? set) (ctob/get-name set) initial-name))) - :schema (cfo/make-token-set-name-schema - (u/locate-tokens-lib file-id) - id) + :schema (fn [_] + (cfo/make-token-set-name-schema + (u/locate-tokens-lib file-id) + id)) :set (fn [_ name] (cond @@ -575,11 +657,12 @@ (fn [_] (let [theme (u/locate-token-theme file-id id)] (:group theme))) - :schema (let [theme (u/locate-token-theme file-id id)] - (cfo/make-token-theme-group-schema - (u/locate-tokens-lib file-id) - (:name theme) - (:id theme))) + :schema (fn [_] + (let [theme (u/locate-token-theme file-id id)] + (cfo/make-token-theme-group-schema + (u/locate-tokens-lib file-id) + (:name theme) + (:id theme)))) :set (fn [_ group] (cond @@ -598,11 +681,12 @@ (fn [_] (let [theme (u/locate-token-theme file-id id)] (:name theme))) - :schema (let [theme (u/locate-token-theme file-id id)] - (cfo/make-token-theme-name-schema - (u/locate-tokens-lib file-id) - (:id theme) - (:group theme))) + :schema (fn [_] + (let [theme (u/locate-token-theme file-id id)] + (cfo/make-token-theme-name-schema + (u/locate-tokens-lib file-id) + (:group theme) + (:id theme)))) :set (fn [_ name] (cond @@ -696,8 +780,14 @@ :else (do (u/check-editable-tokens file-id) - (let [theme (u/locate-token-theme file-id id) + (let [tokens-lib (u/locate-tokens-lib file-id) + theme (u/locate-token-theme file-id id) + names (->> (ctob/get-themes tokens-lib) + (filter #(= (:group theme) (:group %))) + (map :name)) + name (cfh/generate-unique-name (:name theme) names :suffix "copy") theme' (ctob/make-token-theme (-> (datafy theme) + (assoc :name name) (dissoc :id :modified-at)))] (st/emit! (dwtl/create-token-theme theme')) @@ -773,17 +863,12 @@ :addSet {:enumerable false - :schema [:tuple (-> (sm/schema (cfo/make-token-set-schema - (u/locate-tokens-lib file-id) - nil)) - (sm/dissoc-key :id) ;; We don't allow plugins to set the id - ;; Allow an optional `active` flag so a plugin can create - ;; an already-active set in a single call. Newly created - ;; sets are inactive by default (only active sets affect - ;; shapes and reference resolution). `active` is not part - ;; of the token-set data model, so the :fn strips it and - ;; applies it through the set-activation logic. - (sm/merge [:map [:active {:optional true} ::sm/boolean]]))] + :schema (fn [_] + [:tuple (-> (sm/schema (cfo/make-token-set-schema + (u/locate-tokens-lib file-id) + nil)) + (sm/dissoc-key :id) + (sm/merge [:map [:active {:optional true} ::sm/boolean]]))]) :fn (fn [attrs] (cond diff --git a/frontend/src/app/plugins/tracks.cljs b/frontend/src/app/plugins/tracks.cljs index c5f38664f2..b5b62689ca 100644 --- a/frontend/src/app/plugins/tracks.cljs +++ b/frontend/src/app/plugins/tracks.cljs @@ -43,7 +43,7 @@ :set (fn [value] (cond - (not (sm/valid-safe-number? value)) + (not (sm/valid-non-negative-safe-number? value)) (u/not-valid plugin-id :value value) (not (r/check-permission plugin-id "content:write")) diff --git a/frontend/test/frontend_tests/plugins/interactions_test.cljs b/frontend/test/frontend_tests/plugins/interactions_test.cljs index 115786e788..98b7ae12da 100644 --- a/frontend/test/frontend_tests/plugins/interactions_test.cljs +++ b/frontend/test/frontend_tests/plugins/interactions_test.cljs @@ -105,3 +105,23 @@ (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) (t/is (not (throws? #(set! (.-delay inter) 0))) "delay = 0 must be accepted") (t/is (throws? #(set! (.-delay inter) -1)) "negative delay must be rejected"))))) + +(t/deftest interaction-action-ignores-stale-destination + ;; Interactions keep their destination after the action type changes, so a + ;; destination that is no longer valid must not block switching to an action + ;; that has none. + (thw/with-wasm-mocks* + (fn [] + (let [store (ths/setup-store (cthf/sample-file :file1 :page-label :page1)) + ^js context (api/create-context plugin-id) + _ (set! st/state store) + ^js board1 (.createBoard context) + ^js board2 (.createBoard context) + ^js inter (.addInteraction board1 "click" #js {:type "navigate-to" :destination board2})] + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + ;; board2 becomes board1's parent board, an invalid destination for it + (.appendChild board2 board1) + (t/is (not (throws? #(set! (.-action inter) #js {:type "previous-screen"}))) + "an action without destination is accepted") + (t/is (throws? #(set! (.-action inter) #js {:type "navigate-to" :destination board2})) + "navigating to the parent board is still rejected"))))) diff --git a/frontend/test/frontend_tests/plugins/page_test.cljs b/frontend/test/frontend_tests/plugins/page_test.cljs index c7fe33e643..1e5ffd3315 100644 --- a/frontend/test/frontend_tests/plugins/page_test.cljs +++ b/frontend/test/frontend_tests/plugins/page_test.cljs @@ -177,17 +177,20 @@ (first @errors))))))) (t/deftest flow-starting-board-setter-checks-permission - (let [plugin-id "test-plugin" - file-id (uuid/next) - page-id (uuid/next) - flow-id (uuid/next) - errors (atom [])] + ;; The board must be a valid flow starting frame, otherwise the setter + ;; rejects the value before it ever checks the permission. + (let [result (setup-with-board) + file-id (:id (:file result)) + page-id (cthf/current-page-id (:file result)) + board-id (:board-id result) + flow-id (uuid/next) + errors (atom [])] (with-redefs [r/check-permission (constantly false) u/not-valid (mock/stub (fn [pid prop msg] (swap! errors conj [pid prop msg]))) - st/emit! mock/noop - shape/shape-proxy? (constantly true)] - (let [proxy (page/flow-proxy plugin-id file-id page-id flow-id)] - (set! (.-startingBoard proxy) #js {}) + st/emit! mock/noop] + (let [proxy (page/flow-proxy plugin-id file-id page-id flow-id) + board (shape/shape-proxy plugin-id file-id page-id board-id)] + (set! (.-startingBoard proxy) board) (t/is (= 1 (count @errors))) (t/is (= [plugin-id :startingBoard "Plugin doesn't have 'content:write' permission"] (first @errors))))))) @@ -208,16 +211,18 @@ (first @errors))))))) (t/deftest create-flow-checks-permission - (let [plugin-id "test-plugin" - file-id (uuid/next) - page-id (uuid/next) - errors (atom [])] + ;; The frame must be a valid flow starting frame, otherwise createFlow + ;; rejects the argument before it ever checks the permission. + (let [result (setup-with-board) + file-id (:id (:file result)) + page-id (cthf/current-page-id (:file result)) + board-id (:board-id result) + errors (atom [])] (with-redefs [r/check-permission (constantly false) u/not-valid (mock/stub (fn [pid prop msg] (swap! errors conj [pid prop msg]))) - st/emit! mock/noop - shape/shape-proxy? (constantly true)] + st/emit! mock/noop] (let [proxy (page/page-proxy plugin-id file-id page-id) - frame #js {"$id" (uuid/next)}] + frame (shape/shape-proxy plugin-id file-id page-id board-id)] (.createFlow proxy "flow-name" frame) (t/is (= 1 (count @errors))) (t/is (= [plugin-id :createFlow "Plugin doesn't have 'content:write' permission"] diff --git a/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs b/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs index b85f471c13..7dca8b7af8 100644 --- a/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs +++ b/frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs @@ -176,6 +176,7 @@ (with-redefs [u/locate-shape (fn [_file _page id] {:id id :component-id id}) u/locate-library-component (constantly {:id (uuid/next)}) ctk/is-variant? (constantly false) + dwv/valid-components-for-variants? (constantly true) dwv/combine-as-variants (fn [ids opts] (reset! captured {:ids ids :opts opts}) @@ -381,6 +382,7 @@ errors (atom [])] (with-redefs [u/locate-shape (constantly {:id shape-id :component-id shape-id}) u/locate-library-component (constantly {:id (uuid/next)}) + u/page-active? (constantly true) u/not-valid (mock/stub (fn [pid prop msg] (swap! errors conj [pid prop msg]))) r/check-permission (constantly false) st/emit! mock/noop] diff --git a/frontend/test/frontend_tests/plugins/text_test.cljs b/frontend/test/frontend_tests/plugins/text_test.cljs index 3f4d96be9d..49b4dc4ea3 100644 --- a/frontend/test/frontend_tests/plugins/text_test.cljs +++ b/frontend/test/frontend_tests/plugins/text_test.cljs @@ -22,29 +22,42 @@ ;; Regression coverage for issue #9780. ;; ;; `letterSpacing` accepts negative tracking in the product UI (-200..200, -;; see typography.cljs), but the plugin validator regex rejected any leading -;; minus, so negative values were refused. `letter-spacing-re` is the shared -;; predicate behind both the shape- and range-level setters; pin its -;; accept/reject contract here. +;; see typography.cljs), but the plugin setter rejected any leading minus, +;; so negative values were refused. Pin the accept/reject contract of the +;; plugin setter here. -(def ^:private letter-spacing-re @#'plugins.text/letter-spacing-re) +(defn- apply-letter-spacing + "Sets `letterSpacing` on a text range proxy and returns the attributes sent + to the update event, or nil when the plugin rejected the value." + [value] + (let [captured (atom nil) + range (plugins.text/text-range-proxy + plugin-id (random-uuid) (random-uuid) (random-uuid) 0 4)] + (with-redefs [r/check-permission (constantly true) + u/page-active? (constantly true) + u/not-valid (fn [_ _ _] nil) + dwt/update-text-range + (fn [_ _ _ attrs] + (reset! captured attrs) + :update-text-range) + st/emit! mock/noop] + (set! (.-letterSpacing range) value) + @captured))) -(defn- valid? [s] (boolean (re-matches letter-spacing-re s))) +(t/deftest letter-spacing-accepts-negative-values + (t/is (= {:letter-spacing "-0.56"} (apply-letter-spacing "-0.56"))) + (t/is (= {:letter-spacing "-12"} (apply-letter-spacing "-12"))) + (t/is (= {:letter-spacing "-200"} (apply-letter-spacing "-200")))) -(t/deftest letter-spacing-re-accepts-negative-values - (t/is (valid? "-0.56")) - (t/is (valid? "-12")) - (t/is (valid? "-200"))) +(t/deftest letter-spacing-accepts-non-negative-values + (t/is (= {:letter-spacing "0"} (apply-letter-spacing "0"))) + (t/is (= {:letter-spacing "12"} (apply-letter-spacing "12"))) + (t/is (= {:letter-spacing "1.5"} (apply-letter-spacing "1.5")))) -(t/deftest letter-spacing-re-accepts-non-negative-values - (t/is (valid? "0")) - (t/is (valid? "12")) - (t/is (valid? "1.5"))) - -(t/deftest letter-spacing-re-rejects-non-numeric - (t/is (not (valid? "abc"))) - (t/is (not (valid? "1-2"))) - (t/is (not (valid? "--1")))) +(t/deftest letter-spacing-rejects-non-numeric + (t/is (nil? (apply-letter-spacing "abc"))) + (t/is (nil? (apply-letter-spacing "1-2"))) + (t/is (nil? (apply-letter-spacing "--1")))) (t/deftest font-apply-to-text-uses-font-id-not-shape-id diff --git a/frontend/test/frontend_tests/plugins/tokens_test.cljs b/frontend/test/frontend_tests/plugins/tokens_test.cljs index 99ef7f4685..374b7e2cc9 100644 --- a/frontend/test/frontend_tests/plugins/tokens_test.cljs +++ b/frontend/test/frontend_tests/plugins/tokens_test.cljs @@ -143,7 +143,7 @@ (ctob/make-token :id token-id :name "spacing.medium" :type :spacing - :value 16))) + :value "16"))) :status-fn #(ctos/set-tokens-status % #{theme-id} #{set-id})) (ctho/add-frame :frame1 {:layout :flex})) store (ths/setup-store file) @@ -351,17 +351,30 @@ (t/is (not (contains? (-> @captured second :theme :sets) "Primitives"))))))) (t/deftest font-family-token-value-accepts-a-string - (let [file-id (cthi/new-id! :file) - set-id (cthi/new-id! :set) - token-id (cthi/new-id! :token) + ;; The setter validates the candidate token against the file tokens library, + ;; so the token has to live in a real library. + (let [set-id (cthi/new-id! :token-set) + token-id (cthi/new-id! :font-token) + file (-> (cthf/sample-file :file1 :page-label :page1) + (ctht/add-tokens-lib) + (ctht/update-tokens-lib + #(-> % + (ctob/add-set + (ctob/make-token-set :id set-id + :name "fonts")) + (ctob/add-token + set-id + (ctob/make-token :id token-id + :name "font.primary" + :type :font-family + :value ["Inter"]))))) + store (ths/setup-store file) + _ (set! st/state store) + _ (set! st/stream (ptk/input-stream store)) + file-id (:id file) captured (atom nil)] - (with-redefs [u/locate-tokens-lib (constantly nil) - r/check-permission (constantly true) + (with-redefs [r/check-permission (constantly true) u/check-editable-tokens (constantly nil) - u/locate-token (constantly {:id token-id - :name "font.primary" - :type :font-family - :value ["Inter"]}) dwtl/update-token (mock/stub (fn [set-id token-id attrs] (reset! captured {:set-id set-id :token-id token-id diff --git a/frontend/test/frontend_tests/tokens/logic/token_actions_test.cljs b/frontend/test/frontend_tests/tokens/logic/token_actions_test.cljs index 47b3447c68..0377053128 100644 --- a/frontend/test/frontend_tests/tokens/logic/token_actions_test.cljs +++ b/frontend/test/frontend_tests/tokens/logic/token_actions_test.cljs @@ -379,6 +379,37 @@ (t/testing "shapes without layout get ignored" (t/is (nil? (:layout-padding frame-1'))))))))))) +(t/deftest test-apply-negative-spacing-clamps-padding-and-gap + (t/testing "negative spacing tokens write zero padding and gap" + (t/async + done + (let [spacing-token {:name "spacing.negative" + :value "-8" + :type :spacing} + file (-> (setup-file-with-tokens) + (ctho/add-frame :frame-1 {:layout :flex}) + (update-in [:data :tokens-lib] + #(ctob/add-token % (cthi/id :set-a) + (ctob/make-token spacing-token)))) + store (ths/setup-store file) + frame-1 (cths/get-shape file :frame-1) + token (toht/get-token file "spacing.negative") + events [(dwta/apply-token {:shape-ids [(:id frame-1)] + :attributes #{:p1 :p2 :p3 :p4} + :token token + :on-update-shape dwta/update-layout-padding}) + (dwta/apply-token {:shape-ids [(:id frame-1)] + :attributes #{:row-gap :column-gap} + :token token + :on-update-shape dwta/update-layout-gap})]] + (tohs/run-store-async + store done events + (fn [new-state] + (let [file' (ths/get-file-from-state new-state) + frame-1' (cths/get-shape file' :frame-1)] + (t/is (= (:layout-padding frame-1') {:p1 0 :p2 0 :p3 0 :p4 0})) + (t/is (= (:layout-gap frame-1') {:row-gap 0 :column-gap 0}))))))))) + (t/deftest test-apply-sizing (t/testing "applies sizing token and updates the shapes width and height" (t/async diff --git a/plugins/apps/plugin-api-test-suite/src/tests/comments.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/comments.test.ts index ecf00c5255..cc3f471295 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/comments.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/comments.test.ts @@ -156,4 +156,23 @@ describe.skipIfMocked('Comments', () => { cleanup(thread); } }); + + for (const content of [' ', '\n\t', '\u200b']) { + test(`blank comment content ${JSON.stringify(content)} rejects everywhere`, async (ctx) => { + const p = page(ctx); + await expectReject(() => p.addCommentThread(content, { x: 0, y: 0 })); + + const thread = await p.addCommentThread('parent', { x: 12, y: 12 }); + try { + await expectReject(() => thread.reply(content)); + const comments = await thread.findComments(); + expect(comments.length).toBeGreaterThan(0); + expect(() => { + comments[0].content = content; + }).toThrow(); + } finally { + cleanup(thread); + } + }); + } }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/components.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/components.test.ts index 367eaf8267..ecdd6c5118 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/components.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/components.test.ts @@ -222,13 +222,21 @@ describe('Component instances', () => { const inst = comp.instance(); ctx.board.appendChild(inst); - const mainColor = main.fills?.[0]?.fillColor; + const mainFill = main.fills?.[0]; + const mainColor = + typeof mainFill === 'string' ? mainFill : mainFill?.fillColor; inst.fills = [{ fillColor: '#FF0000', fillOpacity: 1 }]; // The override applied (fill getter normalizes to lowercase). - expect(inst.fills?.[0]?.fillColor?.toLowerCase()).toBe('#ff0000'); + const overrideFill = inst.fills?.[0]; + const overrideColor = + typeof overrideFill === 'string' ? overrideFill : overrideFill?.fillColor; + expect(overrideColor?.toLowerCase()).toBe('#ff0000'); inst.resetOverrides(); - expect(inst.fills?.[0]?.fillColor).toBe(mainColor); + const resetFill = inst.fills?.[0]; + expect( + typeof resetFill === 'string' ? resetFill : resetFill?.fillColor, + ).toBe(mainColor); }); test('resetOverrides on a plain shape throws', (ctx) => { @@ -266,6 +274,23 @@ describe('Component instances', () => { ).toThrow(); }); + test('swapComponent rejects a component nesting loop', (ctx) => { + const leaf = ctx.penpot.createRectangle(); + ctx.board.appendChild(leaf); + const inner = ctx.penpot.library.local.createComponent([leaf]); + + const wrapper = ctx.penpot.createBoard(); + ctx.board.appendChild(wrapper); + wrapper.appendChild(inner.instance()); + const outer = ctx.penpot.library.local.createComponent([wrapper]); + const nested = (outer.mainInstance() as Board).children.find((child) => + child.isComponentInstance(), + ); + + expect(nested).toBeDefined(); + if (nested) expect(() => nested.swapComponent(outer)).toThrow(); + }); + test('two instances of one component are independent but share the source', (ctx) => { const comp = makeComponent(ctx); const first = comp.instance(); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/file.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/file.test.ts index 9bff861b82..ccb2b68657 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/file.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/file.test.ts @@ -51,7 +51,9 @@ describe('File', () => { // The exporter service may be unavailable in the headless runner, so a // rejection here is treated as an environment limitation; when it does // run, the result must be a non-empty byte array. - const data = await file.export('penpot', 'detach').catch(() => null); + const data = await file + .export('penpot', 'detach-libraries') + .catch(() => null); if (data) { expect(data.length).toBeGreaterThan(0); } diff --git a/plugins/apps/plugin-api-test-suite/src/tests/fills-strokes.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/fills-strokes.test.ts index 8a01314f53..685d253665 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/fills-strokes.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/fills-strokes.test.ts @@ -267,14 +267,16 @@ describe('Fills & strokes', () => { ]); }); - test('negative strokeWidth is accepted (currently unvalidated)', (ctx) => { - // The plugin API does not constrain strokeWidth to be non-negative, so a - // negative value is stored as-is rather than rejected. This pins the current - // (lenient) behaviour. + test('negative strokeWidth throws', (ctx) => { const r = rect(ctx); - r.strokes = [{ strokeColor: '#000000', strokeWidth: -3 }]; - expect(r.strokes).toHaveLength(1); - expect(typeof r.strokes[0].strokeWidth).toBe('number'); + expect(() => { + r.strokes = [{ strokeColor: '#000000', strokeWidth: -3 }]; + }).toThrow(); + + r.strokes = [{ strokeColor: '#000000', strokeWidth: 1 }]; + expect(() => { + r.strokes[0].strokeWidth = -1; + }).toThrow(); }); test('invalid strokeStyle throws', (ctx) => { diff --git a/plugins/apps/plugin-api-test-suite/src/tests/fonts.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/fonts.test.ts index 7340ddfa94..af1ef08b33 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/fonts.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/fonts.test.ts @@ -119,4 +119,17 @@ describe('Fonts', () => { expect(t.fontVariantId).toBe(variant.fontVariantId); expect(t.fontWeight).toBe(variant.fontWeight); }); + + test('a font rejects a variant owned by another font', (ctx) => { + const fonts = ctx.penpot.fonts.all; + const first = fonts[0]; + const second = fonts.find((font) => font.fontId !== first.fontId); + if (second) { + const t = text(ctx); + expect(() => first.applyToText(t, second.variants[0])).toThrow(); + expect(() => + first.applyToRange(t.getRange(0, 5), second.variants[0]), + ).toThrow(); + } + }); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts index a7fcc88051..9b7a2d6f8a 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/interactions.test.ts @@ -156,7 +156,7 @@ describe('Interactions', () => { test('after-delay trigger carries a delay', (ctx) => { const dest = board(ctx); - const r = rect(ctx); + const r = board(ctx); const interaction = r.addInteraction( 'after-delay', { type: 'navigate-to', destination: dest }, @@ -169,7 +169,7 @@ describe('Interactions', () => { // A zero delay is a valid value (fires immediately), not an error. test('after-delay accepts a zero delay', (ctx) => { const dest = board(ctx); - const r = rect(ctx); + const r = board(ctx); const interaction = r.addInteraction( 'after-delay', { type: 'navigate-to', destination: dest }, @@ -198,7 +198,7 @@ describe('Interactions', () => { // "don't persist" — that is stale: CI confirms they do.) test('interaction delay and action setters persist', (ctx) => { const dest = board(ctx); - const r = rect(ctx); + const r = board(ctx); const interaction = r.addInteraction( 'after-delay', { type: 'navigate-to', destination: dest }, @@ -218,7 +218,7 @@ describe('Interactions', () => { // The delay setter accepts zero (fires immediately) as a valid value. test('delay setter accepts a zero value', (ctx) => { const dest = board(ctx); - const r = rect(ctx); + const r = board(ctx); const interaction = r.addInteraction( 'after-delay', { type: 'navigate-to', destination: dest }, @@ -447,35 +447,205 @@ describe('Interactions', () => { expect(interaction.trigger).toBe('mouse-enter'); }); + test('unknown interaction triggers are rejected', (ctx) => { + const dest = board(ctx); + const r = rect(ctx); + expect(() => + r.addInteraction('unknown-trigger' as unknown as 'click', { + type: 'navigate-to', + destination: dest, + }), + ).toThrow(); + + const interaction = r.addInteraction('click', { + type: 'navigate-to', + destination: dest, + }); + expect(() => { + interaction.trigger = 'unknown-trigger' as unknown as 'click'; + }).toThrow(); + }); + // --------------------------------------------------------------------------- // Edge cases. "fail" tests assert invalid interaction input is // rejected; the "success" test checks several triggers coexisting. // --------------------------------------------------------------------------- - // addInteraction validates the interaction's structure (schema) but not the - // liveness of a navigate destination nor the format of an open-url string, - // so both of these are accepted rather than rejected. These pin the current - // (lenient) behaviour. - test('navigate-to a removed board is accepted (dangling destination)', (ctx) => { + test('navigate-to a removed board throws', (ctx) => { const dest = board(ctx); const r = rect(ctx); dest.remove(); expect(() => r.addInteraction('click', { type: 'navigate-to', destination: dest }), - ).not.toThrow(); + ).toThrow(); }); - test('open-url accepts an arbitrary url string', (ctx) => { + test('open-url rejects invalid input and normalizes a bare hostname', (ctx) => { const r = rect(ctx); + expect(() => + r.addInteraction('click', { + type: 'open-url', + url: 'not a valid url', + }), + ).toThrow(); + expect(() => + r.addInteraction('click', { + type: 'open-url', + url: 'ftp://example.com/file', + }), + ).toThrow(); const interaction = r.addInteraction('click', { type: 'open-url', - url: 'not a valid url', + url: 'example.com/path', }); expect(interaction.action.type).toBe('open-url'); if (interaction.action.type === 'open-url') { - expect(interaction.action.url).toBe('not a valid url'); + expect(interaction.action.url).toBe('http://example.com/path'); } }); + test('after-delay is board-only and initializes its default delay', (ctx) => { + const dest = board(ctx); + const r = rect(ctx); + expect(() => + r.addInteraction('after-delay', { + type: 'navigate-to', + destination: dest, + }), + ).toThrow(); + + const source = board(ctx); + const interaction = source.addInteraction('click', { + type: 'navigate-to', + destination: dest, + }); + interaction.trigger = 'after-delay'; + expect(interaction.delay).toBeCloseTo(600, 0); + expect(() => { + const invalid = r.addInteraction('click', { + type: 'navigate-to', + destination: dest, + }); + invalid.trigger = 'after-delay'; + }).toThrow(); + }); + + test('after-delay creation rejects invalid delays', (ctx) => { + const dest = board(ctx); + const source = board(ctx); + for (const delay of ['bad', 1.5, -1]) { + expect(() => + source.addInteraction( + 'after-delay', + { type: 'navigate-to', destination: dest }, + delay as unknown as number, + ), + ).toThrow(); + } + + const interaction = source.addInteraction( + 'after-delay', + { type: 'navigate-to', destination: dest }, + 10, + ); + for (const delay of ['bad', 1.5, -1]) { + expect(() => { + interaction.delay = delay as unknown as number; + }).toThrow(); + } + }); + + test('navigation destinations must be eligible boards', (ctx) => { + const source = board(ctx); + const child = rect(ctx); + const rectangle = rect(ctx); + expect(() => + source.addInteraction('click', { + type: 'navigate-to', + destination: source, + }), + ).toThrow(); + expect(() => + child.addInteraction('click', { + type: 'navigate-to', + destination: ctx.board, + }), + ).toThrow(); + expect(() => + source.addInteraction('click', { + type: 'navigate-to', + destination: rectangle as unknown as Board, + }), + ).toThrow(); + }); + + test('interaction destinations must belong to the current page', async (ctx) => { + const original = ctx.penpot.currentPage; + expect(original).not.toBeNull(); + if (!original) return; + + const source = rect(ctx); + const localDestination = board(ctx); + const interaction = source.addInteraction('click', { + type: 'navigate-to', + destination: localDestination, + }); + const otherPage = ctx.penpot.createPage(); + try { + await ctx.penpot.openPage(otherPage); + const otherBoard = ctx.penpot.createBoard(); + (otherPage.root as Board).appendChild(otherBoard); + await ctx.penpot.openPage(original); + + expect(() => + source.addInteraction('click', { + type: 'navigate-to', + destination: otherBoard, + }), + ).toThrow(); + expect(() => { + interaction.action = { + type: 'navigate-to', + destination: otherBoard, + }; + }).toThrow(); + } finally { + if (ctx.penpot.currentPage?.id !== original.id) { + await ctx.penpot.openPage(original); + } + otherPage.remove(); + } + }); + + test('push animation is rejected for overlay actions and replacements', (ctx) => { + const overlay = board(ctx); + const r = rect(ctx); + const push = { + type: 'push' as const, + direction: 'left' as const, + duration: 300, + easing: 'linear' as const, + }; + expect(() => + r.addInteraction('click', { + type: 'open-overlay', + destination: overlay, + animation: push, + }), + ).toThrow(); + + const interaction = r.addInteraction('click', { + type: 'navigate-to', + destination: overlay, + }); + expect(() => { + interaction.action = { + type: 'open-overlay', + destination: overlay, + animation: push, + }; + }).toThrow(); + }); + test('several triggers on one shape coexist', (ctx) => { const dest = board(ctx); const r = rect(ctx); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/layout.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/layout.test.ts index 867132714e..37f9a735c2 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/layout.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/layout.test.ts @@ -82,6 +82,34 @@ describe('Layout', () => { expect(flex.leftPadding).toBeCloseTo(4.5, 2); }); + test('every flex gap and padding setter rejects negative values', (ctx) => { + const flex = board(ctx).addFlexLayout(); + expect(() => { + flex.rowGap = -1; + }).toThrow(); + expect(() => { + flex.columnGap = -1; + }).toThrow(); + expect(() => { + flex.verticalPadding = -1; + }).toThrow(); + expect(() => { + flex.horizontalPadding = -1; + }).toThrow(); + expect(() => { + flex.topPadding = -1; + }).toThrow(); + expect(() => { + flex.rightPadding = -1; + }).toThrow(); + expect(() => { + flex.bottomPadding = -1; + }).toThrow(); + expect(() => { + flex.leftPadding = -1; + }).toThrow(); + }); + // paddingType is "simple" (sides mirrored) or "multiple" (each side independent). test('paddingType round-trips', (ctx) => { const flex = board(ctx).addFlexLayout(); @@ -185,6 +213,37 @@ describe('Layout', () => { expect(grid.columns[0].type).toBe('percent'); }); + test('grid track creation and replacement reject negative values', (ctx) => { + const grid = board(ctx).addGridLayout(); + for (const type of ['fixed', 'percent', 'flex'] as const) { + expect(() => grid.addRow(type, -1)).toThrow(); + expect(() => grid.addColumn(type, -1)).toThrow(); + } + expect(() => + grid.addColumn('fixed', 'bad' as unknown as number), + ).toThrow(); + grid.addRow('flex', 1); + grid.addColumn('flex', 1); + expect(() => grid.addRowAtIndex(0, 'fixed', -1)).toThrow(); + expect(() => grid.addColumnAtIndex(0, 'fixed', -1)).toThrow(); + expect(() => grid.setRow(0, 'flex', -1)).toThrow(); + expect(() => grid.setColumn(0, 'flex', -1)).toThrow(); + }); + + test('retained grid track proxies reject negative values', (ctx) => { + const grid = board(ctx).addGridLayout(); + grid.addRow('fixed', 10); + grid.addColumn('fixed', 10); + const row = grid.rows[0]; + const column = grid.columns[0]; + expect(() => { + row.value = -1; + }).toThrow(); + expect(() => { + column.value = -1; + }).toThrow(); + }); + test('removeRow and removeColumn drop tracks', (ctx) => { const grid = board(ctx).addGridLayout(); grid.addRow('flex', 1); @@ -238,6 +297,34 @@ describe('Layout', () => { expect(grid.leftPadding).toBeCloseTo(4.5, 2); }); + test('every grid gap and padding setter rejects negative values', (ctx) => { + const grid = board(ctx).addGridLayout(); + expect(() => { + grid.rowGap = -1; + }).toThrow(); + expect(() => { + grid.columnGap = -1; + }).toThrow(); + expect(() => { + grid.verticalPadding = -1; + }).toThrow(); + expect(() => { + grid.horizontalPadding = -1; + }).toThrow(); + expect(() => { + grid.topPadding = -1; + }).toThrow(); + expect(() => { + grid.rightPadding = -1; + }).toThrow(); + expect(() => { + grid.bottomPadding = -1; + }).toThrow(); + expect(() => { + grid.leftPadding = -1; + }).toThrow(); + }); + // paddingType behaves the same as on flex layouts (see issue #10278). test('paddingType round-trips', (ctx) => { const grid = board(ctx).addGridLayout(); @@ -433,6 +520,29 @@ describe('Layout', () => { } }); + test('every layout child min and max bound rejects negatives', (ctx) => { + const b = board(ctx); + const flex = b.addFlexLayout(); + const rect = ctx.penpot.createRectangle(); + flex.appendChild(rect); + const child = rect.layoutChild; + expect(child).toBeDefined(); + if (child) { + expect(() => { + child.minWidth = -1; + }).toThrow(); + expect(() => { + child.maxWidth = -1; + }).toThrow(); + expect(() => { + child.minHeight = -1; + }).toThrow(); + expect(() => { + child.maxHeight = -1; + }).toThrow(); + } + }); + // marginType is the child-margin counterpart of a layout's paddingType. test('marginType round-trips', (ctx) => { const b = board(ctx); @@ -491,7 +601,7 @@ describe('Layout', () => { b.resize(300, 200); b.addFlexLayout(); - const found = ctx.penpot.currentPage.getShapeById(b.id) as Board; + const found = ctx.penpot.currentPage!.getShapeById(b.id) as Board; expect(found).not.toBeNull(); const child = ctx.penpot.createRectangle(); found.appendChild(child); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/library.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/library.test.ts index 32da4f5a59..1aacbe7780 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/library.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/library.test.ts @@ -206,28 +206,85 @@ describe('Library', () => { const typo = ctx.penpot.library.local.createTypography(); expect(typeof typo.fontFamily).toBe('string'); - typo.fontFamily = 'Arial'; - typo.fontId = 'gfont-arial'; - expect(typo.fontFamily).toBe('Arial'); - expect(typo.fontId).toBe('gfont-arial'); + const font = ctx.penpot.fonts.all[0]; + typo.fontFamily = font.fontFamily; + typo.fontId = font.fontId; + expect(typo.fontFamily).toBe(font.fontFamily); + expect(typo.fontId).toBe(font.fontId); }); test('typography style members round-trip', (ctx) => { const typo = ctx.penpot.library.local.createTypography(); - typo.fontStyle = 'italic'; + const font = + ctx.penpot.fonts.all.find((item) => + item.variants.some((variant) => variant.fontStyle === 'italic'), + ) ?? ctx.penpot.fonts.all[0]; + typo.setFont(font); + const italic = font.variants.find( + (variant) => variant.fontStyle === 'italic', + ); + if (italic) { + typo.fontStyle = italic.fontStyle; + expect(typo.fontStyle).toBe(italic.fontStyle); + expect(typo.fontVariantId).toBe(italic.fontVariantId); + expect(typo.fontWeight).toBe(italic.fontWeight); + } typo.textTransform = 'uppercase'; - typo.fontWeight = '700'; - typo.fontVariantId = 'regular'; + const variant = font.variants[font.variants.length - 1]; + typo.fontWeight = variant.fontWeight; + typo.fontVariantId = variant.fontVariantId; typo.lineHeight = '1.5'; typo.letterSpacing = '1'; - expect(typo.fontStyle).toBe('italic'); expect(typo.textTransform).toBe('uppercase'); - expect(typo.fontWeight).toBe('700'); - expect(typo.fontVariantId).toBe('regular'); - expect(typeof typo.lineHeight).toBe('string'); + expect(typo.fontWeight).toBe(variant.fontWeight); + expect(typo.fontStyle).toBe(variant.fontStyle); + expect(typo.fontVariantId).toBe(variant.fontVariantId); + expect(typo.lineHeight).toBe('1.5'); expect(typeof typo.letterSpacing).toBe('string'); }); + test('typography text values use text validation', (ctx) => { + const typo = ctx.penpot.library.local.createTypography(); + expect(() => { + typo.fontSize = '2'; + }).toThrow(); + expect(() => { + typo.fontSize = '12px'; + }).toThrow(); + expect(() => { + typo.lineHeight = '201'; + }).toThrow(); + expect(() => { + typo.lineHeight = '12px'; + }).toThrow(); + expect(() => { + typo.letterSpacing = '12px'; + }).toThrow(); + expect(() => { + typo.textTransform = 'not-a-transform' as unknown as 'uppercase'; + }).toThrow(); + }); + + test('typography font fields require installed fonts and variants', (ctx) => { + const typo = ctx.penpot.library.local.createTypography(); + expect(() => { + typo.fontId = 'missing-font'; + }).toThrow(); + expect(() => { + typo.fontFamily = 'Missing Font Family'; + }).toThrow(); + expect(() => { + typo.fontVariantId = 'missing-variant'; + }).toThrow(); + + const fonts = ctx.penpot.fonts.all; + const first = fonts[0]; + const second = fonts.find((font) => font.fontId !== first.fontId); + if (second) { + expect(() => typo.setFont(first, second.variants[0])).toThrow(); + } + }); + test('typography setFont updates the font', (ctx) => { const typo = ctx.penpot.library.local.createTypography(); const font = ctx.penpot.fonts.all[0]; diff --git a/plugins/apps/plugin-api-test-suite/src/tests/pages.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/pages.test.ts index 94f2c3842c..7c546a9e6a 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/pages.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/pages.test.ts @@ -1,5 +1,6 @@ import { expect } from '../framework/expect'; import { describe, test } from '../framework/registry'; +import type { Board } from '@penpot/plugin-types'; // Pages, selection and flows. // Most assertions use the active page (`currentPage`) and the scratch board so @@ -16,6 +17,18 @@ describe('Pages', () => { } }); + test('page names are trimmed and cannot be blank', (ctx) => { + const page = ctx.penpot.currentPage; + expect(page).not.toBeNull(); + if (page) { + expect(() => { + page.name = ' '; + }).toThrow(); + page.name = ' Trimmed page '; + expect(page.name).toBe('Trimmed page'); + } + }); + test('createPage and openPage activate a new page', async (ctx) => { const original = ctx.penpot.currentPage; const page = ctx.penpot.createPage(); @@ -206,4 +219,66 @@ describe('Flows', () => { expect(page.flows.length).toBe(before - 1); } }); + + test('flows require a live, unused board on their page', (ctx) => { + const page = ctx.penpot.currentPage; + expect(page).not.toBeNull(); + if (page) { + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + expect(() => + page.createFlow('rect-flow', rect as unknown as Board), + ).toThrow(); + + const removed = ctx.penpot.createBoard(); + ctx.board.appendChild(removed); + removed.remove(); + expect(() => page.createFlow('removed-flow', removed)).toThrow(); + + const target = ctx.penpot.createBoard(); + ctx.board.appendChild(target); + const first = page.createFlow('first-flow', target); + expect(() => page.createFlow('duplicate-flow', target)).toThrow(); + + const secondTarget = ctx.penpot.createBoard(); + ctx.board.appendChild(secondTarget); + const second = page.createFlow('second-flow', secondTarget); + expect(() => { + second.startingBoard = target; + }).toThrow(); + expect(() => { + second.startingBoard = removed; + }).toThrow(); + first.remove(); + second.remove(); + } + }); + + test('flows reject a starting board from another page', async (ctx) => { + const original = ctx.penpot.currentPage; + expect(original).not.toBeNull(); + if (!original) return; + + const localBoard = ctx.penpot.createBoard(); + ctx.board.appendChild(localBoard); + const flow = original.createFlow('cross-page-flow', localBoard); + const otherPage = ctx.penpot.createPage(); + try { + await ctx.penpot.openPage(otherPage); + const otherBoard = ctx.penpot.createBoard(); + (otherPage.root as Board).appendChild(otherBoard); + await ctx.penpot.openPage(original); + + expect(() => original.createFlow('foreign-flow', otherBoard)).toThrow(); + expect(() => { + flow.startingBoard = otherBoard; + }).toThrow(); + } finally { + if (ctx.penpot.currentPage?.id !== original.id) { + await ctx.penpot.openPage(original); + } + flow.remove(); + otherPage.remove(); + } + }); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/shadows-blur.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/shadows-blur.test.ts index 7ede39799a..713862b561 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/shadows-blur.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/shadows-blur.test.ts @@ -58,6 +58,36 @@ describe('Shadows', () => { expect(shadow.style).toBe('inner-shadow'); expect(shadow.hidden).toBe(true); }); + + test('negative shadow blur throws', (ctx) => { + const r = rect(ctx); + expect(() => { + r.shadows = [ + { + style: 'drop-shadow', + offsetX: 0, + offsetY: 0, + blur: -1, + spread: 0, + color: { color: '#000000', opacity: 1 }, + }, + ]; + }).toThrow(); + + r.shadows = [ + { + style: 'drop-shadow', + offsetX: 0, + offsetY: 0, + blur: 1, + spread: 0, + color: { color: '#000000', opacity: 1 }, + }, + ]; + expect(() => { + r.shadows[0].blur = -1; + }).toThrow(); + }); }); describe('Blur', () => { @@ -78,4 +108,14 @@ describe('Blur', () => { expect(r.backgroundBlur).toBeDefined(); expect(r.backgroundBlur && r.backgroundBlur.value).toBeCloseTo(5, 0); }); + + test('negative layer and background blur throw', (ctx) => { + const r = rect(ctx); + expect(() => { + r.blur = { value: -1 }; + }).toThrow(); + expect(() => { + r.backgroundBlur = { value: -1 }; + }).toThrow(); + }); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/shapes-geometry.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/shapes-geometry.test.ts index c31ef3912a..f337796938 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/shapes-geometry.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/shapes-geometry.test.ts @@ -1,5 +1,6 @@ import { expect } from '../framework/expect'; import { describe, test } from '../framework/registry'; +import type { Group } from '@penpot/plugin-types'; import type { TestContext } from '../framework/types'; // Shapes & geometry. @@ -254,6 +255,22 @@ describe('Shapes', () => { expect(r.borderRadiusBottomRight).toBeCloseTo(3.75, 2); expect(r.borderRadiusBottomLeft).toBeCloseTo(0.5, 2); }); + + test('individual corner radii reject negative values', (ctx) => { + const r = rect(ctx); + expect(() => { + r.borderRadiusTopLeft = -1; + }).toThrow(); + expect(() => { + r.borderRadiusTopRight = -1; + }).toThrow(); + expect(() => { + r.borderRadiusBottomRight = -1; + }).toThrow(); + expect(() => { + r.borderRadiusBottomLeft = -1; + }).toThrow(); + }); }); describe('Ordering', () => { @@ -365,7 +382,7 @@ describe('Shapes', () => { expect(group).not.toBeNull(); if (!group) return; - const copy = group.clone(); + const copy = group.clone() as Group; ctx.board.appendChild(copy); expect(copy.id).not.toBe(group.id); expect(copy.children).toHaveLength(group.children.length); @@ -447,6 +464,21 @@ describe('Shapes', () => { }).toThrow(); }); + test('export scale must be positive', (ctx) => { + const r = rect(ctx); + expect(() => { + r.exports = [{ type: 'png', scale: 0, suffix: '' }]; + }).toThrow(); + expect(() => { + r.exports = [{ type: 'png', scale: -1, suffix: '' }]; + }).toThrow(); + + r.exports = [{ type: 'png', scale: 1, suffix: '' }]; + expect(() => { + r.exports[0].scale = 0; + }).toThrow(); + }); + test('resize to zero dimensions throws', (ctx) => { const r = rect(ctx); expect(() => { diff --git a/plugins/apps/plugin-api-test-suite/src/tests/text.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/text.test.ts index 3dbaaf61c9..6b8ed2e448 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/text.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/text.test.ts @@ -302,6 +302,60 @@ describe('Text', () => { }).toThrow(); }); + test('text numeric fields require complete numbers within editor bounds', (ctx) => { + const t = text(ctx); + for (const value of ['.', '-', '12px', '2', '1001']) { + expect(() => { + t.fontSize = value; + }).toThrow(); + } + for (const value of ['.', '-', '12px', '-201', '201']) { + expect(() => { + t.lineHeight = value; + }).toThrow(); + expect(() => { + t.letterSpacing = value; + }).toThrow(); + } + }); + + test('text range numeric fields use the same validation', (ctx) => { + const range = text(ctx, 'Hello').getRange(0, 5); + expect(() => { + range.fontSize = '12px'; + }).toThrow(); + expect(() => { + range.lineHeight = '12px'; + }).toThrow(); + expect(() => { + range.letterSpacing = '12px'; + }).toThrow(); + }); + + test('text and range font fields reject missing fonts and variants', (ctx) => { + const t = text(ctx, 'Hello'); + expect(() => { + t.fontId = 'missing-font'; + }).toThrow(); + expect(() => { + t.fontFamily = 'Missing Font Family'; + }).toThrow(); + expect(() => { + t.fontVariantId = 'missing-variant'; + }).toThrow(); + + const range = t.getRange(0, 5); + expect(() => { + range.fontId = 'missing-font'; + }).toThrow(); + expect(() => { + range.fontFamily = 'Missing Font Family'; + }).toThrow(); + expect(() => { + range.fontVariantId = 'missing-variant'; + }).toThrow(); + }); + test('invalid align value throws', (ctx) => { const t = text(ctx); expect(() => { @@ -314,6 +368,9 @@ describe('Text', () => { expect(() => { t.textTransform = 'UPPERCASE' as unknown as 'uppercase'; }).toThrow(); + expect(() => { + t.getRange(0, 1).textTransform = 'UPPERCASE' as unknown as 'uppercase'; + }).toThrow(); }); test('invalid direction value throws', (ctx) => { diff --git a/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts index 0b3ae9be6c..9a3f65d0d0 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/tokens.test.ts @@ -56,6 +56,20 @@ describe('Tokens', () => { expect(cat.themes.length).toBeGreaterThan(0); expect(cat.getThemeById(theme.id)).toBeDefined(); }); + + test('catalog name validation uses current state and rejects blank names', (ctx) => { + const cat = catalog(ctx); + const name = unique('live-set'); + cat.addSet({ name }); + expect(() => cat.addSet({ name })).toThrow(); + expect(() => cat.addSet({ name: ' ' })).toThrow(); + expect(() => cat.addTheme({ group: '', name: ' ' })).toThrow(); + + const group = unique('live-theme-group'); + const themeName = unique('live-theme'); + cat.addTheme({ group, name: themeName }); + expect(() => cat.addTheme({ group, name: themeName })).toThrow(); + }); }); describe('Set', () => { @@ -70,6 +84,17 @@ describe('Tokens', () => { expect(set.active).toBe(true); }); + test('retained set proxies validate names against current state', (ctx) => { + const first = activeSet(ctx, unique('set')); + const second = activeSet(ctx, unique('set')); + expect(() => { + first.name = second.name; + }).toThrow(); + expect(() => { + first.name = ' '; + }).toThrow(); + }); + // Community report (forum #10700, issue #14): toggling a token set's // active state was said to freeze and roll back when tokens from the set // are bound to shapes. Did not reproduce; kept as a regression pin. @@ -148,15 +173,42 @@ describe('Tokens', () => { }), ).toThrow(); }); + + test('empty composites and line-height without font-size throw', (ctx) => { + const set = activeSet(ctx, unique('set')); + expect(() => + set.addToken({ + type: 'typography', + name: unique('empty-typography.'), + value: {} as never, + }), + ).toThrow(); + expect(() => + set.addToken({ + type: 'shadow', + name: unique('empty-shadow.'), + value: [] as never, + }), + ).toThrow(); + expect(() => + set.addToken({ + type: 'typography', + name: unique('line-height-only.'), + value: { lineHeight: '1.2' } as never, + }), + ).toThrow(); + }); }); describe('Theme', () => { test('group, name and active round-trip', (ctx) => { const theme = catalog(ctx).addTheme({ group: '', name: unique('theme') }); - theme.group = 'brand'; - theme.name = 'dark'; - expect(theme.group).toBe('brand'); - expect(theme.name).toBe('dark'); + const group = unique('brand'); + const name = unique('dark'); + theme.group = group; + theme.name = name; + expect(theme.group).toBe(group); + expect(theme.name).toBe(name); theme.active = true; expect(theme.active).toBe(true); theme.toggleActive(); @@ -191,6 +243,33 @@ describe('Tokens', () => { expect(dup.id).not.toBe(theme.id); dup.remove(); }); + + test('theme names stay unique within their current group', (ctx) => { + const cat = catalog(ctx); + const group = unique('group'); + const first = cat.addTheme({ group, name: unique('theme') }); + const second = cat.addTheme({ group, name: unique('theme') }); + expect(() => { + first.name = second.name; + }).toThrow(); + expect(() => { + first.name = ' '; + }).toThrow(); + + const duplicate = first.duplicate(); + expect(duplicate.name).not.toBe(first.name); + duplicate.remove(); + }); + + test('moving a retained theme cannot create a group/name collision', (ctx) => { + const cat = catalog(ctx); + const name = unique('shared-theme'); + const first = cat.addTheme({ group: unique('group-a'), name }); + const second = cat.addTheme({ group: unique('group-b'), name }); + expect(() => { + first.group = second.group; + }).toThrow(); + }); }); describe('Token', () => { @@ -315,9 +394,131 @@ describe('Tokens', () => { }); const dup = token.duplicate(); expect(dup.id).not.toBe(token.id); + expect(dup.name).not.toBe(token.name); dup.remove(); }); + test('token edits reject missing, self, cyclic, and dotted-name conflicts', (ctx) => { + const set = activeSet(ctx, unique('set')); + const prefix = unique('tree'); + set.addToken({ + type: 'dimension', + name: `${prefix}.child`, + value: '8', + }); + const token = set.addToken({ + type: 'dimension', + name: unique('editable.'), + value: '4', + }); + expect(() => { + token.name = prefix; + }).toThrow(); + expect(() => { + token.value = `{${token.name}}`; + }).toThrow(); + expect(() => { + token.value = '{missing-token}'; + }).toThrow(); + + const other = set.addToken({ + type: 'dimension', + name: unique('other.'), + value: '2', + }); + token.value = `{${other.name}}`; + expect(() => { + other.value = `{${token.name}}`; + }).toThrow(); + }); + + test('retained token proxies validate names against current state', (ctx) => { + const set = activeSet(ctx, unique('set')); + const retained = set.addToken({ + type: 'dimension', + name: unique('retained.'), + value: '1', + }); + const later = set.addToken({ + type: 'dimension', + name: unique('later.'), + value: '2', + }); + expect(() => { + retained.name = later.name; + }).toThrow(); + }); + + test('renaming validates the whole token and rejects a new self-reference', (ctx) => { + const targetName = unique('rename-target'); + activeSet(ctx, unique('target-set')).addToken({ + type: 'dimension', + name: targetName, + value: '8', + }); + const token = activeSet(ctx, unique('source-set')).addToken({ + type: 'dimension', + name: unique('rename-source'), + value: `{${targetName}}`, + }); + + expect(() => { + token.name = targetName; + }).toThrow(); + }); + + test('composite token updates reject empty values and missing font size', (ctx) => { + const set = activeSet(ctx, unique('set')); + const typography = set.addToken({ + type: 'typography', + name: unique('typography.'), + value: { fontSizes: '14', lineHeight: '1.2' } as never, + }) as TokenTypography; + const shadow = set.addToken({ + type: 'shadow', + name: unique('shadow.'), + value: { + color: '#000000', + inset: 'false', + offsetX: '0', + offsetY: '0', + spread: '0', + blur: '1', + }, + }) as TokenShadow; + + expect(() => { + typography.value = {} as never; + }).toThrow(); + expect(() => { + typography.value = { lineHeight: '1.2' } as never; + }).toThrow(); + expect(() => { + shadow.value = []; + }).toThrow(); + }); + + test('tokens with newly broken references cannot be applied', (ctx) => { + const set = activeSet(ctx, unique('set')); + const base = set.addToken({ + type: 'borderRadius', + name: unique('base.'), + value: '8', + }); + const ref = set.addToken({ + type: 'borderRadius', + name: unique('ref.'), + value: `{${base.name}}`, + }); + base.remove(); + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + ctx.penpot.selection = [rect]; + expect(() => ref.applyToShapes([rect])).toThrow(); + expect(() => ref.applyToSelected()).toThrow(); + expect(() => rect.applyToken(ref)).toThrow(); + }); + // Reference resolution — a token referencing another resolves transitively. test('a token referencing another token resolves transitively', (ctx) => { const set = activeSet(ctx, unique('set')); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/value-objects.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/value-objects.test.ts index 88f3eb94c8..f552b2e0fc 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/value-objects.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/value-objects.test.ts @@ -247,13 +247,11 @@ describe('Value objects', () => { } }); - test('negative blur value is accepted (currently unvalidated)', (ctx) => { - // The blur setter does not reject a negative value; this pins the current - // lenient behaviour (a candidate for future hardening). + test('negative blur value throws', (ctx) => { const r = rect(ctx); expect(() => { r.blur = { value: -5 }; - }).not.toThrow(); + }).toThrow(); }); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/variants.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/variants.test.ts index d1e2be5a5a..451e75f91b 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/variants.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/variants.test.ts @@ -232,9 +232,12 @@ describe('Variants', () => { const instance = vc.instance(); ctx.board.appendChild(instance); - // Valid args (nat-int pos, string value): switches to the nearest variant - // with that value at the property position, or no-ops — never throws. - expect(() => instance.switchVariant(0, 'large')).not.toThrow(); + const property = vc.variants!.properties[0]; + const target = + vc.variants!.variantComponents()[1] as LibraryVariantComponent; + expect(() => + instance.switchVariant(0, target.variantProps[property]), + ).not.toThrow(); }); // Community report (forum #10700, issue #3): switchVariant on an instance @@ -256,7 +259,12 @@ describe('Variants', () => { const clonedInstance = cloned.children.find((s) => s.isComponentInstance()); expect(clonedInstance).toBeDefined(); if (clonedInstance) { - expect(() => clonedInstance.switchVariant(0, 'large')).not.toThrow(); + const property = vc.variants!.properties[0]; + const target = + vc.variants!.variantComponents()[1] as LibraryVariantComponent; + expect(() => + clonedInstance.switchVariant(0, target.variantProps[property]), + ).not.toThrow(); } }); @@ -332,6 +340,14 @@ describe('Variants', () => { expect(() => ctx.penpot.createVariantFromComponents([])).toThrow(); }); + test('createVariantFromComponents requires two distinct components', (ctx) => { + const main = componentMain(ctx); + expect(() => ctx.penpot.createVariantFromComponents([main])).toThrow(); + expect(() => + ctx.penpot.createVariantFromComponents([main, main]), + ).toThrow(); + }); + test('removeProperty out of bounds throws', async (ctx) => { const vc = await variantComponent(ctx); const v = vc.variants; @@ -341,6 +357,13 @@ describe('Variants', () => { } }); + test('the last variant property cannot be removed', async (ctx) => { + const vc = await variantComponent(ctx); + const v = vc.variants!; + expect(v.properties).toHaveLength(1); + expect(() => v.removeProperty(0)).toThrow(); + }); + test('renameProperty out of bounds throws', async (ctx) => { const vc = await variantComponent(ctx); const v = vc.variants; @@ -350,8 +373,88 @@ describe('Variants', () => { } }); + test('variant property names are trimmed, nonblank, and at most 60 characters', async (ctx) => { + const vc = await variantComponent(ctx); + const v = vc.variants!; + expect(() => v.renameProperty(0, ' ')).toThrow(); + expect(() => v.renameProperty(0, 'x'.repeat(61))).toThrow(); + v.renameProperty(0, ' Size '); + expect(v.properties[0]).toBe('Size'); + }); + test('setVariantProperty out of bounds throws', async (ctx) => { const vc = await variantComponent(ctx); expect(() => vc.setVariantProperty(999, 'large')).toThrow(); }); + + test('variant values are trimmed and at most 60 characters', async (ctx) => { + const vc = await variantComponent(ctx); + expect(() => vc.setVariantProperty(0, 'x'.repeat(61))).toThrow(); + vc.setVariantProperty(0, ' Large '); + expect(vc.variantProps[vc.variants!.properties[0]]).toBe('Large'); + }); + + test('empty variant values remain allowed after trimming', async (ctx) => { + const vc = await variantComponent(ctx); + expect(() => vc.setVariantProperty(0, ' ')).not.toThrow(); + expect(vc.variantProps[vc.variants!.properties[0]]).toBe(''); + }); + + test('switchVariant rejects mains, bad positions, and unavailable values', async (ctx) => { + const vc = await variantComponent(ctx); + vc.addVariant(); + await waitFor(() => (vc.variants?.variantComponents().length ?? 0) > 1); + const instance = vc.instance(); + ctx.board.appendChild(instance); + expect(() => vc.mainInstance().switchVariant(0, 'Value2')).toThrow(); + expect(() => instance.switchVariant(999, 'Value2')).toThrow(); + expect(() => instance.switchVariant(0, 'missing-value')).toThrow(); + }); + + test('variant combining rejects copies and existing variants', async (ctx) => { + const standard = componentWithMain(ctx); + const other = componentWithMain(ctx); + const copy = other.comp.instance() as Board; + ctx.board.appendChild(copy); + + expect(() => + ctx.penpot.createVariantFromComponents([standard.main, copy]), + ).toThrow(); + expect(() => standard.main.combineAsVariants([copy.id])).toThrow(); + + const variant = await variantComponent(ctx); + expect(() => + ctx.penpot.createVariantFromComponents([ + standard.main, + variant.mainInstance() as Board, + ]), + ).toThrow(); + expect(() => + standard.main.combineAsVariants([variant.mainInstance().id]), + ).toThrow(); + }); + + test('variant combining rejects components from another page', async (ctx) => { + const original = ctx.penpot.currentPage; + expect(original).not.toBeNull(); + if (!original) return; + + const originalMain = componentMain(ctx); + const otherPage = ctx.penpot.createPage(); + try { + await ctx.penpot.openPage(otherPage); + const rect = ctx.penpot.createRectangle(); + (otherPage.root as Board).appendChild(rect); + const other = ctx.penpot.library.local.createComponent([rect]); + const otherMain = other.mainInstance() as Board; + + expect(() => + ctx.penpot.createVariantFromComponents([originalMain, otherMain]), + ).toThrow(); + expect(() => otherMain.combineAsVariants([originalMain.id])).toThrow(); + } finally { + await ctx.penpot.openPage(original); + otherPage.remove(); + } + }); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/viewport-guides.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/viewport-guides.test.ts index 2680c3991e..4feec6e14a 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/viewport-guides.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/viewport-guides.test.ts @@ -1,5 +1,6 @@ import { expect } from '../framework/expect'; import { describe, test } from '../framework/registry'; +import type { Board } from '@penpot/plugin-types'; // Viewport and guides (ruler guides + board guides). @@ -80,6 +81,51 @@ describe('Ruler guides', () => { page.removeRulerGuide(guide); } }); + + test('page ruler guide rejects a removed board', (ctx) => { + const page = ctx.penpot.currentPage; + expect(page).not.toBeNull(); + if (page) { + const removed = ctx.penpot.createBoard(); + ctx.board.appendChild(removed); + removed.remove(); + expect(() => page.addRulerGuide('vertical', 20, removed)).toThrow(); + + const guide = page.addRulerGuide('vertical', 20); + expect(() => { + guide.board = removed; + }).toThrow(); + guide.remove(); + } + }); + + test('ruler guides reject boards from another page', async (ctx) => { + const original = ctx.penpot.currentPage; + expect(original).not.toBeNull(); + if (!original) return; + + const guide = original.addRulerGuide('vertical', 20); + const otherPage = ctx.penpot.createPage(); + try { + await ctx.penpot.openPage(otherPage); + const otherBoard = ctx.penpot.createBoard(); + (otherPage.root as Board).appendChild(otherBoard); + await ctx.penpot.openPage(original); + + expect(() => + original.addRulerGuide('vertical', 20, otherBoard), + ).toThrow(); + expect(() => { + guide.board = otherBoard; + }).toThrow(); + } finally { + if (ctx.penpot.currentPage?.id !== original.id) { + await ctx.penpot.openPage(original); + } + guide.remove(); + otherPage.remove(); + } + }); }); describe('Board guides', () => { @@ -137,4 +183,66 @@ describe('Board guides', () => { } } }); + + test('board guides allow automatic sizes', (ctx) => { + expect(() => { + ctx.board.guides = [ + { + type: 'column', + display: true, + params: { color: { color: '#ff0000', opacity: 1 } }, + }, + { + type: 'row', + display: true, + params: { color: { color: '#00ff00', opacity: 1 } }, + }, + { + type: 'square', + display: true, + params: { color: { color: '#0000ff', opacity: 1 } }, + }, + ]; + }).not.toThrow(); + }); + + test('board guide sizes must meet their positive minimums', (ctx) => { + expect(() => { + ctx.board.guides = [ + { + type: 'column', + display: true, + params: { + color: { color: '#000000', opacity: 1 }, + type: 'stretch', + size: 0.5, + gutter: 0, + }, + }, + ]; + }).toThrow(); + expect(() => { + ctx.board.guides = [ + { + type: 'row', + display: true, + params: { + color: { color: '#000000', opacity: 1 }, + type: 'stretch', + size: 0.5, + gutter: 0, + }, + }, + ]; + }).toThrow(); + expect(() => { + ctx.board.guides = [ + { + type: 'square', + display: true, + params: { color: { color: '#000000', opacity: 1 }, size: 0.009 }, + }, + ]; + }).toThrow(); + }); });