From 7655f854a4ad48eb485c8a7692229952c7eac1c0 Mon Sep 17 00:00:00 2001 From: Alonso Torres Date: Tue, 7 Jul 2026 16:12:12 +0200 Subject: [PATCH] :sparkles: Improved api test suite (#10533) * :bug: Fix plugins api issues * :bug: Fix problem with text weight * :bug: Fix api problems * :sparkles: Updated plugins test suite * :bug: Referential integrity tests * :sparkles: Add resetOverrides method * :bug: Fix plugins components structure poluting --- .../src/app/main/data/workspace/media.cljs | 11 + .../main/data/workspace/thumbnails_wasm.cljs | 3 +- frontend/src/app/plugins/api.cljs | 109 ++++--- frontend/src/app/plugins/exports.cljs | 42 +++ frontend/src/app/plugins/file.cljs | 26 +- frontend/src/app/plugins/flex.cljs | 34 ++- frontend/src/app/plugins/format.cljs | 17 +- frontend/src/app/plugins/grid.cljs | 84 ++++-- frontend/src/app/plugins/library.cljs | 49 +-- frontend/src/app/plugins/page.cljs | 21 ++ frontend/src/app/plugins/shadows.cljs | 60 ++++ frontend/src/app/plugins/shape.cljs | 284 +++++++++++------- frontend/src/app/plugins/text.cljs | 17 +- frontend/src/app/plugins/tokens.cljs | 16 +- frontend/src/app/plugins/tracks.cljs | 63 ++++ frontend/src/app/plugins/utils.cljs | 19 ++ .../frontend_tests/plugins/format_test.cljs | 9 +- .../frontend_tests/plugins/tokens_test.cljs | 8 +- .../plugins/value_objects_test.cljs | 111 +++++++ frontend/test/frontend_tests/runner.cljs | 2 + mcp/packages/plugin/src/PenpotUtils.test.ts | 31 ++ mcp/packages/plugin/src/PenpotUtils.ts | 18 +- plugins/CHANGELOG.md | 30 +- plugins/apps/plugin-api-test-suite/README.md | 30 +- .../apps/plugin-api-test-suite/ci/run-ci.ts | 12 +- .../plugin-api-test-suite/src/ci/headless.ts | 18 +- .../src/framework/registry.ts | 97 ++++-- .../src/framework/runner.ts | 136 +++++++-- .../src/framework/static-coverage.ts | 1 + .../src/framework/types.ts | 12 + .../src/generated/api-surface.json | 118 +++++++- .../apps/plugin-api-test-suite/src/model.ts | 10 +- .../apps/plugin-api-test-suite/src/plugin.ts | 18 +- .../src/tests/comments.test.ts | 11 +- .../src/tests/components.test.ts | 191 +++++++++++- .../src/tests/events.test.ts | 7 +- .../src/tests/file.test.ts | 26 +- .../src/tests/fills-strokes.test.ts | 26 ++ .../src/tests/interactions.test.ts | 19 +- .../src/tests/layout.test.ts | 108 +++++++ .../src/tests/library.test.ts | 50 +++ .../src/tests/misc.test.ts | 106 +++++-- .../src/tests/pages.test.ts | 53 +++- .../src/tests/platform.test.ts | 10 +- .../src/tests/plugin-data.test.ts | 5 +- .../src/tests/shapes-factories.test.ts | 42 ++- .../src/tests/shapes-geometry.test.ts | 135 ++++++++- .../src/tests/shapes-types.test.ts | 17 +- .../src/tests/text.test.ts | 29 +- .../src/tests/tokens.test.ts | 78 ++++- .../src/tests/value-objects.test.ts | 9 +- .../src/tests/variants.test.ts | 198 ++++++++++-- .../src/tests/viewport-guides.test.ts | 6 +- .../plugin-api-test-suite/src/tests/wait.ts | 22 ++ plugins/apps/plugin-api-test-suite/src/ui.ts | 51 +++- plugins/libs/plugin-types/index.d.ts | 67 +++++ 56 files changed, 2347 insertions(+), 435 deletions(-) create mode 100644 frontend/src/app/plugins/exports.cljs create mode 100644 frontend/src/app/plugins/shadows.cljs create mode 100644 frontend/src/app/plugins/tracks.cljs create mode 100644 frontend/test/frontend_tests/plugins/value_objects_test.cljs create mode 100644 plugins/apps/plugin-api-test-suite/src/tests/wait.ts diff --git a/frontend/src/app/main/data/workspace/media.cljs b/frontend/src/app/main/data/workspace/media.cljs index 8c0aa22375..b0fc60445e 100644 --- a/frontend/src/app/main/data/workspace/media.cljs +++ b/frontend/src/app/main/data/workspace/media.cljs @@ -44,6 +44,17 @@ [input] (svgo/optimize input svgo/defaultOptions)) +(defn valid-svg-string? + "True when `text` parses as SVG XML. Lets callers reject malformed markup + up front instead of failing asynchronously inside the import pipeline." + [text] + (and (string? text) + (not (str/blank? text)) + (try + (tubax/xml->clj text) + true + (catch :default _ false)))) + (defn svg->clj [[name text]] (try diff --git a/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs b/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs index 612bdd2e53..853b194bbe 100644 --- a/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs +++ b/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs @@ -81,7 +81,8 @@ (fn [err] (rx/error! subs err))))) - (rx/error! subs "Frame not found"))) + ;; A missing frame. Nothing to render, so end quietly. + (rx/end! subs))) (catch :default err (rx/error! subs err)))))] #(timers/cancel-af! req-id))))) diff --git a/frontend/src/app/plugins/api.cljs b/frontend/src/app/plugins/api.cljs index 7b8c5ef15a..2b65475c22 100644 --- a/frontend/src/app/plugins/api.cljs +++ b/frontend/src/app/plugins/api.cljs @@ -313,43 +313,59 @@ :group (fn [shapes] - (cond - (or (not (array? shapes)) (not (every? shape/shape-proxy? shapes))) - (u/not-valid plugin-id :group-shapes shapes) + (let [valid-shapes? (and (array? shapes) (every? shape/shape-proxy? shapes)) + file-id (:current-file-id @st/state) + page-id (:current-page-id @st/state) + objects (when valid-shapes? (u/locate-objects file-id page-id)) + id (uuid/next) + ids (when valid-shapes? (into #{} (map #(obj/get % "$id")) shapes)) + shape-objs (when valid-shapes? (map #(get objects %) ids))] + (cond + (not valid-shapes?) + (u/not-valid plugin-id :group-shapes shapes) - ;; A group cannot be created from no shapes; per the documented contract - ;; return null instead of a proxy pointing at a shape that never exists. - (zero? (alength shapes)) - nil + ;; A group cannot be created from no shapes; per the documented contract + ;; return null instead of a proxy pointing at a shape that never exists. + (zero? (alength shapes)) + nil - (some #(not (u/page-active? (obj/get % "$page"))) shapes) - (u/not-valid plugin-id :group "Cannot modify a page that is not currently active") + (some #(not (u/page-active? (obj/get % "$page"))) shapes) + (u/not-valid plugin-id :group "Cannot modify a page that is not currently active") - :else - (let [file-id (:current-file-id @st/state) - page-id (:current-page-id @st/state) - id (uuid/next) - ids (into #{} (map #(obj/get % "$id")) shapes)] - (st/emit! (dwg/group-shapes id ids) - (se/event plugin-id "create-shape" :type type)) - (shape/shape-proxy plugin-id file-id page-id id)))) + (some #(u/inside-component-copy? objects %) shape-objs) + (u/not-valid plugin-id :group "Cannot change the structure of a component copy") + + :else + (do + (st/emit! (dwg/group-shapes id ids) + (se/event plugin-id "create-shape" :type type)) + (shape/shape-proxy plugin-id file-id page-id id))))) :ungroup (fn [group & rest] - (cond - (not (shape/shape-proxy? group)) - (u/not-valid plugin-id :ungroup group) + (let [valid-group? (shape/shape-proxy? group) + valid-rest? (every? shape/shape-proxy? rest) + shapes (when (and valid-group? valid-rest?) (concat [group] rest)) + file-id (:current-file-id @st/state) + page-id (:current-page-id @st/state) + objects (when shapes (u/locate-objects file-id page-id)) + ids (when shapes (into #{} (map #(obj/get % "$id")) shapes)) + shape-objs (when shapes (map #(get objects %) ids))] + (cond + (not valid-group?) + (u/not-valid plugin-id :ungroup group) - (and (some? rest) (not (every? shape/shape-proxy? rest))) - (u/not-valid plugin-id :ungroup rest) + (not valid-rest?) + (u/not-valid plugin-id :ungroup rest) - (or (not (u/page-active? (obj/get group "$page"))) - (some #(not (u/page-active? (obj/get % "$page"))) rest)) - (u/not-valid plugin-id :ungroup "Cannot modify a page that is not currently active") + (or (not (u/page-active? (obj/get group "$page"))) + (some #(not (u/page-active? (obj/get % "$page"))) rest)) + (u/not-valid plugin-id :ungroup "Cannot modify a page that is not currently active") - :else - (let [shapes (concat [group] rest) - ids (into #{} (map #(obj/get % "$id")) shapes)] + (some #(u/inside-component-copy? objects %) shape-objs) + (u/not-valid plugin-id :ungroup "Cannot change the structure of a component copy") + + :else (st/emit! (dwg/ungroup-shapes ids))))) :createBoard @@ -418,7 +434,7 @@ :createShapeFromSvg (fn [svg-string] (cond - (or (not (string? svg-string)) (empty? svg-string)) + (not (dwm/valid-svg-string? svg-string)) (u/not-valid plugin-id :createShapeFromSvg svg-string) :else @@ -434,7 +450,7 @@ (js/Promise. (fn [resolve reject] (cond - (or (not (string? svg-string)) (empty? svg-string)) + (not (dwm/valid-svg-string? svg-string)) (do (u/not-valid plugin-id :createShapeFromSvg "Svg not valid") (reject "Svg not valid")) @@ -592,19 +608,28 @@ (page/page-proxy? page) (obj/get page "$id") (string? page) (uuid/parse* page) :else nil)] - (if (nil? id) + (cond + (nil? id) (u/not-valid plugin-id :openPage "Expected a Page object or a page UUID string") - (if (true? new-window) - (do (st/emit! (dcm/go-to-workspace :page-id id ::rt/new-window true)) - (js/Promise.resolve nil)) - (js/Promise. - (fn [resolve _] - (->> st/stream - (rx/filter (ptk/type? ::dwpg/initialized)) - (rx/filter #(= (deref %) id)) - (rx/take 1) - (rx/subs! #(resolve nil))) - (st/emit! (dcm/go-to-workspace :page-id id)))))))) + + (true? new-window) + (do (st/emit! (dcm/go-to-workspace :page-id id ::rt/new-window true)) + (js/Promise.resolve nil)) + + ;; Navigating to the already-active page emits no initialization + ;; event, so resolve right away instead of waiting forever. + (u/page-active? id) + (js/Promise.resolve nil) + + :else + (js/Promise. + (fn [resolve _] + (->> st/stream + (rx/filter (ptk/type? ::dwpg/initialized)) + (rx/filter #(= (deref %) id)) + (rx/take 1) + (rx/subs! #(resolve nil))) + (st/emit! (dcm/go-to-workspace :page-id id))))))) :alignHorizontal (fn [shapes direction] diff --git a/frontend/src/app/plugins/exports.cljs b/frontend/src/app/plugins/exports.cljs new file mode 100644 index 0000000000..0363fdf582 --- /dev/null +++ b/frontend/src/app/plugins/exports.cljs @@ -0,0 +1,42 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns app.plugins.exports + (:require + [app.plugins.format :as format] + [app.plugins.parser :as parser] + [app.util.object :as obj])) + +(defn export-proxy + [export-data on-change!] + (let [state (atom export-data)] + (obj/reify {:name "ExportProxy"} + :type + {:get (fn [] (format/format-key (:type @state))) + :set (fn [v] (swap! state assoc :type (parser/parse-keyword v)) (on-change!))} + + :scale + {:get (fn [] (:scale @state)) + :set (fn [v] (swap! state assoc :scale v) (on-change!))} + + :suffix + {:get (fn [] (:suffix @state)) + :set (fn [v] (swap! state assoc :suffix v) (on-change!))} + + :skipChildren + {:get (fn [] (:skip-children @state)) + :set (fn [v] (swap! state assoc :skip-children v) (on-change!))}))) + +(defn format-exports + ([exports] (format-exports exports nil)) + ([exports commit-fn] + (if (and (some? exports) (fn? commit-fn)) + (let [arr-ref (atom nil) + on-change! (fn [] (commit-fn @arr-ref)) + arr (apply array (mapv #(export-proxy % on-change!) exports))] + (reset! arr-ref arr) + arr) + (format/format-array format/format-export exports)))) diff --git a/frontend/src/app/plugins/file.cljs b/frontend/src/app/plugins/file.cljs index ac6ef089e5..12049611bd 100644 --- a/frontend/src/app/plugins/file.cljs +++ b/frontend/src/app/plugins/file.cljs @@ -8,6 +8,7 @@ (:require [app.common.data :as d] [app.common.data.macros :as dm] + [app.common.files.validate :as cfv] [app.common.time :as ct] [app.common.uuid :as uuid] [app.config :as cf] @@ -55,7 +56,7 @@ (do (swap! data assoc :label value :created-by "user") (->> (rp/cmd! :update-file-snapshot {:id (:id @data) :label value}) (rx/take 1) - (rx/subs! #(st/emit! (se/event "rename-version" :file-id file-id)))))))} + (rx/subs! #(st/emit! (se/event plugin-id "rename-version" :file-id file-id)))))))} :createdBy {:get @@ -148,6 +149,27 @@ (let [file (u/locate-file id)] (apply array (sequence (map #(page/page-proxy plugin-id id %)) (dm/get-in file [:data :pages]))))) + ;; Run referential-integrity validation on the file and return the errors + ;; found (an empty array means the file is valid). Each error carries the + ;; validation `code`, a `hint`, and the offending `shapeId`/`pageId`. + :validate + (fn [] + (let [file (u/locate-file id) + libraries (get @st/state :files)] + (try + (->> (cfv/validate-file file libraries) + (map (fn [{:keys [code hint shape-id page-id]}] + #js {:code (name code) + :hint hint + :shapeId (some-> shape-id str) + :pageId (some-> page-id str)})) + (apply array)) + (catch :default cause + #js [#js {:code "validation-error" + :hint (ex-message cause) + :shapeId nil + :pageId nil}])))) + ;; Plugin data :getPluginData (fn [key] @@ -322,7 +344,7 @@ (fn [resolve reject] (cond (not (r/check-permission plugin-id "content:write")) - (u/reject-not-valid reject :findVersions "Plugin doesn't have 'content:write' permission") + (u/reject-not-valid reject :saveVersion "Plugin doesn't have 'content:write' permission") :else (st/emit! diff --git a/frontend/src/app/plugins/flex.cljs b/frontend/src/app/plugins/flex.cljs index d3067fb7c0..ff1ec594a7 100644 --- a/frontend/src/app/plugins/flex.cljs +++ b/frontend/src/app/plugins/flex.cljs @@ -295,22 +295,28 @@ :appendChild (fn [child] - (cond - (not (shape-proxy? child)) - (u/not-valid plugin-id :appendChild child) + (let [valid-child? (shape-proxy? child) + child-page (when valid-child? (obj/get child "$page")) + child-id (when valid-child? (obj/get child "$id")) + objects (when valid-child? (u/locate-objects file-id page-id)) + shape (when valid-child? (get objects id)) + child-shape (when valid-child? (u/locate-shape file-id page-id child-id)) + index (when valid-child? + (if (and (u/natural-child-ordering? plugin-id) (not (ctl/reverse? shape))) + 0 + (count (:shapes shape))))] + (cond + (not valid-child?) + (u/not-valid plugin-id :appendChild child) - (or (not (u/page-active? page-id)) - (not (u/page-active? (obj/get child "$page")))) - (u/not-valid plugin-id :appendChild "Cannot modify a page that is not currently active") + (or (not (u/page-active? page-id)) + (not (u/page-active? child-page))) + (u/not-valid plugin-id :appendChild "Cannot modify a page that is not currently active") - :else - (let [child-id (obj/get child "$id") - shape (u/locate-shape file-id page-id id) - child-shape (u/locate-shape file-id page-id child-id) - index - (if (and (u/natural-child-ordering? plugin-id) (not (ctl/reverse? shape))) - 0 - (count (:shapes shape)))] + (u/changes-component-copy-structure? objects shape child-shape) + (u/not-valid plugin-id :appendChild "Cannot change the structure of a component copy") + + :else (st/emit! (dwsh/relocate-shapes #{child-id} id index) (se/event plugin-id "add-layout-element" diff --git a/frontend/src/app/plugins/format.cljs b/frontend/src/app/plugins/format.cljs index e920793434..1f0701dd99 100644 --- a/frontend/src/app/plugins/format.cljs +++ b/frontend/src/app/plugins/format.cljs @@ -174,10 +174,6 @@ :hidden hidden :color (format-color color)}))) -(defn format-shadows - [shadows] - (format-array format-shadow shadows)) - ;;export interface Fill { ;; fillColor?: string; ;; fillOpacity?: number; @@ -249,16 +245,13 @@ ;; suffix: string; ;; } (defn format-export - [{:keys [type scale suffix] :as export}] + [{:keys [type scale suffix skip-children] :as export}] (when (some? export) (obj/without-empty #js {:type (format-key type) :scale scale - :suffix suffix}))) - -(defn format-exports - [exports] - (format-array format-export exports)) + :suffix suffix + :skipChildren skip-children}))) ;; export interface GuideColumnParams { ;; color: { color: string; opacity: number }; @@ -409,10 +402,6 @@ #js {:type (-> type format-key) :value value}))) -(defn format-tracks - [tracks] - (format-array format-track tracks)) - ;; export interface Dissolve { ;; type: 'dissolve'; diff --git a/frontend/src/app/plugins/grid.cljs b/frontend/src/app/plugins/grid.cljs index 3ef266c1ad..e0122062db 100644 --- a/frontend/src/app/plugins/grid.cljs +++ b/frontend/src/app/plugins/grid.cljs @@ -12,8 +12,8 @@ [app.main.data.workspace.shape-layout :as dwsl] [app.main.data.workspace.transforms :as dwt] [app.main.store :as st] - [app.plugins.format :as format] [app.plugins.register :as r] + [app.plugins.tracks :as tracks] [app.plugins.utils :as u] [app.util.object :as obj] [potok.v2.core :as ptk])) @@ -53,11 +53,53 @@ :rows {:this true - :get #(-> % u/proxy->shape :layout-grid-rows format/format-tracks)} + :get (fn [self] + (tracks/format-tracks plugin-id file-id page-id id :row + (-> self u/proxy->shape :layout-grid-rows)))} :columns {:this true - :get #(-> % u/proxy->shape :layout-grid-columns format/format-tracks)} + :get (fn [self] + (tracks/format-tracks plugin-id file-id page-id id :column + (-> self u/proxy->shape :layout-grid-columns)))} + + :horizontalSizing + {:this true + :get #(-> % u/proxy->shape :layout-item-h-sizing (d/nilv :fix) d/name) + :set + (fn [_ value] + (let [value (keyword value)] + (cond + (not (contains? ctl/item-h-sizing-types value)) + (u/not-valid plugin-id :horizontalSizing value) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :horizontalSizing "Plugin doesn't have 'content:write' permission") + + (not (u/page-active? page-id)) + (u/not-valid plugin-id :horizontalSizing "Cannot modify a page that is not currently active") + + :else + (st/emit! (dwsl/update-layout #{id} {:layout-item-h-sizing value})))))} + + :verticalSizing + {:this true + :get #(-> % u/proxy->shape :layout-item-v-sizing (d/nilv :fix) d/name) + :set + (fn [_ value] + (let [value (keyword value)] + (cond + (not (contains? ctl/item-v-sizing-types value)) + (u/not-valid plugin-id :verticalSizing value) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :verticalSizing "Plugin doesn't have 'content:write' permission") + + (not (u/page-active? page-id)) + (u/not-valid plugin-id :verticalSizing "Cannot modify a page that is not currently active") + + :else + (st/emit! (dwsl/update-layout #{id} {:layout-item-v-sizing value})))))} :alignItems {:this true @@ -479,25 +521,33 @@ :appendChild (fn [child row column] - (cond - (not (shape-proxy? child)) - (u/not-valid plugin-id :appendChild-child child) + (let [valid-child? (shape-proxy? child) + child-page (when valid-child? (obj/get child "$page")) + child-id (when valid-child? (obj/get child "$id")) + objects (when valid-child? (u/locate-objects file-id page-id)) + shape (when valid-child? (get objects id)) + child-shape (when valid-child? (get objects child-id))] + (cond + (not valid-child?) + (u/not-valid plugin-id :appendChild-child child) - (or (< row 0) (not (sm/valid-safe-int? row))) - (u/not-valid plugin-id :appendChild-row row) + (or (< row 0) (not (sm/valid-safe-int? row))) + (u/not-valid plugin-id :appendChild-row row) - (or (< column 0) (not (sm/valid-safe-int? column))) - (u/not-valid plugin-id :appendChild-column column) + (or (< column 0) (not (sm/valid-safe-int? column))) + (u/not-valid plugin-id :appendChild-column column) - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :appendChild "Plugin doesn't have 'content:write' permission") + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :appendChild "Plugin doesn't have 'content:write' permission") - (or (not (u/page-active? page-id)) - (not (u/page-active? (obj/get child "$page")))) - (u/not-valid plugin-id :appendChild "Cannot modify a page that is not currently active") + (or (not (u/page-active? page-id)) + (not (u/page-active? child-page))) + (u/not-valid plugin-id :appendChild "Cannot modify a page that is not currently active") - :else - (let [child-id (obj/get child "$id")] + (u/changes-component-copy-structure? objects shape child-shape) + (u/not-valid plugin-id :appendChild "Cannot change the structure of a component copy") + + :else (st/emit! (dwt/move-shapes-to-frame #{child-id} id nil [row column]) (ptk/data-event :layout/update {:ids [id]}))))))) diff --git a/frontend/src/app/plugins/library.cljs b/frontend/src/app/plugins/library.cljs index dccce39b97..b97343ec20 100644 --- a/frontend/src/app/plugins/library.cljs +++ b/frontend/src/app/plugins/library.cljs @@ -710,24 +710,26 @@ :removeProperty (fn [pos] - (if (not (nat-int? pos)) - (u/not-valid plugin-id :pos pos) - (st/emit! - (se/event plugin-id "remove-property") - (dwv/remove-property id pos)))) + (let [nprops (->> (get-variant-components file-id id) first :variant-properties count)] + (if (or (not (nat-int? pos)) (>= pos nprops)) + (u/not-valid plugin-id :pos pos) + (st/emit! + (se/event plugin-id "remove-property") + (dwv/remove-property id pos))))) :renameProperty (fn [pos name] - (cond - (not (nat-int? pos)) - (u/not-valid plugin-id :pos pos) + (let [nprops (->> (get-variant-components file-id id) first :variant-properties count)] + (cond + (or (not (nat-int? pos)) (>= pos nprops)) + (u/not-valid plugin-id :pos pos) - (not (string? name)) - (u/not-valid plugin-id :name name) + (not (string? name)) + (u/not-valid plugin-id :name name) - :else - (st/emit! - (dwv/update-property-name id pos name {:trigger "plugin:rename-property"})))))) + :else + (st/emit! + (dwv/update-property-name id pos name {:trigger "plugin:rename-property"}))))))) (set! shape/variant-proxy variant-proxy) @@ -778,7 +780,7 @@ :else (let [component (u/proxy->library-component self) value (dm/str value " / " (:name component))] - (st/emit! (dwl/rename-component id value)))))} + (st/emit! (dwv/rename-comp-or-variant-and-main id value)))))} :remove (fn [] @@ -938,17 +940,18 @@ :setVariantProperty (fn [pos value] - (cond - (not (nat-int? pos)) - (u/not-valid plugin-id :pos (str pos)) + (let [nprops (-> (u/locate-library-component file-id id) :variant-properties count)] + (cond + (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 (string? value)) + (u/not-valid plugin-id :name value) - :else - (st/emit! - (se/event plugin-id "variant-edit-property-value") - (dwv/update-property-value id pos value)))))) + :else + (st/emit! + (se/event plugin-id "variant-edit-property-value") + (dwv/update-property-value id pos 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 9ab02e3b6c..76bc45a832 100644 --- a/frontend/src/app/plugins/page.cljs +++ b/frontend/src/app/plugins/page.cljs @@ -123,6 +123,22 @@ :enumerable false :get #(.getRoot ^js %)} + :remove + (fn [] + (let [pages (-> (u/locate-file file-id) :data :pages)] + (cond + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :remove "Plugin doesn't have 'content:write' permission") + + (nil? (u/locate-page file-id id)) + (u/not-valid plugin-id :remove "Page not found") + + (<= (count pages) 1) + (u/not-valid plugin-id :remove "Cannot remove the last page of the file") + + :else + (st/emit! (dw/delete-page id))))) + :background {:this true :get #(or (-> % u/proxy->page :background) cc/canvas) @@ -275,6 +291,11 @@ (do (st/emit! (dcm/go-to-workspace :page-id id ::rt/new-window true)) (js/Promise.resolve nil)) + ;; Navigating to the already-active page emits no initialization + ;; event, so resolve right away instead of waiting forever. + (u/page-active? id) + (js/Promise.resolve nil) + :else (js/Promise. (fn [resolve _] diff --git a/frontend/src/app/plugins/shadows.cljs b/frontend/src/app/plugins/shadows.cljs new file mode 100644 index 0000000000..866ae5658d --- /dev/null +++ b/frontend/src/app/plugins/shadows.cljs @@ -0,0 +1,60 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns app.plugins.shadows + (:require + [app.plugins.format :as format] + [app.plugins.parser :as parser] + [app.util.object :as obj])) + +(defn shadow-proxy + [shadow-data on-change!] + (let [state (atom shadow-data)] + (obj/reify {:name "ShadowProxy"} + :id + {:get (fn [] (format/format-id (:id @state))) + :set (fn [v] (swap! state assoc :id (parser/parse-id v)) (on-change!))} + + :style + {:get (fn [] (format/format-key (:style @state))) + :set (fn [v] (swap! state assoc :style (parser/parse-keyword v)) (on-change!))} + + :offsetX + {:get (fn [] (:offset-x @state)) + :set (fn [v] (swap! state assoc :offset-x v) (on-change!))} + + :offsetY + {:get (fn [] (:offset-y @state)) + :set (fn [v] (swap! state assoc :offset-y v) (on-change!))} + + :blur + {:get (fn [] (:blur @state)) + :set (fn [v] (swap! state assoc :blur v) (on-change!))} + + :spread + {:get (fn [] (:spread @state)) + :set (fn [v] (swap! state assoc :spread v) (on-change!))} + + :hidden + {:get (fn [] (:hidden @state)) + :set (fn [v] (swap! state assoc :hidden v) (on-change!))} + + ;; The color is returned as a plain snapshot; reconfiguring it means + ;; reassigning the whole `color` object. + :color + {:get (fn [] (format/format-color (:color @state))) + :set (fn [v] (swap! state assoc :color (parser/parse-color v)) (on-change!))}))) + +(defn format-shadows + ([shadows] (format-shadows shadows nil)) + ([shadows commit-fn] + (if (and (some? shadows) (fn? commit-fn)) + (let [arr-ref (atom nil) + on-change! (fn [] (commit-fn @arr-ref)) + arr (apply array (mapv #(shadow-proxy % on-change!) shadows))] + (reset! arr-ref arr) + arr) + (format/format-array format/format-shadow shadows)))) diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index 2e9366941d..4a1ea9bdf9 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -50,6 +50,7 @@ [app.main.data.workspace.variants :as dwv] [app.main.repo :as rp] [app.main.store :as st] + [app.plugins.exports :as exports] [app.plugins.fills :as fills] [app.plugins.flex :as flex] [app.plugins.format :as format] @@ -57,6 +58,7 @@ [app.plugins.parser :as parser] [app.plugins.register :as r] [app.plugins.ruler-guides :as rg] + [app.plugins.shadows :as shadows] [app.plugins.strokes :as strokes] [app.plugins.system-events :as se] [app.plugins.text :as text] @@ -225,6 +227,40 @@ :else (st/emit! (dwsh/update-shapes [id] #(assoc % :strokes value)))))) +(defn commit-shadows! + [plugin-id ^js self value] + (let [id (obj/get self "$id") + value (mapv #(shadow-defaults (parser/parse-shadow %)) value)] + (cond + (not (sm/validate [:vector ctss/schema:shadow] value)) + (u/not-valid plugin-id :shadows value) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :shadows "Plugin doesn't have 'content:write' permission") + + (not (u/page-active? (obj/get self "$page"))) + (u/not-valid plugin-id :shadows "Cannot modify a page that is not currently active") + + :else + (st/emit! (dwsh/update-shapes [id] #(assoc % :shadow value)))))) + +(defn commit-exports! + [plugin-id ^js self value] + (let [id (obj/get self "$id") + value (parser/parse-exports value)] + (cond + (not (sm/validate [:vector ctse/schema:export] value)) + (u/not-valid plugin-id :exports value) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :exports "Plugin doesn't have 'content:write' permission") + + (not (u/page-active? (obj/get self "$page"))) + (u/not-valid plugin-id :exports "Cannot modify a page that is not currently active") + + :else + (st/emit! (dwsh/update-shapes [id] #(assoc % :exports value)))))) + (defn shape-proxy? [p] (obj/type-of? p "ShapeProxy")) @@ -232,6 +268,20 @@ (defn token-proxy? [t] (obj/type-of? t "TokenProxy")) +(defn- z-order-location + "Map a plugin z-order intent (:top/:bottom/:up/:down) to the internal + vertical-order location. Flex layouts store their children in reverse of the + order the plugin exposes (see the `:children` getter), so front/back and + forward/backward are inverted for a flex parent under natural child ordering." + [plugin-id file-id page-id id loc] + (let [shape (u/locate-shape file-id page-id id) + parent (u/locate-shape file-id page-id (:parent-id shape)) + reversed? (and (u/natural-child-ordering? plugin-id) + (ctl/flex-layout? parent))] + (if reversed? + (case loc :top :bottom :bottom :top :up :down :down :up) + loc))) + (defn shape-proxy ([plugin-id id] (shape-proxy plugin-id (:current-file-id @st/state) (:current-page-id @st/state) id)) @@ -551,23 +601,10 @@ :shadows {:this true - :get #(-> % u/proxy->shape :shadow format/format-shadows) - :set - (fn [self value] - (let [id (obj/get self "$id") - value (mapv #(shadow-defaults (parser/parse-shadow %)) value)] - (cond - (not (sm/validate [:vector ctss/schema:shadow] value)) - (u/not-valid plugin-id :shadows value) - - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :shadows "Plugin doesn't have 'content:write' permission") - - (not (u/page-active? page-id)) - (u/not-valid plugin-id :shadows "Cannot modify a page that is not currently active") - - :else - (st/emit! (dwsh/update-shapes [id] #(assoc % :shadow value))))))} + :get (fn [^js self] + (shadows/format-shadows (-> self u/proxy->shape :shadow) + #(commit-shadows! plugin-id self %))) + :set (fn [self value] (commit-shadows! plugin-id self value))} :blur {:this true @@ -617,23 +654,10 @@ :exports {:this true - :get #(-> % u/proxy->shape :exports format/format-exports) - :set - (fn [self value] - (let [id (obj/get self "$id") - value (parser/parse-exports value)] - (cond - (not (sm/validate [:vector ctse/schema:export] value)) - (u/not-valid plugin-id :exports value) - - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :exports "Plugin doesn't have 'content:write' permission") - - (not (u/page-active? page-id)) - (u/not-valid plugin-id :exports "Cannot modify a page that is not currently active") - - :else - (st/emit! (dwsh/update-shapes [id] #(assoc % :exports value))))))} + :get (fn [^js self] + (exports/format-exports (-> self u/proxy->shape :exports) + #(commit-exports! plugin-id self %))) + :set (fn [self value] (commit-exports! plugin-id self value))} ;; Geometry properties :x @@ -842,7 +866,7 @@ :set (fn [self value] (cond - (not (number? value)) + (not (sm/valid-safe-number? value)) (u/not-valid plugin-id :rotation value) (not (r/check-permission plugin-id "content:write")) @@ -960,17 +984,38 @@ (u/not-valid plugin-id :resize "Cannot modify a page that is not currently active") :else - (st/emit! (dw/update-dimensions [id] :width width) - (dw/update-dimensions [id] :height height)))) + ;; A layout container that hugs its content (or a hugging/filling + ;; layout child) ignores explicit dimensions and snaps back, so + ;; switch the non-fixed axes to fixed sizing first, mirroring an + ;; interactive drag resize. The sizing change must commit before + ;; the resize, otherwise the layout reflows the new size away. + (let [shape (u/locate-shape file-id page-id id) + objects (u/locate-objects file-id page-id) + layout? (or (ctl/any-layout-immediate-child? objects shape) + (ctl/any-layout? shape)) + sizing (cond-> {} + (and layout? (not= (:layout-item-h-sizing shape) :fix)) + (assoc :layout-item-h-sizing :fix) + + (and layout? (not= (:layout-item-v-sizing shape) :fix)) + (assoc :layout-item-v-sizing :fix))] + (apply st/emit! + (cond-> [] + (seq sizing) + (conj (dwsl/update-layout #{id} sizing)) + + :always + (conj (dw/update-dimensions [id] :width width) + (dw/update-dimensions [id] :height height))))))) :rotate (fn [angle center] (let [center (when center {:x (obj/get center "x") :y (obj/get center "y")})] (cond - (not (number? angle)) + (not (sm/valid-safe-number? angle)) (u/not-valid plugin-id :rotate-angle angle) - (and (some? center) (or (not (number? (:x center))) (not (number? (:y center))))) + (and (some? center) (or (not (sm/valid-safe-number? (:x center))) (not (sm/valid-safe-number? (:y center))))) (u/not-valid plugin-id :rotate-center center) (not (r/check-permission plugin-id "content:write")) @@ -1109,7 +1154,16 @@ :appendChild (fn [child] - (let [shape (u/locate-shape file-id page-id id)] + (let [shape (u/locate-shape file-id page-id id) + valid-child? (shape-proxy? child) + child-page (when valid-child? (obj/get child "$page")) + child-id (when valid-child? (obj/get child "$id")) + objects (when valid-child? (u/locate-objects file-id page-id)) + child-shape (when valid-child? (u/locate-shape file-id page-id child-id)) + is-reversed? (ctl/flex-layout? shape) + index (if (or (not (u/natural-child-ordering? plugin-id)) is-reversed?) + 0 + (count (:shapes shape)))] (cond (and (not (cfh/frame-shape? shape)) (not (cfh/group-shape? shape)) @@ -1117,33 +1171,38 @@ (not (cfh/bool-shape? shape))) (u/not-valid plugin-id :appendChild (:type shape)) - (not (shape-proxy? child)) + (not valid-child?) (u/not-valid plugin-id :appendChild-child child) (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :appendChild "Plugin doesn't have 'content:write' permission") (or (not (u/page-active? page-id)) - (not (u/page-active? (obj/get child "$page")))) + (not (u/page-active? child-page))) (u/not-valid plugin-id :appendChild "Cannot modify a page that is not currently active") + (u/changes-component-copy-structure? objects shape child-shape) + (u/not-valid plugin-id :appendChild "Cannot change the structure of a component copy") + :else - (let [child-id (obj/get child "$id") - child-shape (u/locate-shape file-id page-id child-id) - is-reversed? (ctl/flex-layout? shape) - index - (if (or (not (u/natural-child-ordering? plugin-id)) is-reversed?) - 0 - (count (:shapes shape)))] - (st/emit! - (dwsh/relocate-shapes #{child-id} id index) - (se/event plugin-id (if (ctl/any-layout? shape) "add-layout-element" "add-element") - :type (:type child-shape) - :parent-type (:type shape))))))) + (st/emit! + (dwsh/relocate-shapes #{child-id} id index) + (se/event plugin-id (if (ctl/any-layout? shape) "add-layout-element" "add-element") + :type (:type child-shape) + :parent-type (:type shape)))))) :insertChild (fn [index child] - (let [shape (u/locate-shape file-id page-id id)] + (let [shape (u/locate-shape file-id page-id id) + valid-child? (shape-proxy? child) + child-page (when valid-child? (obj/get child "$page")) + child-id (when valid-child? (obj/get child "$id")) + objects (when valid-child? (u/locate-objects file-id page-id)) + child-shape (when valid-child? (u/locate-shape file-id page-id child-id)) + is-reversed? (ctl/flex-layout? shape) + index (if (or (not (u/natural-child-ordering? plugin-id)) is-reversed?) + (- (count (:shapes shape)) index) + index)] (cond (and (not (cfh/frame-shape? shape)) (not (cfh/group-shape? shape)) @@ -1151,29 +1210,25 @@ (not (cfh/bool-shape? shape))) (u/not-valid plugin-id :insertChild (:type shape)) - (not (shape-proxy? child)) + (not valid-child?) (u/not-valid plugin-id :insertChild-child child) (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :insertChild "Plugin doesn't have 'content:write' permission") (or (not (u/page-active? page-id)) - (not (u/page-active? (obj/get child "$page")))) + (not (u/page-active? child-page))) (u/not-valid plugin-id :insertChild "Cannot modify a page that is not currently active") + (u/changes-component-copy-structure? objects shape child-shape) + (u/not-valid plugin-id :insertChild "Cannot change the structure of a component copy") + :else - (let [child-id (obj/get child "$id") - child-shape (u/locate-shape file-id page-id child-id) - is-reversed? (ctl/flex-layout? shape) - index - (if (or (not (u/natural-child-ordering? plugin-id)) is-reversed?) - (- (count (:shapes shape)) index) - index)] - (st/emit! - (dwsh/relocate-shapes #{child-id} id index) - (se/event plugin-id (if (ctl/any-layout? shape) "add-layout-element" "add-element") - :type (:type child-shape) - :parent-type (:type shape))))))) + (st/emit! + (dwsh/relocate-shapes #{child-id} id index) + (se/event plugin-id (if (ctl/any-layout? shape) "add-layout-element" "add-element") + :type (:type child-shape) + :parent-type (:type shape)))))) ;; Only for frames :addFlexLayout @@ -1280,7 +1335,11 @@ (u/not-valid plugin-id :getRange-end end) :else - (text/text-range-proxy plugin-id file-id page-id id start end)))) + ;; Clamp the end to the actual character count so an + ;; out-of-bounds range yields the trailing text instead of + ;; reading past the content. + (let [end (min end (count (txt/content->text (:content shape))))] + (text/text-range-proxy plugin-id file-id page-id id start end))))) :applyTypography (fn [typography] @@ -1305,34 +1364,39 @@ ;; Change index method :setParentIndex (fn [index] - (cond - (not (sm/valid-safe-int? index)) - (u/not-valid plugin-id :setParentIndex index) + (let [objects (u/locate-objects file-id page-id) + shape (get objects id)] + (cond + (not (sm/valid-safe-int? index)) + (u/not-valid plugin-id :setParentIndex index) - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :setParentIndex "Plugin doesn't have 'content:write' permission") + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :setParentIndex "Plugin doesn't have 'content:write' permission") - (not (u/page-active? page-id)) - (u/not-valid plugin-id :setParentIndex "Cannot modify a page that is not currently active") + (not (u/page-active? page-id)) + (u/not-valid plugin-id :setParentIndex "Cannot modify a page that is not currently active") - :else - (st/emit! (dw/set-shape-index file-id page-id id index)))) + (u/inside-component-copy? objects shape) + (u/not-valid plugin-id :setParentIndex "Cannot change the structure of a component copy") + + :else + (st/emit! (dw/set-shape-index file-id page-id id index))))) :bringForward (fn [] - (st/emit! (dw/vertical-order-selected :up [id]))) + (st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :up) [id]))) :sendBackward (fn [] - (st/emit! (dw/vertical-order-selected :down [id]))) + (st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :down) [id]))) :bringToFront (fn [] - (st/emit! (dw/vertical-order-selected :top [id]))) + (st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :top) [id]))) :sendToBack (fn [] - (st/emit! (dw/vertical-order-selected :bottom [id]))) + (st/emit! (dw/vertical-order-selected (z-order-location plugin-id file-id page-id id :bottom) [id]))) ;; COMPONENTS :isComponentInstance @@ -1429,6 +1493,22 @@ (obj/get component "$id") true))))) + :resetOverrides + (fn [] + (let [shape (u/locate-shape file-id page-id id)] + (cond + (not (u/page-active? page-id)) + (u/not-valid plugin-id :resetOverrides "Cannot modify a page that is not currently active") + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :resetOverrides "Plugin doesn't have 'content:write' permission") + + (not (ctk/in-component-copy? shape)) + (u/not-valid plugin-id :resetOverrides "The shape is not a component copy instance") + + :else + (st/emit! (dwl/reset-component id))))) + ;; Export :export (fn [value] @@ -1683,29 +1763,31 @@ :set (fn [^js self children] - (cond - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :children "Plugin doesn't have 'content:write' permission") + (let [valid-children? (every? shape-proxy? children) + shape (u/proxy->shape self) + file-id (obj/get self "$file") + page-id (obj/get self "$page") + reverse-fn (if (u/natural-child-ordering? plugin-id) reverse identity) + ids (when valid-children? + (->> children reverse-fn (map #(obj/get % "$id"))))] + (cond + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :children "Plugin doesn't have 'content:write' permission") - (not (u/page-active? page-id)) - (u/not-valid plugin-id :children "Cannot modify a page that is not currently active") + (not (u/page-active? page-id)) + (u/not-valid plugin-id :children "Cannot modify a page that is not currently active") - (not (every? shape-proxy? children)) - (u/not-valid plugin-id :children "Every children needs to be shape proxies") + (not valid-children?) + (u/not-valid plugin-id :children "Every children needs to be shape proxies") - :else - (let [shape (u/proxy->shape self) - file-id (obj/get self "$file") - page-id (obj/get self "$page") - reverse-fn (if (u/natural-child-ordering? plugin-id) reverse identity) - ids (->> children reverse-fn (map #(obj/get % "$id")))] + (u/component-copy-container? shape) + (u/not-valid plugin-id :children "Cannot change the structure of a component copy") - (cond - (not= (set ids) (set (:shapes shape))) - (u/not-valid plugin-id :children "Not all children are present in the input") + (not= (set ids) (set (:shapes shape))) + (u/not-valid plugin-id :children "Not all children are present in the input") - :else - (st/emit! (dw/reorder-children file-id page-id (:id shape) ids))))))})) + :else + (st/emit! (dw/reorder-children file-id page-id (:id shape) ids)))))})) (cond-> (cfh/frame-shape? data) (-> (crc/add-properties! diff --git a/frontend/src/app/plugins/text.cljs b/frontend/src/app/plugins/text.cljs index b584625399..177aa34c19 100644 --- a/frontend/src/app/plugins/text.cljs +++ b/frontend/src/app/plugins/text.cljs @@ -56,6 +56,19 @@ :font-style (:style variant) :font-weight (:weight variant)})) +(defn- unsupported-weight-message + "Validation message for a font weight the current font has no variant for, + listing the weights the font supports." + [font value] + (let [weights (->> (:variants font) + (map :weight) + (distinct) + (sort-by #(or (d/parse-integer %) 0)) + (str/join ", "))] + (cond-> (dm/str "Font weight '" value "' not supported for the current font") + (seq weights) + (str ". Supported weights: " weights)))) + (defn- text-props [shape] (d/merge @@ -225,7 +238,7 @@ (fonts/find-variant font {:weight weight}))] (cond (nil? variant) - (u/not-valid plugin-id :fontWeight (dm/str "Font weight '" value "' not supported for the current font")) + (u/not-valid plugin-id :fontWeight (unsupported-weight-message font value)) (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :fontWeight "Plugin doesn't have 'content:write' permission") @@ -584,7 +597,7 @@ (fonts/find-variant font {:weight weight}))] (cond (nil? variant) - (u/not-valid plugin-id :fontWeight (dm/str "Font weight '" value "' not supported for the current font")) + (u/not-valid plugin-id :fontWeight (unsupported-weight-message font value)) (not (r/check-permission plugin-id "content:write")) (u/not-valid plugin-id :fontWeight "Plugin doesn't have 'content:write' permission") diff --git a/frontend/src/app/plugins/tokens.cljs b/frontend/src/app/plugins/tokens.cljs index e832e7750a..64bc69695f 100644 --- a/frontend/src/app/plugins/tokens.cljs +++ b/frontend/src/app/plugins/tokens.cljs @@ -32,15 +32,15 @@ :r3 :border-radius-bottom-right :r4 :border-radius-bottom-left - :p1 :padding-top-left - :p2 :padding-top-right - :p3 :padding-bottom-right - :p4 :padding-bottom-left + :p1 :padding-top + :p2 :padding-right + :p3 :padding-bottom + :p4 :padding-left - :m1 :margin-top-left - :m2 :margin-top-right - :m3 :margin-bottom-right - :m4 :margin-bottom-left}) + :m1 :margin-top + :m2 :margin-right + :m3 :margin-bottom + :m4 :margin-left}) (def ^:private map:token-attr-plugin->token-attr (merge diff --git a/frontend/src/app/plugins/tracks.cljs b/frontend/src/app/plugins/tracks.cljs new file mode 100644 index 0000000000..643f6657e5 --- /dev/null +++ b/frontend/src/app/plugins/tracks.cljs @@ -0,0 +1,63 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns app.plugins.tracks + (:require + [app.common.schema :as sm] + [app.common.types.shape.layout :as ctl] + [app.main.data.workspace.shape-layout :as dwsl] + [app.main.store :as st] + [app.plugins.format :as format] + [app.plugins.register :as r] + [app.plugins.utils :as u] + [app.util.object :as obj])) + +(defn track-proxy + [plugin-id file-id page-id shape-id kind index] + (let [prop (case kind :row :layout-grid-rows :column :layout-grid-columns) + locate (fn [] (-> (u/locate-shape file-id page-id shape-id) (get prop) (nth index nil)))] + (obj/reify {:name "TrackProxy"} + :type + {:get (fn [] (-> (locate) :type format/format-key)) + :set + (fn [value] + (let [type (keyword value)] + (cond + (not (contains? ctl/grid-track-types type)) + (u/not-valid plugin-id :type value) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :type "Plugin doesn't have 'content:write' permission") + + (not (u/page-active? page-id)) + (u/not-valid plugin-id :type "Cannot modify a page that is not currently active") + + :else + (st/emit! (dwsl/change-layout-track #{shape-id} kind index {:type type})))))} + + :value + {:get (fn [] (:value (locate))) + :set + (fn [value] + (cond + (not (sm/valid-safe-number? value)) + (u/not-valid plugin-id :value value) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :value "Plugin doesn't have 'content:write' permission") + + (not (u/page-active? page-id)) + (u/not-valid plugin-id :value "Cannot modify a page that is not currently active") + + :else + (st/emit! (dwsl/change-layout-track #{shape-id} kind index {:value value}))))}))) + +(defn format-tracks + [plugin-id file-id page-id shape-id kind tracks] + (apply array + (map-indexed (fn [index _] + (track-proxy plugin-id file-id page-id shape-id kind index)) + tracks))) diff --git a/frontend/src/app/plugins/utils.cljs b/frontend/src/app/plugins/utils.cljs index f55d63372e..0bbf358e20 100644 --- a/frontend/src/app/plugins/utils.cljs +++ b/frontend/src/app/plugins/utils.cljs @@ -139,6 +139,25 @@ (when (and (some? file-id) (some? page-id) (some? id)) (locate-shape file-id page-id id)))) +(defn inside-component-copy? + "True when `shape` is nested inside a component copy. The copy root itself is + movable as a whole; its descendants are structural copy content and must not + be reparented by the Plugin API." + [objects shape] + (boolean (ctn/has-any-copy-parent? objects shape))) + +(defn component-copy-container? + "True when changing `shape`'s children would alter a component copy structure." + [shape] + (boolean (ctk/in-component-copy? shape))) + +(defn changes-component-copy-structure? + "Returns true when moving `child` into `parent` would either alter a copy + container or move an existing child out of/within a component copy." + [objects parent child] + (or (component-copy-container? parent) + (inside-component-copy? objects child))) + (defn proxy->library-color [proxy] (let [file-id (obj/get proxy "$file") diff --git a/frontend/test/frontend_tests/plugins/format_test.cljs b/frontend/test/frontend_tests/plugins/format_test.cljs index fd52f1f3fe..f426941acb 100644 --- a/frontend/test/frontend_tests/plugins/format_test.cljs +++ b/frontend/test/frontend_tests/plugins/format_test.cljs @@ -6,7 +6,10 @@ (ns frontend-tests.plugins.format-test (:require + [app.plugins.exports :as exports] [app.plugins.format :as format] + [app.plugins.shadows :as shadows] + [app.plugins.tracks :as tracks] [cljs.test :as t :include-macros true])) (t/deftest test-format-array-always-returns-array @@ -33,10 +36,10 @@ ;; Each wrapper backs a non-nullable array-typed Plugin API property and ;; must return an empty array (never nil) when the source collection is nil. (t/are [result] (and (array? result) (= 0 (.-length result))) - (format/format-shadows nil) - (format/format-exports nil) + (shadows/format-shadows nil) + (exports/format-exports nil) (format/format-frame-guides nil) - (format/format-tracks nil) + (tracks/format-tracks nil nil nil nil nil nil) (format/format-path-content nil))) (t/deftest test-format-color-result-uses-shapes-info-key diff --git a/frontend/test/frontend_tests/plugins/tokens_test.cljs b/frontend/test/frontend_tests/plugins/tokens_test.cljs index fe04a6a5e3..f064f234e0 100644 --- a/frontend/test/frontend_tests/plugins/tokens_test.cljs +++ b/frontend/test/frontend_tests/plugins/tokens_test.cljs @@ -68,9 +68,7 @@ (t/is (= :r1 (ptok/token-attr-plugin->token-attr :border-radius-top-left))) (t/is (= :r2 (ptok/token-attr-plugin->token-attr :border-radius-top-right))) (t/is (= :r3 (ptok/token-attr-plugin->token-attr :border-radius-bottom-right))) - (t/is (= :r4 (ptok/token-attr-plugin->token-attr :border-radius-bottom-left))) - (t/is (= :p1 (ptok/token-attr-plugin->token-attr :padding-top-left))) - (t/is (= :m3 (ptok/token-attr-plugin->token-attr :margin-bottom-right)))) + (t/is (= :r4 (ptok/token-attr-plugin->token-attr :border-radius-bottom-left)))) (t/deftest token-attr-plugin->token-attr-resolves-padding-margin-side-aliases (t/is (= :p1 (ptok/token-attr-plugin->token-attr :padding-top))) @@ -88,7 +86,7 @@ (t/is (= :stroke-color (ptok/token-attr-plugin->token-attr "stroke-color"))) ;; Verbose plugin aliases work via the string path too. (t/is (= :r1 (ptok/token-attr-plugin->token-attr "border-radius-top-left"))) - (t/is (= :m3 (ptok/token-attr-plugin->token-attr "margin-bottom-right")))) + (t/is (= :m3 (ptok/token-attr-plugin->token-attr "margin-bottom")))) (t/deftest token-attr?-accepts-keyword-input (t/is (true? (boolean (ptok/token-attr? :fill)))) @@ -143,7 +141,7 @@ (fn [] (let [shape-id (cthi/id :frame1) page-id (cthf/current-page-id file)] - (t/is (= "spacing.medium" (.. shape -tokens -paddingTopLeft))) + (t/is (= "spacing.medium" (.. shape -tokens -paddingTop))) (t/is (= "spacing.medium" (get-in @store [:files (:id file) :data :pages-index page-id diff --git a/frontend/test/frontend_tests/plugins/value_objects_test.cljs b/frontend/test/frontend_tests/plugins/value_objects_test.cljs new file mode 100644 index 0000000000..e827e0cfc1 --- /dev/null +++ b/frontend/test/frontend_tests/plugins/value_objects_test.cljs @@ -0,0 +1,111 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +;; Value-object proxies (shadows, exports, grid tracks) returned by the Plugin +;; API. `format-shadows`, `format-exports` and `format-tracks` hand back live +;; proxies whose member setters persist back to the located shape. Each test +;; mutates a member on the returned proxy and reads the value back through a +;; fresh proxy from the live store, so a detached-snapshot regression fails. +(ns frontend-tests.plugins.value-objects-test + (:require + [app.common.test-helpers.files :as cthf] + [app.main.store :as st] + [app.plugins.api :as api] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.state :as ths] + [frontend-tests.helpers.wasm :as thw] + [potok.v2.core :as ptk])) + +(def ^:private plugin-id "00000000-0000-0000-0000-000000000000") + +(defn- setup-context [] + (let [store (ths/setup-store (cthf/sample-file :file1 :page-label :page1)) + _ (set! st/state store) + _ (set! st/stream (ptk/input-stream store)) + context (api/create-context plugin-id)] + {:store store :context context})) + +;; --------------------------------------------------------------------------- +;; Shadows +;; --------------------------------------------------------------------------- + +(t/deftest shadow-proxy-member-setters-persist-to-the-shape + (thw/with-wasm-mocks* + (fn [] + (let [{:keys [context]} (setup-context) + ^js rect (.createRectangle ^js context)] + (set! (.-shadows rect) + #js [#js {:style "drop-shadow" + :offsetX 1 :offsetY 1 :blur 2 :spread 0 :hidden false + :color #js {:color "#000000" :opacity 1}}]) + (let [^js shadow (aget (.-shadows rect) 0)] + (set! (.-style shadow) "inner-shadow") + (set! (.-offsetX shadow) 5) + (set! (.-offsetY shadow) 6) + (set! (.-blur shadow) 7) + (set! (.-spread shadow) 2) + (set! (.-hidden shadow) true)) + (let [^js persisted (aget (.-shadows rect) 0)] + (t/is (= "inner-shadow" (.-style persisted))) + (t/is (= 5 (.-offsetX persisted))) + (t/is (= 6 (.-offsetY persisted))) + (t/is (= 7 (.-blur persisted))) + (t/is (= 2 (.-spread persisted))) + (t/is (= true (.-hidden persisted)))))))) + +;; --------------------------------------------------------------------------- +;; Exports +;; --------------------------------------------------------------------------- + +(t/deftest export-proxy-member-setters-persist-to-the-shape + (thw/with-wasm-mocks* + (fn [] + (let [{:keys [context]} (setup-context) + ^js rect (.createRectangle ^js context)] + (set! (.-exports rect) + #js [#js {:type "png" :scale 1 :suffix "" :skipChildren false}]) + (let [^js export (aget (.-exports rect) 0)] + (set! (.-type export) "jpeg") + (set! (.-scale export) 2) + (set! (.-suffix export) "@2x") + (set! (.-skipChildren export) true)) + (let [^js persisted (aget (.-exports rect) 0)] + (t/is (= "jpeg" (.-type persisted))) + (t/is (= 2 (.-scale persisted))) + (t/is (= "@2x" (.-suffix persisted))) + (t/is (= true (.-skipChildren persisted)))))))) + +;; --------------------------------------------------------------------------- +;; Grid tracks +;; --------------------------------------------------------------------------- + +(defn- setup-grid [] + (let [{:keys [store context]} (setup-context) + ^js board (.createBoard ^js context) + ^js grid (.addGridLayout board)] + {:store store :context context :board board :grid grid})) + +(t/deftest track-proxy-member-setters-update-the-track + (thw/with-wasm-mocks* + (fn [] + (let [{:keys [^js grid]} (setup-grid)] + (.addColumn grid "flex" 1) + (let [^js track (aget (.-columns grid) 0)] + (set! (.-type track) "fixed") + (set! (.-value track) 120)) + (let [^js persisted (aget (.-columns grid) 0)] + (t/is (= "fixed" (.-type persisted))) + (t/is (= 120 (.-value persisted)))))))) + +(t/deftest track-proxy-member-setters-reject-invalid-input + (thw/with-wasm-mocks* + (fn [] + (let [{:keys [store ^js grid]} (setup-grid)] + (swap! store assoc-in [:plugins :flags plugin-id :throw-validation-errors] true) + (.addColumn grid "flex" 1) + (let [^js track (aget (.-columns grid) 0)] + (t/is (thrown? js/Error (set! (.-type track) "not-a-track-type"))) + (t/is (thrown? js/Error (set! (.-value track) "not-a-number")))))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 69c439dab2..0e92dd804a 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -42,6 +42,7 @@ [frontend-tests.plugins.text-test] [frontend-tests.plugins.tokens-test] [frontend-tests.plugins.utils-test] + [frontend-tests.plugins.value-objects-test] [frontend-tests.render-wasm.process-objects-test] [frontend-tests.svg-fills-test] [frontend-tests.tokens.import-export-test] @@ -114,6 +115,7 @@ 'frontend-tests.plugins.text-test 'frontend-tests.plugins.tokens-test 'frontend-tests.plugins.utils-test + 'frontend-tests.plugins.value-objects-test 'frontend-tests.svg-fills-test 'frontend-tests.tokens.import-export-test 'frontend-tests.tokens.logic.token-actions-test diff --git a/mcp/packages/plugin/src/PenpotUtils.test.ts b/mcp/packages/plugin/src/PenpotUtils.test.ts index 62dcb60a80..1ed2d51ee0 100644 --- a/mcp/packages/plugin/src/PenpotUtils.test.ts +++ b/mcp/packages/plugin/src/PenpotUtils.test.ts @@ -130,3 +130,34 @@ test("createVariantContainer — returns the container from createVariantFromCom assert.equal(result, container); }); + +// --------------------------------------------------------------------------- +// findShapes root handling +// (community report https://community.penpot.app/t/mcp-api-issue-list/10700 +// issue #15: including the matching root itself is a footgun — a predicate +// meant for descendants can accidentally select and mutate the container). +// --------------------------------------------------------------------------- + +function makeShapeTree() { + const childA = { name: "child-a", fills: [{}] }; + const childB = { name: "child-b", fills: [{}] }; + const root = { name: "root", fills: [{}], children: [childA, childB] }; + return { root, childA, childB }; +} + +test("findShapes — matches descendants of the given root", () => { + const { root, childA, childB } = makeShapeTree(); + + const result = PenpotUtils.findShapes((s: any) => s.fills.length > 0, root as any); + + assert.ok(result.includes(childA as any)); + assert.ok(result.includes(childB as any)); +}); + +test("findShapes — excludes the root shape itself from the results", () => { + const { root } = makeShapeTree(); + + const result = PenpotUtils.findShapes((s: any) => s.fills.length > 0, root as any); + + assert.ok(!result.includes(root as any)); +}); diff --git a/mcp/packages/plugin/src/PenpotUtils.ts b/mcp/packages/plugin/src/PenpotUtils.ts index ae012608ac..e5f4d525f0 100644 --- a/mcp/packages/plugin/src/PenpotUtils.ts +++ b/mcp/packages/plugin/src/PenpotUtils.ts @@ -87,16 +87,20 @@ export class PenpotUtils { public static findShapes(predicate: (shape: Shape) => boolean, root: Shape | null = null): Shape[] { let result = new Array(); - let find = function (shape: Shape | null) { + // Walk the descendants of the given shape, testing the predicate on each + // one. The starting shape is never tested itself: findShapes searches + // *within* a container, so a predicate meant for descendants cannot + // accidentally select (and later mutate) the container it was handed. + let walk = function (shape: Shape | null) { if (!shape) { return; } - if (predicate(shape)) { - result.push(shape); - } if ("children" in shape && shape.children) { for (let child of shape.children) { - find(child); + if (predicate(child)) { + result.push(child); + } + walk(child); } } }; @@ -105,11 +109,11 @@ export class PenpotUtils { const pages = penpot.currentFile?.pages; if (pages) { for (let page of pages) { - find(page.root); + walk(page.root); } } } else { - find(root); + walk(root); } return result; } diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index 5504c20dac..00095c38b5 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -4,14 +4,22 @@ - **plugins-runtime**: changes outside the current page now raise a validation error when the target belongs to a page that is not currently active, instead of silently operating on the active page. - **plugin-types**: Change return type of `combineAsVariants` +- **plugin-types**: Removed the `type` (`'layer-blur'`) property from the `Blur` interface; a blur is now described only by `id`, `value` and `hidden`. - **plugin-types:** Deprecate the legacy `Image` shape interface — image shapes exist only for backward compatibility with old files; new images are embedded in a `Fill` via its `fillImage` (an `ImageData`). - We've solved several inconsistencies accross the API, if you relied on an undocumented property or method be aware that might have changed. ### 🚀 Features +- **plugin-types**: Added the design **tokens API**: `Library.tokens` (a `TokenCatalog` exposing `themes`/`sets`, `addTheme`, `addSet`, `getThemeById`, `getSetById`), the `TokenSet` and `TokenTheme` interfaces, the full `Token` union (`TokenColor`, `TokenDimension`, `TokenBorderRadius`, `TokenShadow`, `TokenTypography`, `TokenFontFamilies`, etc.) with `value`/`resolvedValue`/`resolvedValueString`, `Shape.tokens` and `Shape.applyToken()`, per-token `applyToShapes()`/`applyToSelected()`, and the `TokenType` and `TokenProperty` types. Notable behaviors: + - `TokenCatalog.addSet` accepts an optional `active` flag to create an already-active set (sets are inactive by default). + - `TokenTheme.addSet` and `TokenTheme.removeSet` accept a token set id (`string`) in addition to a `TokenSet`. + - `TokenSet.addToken` resolves references against all token sets, allowing references to tokens in inactive sets. + - A `fontFamilies` token's `resolvedValue` returns the resolved family list as `string[]`. +- **plugin-types**: Added `backgroundBlur` property for shapes (a `Blur` applied to the content behind the shape) +- **plugin-types**: Added a `hidden` flag to `penpot.ui.open()` options to open the plugin without showing the modal - **plugins-runtime**: Added `version` field that returns the current version - **plugins-runtime**: Added optional parameter `throwOnError` to `penpot.ui.sendMessage` (default false, backwards-compatible) -- **plugin-types**: Added a flags subcontexts with the flag `naturalChildrenOrdering` +- **plugin-types**: Added a `penpot.flags` subcontext with the flag `naturalChildOrdering` - **plugin-types**: Added flag `throwValidationErrors` to enable exceptions on validation - **plugin-types**: `penpot.openPage()` now returns `Promise` and should be awaited before performing operations on the new page - **plugin-types:** Change `LibraryComponent.isVariant()` return type to type guard `this is LibraryVariantComponent` @@ -19,19 +27,31 @@ - **plugin-types**: Added `textBounds` property for text shapes - **plugin-types**: Fix missing `webp` export format in `Export.type` - **plugin-types**: Added `fixedWhenScrolling` property for shapes -- **plugin-runtime:** `addToken` now resolves references against all token sets, allowing references to tokens in inactive sets -- **plugin-types:** `TokenCatalog.addSet` now accepts an optional `active` flag to create an already-active set (sets are inactive by default) -- **plugin-types:** `TokenTheme.addSet` and `TokenTheme.removeSet` now accept a token set id (`string`) in addition to a `TokenSet` -- **plugin-runtime:** A `fontFamilies` token's `resolvedValue` now returns the documented `string[]` (the resolved family list) instead of leaking the raw tokenscript list symbol +- **plugin-types**: Added `Page.remove()` to remove a page from the file (the last remaining page cannot be removed; removing the active page activates another one) +- **plugin-types**: Added `RulerGuide.remove()` to remove a ruler guide +- **plugin-types**: Added `File.validate()` to run the file's referential-integrity validation and return the list of errors found (empty when the file is valid) — the same errors the backend rejects on save +- **plugin-types**: Added `Shape.resetOverrides()` to restore a component copy's attributes (and its children's) to the linked main component, like the "reset overrides" action on the Penpot interface ### 🩹 Fixes +- **plugins-runtime**: The validation error raised when setting a `fontWeight` the current font has no variant for now lists the weights the font supports. - **plugins-runtime**: Fix inverted validation that rejected valid values (and accepted invalid ones) on text range `align`, `direction`, `textDecoration`, `letterSpacing` and on layout child `zIndex`. - **plugins-runtime**: Array-typed properties (e.g. `page.flows`, `shape.exports`, `shape.shadows`, layout `rows`/`columns`, ruler guides, path `commands`) now always return an array, returning an empty array instead of `null` when there are no items +- **plugin-types**: Fix `CommonLayout.horizontalSizing`/`verticalSizing` values, which were typed as `'fit-content'` but the runtime uses `'fix'` (now `'fix' | 'fill' | 'auto'`) +- **plugin-types**: Fix `Shape.layoutCell` type, which pointed to `LayoutChildProperties` instead of `LayoutCellProperties` +- **plugin-types**: Mark the interaction `animation` as optional (`animation?: Animation`) to match interactions that have no animation - **plugin-types**: Fix penpot.openPage() to navigate in same tab by default - **plugin-types**: Rename `LibraryTypography.fontFamilies` to `fontFamily` to match the runtime (it holds a single font family, not an array) - **plugin-runtime:** Setting a `LibraryColor`'s `gradient` or `image` now clears the other color representations (solid/gradient/image are mutually exclusive), so the result is a valid color instead of being rejected with "expected valid color" - **plugin-types:** Mark members that have no runtime setter as `readonly`, fixing a mismatch where they were typed as writable: font metadata (`Font.*`, `FontVariant.*`, `FontsContext.all`), the `Ellipse`/`Image`/`SvgRaw` `type` discriminants (now consistent with the other shapes), `File.name`/`pages`/`revn`, `Page.root`, `TokenTheme.activeSets`, `Variants.properties`, `ImageData.*`, the board guide value objects (`GuideColumn`/`GuideRow`/`GuideSquare` and their params — `board.guides` returns a formatted snapshot, so reconfiguring means reassigning the whole array), the `Point` and `Bounds` value objects, the `Penpot.ui`/`Penpot.utils` subcontexts, the derived `Boolean` path data (`d`/`content`/`commands` are computed from the operands; `Boolean` is not editable like a `Path`), and the `EventsMap` event entries (a type-only event→callback map, never assigned). Members that do expose a setter stay writable: `Board.children`, `Path.d`/`content`/`commands` and `FileVersion.label`. +- **plugins-runtime**: `Shape.rotation` (and `Shape.rotate()`) now reject `NaN`/non-finite values instead of letting them reach the geometry layer as an invalid move vector (which surfaced as an error toast). +- **plugins-runtime**: The positional variant-property operations (`Variants.removeProperty`, `Variants.renameProperty`, `LibraryVariantComponent.setVariantProperty`) now reject an out-of-range `pos` instead of letting it reach the data layer as an index-out-of-bounds error (which surfaced as an error toast). +- **plugins-runtime**: `createShapeFromSvg`/`createShapeFromSvgWithImages` now reject malformed SVG markup up front instead of failing asynchronously inside the import pipeline (which surfaced as an "SVG is invalid or malformed" error toast). +- **plugins-runtime**: Setting `FileVersion.label` no longer raises an internal error toast after renaming the version (the internal rename event was built with a misplaced argument). +- **plugins-runtime**: `Text.getRange(start, end)` now clamps an `end` past the text length to the character count instead of throwing an internal `TypeError` when reading the range's `characters`. +- **plugins-runtime**: `penpot.openPage()` (and `Page.openPage()`) now resolves immediately when the target page is already active, instead of waiting forever for a page-initialization event that never fires. +- **plugins-runtime**: `Shape.shadows`, `Shape.exports` and grid `rows`/`columns` now return live proxies, so writing a member on a returned shadow/export/track (e.g. `shape.shadows[0].blur = 7`) persists to the shape instead of mutating a detached snapshot that was silently discarded. The shadow `color` remains a plain snapshot (reconfigure it by assigning `shadow.color`). +- **plugins-runtime**: Setting a variant component's `path` now renames the whole variant (its container and every main instance), like the `name` setter already did, instead of renaming only the component and leaving the file referentially inconsistent (which the backend rejected on save with a `variant-component-bad-name` error). ## 1.4.2 (2026-01-21) diff --git a/plugins/apps/plugin-api-test-suite/README.md b/plugins/apps/plugin-api-test-suite/README.md index 82313a1060..c9b0b8cdf5 100644 --- a/plugins/apps/plugin-api-test-suite/README.md +++ b/plugins/apps/plugin-api-test-suite/README.md @@ -152,6 +152,32 @@ In the UI each group header shows an aggregate **status dot** rolled up from its tests: it turns purple while any test in the group is running, red if any failed, green only once every test passed, and grey until then. +### Focusing and debugging tests + +Two chainable modifiers help while developing a single case: + +```ts +// Focus the run on this test — every non-`.only` test is skipped. +test.only('the case under investigation', (ctx) => { + /* … */ +}); + +// Keep the scratch board (and selection / active page) after the test so the +// result can be inspected in the workspace instead of being torn down. +test.nocleanup('leaves its board behind', (ctx) => { + /* … */ +}); + +// They compose in any order, and with `skipIfMocked`: +test.only.nocleanup('inspect this one', (ctx) => { + /* … */ +}); +``` + +`.only` is a source-level focus, like `it.only` in other frameworks: leaving it in +place limits every run (including CI) to the focused tests, so remove it before +committing. + ### The test context (`ctx`) `fn` receives a `TestContext` (`src/framework/types.ts`): @@ -339,9 +365,7 @@ count: When a member is confirmed broken, add a test that asserts its **correct** behaviour and comment it as blocked-by-bug; it stays red until the API is fixed and then turns -green (at which point drop the "API bug" framing). There are currently no such red -tests — e.g. the `fontFamilies` token `resolvedValue` bug (it used to leak the raw -tokenscript structure instead of `string[]`) has since been fixed. +green (at which point drop the "API bug" framing). ### d.ts / runtime mismatches diff --git a/plugins/apps/plugin-api-test-suite/ci/run-ci.ts b/plugins/apps/plugin-api-test-suite/ci/run-ci.ts index 5b03d30386..41a544d4f3 100644 --- a/plugins/apps/plugin-api-test-suite/ci/run-ci.ts +++ b/plugins/apps/plugin-api-test-suite/ci/run-ci.ts @@ -15,6 +15,8 @@ import type { CoverageReport, TestResult } from '../src/framework/types'; // file, and drives the real backend + frontend end-to-end. // Required env: E2E_LOGIN_EMAIL, E2E_LOGIN_PASSWORD. // Optional env: PENPOT_BASE_URL (default https://localhost:3449). +// Optional env: TEST_FILTER — run only tests whose group or name contains +// the given substring (case-insensitive). // // - MOCKED (`MOCK_BACKEND=1`): serves the prebuilt frontend bundle via the e2e // static server and intercepts every backend RPC with Playwright `page.route`, @@ -369,9 +371,13 @@ async function main() { // The bundle runs inside an SES Compartment (its own `globalThis`), so a page // `addInitScript` global can't reach it. Prepend the mocked flag straight into // the evaluated code so the bundle's `runTests` excludes `skipIfMocked` tests. - const injectedCode = MOCKED - ? `globalThis.__PLUGIN_SUITE_MOCKED__ = true;\n${bundle}` - : bundle; + const filter = process.env['TEST_FILTER']; + const flags = + (MOCKED ? 'globalThis.__PLUGIN_SUITE_MOCKED__ = true;\n' : '') + + (filter + ? `globalThis.__PLUGIN_SUITE_FILTER__ = ${JSON.stringify(filter)};\n` + : ''); + const injectedCode = flags ? `${flags}${bundle}` : bundle; const results: TestResult[] = []; let coverage: CoverageReport | null = null; diff --git a/plugins/apps/plugin-api-test-suite/src/ci/headless.ts b/plugins/apps/plugin-api-test-suite/src/ci/headless.ts index 9374ab4e48..c983173532 100644 --- a/plugins/apps/plugin-api-test-suite/src/ci/headless.ts +++ b/plugins/apps/plugin-api-test-suite/src/ci/headless.ts @@ -1,4 +1,5 @@ import { runTests } from '../framework/runner'; +import { getTests } from '../framework/registry'; // In-sandbox CI entry point. Built as a standalone IIFE bundle (headless.js) and // evaluated inside a real Penpot plugin sandbox by the out-of-sandbox driver @@ -16,10 +17,25 @@ async function main() { globalThis as unknown as { __PLUGIN_SUITE_MOCKED__?: boolean } ).__PLUGIN_SUITE_MOCKED__; + // Set by the driver from TEST_FILTER: run only tests whose group or name + // contains the given substring (case-insensitive). + const filter = ( + globalThis as unknown as { __PLUGIN_SUITE_FILTER__?: string } + ).__PLUGIN_SUITE_FILTER__?.toLowerCase(); + const ids = filter + ? getTests() + .filter( + (t) => + t.name.toLowerCase().includes(filter) || + t.group.toLowerCase().includes(filter), + ) + .map((t) => t.id) + : ('all' as const); + // Stream each result as it completes (not just at the end) so the runner sees // progress and partial output survives if a later test hangs to its timeout. const { summary, coverage, skipped } = await runTests( - 'all', + ids, (result) => { if (result.status !== 'running') { console.log('__TEST_RESULT__ ' + JSON.stringify(result)); diff --git a/plugins/apps/plugin-api-test-suite/src/framework/registry.ts b/plugins/apps/plugin-api-test-suite/src/framework/registry.ts index da01c57430..77bd572948 100644 --- a/plugins/apps/plugin-api-test-suite/src/framework/registry.ts +++ b/plugins/apps/plugin-api-test-suite/src/framework/registry.ts @@ -57,7 +57,18 @@ export const describe: { }, }); -function registerTest(name: string, fn: TestFn, mockedSkip: boolean): void { +/** Per-test flags applied through the `test` modifier getters. */ +interface TestModifiers { + only: boolean; + noCleanup: boolean; + mockedSkip: boolean; +} + +function registerTest( + name: string, + fn: TestFn, + mods: Partial, +): void { const base = slugify(name) || 'test'; let id = base; let n = 2; @@ -68,7 +79,61 @@ function registerTest(name: string, fn: TestFn, mockedSkip: boolean): void { const group = groupStack.length ? groupStack.join(GROUP_SEPARATOR) : DEFAULT_GROUP; - registry.push({ id, name, group, fn, mockedSkip }); + registry.push({ + id, + name, + group, + fn, + mockedSkip: Boolean(mods.mockedSkip) || skipMockedDepth > 0, + only: mods.only, + noCleanup: mods.noCleanup, + }); +} + +/** + * Registrar for a single test. Callable as `test(name, fn)` and chainable + * through modifier getters so flags compose, e.g. `test.only.nocleanup(name, fn)`. + */ +export interface TestRegistrar { + (name: string, fn: TestFn): void; + /** + * Focuses the run on tests marked `.only`; every other test is skipped. A dev + * aid for isolating a single case (see {@link TestCase.only}). + */ + readonly only: TestRegistrar; + /** + * Leaves the scratch board and shared state in place after the test instead of + * tearing them down, so the result can be inspected (see {@link TestCase.noCleanup}). + */ + readonly nocleanup: TestRegistrar; + /** + * Registers a test that is excluded when running against a mocked backend + * (see {@link TestCase.mockedSkip}). + */ + skipIfMocked(name: string, fn: TestFn): void; +} + +function makeRegistrar(mods: Partial): TestRegistrar { + const register = ((name: string, fn: TestFn): void => + registerTest(name, fn, mods)) as TestRegistrar; + // Define the modifier getters directly rather than via Object.assign: assign + // would *read* each getter to copy its value, recursing without end. + Object.defineProperties(register, { + only: { + get: () => makeRegistrar({ ...mods, only: true }), + configurable: true, + }, + nocleanup: { + get: () => makeRegistrar({ ...mods, noCleanup: true }), + configurable: true, + }, + skipIfMocked: { + value: (name: string, fn: TestFn): void => + registerTest(name, fn, { ...mods, mockedSkip: true }), + configurable: true, + }, + }); + return register; } /** @@ -76,28 +141,26 @@ function registerTest(name: string, fn: TestFn, mockedSkip: boolean): void { * `tests/*.test.ts` files. Ids are derived from the name and de-duplicated so * the UI and runner can address each test unambiguously. * - * `test.skipIfMocked(name, fn)` registers a single test that is excluded when - * running against a mocked backend (see {@link TestCase.mockedSkip}). + * Modifiers compose through chainable getters: + * - `test.only(name, fn)` focuses the run on this test. + * - `test.nocleanup(name, fn)` keeps the scratch board after the test. + * - `test.skipIfMocked(name, fn)` excludes it from a mocked-backend run. */ -export const test: { - (name: string, fn: TestFn): void; - skipIfMocked(name: string, fn: TestFn): void; -} = Object.assign( - (name: string, fn: TestFn): void => - registerTest(name, fn, skipMockedDepth > 0), - { - skipIfMocked(name: string, fn: TestFn): void { - registerTest(name, fn, true); - }, - }, -); +export const test: TestRegistrar = makeRegistrar({}); export function getTests(): TestCase[] { return registry.slice(); } +/** + * Test descriptions for the UI. When any test is marked `.only`, only those are + * returned so the UI shows the focused set exclusively (matching the run focus). + */ export function getTestMetas(): TestMeta[] { - return registry.map(({ id, name, group }) => ({ id, name, group })); + const source = registry.some((t) => t.only) + ? registry.filter((t) => t.only) + : registry; + return source.map(({ id, name, group }) => ({ id, name, group })); } /** diff --git a/plugins/apps/plugin-api-test-suite/src/framework/runner.ts b/plugins/apps/plugin-api-test-suite/src/framework/runner.ts index 218a95e2f8..9c20d38969 100644 --- a/plugins/apps/plugin-api-test-suite/src/framework/runner.ts +++ b/plugins/apps/plugin-api-test-suite/src/framework/runner.ts @@ -10,6 +10,62 @@ import type { const SCRATCH_NAME = '__api_test_scratch__'; +// Every test runs an extra postcondition: after it we assert it did not leave +// the file referentially inconsistent (a class of bug local property round-trips +// can't catch — see `File.validate`). Cheap enough (~1ms/call) to run on all +// tests, and widens the net to non-component corruption (e.g. layout). + +/** Stable signature for a validation error so results can be diffed across runs. */ +function errorSignature(e: { + code: string; + shapeId: string | null; + pageId: string | null; +}): string { + return `${e.code}:${e.shapeId ?? ''}:${e.pageId ?? ''}`; +} + +// Runs `File.validate` on the current file, or returns null when the running +// frontend predates the method (keeps the suite working against older builds). +// Uses the *raw* penpot so it isn't credited toward coverage — a dedicated test +// in file.test.ts exercises `File.validate` for coverage. +function fileValidate(): FileValidationError[] | null { + const file = penpot.currentFile as + { validate?: () => FileValidationError[] } | null | undefined; + if (!file || typeof file.validate !== 'function') return null; + return file.validate(); +} + +interface FileValidationError { + code: string; + shapeId: string | null; + pageId: string | null; +} + +// Snapshot of the file's current referential-integrity errors, by signature. +// Null when validation is unavailable, which disables the postcondition. +function integritySignatures(): Set | null { + const errors = fileValidate(); + return errors ? new Set(errors.map(errorSignature)) : null; +} + +// Fails the test if it introduced referential-integrity errors not present in +// `before`. Diffing against a baseline tolerates pre-existing corruption (e.g. a +// `.nocleanup` test that intentionally leaves a broken file behind) so one bad +// test doesn't cascade into every later variant/component test. +function assertNoNewIntegrityErrors(before: Set): void { + const errors = fileValidate(); + if (!errors) return; + const introduced = errors.filter((e) => !before.has(errorSignature(e))); + if (introduced.length > 0) { + const summary = introduced + .map((e) => (e.shapeId ? `${e.code} (${e.shapeId})` : e.code)) + .join(', '); + throw new Error( + `Test left the file referentially inconsistent: ${summary}`, + ); + } +} + // A single test must never freeze the whole run. Some plugin API calls can hang // indefinitely (e.g. an async op whose completion event never fires), so each // test is raced against this timeout and turned into a failure if it exceeds it. @@ -21,6 +77,8 @@ export interface RunOutput { coverage: CoverageReport; /** Names of tests excluded because of {@link RunOptions.skipMocked}. */ skipped: string[]; + /** True when the run ended early because {@link RunOptions.shouldStop} asked it to. */ + stopped: boolean; } export interface RunOptions { @@ -30,6 +88,12 @@ export interface RunOptions { * {@link RunOutput.skipped}. */ skipMocked?: boolean; + /** + * Checked before each test; returning true stops the run after the current + * test finishes. Lets the UI cancel a long run without leaving a half-created + * scratch board behind. + */ + shouldStop?: () => boolean; } export type ResultReporter = (result: TestResult) => void; @@ -75,12 +139,17 @@ export async function runTests( ): Promise { const all = getTests(); const requested = ids === 'all' ? all : all.filter((t) => ids.includes(t.id)); + // `.only` focuses the run: when any requested test is marked only, restrict the + // run to those and drop the rest. A source-level dev aid for isolating a case. + const focused = requested.some((t) => t.only) + ? requested.filter((t) => t.only) + : requested; const skipped = options?.skipMocked - ? requested.filter((t) => t.mockedSkip).map((t) => t.name) + ? focused.filter((t) => t.mockedSkip).map((t) => t.name) : []; const selected = options?.skipMocked - ? requested.filter((t) => !t.mockedSkip) - : requested; + ? focused.filter((t) => !t.mockedSkip) + : focused; const recorder = createRecorder(penpot, apiSurface as ApiSurface); @@ -103,7 +172,13 @@ export async function runTests( const results: TestResult[] = []; + let stopped = false; for (const testCase of selected) { + if (options?.shouldStop?.()) { + stopped = true; + break; + } + onResult?.({ id: testCase.id, name: testCase.name, @@ -118,6 +193,10 @@ export async function runTests( let rawBoard: ReturnType | undefined; let result: TestResult; + // Baseline the file's integrity errors before the test so we can attribute + // only newly-introduced ones to it. + const integrityBaseline = integritySignatures(); + try { rawBoard = penpot.createBoard(); rawBoard.name = SCRATCH_NAME; @@ -126,6 +205,8 @@ export async function runTests( testCase.fn({ penpot: recorder.proxy, board }), TEST_TIMEOUT_MS, ); + // Postcondition: the test must not have broken referential integrity. + if (integrityBaseline) assertNoNewIntegrityErrors(integrityBaseline); result = { id: testCase.id, name: testCase.name, @@ -141,30 +222,41 @@ export async function runTests( durationMs: Date.now() - start, }; } finally { - try { - rawBoard?.remove(); - } catch { - // best-effort cleanup; never fail a test because teardown failed - } - // Reset shared state so the next test starts clean. All best-effort: a - // teardown failure must never turn into a test failure. - try { - penpot.selection = []; - } catch { - /* ignore */ - } - try { - const active = penpot.currentPage; - if (homePage && active && active.id !== homePage.id) { - await penpot.openPage(homePage); + // `test.nocleanup` keeps the scratch board and shared state as the test + // left them so the result can be inspected in the workspace. + if (!testCase.noCleanup) { + try { + rawBoard?.remove(); + } catch { + // best-effort cleanup; never fail a test because teardown failed + } + // Reset shared state so the next test starts clean. All best-effort: a + // teardown failure must never turn into a test failure. + try { + penpot.selection = []; + } catch { + /* ignore */ + } + try { + const active = penpot.currentPage; + if (homePage && active && active.id !== homePage.id) { + await penpot.openPage(homePage); + } + } catch { + /* ignore */ } - } catch { - /* ignore */ } } results.push(result); onResult?.(result); + + // Yield a macrotask between tests so the workspace can flush pending + // renders. Sync tests only yield microtasks, and hundreds of back-to-back + // mutation bursts starve React's commit cycle until it trips its + // nested-update limit, surfacing "Maximum update depth exceeded" error + // toasts in the host app (and, more rarely, wasm render errors). + await new Promise((resolve) => setTimeout(resolve, 0)); } const summary: RunSummary = { @@ -175,5 +267,5 @@ export async function runTests( const coverage = computeCoverage(recorder.accessed, apiSurface as ApiSurface); - return { results, summary, coverage, skipped }; + return { results, summary, coverage, skipped, stopped }; } diff --git a/plugins/apps/plugin-api-test-suite/src/framework/static-coverage.ts b/plugins/apps/plugin-api-test-suite/src/framework/static-coverage.ts index ecc1e86736..6116f703e3 100644 --- a/plugins/apps/plugin-api-test-suite/src/framework/static-coverage.ts +++ b/plugins/apps/plugin-api-test-suite/src/framework/static-coverage.ts @@ -65,6 +65,7 @@ export const STATIC_COVERAGE: ReadonlySet = new Set([ // via VariantContainer.variants in variants.test.ts. 'LibraryVariantComponent.variants#get', 'LibraryVariantComponent.variantProps#get', + 'LibraryVariantComponent.variantError#get', 'LibraryVariantComponent.addVariant#call', 'LibraryVariantComponent.setVariantProperty#call', ]); diff --git a/plugins/apps/plugin-api-test-suite/src/framework/types.ts b/plugins/apps/plugin-api-test-suite/src/framework/types.ts index 94b19cbe8b..665597478b 100644 --- a/plugins/apps/plugin-api-test-suite/src/framework/types.ts +++ b/plugins/apps/plugin-api-test-suite/src/framework/types.ts @@ -27,6 +27,18 @@ export interface TestCase { * or `describe.skipIfMocked`. */ mockedSkip?: boolean; + /** + * When true, the run is focused on this test (and any other `.only`): all + * non-`only` tests are skipped. A dev aid for isolating a single case, set via + * `test.only`. + */ + only?: boolean; + /** + * When true, the scratch board and shared state (selection, active page) are + * left in place after the test instead of being torn down, so the result can + * be inspected in the workspace. Set via `test.nocleanup`. + */ + noCleanup?: boolean; } /** Lightweight test description sent to the UI (no function). */ diff --git a/plugins/apps/plugin-api-test-suite/src/generated/api-surface.json b/plugins/apps/plugin-api-test-suite/src/generated/api-surface.json index 783f45d044..69dc9d5546 100644 --- a/plugins/apps/plugin-api-test-suite/src/generated/api-surface.json +++ b/plugins/apps/plugin-api-test-suite/src/generated/api-surface.json @@ -149,8 +149,10 @@ "name", "pages", "revn", - "saveVersion" + "saveVersion", + "validate" ], + "FileValidationError": ["code", "hint", "pageId", "shapeId"], "FileVersion": [ "createdAt", "createdBy", @@ -350,6 +352,7 @@ "getShapeById", "id", "name", + "remove", "removeCommentThread", "removeFlow", "removeRulerGuide", @@ -371,7 +374,7 @@ "PreviousScreen": ["type"], "Push": ["direction", "duration", "easing", "type"], "Rectangle": ["fills", "type"], - "RulerGuide": ["board", "orientation", "position"], + "RulerGuide": ["board", "orientation", "position", "remove"], "Shadow": [ "blur", "color", @@ -436,6 +439,7 @@ "proportionLock", "remove", "removeInteraction", + "resetOverrides", "resize", "rotate", "rotation", @@ -1104,6 +1108,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -1598,6 +1608,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -2718,6 +2734,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -2964,6 +2986,12 @@ "type": "FileVersion", "array": false }, + "validate": { + "decl": "File", + "kind": "method", + "type": "FileValidationError", + "array": true + }, "getPluginData": { "decl": "PluginData", "kind": "method", @@ -3001,6 +3029,32 @@ "array": true } }, + "FileValidationError": { + "code": { + "decl": "FileValidationError", + "kind": "get", + "type": null, + "array": false + }, + "hint": { + "decl": "FileValidationError", + "kind": "get", + "type": null, + "array": false + }, + "shapeId": { + "decl": "FileValidationError", + "kind": "get", + "type": null, + "array": false + }, + "pageId": { + "decl": "FileValidationError", + "kind": "get", + "type": null, + "array": false + } + }, "FileVersion": { "label": { "decl": "FileVersion", @@ -3910,6 +3964,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -4488,6 +4548,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -5658,6 +5724,12 @@ "type": "Shape", "array": false }, + "remove": { + "decl": "Page", + "kind": "method", + "type": null, + "array": false + }, "getShapeById": { "decl": "Page", "kind": "method", @@ -6098,6 +6170,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -6922,6 +7000,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -7073,6 +7157,12 @@ "kind": "getset", "type": "Board", "array": false + }, + "remove": { + "decl": "RulerGuide", + "kind": "method", + "type": null, + "array": false } }, "Shadow": { @@ -7438,6 +7528,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -7990,6 +8086,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -8550,6 +8652,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -10842,6 +10950,12 @@ "type": null, "array": false }, + "resetOverrides": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", diff --git a/plugins/apps/plugin-api-test-suite/src/model.ts b/plugins/apps/plugin-api-test-suite/src/model.ts index a86cb3eb19..d09e7b6e5f 100644 --- a/plugins/apps/plugin-api-test-suite/src/model.ts +++ b/plugins/apps/plugin-api-test-suite/src/model.ts @@ -15,13 +15,19 @@ export interface RunMessage { ids: string[] | 'all'; } +/** Requests that an in-progress run stop after the current test finishes. */ +export interface StopMessage { + type: 'stop'; +} + /** Carries the freshly built tests bundle source to be evaluated in the sandbox. */ export interface ReloadTestsMessage { type: 'reloadTests'; code: string; } -export type UIToPluginMessage = ReadyMessage | RunMessage | ReloadTestsMessage; +export type UIToPluginMessage = + ReadyMessage | RunMessage | StopMessage | ReloadTestsMessage; // Messages sent from the plugin sandbox to the UI iframe. export interface TestsMessage { @@ -38,6 +44,8 @@ export interface RunCompleteMessage { type: 'runComplete'; summary: RunSummary; coverage: CoverageReport; + /** True when the run ended early because the user pressed Stop. */ + stopped?: boolean; } export interface ThemeMessage { diff --git a/plugins/apps/plugin-api-test-suite/src/plugin.ts b/plugins/apps/plugin-api-test-suite/src/plugin.ts index db4f548bbb..30f3215530 100644 --- a/plugins/apps/plugin-api-test-suite/src/plugin.ts +++ b/plugins/apps/plugin-api-test-suite/src/plugin.ts @@ -16,17 +16,29 @@ function send(message: PluginToUIMessage) { penpot.ui.sendMessage(message); } +// Set by a `stop` message and read between tests by the runner. Reset at the +// start of every run. +let stopRequested = false; + penpot.ui.onMessage(async (message) => { if (message.type === 'ready') { send({ type: 'tests', tests: getTestMetas() }); return; } + if (message.type === 'stop') { + stopRequested = true; + return; + } + if (message.type === 'run') { - const { summary, coverage } = await runTests(message.ids, (result) => - send({ type: 'result', result }), + stopRequested = false; + const { summary, coverage, stopped } = await runTests( + message.ids, + (result) => send({ type: 'result', result }), + { shouldStop: () => stopRequested }, ); - send({ type: 'runComplete', summary, coverage }); + send({ type: 'runComplete', summary, coverage, stopped }); return; } 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 bc53f49d26..ecf00c5255 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 @@ -2,6 +2,7 @@ import { expect, expectReject } from '../framework/expect'; import { describe, test } from '../framework/registry'; import type { CommentThread, Page } from '@penpot/plugin-types'; import type { TestContext } from '../framework/types'; +import { waitFor } from './wait'; // Comments. // Comment threads are created on the current page. Thread/comment removal is @@ -14,10 +15,6 @@ function page(ctx: TestContext): Page { return p; } -function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); -} - function cleanup(thread: CommentThread): void { try { thread.remove(); @@ -97,7 +94,7 @@ describe.skipIfMocked('Comments', () => { const comment = comments[0]; comment.content = 'edited content'; // The content setter persists via an async RPC before updating locally. - await sleep(300); + await waitFor(() => comment.content === 'edited content'); expect(comment.content).toBe('edited content'); expect(comment.user).toBeDefined(); } finally { @@ -111,8 +108,10 @@ describe.skipIfMocked('Comments', () => { try { const reply = await thread.reply('to be removed'); await reply.remove(); + // The root comment survives but the removed reply is gone. const comments = await thread.findComments(); expect(comments.length).toBeGreaterThan(0); + expect(comments.some((c) => c.content === 'to be removed')).toBe(false); } finally { cleanup(thread); } @@ -136,6 +135,8 @@ describe.skipIfMocked('Comments', () => { y: 70, }); await p.removeCommentThread(thread); + const threads = await p.findCommentThreads(); + expect(threads.every((t) => t.seqNumber !== thread.seqNumber)).toBe(true); }); // --------------------------------------------------------------------------- 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 4982bc574c..367eaf8267 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 @@ -1,8 +1,11 @@ import { expect } from '../framework/expect'; import { describe, test } from '../framework/registry'; +import { waitFor } from './wait'; import type { Board, Shape } from '@penpot/plugin-types'; import type { TestContext } from '../framework/types'; +const COPY_STRUCTURE_ERROR = 'Cannot change the structure of a component copy'; + // Component instances and the ShapeBase component methods. // A component is built from a rectangle and instantiated; the instance exposes // the component predicates and navigation methods. @@ -20,6 +23,35 @@ function instanceOf(ctx: TestContext): Shape { return inst; } +function nestedComponentMain(ctx: TestContext): Board { + const nested = makeComponent(ctx).instance(); + const host = ctx.penpot.createBoard(); + ctx.board.appendChild(host); + host.appendChild(nested); + return ctx.penpot.library.local + .createComponent([host]) + .mainInstance() as Board; +} + +function boardComponentMainWithChildren(ctx: TestContext): Board { + const first = ctx.penpot.createRectangle(); + const second = ctx.penpot.createRectangle(); + const host = ctx.penpot.createBoard(); + ctx.board.appendChild(host); + host.appendChild(first); + host.appendChild(second); + return ctx.penpot.library.local + .createComponent([host]) + .mainInstance() as Board; +} + +function componentMainFromClonedNestedMain(ctx: TestContext): Board { + const cloned = nestedComponentMain(ctx).clone(); + return ctx.penpot.library.local + .createComponent([cloned]) + .mainInstance() as Board; +} + describe('Component instances', () => { test('component predicates identify an instance', (ctx) => { const inst = instanceOf(ctx); @@ -39,6 +71,125 @@ describe('Component instances', () => { expect(inst.componentRefShape()).toBeDefined(); }); + // Community report (forum #10700, issue #8): cloning a component's main + // instance was said to yield a shape with null type/name that appendChild + // silently drops. Did not reproduce (clone attaches to the same parent and + // can be re-parented); kept as a regression pin. + test('cloning a component main instance yields a valid shape', (ctx) => { + const comp = makeComponent(ctx); + const main = comp.mainInstance(); + const copy = main.clone(); + expect(copy.type).not.toBeNull(); + expect(copy.type).toBe(main.type); + expect(copy.name).not.toBeNull(); + expect(copy.id).not.toBe(main.id); + + const target = ctx.penpot.createBoard(); + ctx.board.appendChild(target); + target.resize(200, 200); + const before = target.children.length; + target.appendChild(copy); + expect(target.children.length).toBe(before + 1); + expect(target.children.some((s) => s.id === copy.id)).toBe(true); + }); + + test('appendChild rejects moving children out of a cloned main instance', (ctx) => { + const main = nestedComponentMain(ctx); + const cloned = main.clone() as Board; + const child = cloned.children[0]; + expect(child).toBeDefined(); + + expect(() => ctx.board.appendChild(child)).toThrow(COPY_STRUCTURE_ERROR); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + + test('insertChild rejects moving children out of a cloned main instance', (ctx) => { + const main = nestedComponentMain(ctx); + const cloned = main.clone() as Board; + const child = cloned.children[0]; + const target = ctx.penpot.createBoard(); + ctx.board.appendChild(target); + expect(child).toBeDefined(); + + expect(() => target.insertChild(0, child)).toThrow(COPY_STRUCTURE_ERROR); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + + test('children assignment rejects reordering a cloned main instance', (ctx) => { + const main = nestedComponentMain(ctx); + const cloned = main.clone() as Board; + + expect(() => { + cloned.children = [...cloned.children]; + }).toThrow(COPY_STRUCTURE_ERROR); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + + test('setParentIndex rejects reordering children inside a component copy', (ctx) => { + const main = boardComponentMainWithChildren(ctx); + const copy = main.component()?.instance() as Board | undefined; + expect(copy).toBeDefined(); + if (!copy) return; + + ctx.board.appendChild(copy); + const child = copy.children[1]; + expect(child).toBeDefined(); + + expect(() => child.setParentIndex(0)).toThrow(COPY_STRUCTURE_ERROR); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + + test('group rejects children inside a cloned main instance', (ctx) => { + const main = nestedComponentMain(ctx); + const cloned = main.clone() as Board; + const child = cloned.children[0]; + expect(child).toBeDefined(); + + expect(() => ctx.penpot.group([child])).toThrow(COPY_STRUCTURE_ERROR); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + + test('remove hides children inside a cloned main instance', (ctx) => { + const main = nestedComponentMain(ctx); + const cloned = main.clone() as Board; + const child = cloned.children[0]; + expect(child).toBeDefined(); + + child.remove(); + expect(child.hidden).toBe(true); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + + test('detaching a cloned main instance makes child reparenting safe', (ctx) => { + const main = nestedComponentMain(ctx); + const cloned = main.clone() as Board; + cloned.detach(); + const child = cloned.children[0]; + expect(child).toBeDefined(); + + ctx.board.appendChild(child); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + + test('cloned nested main instances can be used as variant component sources', async (ctx) => { + const mainA = componentMainFromClonedNestedMain(ctx); + const mainB = componentMainFromClonedNestedMain(ctx); + const container = ctx.penpot.createVariantFromComponents([mainA, mainB]); + + await waitFor( + () => (container.variants?.variantComponents().length ?? 0) >= 2, + ); + const errors = ctx.penpot.currentFile?.validate() ?? []; + expect(errors.map((e) => e.code)).toEqual([]); + }); + test('component() returns the library component', (ctx) => { const inst = instanceOf(ctx); const comp = inst.component(); @@ -65,6 +216,27 @@ describe('Component instances', () => { } }); + test('resetOverrides restores a copy to its main component', (ctx) => { + const comp = makeComponent(ctx); + const main = comp.mainInstance(); + const inst = comp.instance(); + ctx.board.appendChild(inst); + + const mainColor = main.fills?.[0]?.fillColor; + inst.fills = [{ fillColor: '#FF0000', fillOpacity: 1 }]; + // The override applied (fill getter normalizes to lowercase). + expect(inst.fills?.[0]?.fillColor?.toLowerCase()).toBe('#ff0000'); + + inst.resetOverrides(); + expect(inst.fills?.[0]?.fillColor).toBe(mainColor); + }); + + test('resetOverrides on a plain shape throws', (ctx) => { + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + expect(() => rect.resetOverrides()).toThrow(); + }); + // --------------------------------------------------------------------------- // Edge cases. "fail" tests exercise the component methods on shapes // that are not component instances (documented null/self returns, invalid @@ -115,21 +287,10 @@ describe('Component instances', () => { expect(c1.id).toBe(c2.id); } }); -}); -describe('Shape interactions cleanup', () => { - test('removeInteraction removes an interaction from a shape', (ctx) => { - const dest = ctx.penpot.createBoard(); - ctx.board.appendChild(dest as Board); - const rect = ctx.penpot.createRectangle(); - ctx.board.appendChild(rect); - - const interaction = rect.addInteraction('click', { - type: 'navigate-to', - destination: dest, - }); - const before = rect.interactions.length; - rect.removeInteraction(interaction); - expect(rect.interactions.length).toBe(before - 1); + test('detaching a copy breaks the component link', (ctx) => { + const inst = instanceOf(ctx); + inst.detach(); + expect(inst.isComponentInstance()).toBeFalsy(); }); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/events.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/events.test.ts index 8d1fddf66a..23203c88f9 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/events.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/events.test.ts @@ -1,5 +1,6 @@ import { expect } from '../framework/expect'; import { describe, test } from '../framework/registry'; +import { waitFor } from './wait'; // Events. // Listeners are registered with `on`, triggered by mutating state, and removed @@ -20,7 +21,7 @@ describe('Events', () => { }); ctx.penpot.selection = [rect]; - await sleep(150); + await waitFor(() => received !== null); ctx.penpot.off(listenerId); expect(received).not.toBeNull(); @@ -43,7 +44,7 @@ describe('Events', () => { ); rect.name = 'changed-name'; - await sleep(150); + await waitFor(() => fired); ctx.penpot.off(listenerId); expect(fired).toBe(true); @@ -60,6 +61,8 @@ describe('Events', () => { ctx.penpot.off(listenerId); ctx.penpot.selection = [rect]; + // Asserting a non-event: there is no positive condition to poll for, so + // give the debounce a fixed window to (not) fire. await sleep(150); expect(count).toBe(0); 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 5f8f4a8b11..9bff861b82 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 @@ -30,6 +30,21 @@ describe('File', () => { } }); + test('validate reports referential-integrity errors', (ctx) => { + const file = ctx.penpot.currentFile; + expect(file).not.toBeNull(); + if (file) { + const errors = file.validate(); + expect(Array.isArray(errors)).toBe(true); + // A healthy scratch file has no integrity errors. Each error (if any) + // exposes a string code and hint. + for (const e of errors) { + expect(typeof e.code).toBe('string'); + expect(typeof e.hint).toBe('string'); + } + } + }); + test('export returns binary data', async (ctx) => { const file = ctx.penpot.currentFile; if (file) { @@ -55,12 +70,21 @@ describe('File', () => { expect(version.label).toBe('plugin-test-version'); expect(version.isAutosave).toBe(false); - // Relabel the saved version (covers FileVersion.label set). + // Relabel the saved version (covers FileVersion.label set). The + // proxy's own `label` reflects the write immediately from its local + // cache, so also fetch the versions again and assert the rename + // actually reached the backend. version.label = 'plugin-test-version-renamed'; expect(version.label).toBe('plugin-test-version-renamed'); + // The rename is persisted asynchronously; give it a moment to land. + await new Promise((resolve) => setTimeout(resolve, 500)); const versions = await file.findVersions(); expect(versions.length).toBeGreaterThan(0); + const renamed = versions.filter( + (v) => v.label === 'plugin-test-version-renamed', + ); + expect(renamed).toHaveLength(1); // Clean up the version we just created. await version.remove(); 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 bf4ef96cf0..8a01314f53 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 @@ -119,6 +119,32 @@ describe('Fills & strokes', () => { } }); + test('a gradient-only fill reads back no solid fillColor', (ctx) => { + const r = rect(ctx); + r.fills = [ + { + fillColorGradient: { + type: 'linear', + startX: 0, + startY: 0, + endX: 1, + endY: 1, + width: 1, + stops: [ + { color: '#ff0000', opacity: 1, offset: 0 }, + { color: '#0000ff', opacity: 1, offset: 1 }, + ], + }, + }, + ]; + const fills = r.fills; + if (Array.isArray(fills)) { + expect(fills[0].fillColorGradient).toBeDefined(); + // A gradient fill carries no solid color. + expect(fills[0].fillColor).toBeFalsy(); + } + }); + test('fillOpacity above 1 throws', (ctx) => { const r = rect(ctx); expect(() => { 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 842a395031..fe18ca4e41 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 @@ -208,8 +208,11 @@ describe('Interactions', () => { interaction.delay = 250; interaction.action = { type: 'previous-screen' }; - expect(interaction.delay).toBeCloseTo(250, 0); - expect(interaction.action.type).toBe('previous-screen'); + // Re-fetch from the shape so we prove persistence, not that the proxy + // reflects its own write. + const persisted = r.interactions[0]; + expect(persisted.delay).toBeCloseTo(250, 0); + expect(persisted.action.type).toBe('previous-screen'); }); // The delay setter accepts zero (fires immediately) as a valid value. @@ -226,6 +229,18 @@ describe('Interactions', () => { expect(interaction.delay).toBeCloseTo(0, 0); }); + test('removeInteraction removes an interaction from a shape', (ctx) => { + const dest = board(ctx); + const r = rect(ctx); + const interaction = r.addInteraction('click', { + type: 'navigate-to', + destination: dest, + }); + const before = r.interactions.length; + r.removeInteraction(interaction); + expect(r.interactions.length).toBe(before - 1); + }); + describe('Animations', () => { test('dissolve animation round-trips', (ctx) => { const dest = board(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 21471365bc..b42cb65fab 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 @@ -388,6 +388,26 @@ describe('Layout', () => { expect(child.minHeight).toBeCloseTo(20, 0); } }); + + // Community report (forum #10700, issue #12): layoutChild was said to be + // null for children appended to a flex board that is re-found (fresh + // proxy) instead of using the creation-time reference. Did not reproduce; + // kept as a regression pin. + test('layoutChild is available on children of a re-found flex board', (ctx) => { + const b = board(ctx); + b.resize(300, 200); + b.addFlexLayout(); + + const found = ctx.penpot.currentPage.getShapeById(b.id) as Board; + expect(found).not.toBeNull(); + const child = ctx.penpot.createRectangle(); + found.appendChild(child); + expect(child.layoutChild).toBeDefined(); + if (child.layoutChild) { + child.layoutChild.horizontalSizing = 'fill'; + expect(child.layoutChild.horizontalSizing).toBe('fill'); + } + }); }); describe('Cell', () => { @@ -413,6 +433,94 @@ describe('Layout', () => { expect(cell.columnSpan).toBeCloseTo(2, 0); } }); + + // Community report (forum #10700, issue #2): columnSpan was said to expand + // the grid's column count and scramble the other cells' positions. Did not + // reproduce; kept as a regression pin. + test('columnSpan spans a cell without changing column count or other cells', (ctx) => { + const b = board(ctx); + b.resize(600, 400); + const grid = b.addGridLayout(); + grid.addColumn('flex', 1); + grid.addColumn('flex', 1); + grid.addColumn('flex', 1); + grid.addRow('flex', 1); + grid.addRow('flex', 1); + + const a = ctx.penpot.createRectangle(); + const c = ctx.penpot.createRectangle(); + const d = ctx.penpot.createRectangle(); + grid.appendChild(a, 1, 1); + grid.appendChild(c, 1, 3); + grid.appendChild(d, 2, 2); + + const cellA = a.layoutCell; + expect(cellA).toBeDefined(); + if (cellA) { + cellA.columnSpan = 2; + } + + expect(grid.columns.length).toBe(3); + expect(grid.rows.length).toBe(2); + if (cellA) { + expect(cellA.row).toBeCloseTo(1, 0); + expect(cellA.column).toBeCloseTo(1, 0); + expect(cellA.columnSpan).toBeCloseTo(2, 0); + } + const cellC = c.layoutCell; + const cellD = d.layoutCell; + expect(cellC).toBeDefined(); + expect(cellD).toBeDefined(); + if (cellC && cellD) { + expect(cellC.row).toBeCloseTo(1, 0); + expect(cellC.column).toBeCloseTo(3, 0); + expect(cellD.row).toBeCloseTo(2, 0); + expect(cellD.column).toBeCloseTo(2, 0); + } + }); + }); + + describe('Sizing', () => { + // Community report (forum #10700, feature request C): resize on a board + // whose layout hugs content (auto sizing) must behave like the UI — switch + // the sizing to fixed and apply the requested dimensions. This once snapped + // back to the hugged content (flex) or left the sizing 'auto' (grid). Fixed; + // kept as a regression pin. + test('resize on an auto-sized flex board switches sizing to fixed', (ctx) => { + const b = board(ctx); + b.resize(300, 200); + const flex = b.addFlexLayout(); + flex.horizontalSizing = 'auto'; + flex.verticalSizing = 'auto'; + const child = ctx.penpot.createRectangle(); + child.resize(50, 50); + b.appendChild(child); + + b.resize(500, 400); + expect(b.width).toBeCloseTo(500, 0); + expect(b.height).toBeCloseTo(400, 0); + expect(flex.horizontalSizing).toBe('fix'); + expect(flex.verticalSizing).toBe('fix'); + }); + + test('resize on an auto-sized grid board switches sizing to fixed', (ctx) => { + const b = board(ctx); + b.resize(300, 200); + const grid = b.addGridLayout(); + grid.addColumn('auto'); + grid.addRow('auto'); + grid.horizontalSizing = 'auto'; + grid.verticalSizing = 'auto'; + const child = ctx.penpot.createRectangle(); + child.resize(50, 50); + grid.appendChild(child, 1, 1); + + b.resize(500, 400); + expect(b.width).toBeCloseTo(500, 0); + expect(b.height).toBeCloseTo(400, 0); + expect(grid.horizontalSizing).toBe('fix'); + expect(grid.verticalSizing).toBe('fix'); + }); }); describe('Switching type', () => { 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 f28e3263b2..5f16958282 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 @@ -126,6 +126,16 @@ describe('Library', () => { color.image = image; expect(color.image).toBeDefined(); }); + + // `name`/`path` and the PluginData methods are inherited (LibraryElement / + // PluginData) but implemented per concrete type in the cljs binding + // (lib-color-proxy), so they must be exercised on each type separately. + test('color plugin data round-trips', (ctx) => { + const color = ctx.penpot.library.local.createColor(); + color.setPluginData('k', 'v'); + expect(color.getPluginData('k')).toBe('v'); + expect(color.getPluginDataKeys()).toContain('k'); + }); }); describe('Typographies', () => { @@ -143,6 +153,23 @@ describe('Library', () => { expect(typeof typo.fontId).toBe('string'); }); + // `name`/`path` and PluginData are implemented per concrete type + // (lib-typography-proxy); exercise them on a typography specifically. + test('typography name and path round-trip', (ctx) => { + const typo = ctx.penpot.library.local.createTypography(); + typo.name = 'Heading'; + typo.path = 'Text'; + expect(typo.name).toBe('Heading'); + expect(typo.path).toBe('Text'); + }); + + test('typography plugin data round-trips', (ctx) => { + const typo = ctx.penpot.library.local.createTypography(); + typo.setPluginData('k', 'v'); + expect(typo.getPluginData('k')).toBe('v'); + expect(typo.getPluginDataKeys()).toContain('k'); + }); + test('typography fontFamily and fontId round-trip', (ctx) => { const typo = ctx.penpot.library.local.createTypography(); expect(typeof typo.fontFamily).toBe('string'); @@ -206,6 +233,29 @@ describe('Library', () => { expect(comp.isVariant()).toBe(false); }); + // `name`/`path` and PluginData are implemented per concrete type + // (lib-component-proxy). `component.path` in particular had no test before, + // which is how the variant `.path` corruption slipped through — exercise it + // on a plain (non-variant) component here. + test('component name and path round-trip', (ctx) => { + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + const comp = ctx.penpot.library.local.createComponent([rect]); + comp.name = 'Button'; + comp.path = 'Controls'; + expect(comp.name).toBe('Button'); + expect(comp.path).toBe('Controls'); + }); + + test('component plugin data round-trips', (ctx) => { + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + const comp = ctx.penpot.library.local.createComponent([rect]); + comp.setPluginData('k', 'v'); + expect(comp.getPluginData('k')).toBe('v'); + expect(comp.getPluginDataKeys()).toContain('k'); + }); + test('component instance and mainInstance return shapes', (ctx) => { const rect = ctx.penpot.createRectangle(); ctx.board.appendChild(rect); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/misc.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/misc.test.ts index cabb0869b4..46e4150123 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/misc.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/misc.test.ts @@ -58,8 +58,12 @@ describe('Misc', () => { // Boolean fills round-trip; d/content/commands are derived from the // operands and not independently settable (see coverage notes). bool.fills = [{ fillColor: '#abcdef', fillOpacity: 1 }]; - void bool.content; expect(bool.fills).toHaveLength(1); + // The derived path data reflects the two operands. + expect(typeof bool.d).toBe('string'); + expect(bool.d.length).toBeGreaterThan(0); + expect(bool.commands.length).toBeGreaterThan(0); + expect(bool.children).toHaveLength(2); } }); @@ -80,6 +84,10 @@ describe('Misc', () => { }); describe('Export settings setters', () => { + // `shape.exports` returns live export proxies: writing through a returned + // export persists to the shape. This reads the export back from the shape + // (a fresh proxy) instead of asserting on the object we wrote to, so a + // detached-snapshot regression would fail here. test('export members round-trip on the returned export', (ctx) => { const r = rect(ctx); r.exports = [{ type: 'png', scale: 1, suffix: '', skipChildren: false }]; @@ -88,11 +96,17 @@ describe('Misc', () => { exp.scale = 2; exp.suffix = '@2x'; exp.skipChildren = true; - expect(exp.type).toBe('jpeg'); - expect(exp.scale).toBeCloseTo(2, 0); + const persisted = r.exports[0]; + expect(persisted.type).toBe('jpeg'); + expect(persisted.scale).toBeCloseTo(2, 0); + expect(persisted.suffix).toBe('@2x'); + expect(persisted.skipChildren).toBe(true); }); }); + // Coverage-only re-exercise of gradient/shadow members. The behavioural + // assertions live in `value-objects.test.ts` (gradients, shadow color) and + // `shadows-blur.test.ts` (shadow scalars); look there for a regression. describe('Gradient and shadow leftovers', () => { test('gradient endpoints and stops round-trip', (ctx) => { const r = rect(ctx); @@ -127,6 +141,10 @@ describe('Misc', () => { } }); + // `shape.shadows` returns live shadow proxies: writing through a returned + // shadow persists to the shape. The shadow `color`, however, is a plain + // snapshot, so setting a gradient on it is lost and the solid color set via + // `shadow.color` survives. This reads the shadow back from the shape. test('shadow color and id round-trip', (ctx) => { const r = rect(ctx); r.shadows = [ @@ -161,6 +179,36 @@ describe('Misc', () => { void color.gradient; } expect(r.shadows).toHaveLength(1); + const persisted = r.shadows[0].color; + expect(persisted).toBeDefined(); + if (persisted) { + expect(persisted.color).toBe('#ff00ff'); + expect(persisted.opacity).toBeCloseTo(0.5, 2); + } + }); + + // The scalar shadow members persist to the shape through the live proxy. + test('shadow scalar members persist to the shape', (ctx) => { + const r = rect(ctx); + r.shadows = [ + { + style: 'drop-shadow', + offsetX: 1, + offsetY: 1, + blur: 2, + spread: 0, + hidden: false, + color: { color: '#000000', opacity: 1 }, + }, + ]; + const shadow = r.shadows[0]; + shadow.offsetX = 9; + shadow.blur = 7; + shadow.hidden = true; + const persisted = r.shadows[0]; + expect(persisted.offsetX).toBeCloseTo(9, 0); + expect(persisted.blur).toBeCloseTo(7, 0); + expect(persisted.hidden).toBe(true); }); }); @@ -182,36 +230,38 @@ describe('Misc', () => { }); describe('Layout leftovers', () => { - test('flex padding and child margins are readable', (ctx) => { + test('flex padding and child margins round-trip', (ctx) => { const board = ctx.penpot.createBoard(); ctx.board.appendChild(board); const flex = board.addFlexLayout(); flex.horizontalPadding = 4; flex.verticalPadding = 6; - void flex.horizontalPadding; - void flex.verticalPadding; + expect(flex.horizontalPadding).toBeCloseTo(4, 0); + expect(flex.verticalPadding).toBeCloseTo(6, 0); const child = ctx.penpot.createRectangle(); flex.appendChild(child); const lc = child.layoutChild; + expect(lc).toBeDefined(); if (lc) { - lc.horizontalMargin = 1; - lc.verticalMargin = 2; lc.topMargin = 3; lc.rightMargin = 4; lc.bottomMargin = 5; lc.leftMargin = 6; lc.maxHeight = 100; lc.minWidth = 10; - void lc.horizontalMargin; - void lc.verticalMargin; - void lc.leftMargin; - void lc.rightMargin; - void lc.bottomMargin; - void lc.maxHeight; - void lc.minWidth; + expect(lc.topMargin).toBeCloseTo(3, 0); + expect(lc.rightMargin).toBeCloseTo(4, 0); + expect(lc.bottomMargin).toBeCloseTo(5, 0); + expect(lc.leftMargin).toBeCloseTo(6, 0); + expect(lc.maxHeight).toBeCloseTo(100, 0); + expect(lc.minWidth).toBeCloseTo(10, 0); + // The combined setters overwrite the per-side values just set. + lc.horizontalMargin = 7; + lc.verticalMargin = 8; + expect(lc.horizontalMargin).toBeCloseTo(7, 0); + expect(lc.verticalMargin).toBeCloseTo(8, 0); } - expect(board.type).toBe('board'); }); test('grid cell properties round-trip', (ctx) => { @@ -223,18 +273,21 @@ describe('Misc', () => { const child = ctx.penpot.createRectangle(); grid.appendChild(child, 1, 1); const cell = child.layoutCell; + expect(cell).toBeDefined(); if (cell) { cell.areaName = 'header'; + expect(cell.areaName).toBe('header'); cell.position = 'auto'; - void cell.areaName; - void cell.position; - void cell.rowSpan; + expect(cell.position).toBe('auto'); + expect(cell.rowSpan).toBeCloseTo(1, 0); } - expect(board.type).toBe('board'); }); }); describe('Track', () => { + // `grid.rows` returns live track proxies: writing through a returned track + // persists to the grid. This reads the track back from the grid (a fresh + // proxy) instead of asserting on the object we wrote to. test('grid track members round-trip on the returned track', (ctx) => { const board = ctx.penpot.createBoard(); ctx.board.appendChild(board); @@ -243,8 +296,9 @@ describe('Misc', () => { const track = grid.rows[0]; track.type = 'fixed'; track.value = 80; - expect(track.type).toBe('fixed'); - expect(track.value).toBeCloseTo(80, 0); + const persisted = grid.rows[0]; + expect(persisted.type).toBe('fixed'); + expect(persisted.value).toBeCloseTo(80, 0); }); }); @@ -258,11 +312,15 @@ describe('Misc', () => { const cmd = commands[0]; void cmd.command; void cmd.params; + // Command objects are snapshots: mutating one only changes the local + // array; the edit reaches the shape by reassigning the whole command + // list (Path.commands set). cmd.command = 'line-to'; cmd.params = { x: 5, y: 5 }; expect(cmd.command).toBe('line-to'); - // Reassign the whole command list (Path.commands set). + expect(path.commands[0].command).toBe('move-to'); path.commands = commands; + expect(path.commands[0].command).toBe('line-to'); }); }); @@ -277,6 +335,8 @@ describe('Misc', () => { }); }); + // Coverage-only reads of interaction/action fields; the behavioural + // round-trips are owned by `interactions.test.ts`. describe('Interaction reads', () => { test('overlay action fields are readable', (ctx) => { const overlay = ctx.penpot.createBoard(); 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 1cd3463d7f..94f2c3842c 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 @@ -3,8 +3,8 @@ import { describe, test } from '../framework/registry'; // Pages, selection and flows. // Most assertions use the active page (`currentPage`) and the scratch board so -// the user's file is left clean. createPage/openPage necessarily leave a page -// behind (the API has no removePage), so the active page is restored afterwards. +// the user's file is left clean. Tests that create a page restore the active +// page and remove the created one afterwards. describe('Pages', () => { test('currentPage exposes id and name', (ctx) => { @@ -26,7 +26,29 @@ describe('Pages', () => { const active = ctx.penpot.currentPage; expect(active && active.id).toBe(page.id); - // Restore the originally active page so other tests aren't affected. + // Restore the originally active page and remove the created page so the + // file is left clean. + if (original) await ctx.penpot.openPage(original); + page.remove(); + const pages = ctx.penpot.currentFile?.pages ?? []; + expect(pages.some((p) => p.id === page.id)).toBe(false); + }); + + test('removing the active page activates another page', async (ctx) => { + const original = ctx.penpot.currentPage; + const page = ctx.penpot.createPage(); + await ctx.penpot.openPage(page); + page.remove(); + // The switch to another page happens asynchronously; wait for it. + const start = Date.now(); + let active = ctx.penpot.currentPage; + while (active && active.id === page.id && Date.now() - start < 2000) { + await new Promise((resolve) => setTimeout(resolve, 50)); + active = ctx.penpot.currentPage; + } + expect(active).not.toBeNull(); + expect(active && active.id).not.toBe(page.id); + // The workspace activates the first remaining page; go back home. if (original) await ctx.penpot.openPage(original); }); @@ -104,6 +126,31 @@ describe('Selection', () => { ctx.penpot.selection = [rect, rect]; expect(ctx.penpot.selection).toHaveLength(1); }); + + // The selection setter does not validate liveness: a removed shape reference + // is still accepted into the selection (currently unvalidated). + test('selection accepts a removed shape (currently unvalidated)', (ctx) => { + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + rect.remove(); + ctx.penpot.selection = [rect]; + expect(ctx.penpot.selection.some((s) => s.id === rect.id)).toBe(true); + }); + + // A shape living on a non-active page cannot be selected on the current page; + // the setter filters it out. + test('selection ignores a shape from a non-active page', async (ctx) => { + const original = ctx.penpot.currentPage; + const page = ctx.penpot.createPage(); + await ctx.penpot.openPage(page); + const other = ctx.penpot.createRectangle(); + if (original) await ctx.penpot.openPage(original); + + ctx.penpot.selection = [other]; + expect(ctx.penpot.selection.some((s) => s.id === other.id)).toBe(false); + + page.remove(); + }); }); describe('Flows', () => { diff --git a/plugins/apps/plugin-api-test-suite/src/tests/platform.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/platform.test.ts index ba318d0599..f77e063304 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/platform.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/platform.test.ts @@ -35,9 +35,13 @@ describe('Platform', () => { expect(['light', 'dark']).toContain(ctx.penpot.theme); }); - test('flags are readable', (ctx) => { - expect(typeof ctx.penpot.flags.naturalChildOrdering).toBe('boolean'); - expect(typeof ctx.penpot.flags.throwValidationErrors).toBe('boolean'); + // The whole suite is written against both flags being on (README "Runtime + // details"): invalid input throws and `children` is z-index ordered. Pin + // them so a runner-setup regression fails here instead of silently + // changing what dozens of tests mean. + test('runner pins naturalChildOrdering and throwValidationErrors on', (ctx) => { + expect(ctx.penpot.flags.naturalChildOrdering).toBe(true); + expect(ctx.penpot.flags.throwValidationErrors).toBe(true); }); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/plugin-data.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/plugin-data.test.ts index e5c02be79b..254717e63e 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/plugin-data.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/plugin-data.test.ts @@ -36,10 +36,13 @@ describe('Plugin data', () => { // --------------------------------------------------------------------------- test('setPluginData with an empty key is accepted (currently unvalidated)', (ctx) => { // An empty key is not rejected; this pins the current lenient behaviour - // (a candidate for future hardening). + // (a candidate for future hardening): the value is stored and retrievable + // under the empty key like any other. const rect = ctx.penpot.createRectangle(); ctx.board.appendChild(rect); expect(() => rect.setPluginData('', 'value')).not.toThrow(); + expect(rect.getPluginData('')).toBe('value'); + expect(rect.getPluginDataKeys()).toContain(''); }); test('setPluginData with a non-string value throws', (ctx) => { diff --git a/plugins/apps/plugin-api-test-suite/src/tests/shapes-factories.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/shapes-factories.test.ts index 89209a0906..6dc8be8fa5 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/shapes-factories.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/shapes-factories.test.ts @@ -118,19 +118,14 @@ describe('Shapes', () => { if (bool) { ctx.board.appendChild(bool); expect(typeof bool.d).toBe('string'); - expect(typeof bool.toD()).toBe('string'); } } }); - test('createShapeFromSvg is lenient with unparseable markup', (ctx) => { - // The SVG importer is permissive: it still produces a group for input - // that is not valid SVG rather than returning null. - const group = ctx.penpot.createShapeFromSvg('not svg at all'); - expect(group).not.toBeNull(); - if (group) { - ctx.board.appendChild(group); - } + test('createShapeFromSvg rejects unparseable markup', (ctx) => { + // Malformed SVG is rejected up front rather than failing asynchronously + // inside the import pipeline (which would surface as an error toast). + expect(() => ctx.penpot.createShapeFromSvg('not svg at all')).toThrow(); }); // Success edges — non-trivial valid construction. @@ -229,8 +224,10 @@ describe('Shapes', () => { if (group) { const before = ctx.board.children.length; ctx.penpot.ungroup(group); - // After ungroup the two shapes should be back on the board directly. - expect(ctx.board.children.length).toBeGreaterThan(before - 1); + // The group dissolves into its two children (net +1 on the board) and + // the group itself is gone. + expect(ctx.board.children.length).toBe(before + 1); + expect(ctx.board.children.some((c) => c.id === group.id)).toBe(false); } }); @@ -269,35 +266,48 @@ describe('Shapes', () => { expect(a.y).toBeCloseTo(b.y, 0); }); - test('distributeHorizontal runs without error', (ctx) => { + test('distributeHorizontal spaces the shapes evenly', (ctx) => { const a = ctx.penpot.createRectangle(); const b = ctx.penpot.createRectangle(); const c = ctx.penpot.createRectangle(); a.x = 0; b.x = 50; c.x = 300; + a.resize(20, 20); + b.resize(20, 20); + c.resize(20, 20); ctx.board.appendChild(a); ctx.board.appendChild(b); ctx.board.appendChild(c); ctx.penpot.distributeHorizontal([a, b, c]); - // Middle shape should end up between the outer two. - expect(b.x).toBeGreaterThan(a.x); + // The outer shapes stay in place and the middle one lands so the gaps + // between consecutive shapes are equal. + const xs = [a.x, b.x, c.x].sort((p, q) => p - q); + expect(xs[1] - xs[0]).toBeCloseTo(xs[2] - xs[1], 0); + expect(xs[0]).toBeCloseTo(0, 0); + expect(xs[2]).toBeCloseTo(300, 0); }); - test('distributeVertical runs without error', (ctx) => { + test('distributeVertical spaces the shapes evenly', (ctx) => { const a = ctx.penpot.createRectangle(); const b = ctx.penpot.createRectangle(); const c = ctx.penpot.createRectangle(); a.y = 0; b.y = 50; c.y = 300; + a.resize(20, 20); + b.resize(20, 20); + c.resize(20, 20); ctx.board.appendChild(a); ctx.board.appendChild(b); ctx.board.appendChild(c); ctx.penpot.distributeVertical([a, b, c]); - expect(b.y).toBeGreaterThan(a.y); + const ys = [a.y, b.y, c.y].sort((p, q) => p - q); + expect(ys[1] - ys[0]).toBeCloseTo(ys[2] - ys[1], 0); + expect(ys[0]).toBeCloseTo(0, 0); + expect(ys[2]).toBeCloseTo(300, 0); }); // Edge cases. 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 dfeae815f6..c31ef3912a 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 @@ -38,7 +38,9 @@ describe('Shapes', () => { const b = rect(ctx); // Two siblings on a fresh board occupy indices 0 and 1 (direction depends // on naturalChildOrdering, so assert the set rather than which is which). - expect([a.parentIndex, b.parentIndex].sort()).toEqual([0, 1]); + expect([a.parentIndex, b.parentIndex].sort((x, y) => x - y)).toEqual([ + 0, 1, + ]); }); }); @@ -269,16 +271,12 @@ describe('Shapes', () => { void b; const last = ctx.board.children.length - 1; + // naturalChildOrdering is pinned on, so the last index renders in front. a.bringToFront(); - const front = a.parentIndex; - expect(front === 0 || front === last).toBe(true); + expect(a.parentIndex).toBe(last); a.sendToBack(); - const back = a.parentIndex; - expect(back === 0 || back === last).toBe(true); - - // Front and back must be different extremes. - expect(front).not.toBe(back); + expect(a.parentIndex).toBe(0); }); test('bringForward / sendBackward move the shape one step', (ctx) => { @@ -290,11 +288,63 @@ describe('Shapes', () => { const start = a.parentIndex; a.bringForward(); - expect(Math.abs(a.parentIndex - start)).toBe(1); + expect(a.parentIndex).toBe(start + 1); const mid = a.parentIndex; a.sendBackward(); - expect(Math.abs(a.parentIndex - mid)).toBe(1); + expect(a.parentIndex).toBe(mid - 1); + }); + + // Community report (forum #10700, issue #16): bringToFront once failed to + // reorder shapes inside a flex container (it worked on plain boards and grid + // containers). Fixed; kept as a regression pin. + test('bringToFront reorders children inside a flex container', (ctx) => { + const b = ctx.penpot.createBoard(); + ctx.board.appendChild(b); + b.resize(300, 200); + b.addFlexLayout(); + + const r1 = ctx.penpot.createRectangle(); + const r2 = ctx.penpot.createRectangle(); + const r3 = ctx.penpot.createRectangle(); + b.appendChild(r1); + b.appendChild(r2); + b.appendChild(r3); + + r1.bringToFront(); + // naturalChildOrdering is on: the last element renders in front. + expect(b.children[b.children.length - 1].id).toBe(r1.id); + }); + + // Issue #16, grid variant: reorders correctly. Kept as a regression pin. + test('bringToFront reorders children inside a grid container', (ctx) => { + const b = ctx.penpot.createBoard(); + ctx.board.appendChild(b); + b.resize(300, 200); + const grid = b.addGridLayout(); + grid.addColumn('flex', 1); + grid.addColumn('flex', 1); + grid.addRow('flex', 1); + + const r1 = ctx.penpot.createRectangle(); + const r2 = ctx.penpot.createRectangle(); + grid.appendChild(r1, 1, 1); + grid.appendChild(r2, 1, 2); + + r1.bringToFront(); + expect(b.children[b.children.length - 1].id).toBe(r1.id); + }); + + // Locks the ordering semantics the whole suite relies on: with + // naturalChildOrdering on, the last child renders in front. If the flag + // default ever regresses, this fails loudly. + test('last child renders in front under naturalChildOrdering', (ctx) => { + const a = rect(ctx); + const b = rect(ctx); + a.bringToFront(); + const children = ctx.board.children; + expect(children[children.length - 1].id).toBe(a.id); + expect(a.parentIndex).toBeGreaterThan(b.parentIndex); }); }); @@ -308,12 +358,43 @@ describe('Shapes', () => { expect(copy.type).toBe('rectangle'); }); + test('clone deep-copies a group with its children', (ctx) => { + const a = rect(ctx); + const b = rect(ctx); + const group = ctx.penpot.group([a, b]); + expect(group).not.toBeNull(); + if (!group) return; + + const copy = group.clone(); + ctx.board.appendChild(copy); + expect(copy.id).not.toBe(group.id); + expect(copy.children).toHaveLength(group.children.length); + + // Every cloned descendant is a fresh shape, none sharing an id with the + // originals. + const originalIds = new Set(group.children.map((c) => c.id)); + for (const child of copy.children) { + expect(originalIds.has(child.id)).toBe(false); + } + }); + test('remove detaches the shape from its parent', (ctx) => { const r = rect(ctx); const before = ctx.board.children.length; r.remove(); expect(ctx.board.children.length).toBe(before - 1); }); + + test('remove makes the shape unreachable by id', (ctx) => { + const r = rect(ctx); + const id = r.id; + r.remove(); + const page = ctx.penpot.currentPage; + expect(page).not.toBeNull(); + if (page) { + expect(page.getShapeById(id)).toBeNull(); + } + }); }); // --------------------------------------------------------------------------- @@ -343,13 +424,13 @@ describe('Shapes', () => { }).toThrow(); }); - test('NaN rotation is accepted (currently unvalidated)', (ctx) => { - // The rotation setter does not reject NaN; this pins the current lenient - // behaviour (a candidate for future hardening). + test('NaN rotation throws', (ctx) => { + // The rotation setter rejects non-finite numbers; a NaN would otherwise + // reach the geometry layer as an invalid move vector. const r = rect(ctx); expect(() => { r.rotation = NaN; - }).not.toThrow(); + }).toThrow(); }); test('invalid blendMode throws', (ctx) => { @@ -366,13 +447,33 @@ describe('Shapes', () => { }).toThrow(); }); + test('resize to zero dimensions throws', (ctx) => { + const r = rect(ctx); + expect(() => { + r.resize(0, 0); + }).toThrow(); + }); + + test('resize with negative dimensions throws', (ctx) => { + const r = rect(ctx); + expect(() => { + r.resize(-100, -50); + }).toThrow(); + }); + test('setParentIndex with a negative index is accepted (currently unvalidated)', (ctx) => { // setParentIndex does not reject a negative index; this pins the current - // lenient behaviour (a candidate for future hardening). + // lenient behaviour (a candidate for future hardening) and that the + // hierarchy survives it: the shape stays a child and no sibling is + // duplicated or dropped. const a = rect(ctx); const b = rect(ctx); void b; + const countBefore = ctx.board.children.length; expect(() => a.setParentIndex(-1)).not.toThrow(); + expect(a.parent?.id).toBe(ctx.board.id); + expect(ctx.board.children).toHaveLength(countBefore); + expect(ctx.board.children.filter((c) => c.id === a.id)).toHaveLength(1); }); }); @@ -405,7 +506,9 @@ describe('Shapes', () => { void c; b.setParentIndex(0); expect(b.parentIndex).toBe(0); - const indices = ctx.board.children.map((s) => s.parentIndex).sort(); + const indices = ctx.board.children + .map((s) => s.parentIndex) + .sort((x, y) => x - y); expect(indices).toEqual([0, 1, 2]); }); diff --git a/plugins/apps/plugin-api-test-suite/src/tests/shapes-types.test.ts b/plugins/apps/plugin-api-test-suite/src/tests/shapes-types.test.ts index dcd7c82dd5..33bdaa9487 100644 --- a/plugins/apps/plugin-api-test-suite/src/tests/shapes-types.test.ts +++ b/plugins/apps/plugin-api-test-suite/src/tests/shapes-types.test.ts @@ -168,7 +168,6 @@ describe('Shapes', () => { ctx.board.appendChild(bool); expect(bool.children.length).toBeGreaterThan(1); expect(typeof bool.d).toBe('string'); - expect(typeof bool.toD()).toBe('string'); expect(Array.isArray(bool.commands)).toBe(true); } }); @@ -181,7 +180,6 @@ describe('Shapes', () => { path.d = 'M0 0 L10 0 L10 10 Z'; expect(path.d).toContain('M'); expect(path.commands.length).toBeGreaterThan(0); - expect(typeof path.toD()).toBe('string'); }); test('content alias is readable and writable', (ctx) => { @@ -227,21 +225,26 @@ describe('Shapes', () => { // --------------------------------------------------------------------------- describe('Hierarchy — circular references', () => { // The plugin appendChild does not explicitly reject cycle-creating moves; - // the underlying relocate handles them without throwing. These pin that the - // call is not rejected (cycle-prevention at the API boundary is a candidate - // for future hardening). - test('appending a board into itself does not throw', (ctx) => { + // the underlying relocate drops them without throwing, leaving the + // hierarchy untouched. These pin both halves: the call is not rejected + // (cycle-prevention at the API boundary is a candidate for future + // hardening) AND the tree stays intact. + test('appending a board into itself is a safe no-op', (ctx) => { const board = ctx.penpot.createBoard(); ctx.board.appendChild(board); expect(() => board.appendChild(board)).not.toThrow(); + expect(board.parent?.id).toBe(ctx.board.id); + expect(board.children.some((c) => c.id === board.id)).toBe(false); }); - test('appending an ancestor into its descendant does not throw', (ctx) => { + test('appending an ancestor into its descendant is a safe no-op', (ctx) => { const outer = ctx.penpot.createBoard(); const inner = ctx.penpot.createBoard(); ctx.board.appendChild(outer); outer.appendChild(inner); expect(() => inner.appendChild(outer)).not.toThrow(); + expect(outer.parent?.id).toBe(ctx.board.id); + expect(inner.parent?.id).toBe(outer.id); }); }); 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 21a4efbe38..3dbaaf61c9 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 @@ -22,6 +22,27 @@ describe('Text', () => { expect(t.characters).toBe('Updated content'); }); + // Community report (forum #10700, issue #7): a text shape must always have + // content, so assigning an empty string to characters is rejected by + // validation (by design). To make a text disappear, hide or remove the shape. + test('characters rejects an empty string', (ctx) => { + const t = text(ctx, 'hello'); + expect(() => { + t.characters = ''; + }).toThrow(); + expect(t.characters).toBe('hello'); + }); + + // Community report (forum #10700, feature request D): rejecting a font + // weight the current font has no variant for must tell the caller which + // weights are supported. + test('unsupported fontWeight error lists the supported weights', (ctx) => { + const t = text(ctx, 'hello'); + expect(() => { + t.fontWeight = '123'; + }).toThrow('Supported weights:'); + }); + test('growType round-trips', (ctx) => { const t = text(ctx); t.growType = 'auto-height'; @@ -202,7 +223,7 @@ describe('Text', () => { expect(range.characters.length).toBeGreaterThan(0); }); - test('range font properties can be set', (ctx) => { + test('range font property setters are exercised (coverage only)', (ctx) => { const t = text(ctx, 'Hello Penpot'); const range = t.getRange(0, 5); const font = ctx.penpot.fonts.all[0]; @@ -247,13 +268,17 @@ describe('Text', () => { }); test('getRange beyond the text length is clamped (not rejected)', (ctx) => { - // An end index past the text length is clamped rather than rejected. + // An end index past the text length is clamped rather than rejected: the + // range object is returned and reading `characters` yields the clamped + // text. (Reading `characters` once crashed with a TypeError on an internal + // null; fixed, kept as a regression pin.) const t = text(ctx, 'Hello Penpot'); let range: ReturnType | null = null; expect(() => { range = t.getRange(0, 999); }).not.toThrow(); expect(range).not.toBeNull(); + expect(range!.characters).toBe('Hello Penpot'); }); test('empty fontSize 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 8bd2e0f790..0b3ae9be6c 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 @@ -9,6 +9,7 @@ import type { TokenTypography, } from '@penpot/plugin-types'; import type { TestContext } from '../framework/types'; +import { waitFor } from './wait'; // Design tokens. // The token catalog is reached through the local library. Sets/themes/tokens are @@ -69,6 +70,29 @@ describe('Tokens', () => { expect(set.active).toBe(true); }); + // 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. + test('toggleActive persists when the set has bound tokens', async (ctx) => { + const set = activeSet(ctx, unique('set')); + const token = set.addToken({ + type: 'borderRadius', + name: unique('radius.'), + value: '12', + }); + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + rect.applyToken(token); + await sleep(300); + + set.toggleActive(); + await waitFor(() => set.active === false); + expect(set.active).toBe(false); + set.toggleActive(); + await waitFor(() => set.active === true); + expect(set.active).toBe(true); + }); + test('addToken adds a token and lists it', (ctx) => { const set = activeSet(ctx, unique('set')); const token = set.addToken({ @@ -143,7 +167,7 @@ describe('Tokens', () => { const theme = cat.addTheme({ group: '', name: unique('theme') }); const set = cat.addSet({ name: unique('set'), active: false }); theme.addSet(set); - await sleep(300); + await waitFor(() => theme.activeSets.length > 0); expect(theme.activeSets.length).toBeGreaterThan(0); theme.removeSet(set); }); @@ -154,10 +178,10 @@ describe('Tokens', () => { const theme = cat.addTheme({ group: '', name: unique('theme') }); const set = cat.addSet({ name: unique('set'), active: false }); theme.addSet(set.id); - await sleep(300); + await waitFor(() => theme.activeSets.length > 0); expect(theme.activeSets.length).toBeGreaterThan(0); theme.removeSet(set.id); - await sleep(300); + await waitFor(() => theme.activeSets.length === 0); expect(theme.activeSets.length).toBe(0); }); @@ -224,7 +248,7 @@ describe('Tokens', () => { const rect = ctx.penpot.createRectangle(); ctx.board.appendChild(rect); token.applyToShapes([rect]); - await sleep(300); + await waitFor(() => Object.keys(rect.tokens).length > 0); expect(Object.keys(rect.tokens).length).toBeGreaterThan(0); }); @@ -239,7 +263,7 @@ describe('Tokens', () => { ctx.board.appendChild(rect); ctx.penpot.selection = [rect]; token.applyToSelected(); - await sleep(300); + await waitFor(() => Object.keys(rect.tokens).length > 0); expect(Object.keys(rect.tokens).length).toBeGreaterThan(0); }); @@ -256,6 +280,32 @@ describe('Tokens', () => { expect(rect.tokens).toBeDefined(); }); + // Community report (forum #10700, issue #9): applyToken with a spacing token + // on 'paddingLeft' once recorded the binding under a bogus + // 'paddingBottomLeft' key (the gap properties bound correctly). Fixed; kept + // as a regression pin. + test('applyToken binds a spacing token to flex padding', async (ctx) => { + const set = activeSet(ctx, unique('set')); + const token = set.addToken({ + type: 'spacing', + name: unique('spacing.'), + value: '8', + }); + + const b = ctx.penpot.createBoard(); + ctx.board.appendChild(b); + b.resize(300, 200); + b.addFlexLayout(); + + b.applyToken(token, ['columnGap']); + b.applyToken(token, ['paddingLeft']); + // Both bindings settle together; wait on columnGap, then assert paddingLeft + // lands under its own key (the previously-misfiled case). + await waitFor(() => Object.keys(b.tokens).includes('columnGap')); + expect(Object.keys(b.tokens)).toContain('columnGap'); + expect(Object.keys(b.tokens)).toContain('paddingLeft'); + }); + test('duplicate and remove a token', (ctx) => { const set = activeSet(ctx, unique('set')); const token = set.addToken({ @@ -318,11 +368,19 @@ describe('Token types', () => { expect(typeof token.type).toBe('string'); void token.value; token.value = value2; - token.name = unique('renamed.'); - expect(typeof token.name).toBe('string'); - // Record the resolvedValue (get) target for every type. fontFamilies - // returns the wrong shape (see the dedicated red test below), but reading - // it no longer throws, so a plain read is enough here. + const renamed = unique('renamed.'); + token.name = renamed; + // The value and name setters round-trip on the live backend. A + // `fontFamilies` value reads back as a family array, so join it before + // comparing to the assigned single family. + const readValue = token.value; + const normalized = Array.isArray(readValue) + ? readValue.join(', ') + : readValue; + expect(normalized).toBe(value2); + expect(token.name).toBe(renamed); + // Record the resolvedValue (get) target for every type; the fontFamilies + // shape is asserted in the dedicated test below. void token.resolvedValue; }); } 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 adcbabd52c..88f3eb94c8 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 @@ -4,10 +4,11 @@ import type { TestContext } from '../framework/types'; import { PNG_1X1 } from './fixtures'; // Value-object property setters. -// Fills/strokes/gradients are returned as live proxies (their setters persist); -// shadows/blur/colors are returned as plain snapshots (setting records the -// member and round-trips on the returned object). Either way every writable -// member is exercised by reading the value object and setting each property. +// Fills/strokes/gradients/shadows are returned as live proxies (their setters +// persist to the shape); blur and the shadow `color` are returned as plain +// snapshots (setting records the member and round-trips on the returned +// object). Either way every writable member is exercised by reading the value +// object and setting each property. function rect(ctx: TestContext) { const r = ctx.penpot.createRectangle(); 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 6afb75f495..d1e2be5a5a 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 @@ -1,6 +1,11 @@ import { expect } from '../framework/expect'; import { describe, test } from '../framework/registry'; -import type { Board, LibraryVariantComponent } from '@penpot/plugin-types'; +import type { + Board, + LibraryComponent, + LibraryVariantComponent, + VariantContainer, +} from '@penpot/plugin-types'; import type { TestContext } from '../framework/types'; // Variants. @@ -15,8 +20,41 @@ function componentMain(ctx: TestContext): Board { return comp.mainInstance() as Board; } -function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); +interface ComponentWithMain { + comp: LibraryComponent; + main: Board; +} + +function componentWithMain(ctx: TestContext): ComponentWithMain { + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + const comp = ctx.penpot.library.local.createComponent([rect]); + return { comp, main: comp.mainInstance() as Board }; +} + +function variantComponentIds(container: VariantContainer): string[] { + const components = container.variants?.variantComponents() ?? []; + return components.map((c) => c.id); +} + +function sameIds(a: string[], b: string[]): boolean { + return a.length === b.length && a.every((id, i) => id === b[i]); +} + +// Polls until the condition holds. On timeout it returns normally so the +// assertion that follows reports the actual mismatch. +async function waitFor( + condition: () => boolean, + attempts = 100, +): Promise { + for (let i = 0; i < attempts; i++) { + try { + if (condition()) return; + } catch { + // Treat a throwing condition as not-yet-ready and keep polling. + } + await new Promise((resolve) => setTimeout(resolve, 20)); + } } // transformInVariant is async, so `variants` is only populated after a tick. @@ -27,7 +65,7 @@ async function variantComponent( ctx.board.appendChild(rect); const comp = ctx.penpot.library.local.createComponent([rect]); comp.transformInVariant(); - await sleep(400); + await waitFor(() => comp.isVariant()); return comp as LibraryVariantComponent; } @@ -57,6 +95,45 @@ describe('Variants', () => { expect(container.isVariantContainer()).toBe(true); }); + // The order of the resulting variant components must follow the input order, + // so that variant property values can be paired positionally (#10506). + test('createVariantFromComponents preserves the input component order', async (ctx) => { + const a = componentWithMain(ctx); + const b = componentWithMain(ctx); + const c = componentWithMain(ctx); + const container = ctx.penpot.createVariantFromComponents([ + c.main, + a.main, + b.main, + ]); + const expected = [c.comp.id, a.comp.id, b.comp.id]; + await waitFor(() => sameIds(variantComponentIds(container), expected)); + expect(variantComponentIds(container)).toEqual(expected); + }); + + test('combineAsVariants orders components head first, then the given ids', async (ctx) => { + const a = componentWithMain(ctx); + const b = componentWithMain(ctx); + const c = componentWithMain(ctx); + const container = a.main.combineAsVariants([c.main.id, b.main.id]); + const expected = [a.comp.id, c.comp.id, b.comp.id]; + await waitFor(() => sameIds(variantComponentIds(container), expected)); + expect(variantComponentIds(container)).toEqual(expected); + }); + + test('combineAsVariants deduplicates repeated ids', async (ctx) => { + const a = componentWithMain(ctx); + const b = componentWithMain(ctx); + const container = a.main.combineAsVariants([ + b.main.id, + a.main.id, + b.main.id, + ]); + const expected = [a.comp.id, b.comp.id]; + await waitFor(() => sameIds(variantComponentIds(container), expected)); + expect(variantComponentIds(container)).toEqual(expected); + }); + test('variant component exposes variant props and Variants', (ctx) => { const rect = ctx.penpot.createRectangle(); ctx.board.appendChild(rect); @@ -97,7 +174,8 @@ describe('Variants', () => { const vc = await variantComponent(ctx); expect(vc.isVariant()).toBe(true); expect(typeof vc.variantProps).toBe('object'); - void vc.variantError; // get only (no runtime setter) + // A well-formed variant carries no error (get only; no runtime setter). + expect(vc.variantError ?? undefined).toBeUndefined(); const v = vc.variants; expect(v).not.toBeNull(); @@ -117,15 +195,16 @@ describe('Variants', () => { const v = vc.variants; expect(v).not.toBeNull(); if (v) { + const before = v.properties.length; v.addProperty(); - await sleep(300); + await waitFor(() => v.properties.length > before); const count = v.properties.length; expect(count).toBeGreaterThan(0); v.renameProperty(0, 'Size'); - await sleep(300); v.removeProperty(count - 1); - await sleep(300); + await waitFor(() => v.properties.length < count); + expect(v.properties.length).toBe(count - 1); } }); @@ -136,12 +215,11 @@ describe('Variants', () => { if (v) { const before = v.variantComponents().length; vc.addVariant(); - await sleep(300); + await waitFor(() => v.variantComponents().length > before); expect(v.variantComponents().length).toBeGreaterThan(before); if (v.properties.length > 0) { vc.setVariantProperty(0, 'large'); - await sleep(300); } } }); @@ -150,7 +228,7 @@ describe('Variants', () => { const vc = await variantComponent(ctx); // Add a second variant so there is another value to switch to. vc.addVariant(); - await sleep(300); + await waitFor(() => (vc.variants?.variantComponents().length ?? 0) > 1); const instance = vc.instance(); ctx.board.appendChild(instance); @@ -159,43 +237,121 @@ describe('Variants', () => { expect(() => instance.switchVariant(0, 'large')).not.toThrow(); }); + // Community report (forum #10700, issue #3): switchVariant on an instance + // living inside a cloned board was said to hang the plugin bridge + // indefinitely. Did not reproduce; kept as a regression pin. + test('switchVariant works on an instance inside a cloned board', async (ctx) => { + const vc = await variantComponent(ctx); + vc.addVariant(); + await waitFor(() => (vc.variants?.variantComponents().length ?? 0) > 1); + + const wrapper = ctx.penpot.createBoard(); + ctx.board.appendChild(wrapper); + wrapper.resize(400, 300); + const instance = vc.instance(); + wrapper.appendChild(instance); + + const cloned = wrapper.clone() as Board; + ctx.board.appendChild(cloned); + const clonedInstance = cloned.children.find((s) => s.isComponentInstance()); + expect(clonedInstance).toBeDefined(); + if (clonedInstance) { + expect(() => clonedInstance.switchVariant(0, 'large')).not.toThrow(); + } + }); + test('utils.types.isVariantComponent identifies a variant component', async (ctx) => { const vc = await variantComponent(ctx); expect(ctx.penpot.utils.types.isVariantComponent(vc)).toBeTruthy(); }); + test('variantError stores an invalid variant name', async (ctx) => { + const mainA = componentMain(ctx); + const mainB = componentMain(ctx); + const container = ctx.penpot.createVariantFromComponents([mainA, mainB]); + await waitFor(() => { + const v = container.variants; + return !!v && v.variantComponents().length > 0; + }); + + const variants = container.variants; + expect(variants).not.toBeNull(); + if (variants) { + const comps = variants.variantComponents(); + expect(comps.length).toBeGreaterThan(0); + + // Renaming a variant's main instance to something that doesn't follow + // the "[property]=[value], …" structure surfaces the rejected name in + // variantError instead of applying it. + const invalidName = 'not a valid variant structure'; + comps[0].mainInstance().name = invalidName; + await waitFor(() => + variants + .variantComponents() + .some( + (c) => (c as LibraryVariantComponent).variantError === invalidName, + ), + ); + + const after = variants.variantComponents(); + expect( + after.map((c) => (c as LibraryVariantComponent).variantError), + ).toContain(invalidName); + } + }); + + // Blocked by API bug: setting `path` on a variant component renames only that + // component (via rename-component), leaving its variant container and main + // instance untouched. For a variant, path/name are shared across the whole + // group, so the container's name must stay equal to the component's full + // path/name. A plain `vc.path === 'Group'` round-trip would pass even with the + // bug (the component *is* renamed); the container is the tell. `.name` routes + // through the variant-aware rename; `.path` must do the same. + test('setting path on a variant component renames the whole variant', async (ctx) => { + const vc = await variantComponent(ctx); + const container = vc.mainInstance().parent as VariantContainer; + + // mergePathItem(path, name): "path / name" when both present, else the leaf. + // Reconstructed from the component so the assertion holds whether the fix + // propagates the path or strips it (as the variant rename currently does). + const fullName = () => (vc.path ? `${vc.path} / ${vc.name}` : vc.name); + + vc.path = 'Group'; + await waitFor(() => container.name === fullName()); + expect(container.name).toBe(fullName()); + }); + // --------------------------------------------------------------------------- // Edge cases. Out-of-bounds property positions and degenerate // container input should be rejected. // --------------------------------------------------------------------------- - // createVariantFromComponents([]) is rejected (validated), but the - // positional property ops do not bounds-check `pos`; an out-of-range index - // is a no-op rather than an error. These pin the current behaviour - // (bounds-checking the position is a candidate for future hardening). + // createVariantFromComponents([]) is rejected (validated). The positional + // property ops bounds-check `pos`; an out-of-range index is rejected rather + // than reaching the data layer (where it would surface as an error toast). test('createVariantFromComponents of an empty array throws', (ctx) => { expect(() => ctx.penpot.createVariantFromComponents([])).toThrow(); }); - test('removeProperty out of bounds is a no-op (not rejected)', async (ctx) => { + test('removeProperty out of bounds throws', async (ctx) => { const vc = await variantComponent(ctx); const v = vc.variants; expect(v).not.toBeNull(); if (v) { - expect(() => v.removeProperty(999)).not.toThrow(); + expect(() => v.removeProperty(999)).toThrow(); } }); - test('renameProperty out of bounds is a no-op (not rejected)', async (ctx) => { + test('renameProperty out of bounds throws', async (ctx) => { const vc = await variantComponent(ctx); const v = vc.variants; expect(v).not.toBeNull(); if (v) { - expect(() => v.renameProperty(999, 'Nope')).not.toThrow(); + expect(() => v.renameProperty(999, 'Nope')).toThrow(); } }); - test('setVariantProperty out of bounds is a no-op (not rejected)', async (ctx) => { + test('setVariantProperty out of bounds throws', async (ctx) => { const vc = await variantComponent(ctx); - expect(() => vc.setVariantProperty(999, 'large')).not.toThrow(); + expect(() => vc.setVariantProperty(999, 'large')).toThrow(); }); }); 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 0745629625..2680c3991e 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 @@ -39,6 +39,7 @@ describe('Ruler guides', () => { expect(guide.orientation).toBe('vertical'); // A board-attached ruler guide exposes its board. void guide.board; + guide.remove(); }); test('board ruler guide can be reassigned to another board', (ctx) => { @@ -47,17 +48,20 @@ describe('Ruler guides', () => { ctx.board.appendChild(other); guide.board = other; expect(guide.board && guide.board.id).toBe(other.id); + guide.remove(); }); test('board ruler guide position round-trips', (ctx) => { const guide = ctx.board.addRulerGuide('vertical', 50); guide.position = 60; expect(guide.position).toBeCloseTo(60, 0); + guide.remove(); }); test('board lists its ruler guides', (ctx) => { - ctx.board.addRulerGuide('horizontal', 30); + const guide = ctx.board.addRulerGuide('horizontal', 30); expect(ctx.board.rulerGuides.length).toBeGreaterThan(0); + guide.remove(); }); test('board removeRulerGuide removes a guide', (ctx) => { diff --git a/plugins/apps/plugin-api-test-suite/src/tests/wait.ts b/plugins/apps/plugin-api-test-suite/src/tests/wait.ts new file mode 100644 index 0000000000..36eecc2bb4 --- /dev/null +++ b/plugins/apps/plugin-api-test-suite/src/tests/wait.ts @@ -0,0 +1,22 @@ +// Shared async wait helpers. Not a `*.test.ts`, so the runner's glob doesn't +// pick it up as a test file; it's only imported by the tests that need it. + +/** + * Polls `predicate` until it returns truthy or the timeout elapses. Prefer this + * over a fixed `sleep` whenever there is an observable post-condition to wait + * for: it settles as soon as the condition holds (faster) and tolerates a slow + * backend (less flaky). It does not throw on timeout — the test's own assertion + * reports the failure. + */ +export async function waitFor( + predicate: () => boolean, + { + timeout = 2000, + interval = 50, + }: { timeout?: number; interval?: number } = {}, +): Promise { + const start = Date.now(); + while (!predicate() && Date.now() - start < timeout) { + await new Promise((resolve) => setTimeout(resolve, interval)); + } +} diff --git a/plugins/apps/plugin-api-test-suite/src/ui.ts b/plugins/apps/plugin-api-test-suite/src/ui.ts index 64dc79b486..4aacfd1a8e 100644 --- a/plugins/apps/plugin-api-test-suite/src/ui.ts +++ b/plugins/apps/plugin-api-test-suite/src/ui.ts @@ -16,6 +16,7 @@ const results = new Map(); const selected = new Set(); const expandedGroups = new Set(); let running = false; +let stopping = false; let reloading = false; let statusText = ''; @@ -160,12 +161,24 @@ function renderHeader(): HTMLElement { } function renderToolbar(): HTMLElement { - const runAll = el('button', { - textContent: 'Run all', - disabled: running || tests.length === 0, - }); - runAll.dataset.appearance = 'primary'; - runAll.addEventListener('click', () => run('all')); + // While a run is in progress the primary action becomes Stop; otherwise it + // runs all tests. + const primary = running + ? el('button', { + textContent: 'Stop', + disabled: stopping, + }) + : el('button', { + textContent: 'Run all', + disabled: tests.length === 0, + }); + primary.dataset.appearance = 'primary'; + if (running) { + primary.dataset.variant = 'destructive'; + primary.addEventListener('click', () => stop()); + } else { + primary.addEventListener('click', () => run('all')); + } const runSelected = el('button', { textContent: 'Run selected', @@ -187,7 +200,7 @@ function renderToolbar(): HTMLElement { reload.addEventListener('click', () => reloadTests()); const toolbar = el('div', { className: 'toolbar' }, [ - runAll, + primary, runSelected, reload, ]); @@ -471,6 +484,8 @@ function renderCoverage(): HTMLElement { function run(ids: string[] | 'all') { if (running) return; running = true; + stopping = false; + statusText = ''; const targetIds = ids === 'all' ? tests.map((t) => t.id) : ids; for (const id of targetIds) { @@ -489,6 +504,19 @@ function run(ids: string[] | 'all') { sendToPlugin({ type: 'run', ids }); } +/** + * Asks the sandbox to stop after the current test. The run keeps going until the + * in-flight test settles, at which point a `runComplete` arrives and clears the + * running state. + */ +function stop() { + if (!running || stopping) return; + stopping = true; + statusText = 'Stopping after the current test…'; + render(); + sendToPlugin({ type: 'stop' }); +} + async function reloadTests() { if (running || reloading) return; reloading = true; @@ -537,7 +565,16 @@ window.addEventListener('message', (event: MessageEvent) => { break; case 'runComplete': running = false; + stopping = false; lastCoverage = message.coverage; + // A stopped run leaves queued tests marked `running`; drop those back to + // pending so the list doesn't show spinners forever. + for (const [id, r] of [...results]) { + if (r.status === 'running') results.delete(id); + } + statusText = message.stopped + ? `Stopped · ${message.summary.passed + message.summary.failed} test(s) run` + : ''; render(); break; case 'reloaded': diff --git a/plugins/libs/plugin-types/index.d.ts b/plugins/libs/plugin-types/index.d.ts index b95e78c9c3..6150e6b136 100644 --- a/plugins/libs/plugin-types/index.d.ts +++ b/plugins/libs/plugin-types/index.d.ts @@ -1654,6 +1654,46 @@ export interface File extends PluginData { * Requires the `content:write` permission. */ saveVersion(label: string): Promise; + + /** + * Runs the referential-integrity validation on the file and returns the list + * of errors found. An empty array means the file is valid. Useful to detect + * inconsistencies (dangling references, broken components/variants, …) that + * the backend would otherwise reject when the file is saved. + * + * @example + * ```js + * const errors = file.validate(); + * if (errors.length) console.error(errors); + * ``` + */ + validate(): FileValidationError[]; +} + +/** + * A single referential-integrity error reported by {@link File.validate}. + */ +export interface FileValidationError { + /** + * The validation error code (e.g. `'variant-component-bad-name'`, + * `'child-not-found'`). + */ + readonly code: string; + + /** + * A human-readable description of the error. + */ + readonly hint: string; + + /** + * The id of the offending shape, when the error is attached to one. + */ + readonly shapeId: string | null; + + /** + * The id of the page the offending shape lives in, when applicable. + */ + readonly pageId: string | null; } /** @@ -3102,6 +3142,20 @@ export interface Page extends PluginData { */ readonly root: Shape; + /** + * Removes the page from the file. The last remaining page of the file + * cannot be removed. If the removed page is the active one, another page + * is activated. + * Requires `content:write` permission. + * + * @example + * ```js + * const page = penpot.createPage(); + * page.remove(); + * ``` + */ + remove(): void; + /** * Retrieves a shape by its unique identifier. * @param id The unique identifier of the shape. @@ -3514,6 +3568,11 @@ export interface RulerGuide { * If the guide is attached to a board this will retrieve the board shape */ board?: Board; + + /** + * Removes the guide from its page. + */ + remove(): void; } /** @@ -3881,6 +3940,14 @@ export interface ShapeBase extends PluginData { */ swapComponent(component: LibraryComponent): void; + /** + * Resets the overrides of the component copy, restoring all its attributes + * (and those of its children) to the ones in the linked main component. + * Similar to the "reset overrides" action on the Penpot interface. + * The current shape must be a component copy instance. + */ + resetOverrides(): void; + /** * Switch a VariantComponent copy to the nearest one that has the specified property value * @param pos The position of the property to update