diff --git a/.agents/skills/create-commit/SKILL.md b/.agents/skills/create-commit/SKILL.md index bf0667ca65..dd85191fd9 100644 --- a/.agents/skills/create-commit/SKILL.md +++ b/.agents/skills/create-commit/SKILL.md @@ -23,10 +23,11 @@ subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly. ## Iron Rules (non-negotiable) -1. **Wrap every body line at 76 characters or fewer.** Count characters, do +1. **Wrap every body line at 80 characters or fewer.** Count characters, do not eyeball. Exceptions: `Signed-off-by:` / `AI-assisted-by:` trailers and lines carrying a URL. This is the rule agents skip most often. -2. **Subject ≤70 chars**, imperative, capitalized, no trailing period. +2. **Subject ≤80 chars**, imperative, capitalized, no trailing period. + `Merge`/`Revert`/`Reapply` commits skip the length check. 3. **Blank line between subject and body.** 4. **Run `./scripts/check-commit` and require exit code 0.** It mechanically checks rules 1–3. A non-zero exit is a hard blocker: fix the message and @@ -41,7 +42,7 @@ subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly. that does not match the stated intent, **STOP** and tell the user before committing. 3. Draft the message following the format in the memory doc, wrapping the body - at 76 characters per line, and run: + at 80 characters per line, and run: ```bash git commit -m "" -m "" ``` diff --git a/.serena/memories/critical-info.md b/.serena/memories/critical-info.md index 64141eed15..1ea20e5c77 100644 --- a/.serena/memories/critical-info.md +++ b/.serena/memories/critical-info.md @@ -12,7 +12,7 @@ You are working on the GitHub project `penpot/penpot`, a monorepo. # Development workflow - Commit/PR/issue creation is **on explicit request only**. Before any of these actions, read the relevant memory — don't infer format from prior examples: - - Before `git commit` → `mem:workflow/creating-commits` (subject/body format, 76-char body wrapping enforced by `scripts/check-commit`, `AI-assisted-by: model-name` trailer) + - Before `git commit` → `mem:workflow/creating-commits` (subject/body format, 80-char body wrapping enforced by `scripts/check-commit`, `AI-assisted-by: model-name` trailer) - Before `gh issue create` → `mem:workflow/creating-issues` (title derivation, body template, labels, Issue Type) - Before `gh pr create` / `gh pr edit` → `mem:workflow/creating-prs` (title format, body structure, "Note:" line) - Before a repo-wide pnpm version update → `mem:workflow/updating-pnpm` (workspace layout, `scripts/sync-pnpm-version` flow, the stamp-missing-field and ignored-builds gotchas, verification steps) diff --git a/.serena/memories/workflow/creating-commits.md b/.serena/memories/workflow/creating-commits.md index a60234819f..678681d4ba 100644 --- a/.serena/memories/workflow/creating-commits.md +++ b/.serena/memories/workflow/creating-commits.md @@ -11,12 +11,11 @@ automatically pull the identity from the local git config `user.name` and `user. ## Message Format ``` -:emoji: Subject line (imperative, capitalized, no period, <=70 chars) +:emoji: Subject line (imperative, capitalized, no period, <=80 chars) Body explaining what changed and why. -Wrap lines at 76 characters — git log adds a -four-space indent, so 76 + 4 fits an 80-column -terminal. Keep each line concise. +Wrap lines at 80 characters. +Keep each line concise. AI-assisted-by: model-name ``` @@ -26,16 +25,17 @@ AI-assisted-by: model-name These rules are not advisory. Do not commit until every one holds. A commit that breaks them is wrong, even if the code is right. -- **Body lines MUST wrap at 76 characters or fewer.** Measure every line; do - not eyeball it. This is the rule most often skipped. Rationale: `git log` - indents the body four spaces, so 76 + 4 fits an 80-column terminal. -- **Subject MUST be ≤70 chars**, imperative, capitalized, no trailing period. +- **Body lines MUST wrap at 80 characters or fewer.** Measure every line; do + not eyeball it. This is the rule most often skipped. +- **Subject MUST be ≤80 chars**, imperative, capitalized, no trailing period. + Merge/Revert/Reapply commits skip the length check: git prepends those + prefixes to an existing subject. - **MUST be a blank line** between subject and body. - **MUST run `scripts/check-commit` and get exit code 0 before finishing.** It mechanically validates the rules above; a failing run is a blocker. - It checks `HEAD` by default: `./scripts/check-commit` - For another commit: `./scripts/check-commit -c ` -- **NEVER** hand-wave the body as "one long line". If a line exceeds 76, +- **NEVER** hand-wave the body as "one long line". If a line exceeds 80, break it at a space. - **NEVER add a `Co-authored-by:` trailer.** It is forbidden; `scripts/check-commit` fails the commit when any line matches it (case-insensitive). - CI runs `scripts/check-commit` on every new commit (`mem:workflow/creating-commits` is enforced by `.github/workflows/commit-checker.yml`), so a forbidden trailer fails the build. diff --git a/.serena/memories/workflow/creating-prs.md b/.serena/memories/workflow/creating-prs.md index f9b41275ef..4d9d0fe8d4 100644 --- a/.serena/memories/workflow/creating-prs.md +++ b/.serena/memories/workflow/creating-prs.md @@ -27,7 +27,7 @@ Always add the PR to the Main project (`--project "Main"`) unless the user expli PR titles follow commit title conventions: ``` -:emoji: Subject line (imperative, capitalized, no period, <=70 chars) +:emoji: Subject line (imperative, capitalized, no period, <=80 chars) ``` See `mem:workflow/creating-commits` for emoji codes. Squash merge uses the PR title as the final commit subject, so title format matters. diff --git a/AGENTS.md b/AGENTS.md index fa95b4c045..e900e32518 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,7 +17,8 @@ - **Read `AGENTS.local.md` after this file whenever it exists.** It carries the developer's steering for this project and overrides this file on prose style and workflow, but not on project policy, where instructions here prevail. -- **Commit message body lines MUST wrap at ≤76 chars** (subject ≤70 chars) and +- **Commit message body lines MUST wrap at ≤80 chars** (subject ≤80 chars, + except `Merge`/`Revert`/`Reapply` which skip the subject length check) and the commit MUST pass `./scripts/check-commit` with exit code 0 before you consider it done. This is mechanically checked — do not eyeball it. - **Read the workflow memory BEFORE the corresponding action**: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2d665b0715..9b23ae0fa9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -124,7 +124,7 @@ commit history — what matters is the [PR title](#title). Pull request titles **must** follow the same convention as commit subjects: ``` -:emoji: Subject line (imperative, capitalized, no period, <=70 chars) +:emoji: Subject line (imperative, capitalized, no period, <=80 chars) ``` Read [Creating Commits](./.serena/memories/workflow/creating-commits.md) @@ -220,8 +220,10 @@ Commit messages must follow this format: - Capitalize the first letter of the subject - Add clear and concise description on the body - Do not end the subject with a period -- Keep the subject to **70 characters** or fewer -- **Wrap body lines at 76 characters or fewer** (trailers and URLs excepted) +- Keep the subject to **80 characters** or fewer + (`Merge`/`Revert`/`Reapply` commits skip this limit, since git prepends + those prefixes to an existing subject) +- **Wrap body lines at 80 characters or fewer** (trailers and URLs excepted) - Separate the subject from the body with a **blank line** - Never add a `Co-authored-by` trailer — it is forbidden and fails `./scripts/check-commit` diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 2113f76ea1..b84faf4356 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -531,7 +531,8 @@ (-> (pcb/empty-changes it nil) (cll/generate-duplicate-component library component-id new-component-id))] (rx/of - (ptk/data-event :layout/update {:ids [(:id main-instance)]}) + (ptk/data-event :layout/update {:page-id (:main-instance-page (get-in library [:data :components component-id])) + :ids [(:id main-instance)]}) (dch/commit-changes changes))))))) (defn delete-component diff --git a/frontend/src/app/plugins/library.cljs b/frontend/src/app/plugins/library.cljs index 661e3d9b0d..555eea5559 100644 --- a/frontend/src/app/plugins/library.cljs +++ b/frontend/src/app/plugins/library.cljs @@ -19,6 +19,7 @@ [app.common.types.variant :as ctv] [app.common.uuid :as uuid] [app.main.data.plugins :as dp] + [app.main.data.workspace :as dw] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.texts :as dwt] [app.main.data.workspace.variants :as dwv] @@ -845,12 +846,81 @@ :remove (fn [] - (cond - (not (r/check-permission plugin-id "library:write")) - (u/not-valid plugin-id :remove "Plugin doesn't have 'library:write' permission") + (let [component (u/locate-library-component file-id id)] + (cond + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :remove "Plugin doesn't have 'library:write' permission") - :else - (st/emit! (dwl/delete-component {:id id})))) + (not= file-id (:current-file-id @st/state)) + (u/not-valid plugin-id :remove "Can only remove components from the current file's library") + + (nil? component) + (u/not-valid plugin-id :remove "Component does not exist") + + (not (:deleted component)) + (st/emit! (-> (dwl/delete-component {:id id}) + (se/add-event plugin-id)))))) + + :annotation + {:get (fn [] (:annotation (u/locate-library-component file-id id))) + :set + (fn [value] + (let [component (u/locate-library-component file-id id)] + (cond + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :annotation "Plugin doesn't have 'library:write' permission") + + (not= file-id (:current-file-id @st/state)) + (u/not-valid plugin-id :annotation "Can only edit annotations in the current file's library") + + (nil? component) + (u/not-valid plugin-id :annotation "Component does not exist") + + (:deleted component) + (u/not-valid plugin-id :annotation "Cannot edit a deleted component") + + (not (or (nil? value) (and (string? value) (<= (count value) 300)))) + (u/not-valid plugin-id :annotation "Expected null or a string of at most 300 characters") + + :else + (st/emit! (-> (dw/update-component-annotation id value) + (se/add-event plugin-id))))))} + + :duplicate + (fn [] + (let [component (u/locate-library-component file-id id)] + (cond + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :duplicate "Plugin doesn't have 'library:write' permission") + + (not= file-id (:current-file-id @st/state)) + (u/not-valid plugin-id :duplicate "Can only duplicate components in the current file's library") + + (nil? component) + (u/not-valid plugin-id :duplicate "Component does not exist") + + (:deleted component) + (u/not-valid plugin-id :duplicate "Cannot duplicate a deleted component") + + :else + (let [new-id (uuid/next)] + (st/emit! (-> (dwl/duplicate-component file-id id new-id) + (se/add-event plugin-id))) + (lib-component-proxy plugin-id file-id new-id))))) + + :restore + (fn [] + (let [component (u/locate-library-component file-id id)] + (cond + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :restore "Plugin doesn't have 'library:write' permission") + + (nil? component) + (u/not-valid plugin-id :restore "Component does not exist") + + (:deleted component) + (st/emit! (-> (dwl/restore-component file-id id) + (se/add-event plugin-id)))))) :instance (fn [] @@ -1080,6 +1150,16 @@ (map #(lib-component-proxy plugin-id file-id %)))] (apply array components)))} + :deletedComponents + {:get + (fn [] + (let [components (->> (u/locate-file file-id) + :data + :components + (filter (comp :deleted second)) + (map #(lib-component-proxy plugin-id file-id (first %))))] + (apply array components)))} + :tokens {:this true :get diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index 6be18bd4fa..b34b329598 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -1587,6 +1587,38 @@ target-id true))))) + :updateMain + (fn [] + (let [shape (u/locate-shape file-id page-id id) + objects (u/locate-objects file-id page-id) + component (when (:component-id shape) + (u/locate-library-component (:component-file shape) (:component-id shape)))] + (cond + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :updateMain "Plugin doesn't have 'content:write' permission") + + (not (r/check-permission plugin-id "library:write")) + (u/not-valid plugin-id :updateMain "Plugin doesn't have 'library:write' permission") + + (not= file-id (:current-file-id @st/state)) + (u/not-valid plugin-id :updateMain "Can only update a component from a copy in the current file") + + (not (u/page-active? page-id)) + (u/not-valid plugin-id :updateMain "Cannot modify a page that is not currently active") + + (not (and (ctk/instance-head? shape) (ctk/in-component-copy? shape))) + (u/not-valid plugin-id :updateMain "The shape is not a component copy head") + + (u/inside-component-copy? objects shape) + (u/not-valid plugin-id :updateMain "Cannot update the main from a shape inside another component copy") + + (or (nil? component) (:deleted component)) + (u/not-valid plugin-id :updateMain "The main component does not exist or is deleted") + + :else + (st/emit! (-> (dwl/update-component-sync id (:component-file shape)) + (se/add-event plugin-id)))))) + :resetOverrides (fn [] (let [shape (u/locate-shape file-id page-id id)] diff --git a/frontend/test/frontend_tests/plugins/library_test.cljs b/frontend/test/frontend_tests/plugins/library_test.cljs index 25122a9104..8edd8a0e30 100644 --- a/frontend/test/frontend_tests/plugins/library_test.cljs +++ b/frontend/test/frontend_tests/plugins/library_test.cljs @@ -6,6 +6,12 @@ (ns frontend-tests.plugins.library-test (:require + [app.common.files.changes-builder :as pcb] + [app.common.logic.shapes :as cls] + [app.common.test-helpers.components :as cthc] + [app.common.test-helpers.files :as cthf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as cths] [app.common.types.component :as ctk] [app.common.uuid :as uuid] [app.main.data.event :as ev] @@ -14,8 +20,10 @@ [app.main.data.workspace.variants :as dwv] [app.main.repo :as rp] [app.main.store :as st] + [app.main.worker :as mw] [app.plugins.library :as library] [app.plugins.register :as r] + [app.plugins.shape :as shape] [app.plugins.text :as text] [app.plugins.utils :as u] [app.util.object :as obj] @@ -24,10 +32,163 @@ [cuerdas.core :as str] [frontend-tests.helpers.async :as async] [frontend-tests.helpers.mock :as mock] + [frontend-tests.helpers.state :as ths] [potok.v2.core :as ptk])) (def ^:private plugin-id "00000000-0000-0000-0000-000000000000") +(defn- file-with-component + [] + (-> (cthf/sample-file :restore-file :page-label :main-page) + (cths/add-sample-shape :main :type :frame) + (cths/add-sample-shape :child :parent-label :main) + (cthc/make-component :restorable :main) + (cthc/instantiate-component :restorable :copy))) + +(defn- file-with-deleted-component + [] + (let [file (file-with-component) + page (cthf/current-page file) + changes (-> (pcb/empty-changes nil (:id page)) + (cls/generate-delete-shapes (:data file) page (:objects page) #{(thi/id :main)} {}) + second)] + (cthf/apply-changes file changes))) + +(t/deftest ^:async component-restore-preserves-the-users-view + (doseq [scenario [:same-page :other-page :linked-library]] + (let [file (cond-> (file-with-deleted-component) + (= scenario :other-page) (cthf/add-sample-page :other-page)) + store (if (= scenario :linked-library) + (ths/setup-store (cthf/sample-file :local-file) + {:renderer :svg :libraries [file]}) + (ths/setup-store file {:renderer :svg})) + view {:selected #{(thi/id :copy)} + :zoom 2 + :vport {:x 100 :y 200 :width 800 :height 600} + :vbox {:x 50 :y 100 :width 400 :height 300}} + view-of #(select-keys % [:current-file-id :current-page-id :workspace-local])] + (ptk/emit! store #(assoc % :workspace-local view)) + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + (let [before (view-of @store)] + (await + (mock/with-mocks* + {st/state store + st/emit! (mock/stub (fn [& events] (apply ptk/emit! store events))) + mw/ask! (mock/stub (fn [_] (rx/empty))) + shape/lib-component-proxy library/lib-component-proxy} + (let [copy (shape/shape-proxy plugin-id (:id file) (thi/id :main-page) (thi/id :copy)) + component (.component copy)] + (t/is (true? (get-in @store [:files (:id file) :data :components (thi/id :restorable) :deleted]))) + (.restore component) + (await (async/wait-for #(not (get-in @store [:files (:id file) :data :components (thi/id :restorable) :deleted])) + "component restored")) + (.restore component) + (await (async/settle)) + (t/is (= (thi/id :main) + (get-in @store [:files (:id file) :data :pages-index (thi/id :main-page) :objects (thi/id :main) :id]))) + (t/is (= (thi/id :child) + (get-in @store [:files (:id file) :data :pages-index (thi/id :main-page) :objects (thi/id :child) :id]))) + (t/is (= before (view-of @store))) + (t/is (= [(str (thi/id :restorable))] + (mapv #(.-id %) (.-components (library/library-proxy plugin-id (:id file))))))))))))) + +(t/deftest ^:async component-restore-checks-library-write-permission + (let [file (file-with-deleted-component) + store (ths/setup-store file {:renderer :svg}) + events (atom [])] + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + (await + (mock/with-mocks* + {st/state store + st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + r/check-permission (mock/stub (constantly false))} + (let [component (library/lib-component-proxy plugin-id (:id file) (thi/id :restorable))] + (t/is (thrown-with-msg? js/Error #"library:write" (.restore component))) + (await (async/settle)) + (t/is (empty? @events)) + (t/is (true? (get-in @store [:files (:id file) :data :components (thi/id :restorable) :deleted])))))))) + +(t/deftest ^:async component-restore-rejects-a-missing-component + (let [file (cthf/sample-file :restore-file) + store (ths/setup-store file {:renderer :svg})] + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + (await + (mock/with-mocks* + {st/state store} + (let [component (library/lib-component-proxy plugin-id (:id file) (uuid/next))] + (t/is (thrown-with-msg? js/Error #"Component does not exist" (.restore component)))))))) + +(def ^:private component-mutations + [["remove" #(.remove %)] + ["duplicate" #(.duplicate %)] + ["annotation" #(set! (.-annotation %) "Usage")]]) + +(t/deftest ^:async component-mutations-check-library-write-permission + (let [file (file-with-component) + store (ths/setup-store file {:renderer :svg}) + before @store + events (atom [])] + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + (await + (mock/with-mocks* + {st/state store + st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + r/check-permission (mock/stub (constantly false))} + (let [component (library/lib-component-proxy plugin-id (:id file) (thi/id :restorable))] + (doseq [[operation mutate] component-mutations] + (t/testing operation + (t/is (thrown-with-msg? js/Error #"library:write" (mutate component))))) + (await (async/settle)) + (t/is (empty? @events)) + (t/is (= (:files before) (:files @store)))))))) + +(t/deftest ^:async component-mutations-reject-linked-libraries + (let [library (file-with-component) + file (cthf/sample-file :consumer-file) + store (ths/setup-store file {:renderer :svg :libraries [library]}) + before @store + events (atom [])] + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + (await + (mock/with-mocks* + {st/state store + st/emit! (mock/stub (fn [& emitted] (swap! events into emitted)))} + (let [component (library/lib-component-proxy plugin-id (:id library) (thi/id :restorable))] + (doseq [[operation mutate] component-mutations] + (t/testing operation + (t/is (thrown-with-msg? js/Error #"current file" (mutate component))))) + (await (async/settle)) + (t/is (empty? @events)) + (t/is (= (:files before) (:files @store)))))))) + +(t/deftest ^:async component-mutations-reject-missing-components + (let [file (cthf/sample-file :restore-file) + store (ths/setup-store file {:renderer :svg})] + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + (await + (mock/with-mocks* + {st/state store} + (let [component (library/lib-component-proxy plugin-id (:id file) (uuid/next))] + (doseq [[operation mutate] component-mutations] + (t/testing operation + (t/is (thrown-with-msg? js/Error #"Component does not exist" (mutate component)))))))))) + +(t/deftest ^:async update-main-checks-content-and-library-write-permissions + (doseq [denied ["content:write" "library:write"]] + (let [file (file-with-component) + store (ths/setup-store file {:renderer :svg}) + events (atom [])] + (ptk/emit! store #(assoc-in % [:plugins :flags plugin-id :throw-validation-errors] true)) + (await + (mock/with-mocks* + {st/state store + st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + r/check-permission (mock/stub (fn [_ permission] (not= permission denied)))} + (let [copy (shape/shape-proxy plugin-id (:id file) (thi/id :main-page) (thi/id :copy))] + (t/is (thrown-with-msg? js/Error (re-pattern denied) (.updateMain copy))) + (await (async/settle)) + (t/is (empty? @events)))))))) + (t/deftest library-asset-proxies-expose-library-id (let [file-id (random-uuid) id (random-uuid)] diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index a6bbcde71f..33623eea15 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -4,6 +4,8 @@ - **plugin-types, plugins-runtime:** `penpotMgmt` returns `Team`, `Project` and `ProjectFile` objects. Assigning `name` renames teams, projects and files, `project.pinned` pins a project and `file.shared` shares a file as a team library; `createProject`, `createFile`, `moveTo`, `duplicate`, `remove` and `restore` live on the objects. Added `penpotMgmt.listTeams`, `penpotMgmt.createTeam`, `penpotMgmt.getFile`, `listDeletedProjects` and `listDeletedFiles`, and the `manage:projects`, `manage:teams` and `manage:delete` permissions. Restoration uses the dashboard's Trash operation; projects need recoverable files and empty projects cannot be restored; the API exposes no permanent deletion method. Team deletion is not exposed. - **plugins-runtime, plugin-types**: Added global plugins (`scope: "global"`) that remain active across dashboard and workspace navigation until closed or logged out, with `penpotMgmt` for project and file discovery, file navigation and workspace events. Global scope adds the `allow:global` permission, which users must accept when installing the plugin. +- **plugin-types:** Added `LibraryComponent.restore()` to restore a deleted component and its main instance without changing the current page, selection, zoom, or viewport. Requires `library:write` permission. +- **plugin-types:** Added `Shape.updateMain()`, `LibraryComponent.duplicate()`, writable component `annotation`, and `Library.deletedComponents`. Declared the existing `LibraryComponent.remove()` method. - **plugin-types:** Added `paddingType` (`'simple' | 'multiple'`) to flex and grid layouts and `marginType` (`'simple' | 'multiple'`) to layout children, exposing whether the four padding/margin sides are mirrored or honoured independently. - **plugin-types**: Added `waitForLayoutUpdate` to wait until pending layout updates have finished. It rejects when the optional timeout elapses, defaulting to 30 seconds so a wait never hangs. - **plugin-types**: Added `waitForLayoutUpdate` to the `Shape` interface to wait until the pending layout updates of a shape and its children have finished @@ -14,6 +16,7 @@ - **plugin-types, plugin-runtime:** Dashboard property setters now update values after backend success and log failures without interrupting the workspace. File moves reject projects in other teams, backend rejections include their type and code, and `project.fileCount` stays at its listing snapshot. Duplicated projects include their file count and pinned state. - **plugin-runtime:** Cancel plugin loads before they execute after unloading or logout, and allow reopening plugins that close during startup. - **plugins-runtime**: `library.connectLibrary()` now resolves once the library's assets are loaded; it used to resolve with an empty library the first time. `library.connected` and `library.availableLibraries()` ignore files opened earlier in the same tab, which used to show up as connected and be left out of the available libraries. +- **plugins-runtime:** `LibraryComponent.remove()` now rejects linked-library and missing component handles, and does nothing for an already deleted component. - **plugins-runtime**: An interaction obtained from `Shape.interactions` now keeps addressing that interaction instead of the position it held when the array was read. Removing every interaction of a shape from a single read removes all of them rather than leaving some behind, and writing through a held interaction after an earlier one is removed no longer lands on a different interaction. - **plugins-runtime**: `Shape.removeInteraction()` now rejects an interaction belonging to a different shape with a validation error, instead of removing whichever interaction sat at the same position on the target shape. - **plugins-runtime**: Writing `trigger`, `delay` or `action` on an interaction the shape no longer has now raises a validation error. The write used to be sent to the workspace with no position to apply it at, where it failed out of the plugin's reach: nothing was written and nothing was reported. 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 1025e9d764..31358b874b 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 @@ -275,6 +275,7 @@ "createColor", "createComponent", "createTypography", + "deletedComponents", "id", "name", "tokens", @@ -289,9 +290,13 @@ "opacity" ], "LibraryComponent": [ + "annotation", + "duplicate", "instance", "isVariant", "mainInstance", + "remove", + "restore", "transformInVariant" ], "LibraryContext": [ @@ -491,6 +496,7 @@ "swapComponent", "switchVariant", "tokens", + "updateMain", "visible", "waitForLayoutUpdate", "width", @@ -1171,6 +1177,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -1677,6 +1689,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -2827,6 +2845,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -4087,6 +4111,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -4677,6 +4707,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -5059,6 +5095,12 @@ "type": "LibraryComponent", "array": true }, + "deletedComponents": { + "decl": "Library", + "kind": "get", + "type": "LibraryComponent", + "array": true + }, "tokens": { "decl": "Library", "kind": "get", @@ -5219,6 +5261,30 @@ } }, "LibraryComponent": { + "annotation": { + "decl": "LibraryComponent", + "kind": "getset", + "type": null, + "array": false + }, + "duplicate": { + "decl": "LibraryComponent", + "kind": "method", + "type": "LibraryComponent", + "array": false + }, + "remove": { + "decl": "LibraryComponent", + "kind": "method", + "type": null, + "array": false + }, + "restore": { + "decl": "LibraryComponent", + "kind": "method", + "type": null, + "array": false + }, "instance": { "decl": "LibraryComponent", "kind": "method", @@ -5589,6 +5655,30 @@ "type": null, "array": false }, + "annotation": { + "decl": "LibraryComponent", + "kind": "getset", + "type": null, + "array": false + }, + "duplicate": { + "decl": "LibraryComponent", + "kind": "method", + "type": "LibraryComponent", + "array": false + }, + "remove": { + "decl": "LibraryComponent", + "kind": "method", + "type": null, + "array": false + }, + "restore": { + "decl": "LibraryComponent", + "kind": "method", + "type": null, + "array": false + }, "instance": { "decl": "LibraryComponent", "kind": "method", @@ -6311,6 +6401,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -7333,6 +7429,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -7867,6 +7969,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -8467,6 +8575,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -9077,6 +9191,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", @@ -11387,6 +11507,12 @@ "type": null, "array": false }, + "updateMain": { + "decl": "ShapeBase", + "kind": "method", + "type": null, + "array": false + }, "switchVariant": { "decl": "ShapeBase", "kind": "method", 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 ecdd6c5118..92d68fd6aa 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 @@ -199,6 +199,66 @@ describe('Component instances', () => { } }); + test('restore restores a deleted component without changing the view', async (ctx) => { + const originalPage = ctx.penpot.currentPage; + expect(originalPage).not.toBeNull(); + if (!originalPage) return; + + const comp = makeComponent(ctx); + const main = comp.mainInstance() as Board; + const mainId = main.id; + const childIds = main.children.map((child) => child.id); + const inst = comp.instance(); + ctx.board.appendChild(inst); + comp.remove(); + await ctx.penpot.waitForLayoutUpdate(); + expect( + ctx.penpot.library.local.components.some((c) => c.id === comp.id), + ).toBe(false); + expect(originalPage.getShapeById(mainId)).toBeNull(); + + const otherPage = ctx.penpot.createPage(); + try { + await ctx.penpot.openPage(otherPage); + const selected = ctx.penpot.createRectangle(); + (otherPage.root as Board).appendChild(selected); + ctx.penpot.selection = [selected]; + ctx.penpot.viewport.zoom = 2; + ctx.penpot.viewport.center = { x: 300, y: 200 }; + await ctx.penpot.waitForLayoutUpdate(); + + const zoom = ctx.penpot.viewport.zoom; + const center = ctx.penpot.viewport.center; + const deleted = inst.component(); + expect(deleted).not.toBeNull(); + if (!deleted) return; + + deleted.restore(); + await waitFor(() => + ctx.penpot.library.local.components.some((c) => c.id === comp.id), + ); + await ctx.penpot.waitForLayoutUpdate(); + + expect( + ctx.penpot.library.local.components.some((c) => c.id === comp.id), + ).toBe(true); + expect(originalPage.getShapeById(mainId)?.id).toBe(mainId); + expect( + (deleted.mainInstance() as Board).children.map((child) => child.id), + ).toEqual(childIds); + expect(ctx.penpot.currentPage?.id).toBe(otherPage.id); + expect(ctx.penpot.selection.map((shape) => shape.id)).toEqual([ + selected.id, + ]); + expect(ctx.penpot.viewport.zoom).toBeCloseTo(zoom); + expect(ctx.penpot.viewport.center.x).toBeCloseTo(center.x); + expect(ctx.penpot.viewport.center.y).toBeCloseTo(center.y); + } finally { + await ctx.penpot.openPage(originalPage); + otherPage.remove(); + } + }); + test('detach turns an instance into a basic shape', (ctx) => { const inst = instanceOf(ctx); inst.detach(); @@ -319,3 +379,177 @@ describe('Component instances', () => { expect(inst.isComponentInstance()).toBeFalsy(); }); }); + +describe('Component assets', () => { + test('duplicate creates an independent component with its annotation', async (ctx) => { + const source = makeComponent(ctx); + source.annotation = 'Button usage'; + const original = source.mainInstance() as Board; + const duplicate = source.duplicate(); + await ctx.penpot.waitForLayoutUpdate(); + const main = duplicate.mainInstance() as Board; + ctx.board.appendChild(main); + + expect(duplicate.id).not.toBe(source.id); + expect(main.id).not.toBe(original.id); + expect(main.isComponentMainInstance()).toBe(true); + expect(main.component()?.id).toBe(duplicate.id); + expect(main.children).toHaveLength(original.children.length); + expect(main.children[0].id).not.toBe(original.children[0].id); + expect(duplicate.annotation).toBe('Button usage'); + duplicate.name = 'Duplicate button'; + expect(source.name).not.toBe(duplicate.name); + }); + + test('annotations can be created, edited, and removed', async (ctx) => { + const comp = makeComponent(ctx); + expect(comp.annotation).toBeNull(); + comp.annotation = 'Use for primary actions'; + expect(comp.annotation).toBe('Use for primary actions'); + comp.annotation = 'Use for secondary actions'; + expect(comp.annotation).toBe('Use for secondary actions'); + expect(() => { + comp.annotation = 42 as unknown as string; + }).toThrow(); + expect(() => { + comp.annotation = 'x'.repeat(301); + }).toThrow(); + expect(comp.annotation).toBe('Use for secondary actions'); + comp.annotation = null; + await ctx.penpot.waitForLayoutUpdate(); + expect(comp.annotation).toBeNull(); + }); + + test('deletedComponents lists and restores a component without surviving copies', async (ctx) => { + const comp = makeComponent(ctx); + const mainId = comp.mainInstance().id; + const page = ctx.penpot.currentPage; + comp.remove(); + await ctx.penpot.waitForLayoutUpdate(); + expect(page?.getShapeById(mainId)).toBeNull(); + expect( + ctx.penpot.library.local.components.some((c) => c.id === comp.id), + ).toBe(false); + + const deleted = ctx.penpot.library.local.deletedComponents.find( + (c) => c.id === comp.id, + ); + expect(deleted).toBeDefined(); + if (!deleted) return; + deleted.remove(); + deleted.restore(); + await ctx.penpot.waitForLayoutUpdate(); + expect(page?.getShapeById(mainId)?.id).toBe(mainId); + expect( + ctx.penpot.library.local.components.some((c) => c.id === comp.id), + ).toBe(true); + expect( + ctx.penpot.library.local.deletedComponents.some((c) => c.id === comp.id), + ).toBe(false); + }); + + test('updateMain applies a copy override to the main and other copies', async (ctx) => { + const childColor = (board: Board) => { + const fill = board.children[0].fills[0]; + return typeof fill === 'string' ? fill : fill?.fillColor; + }; + const comp = makeComponent(ctx); + const main = comp.mainInstance() as Board; + const copy = comp.instance() as Board; + const other = comp.instance() as Board; + ctx.board.appendChild(copy); + ctx.board.appendChild(other); + copy.children[0].fills = [{ fillColor: '#ff0000', fillOpacity: 1 }]; + copy.updateMain(); + await ctx.penpot.waitForLayoutUpdate(); + await waitFor(() => childColor(main) === '#ff0000'); + expect(childColor(main)).toBe('#ff0000'); + expect(childColor(other)).toBe('#ff0000'); + + main.children[0].fills = [{ fillColor: '#00ff00', fillOpacity: 1 }]; + await waitFor(() => childColor(copy) === '#00ff00'); + expect(childColor(copy)).toBe('#00ff00'); + }); + + test('updateMain rejects plain shapes, main instances, and copy children', (ctx) => { + const rect = ctx.penpot.createRectangle(); + ctx.board.appendChild(rect); + expect(() => rect.updateMain()).toThrow(); + const comp = makeComponent(ctx); + expect(() => comp.mainInstance().updateMain()).toThrow(); + const copy = comp.instance() as Board; + ctx.board.appendChild(copy); + expect(() => copy.children[0].updateMain()).toThrow(); + }); + + test('deleted components reject duplication, annotation edits, and updateMain', async (ctx) => { + const comp = makeComponent(ctx); + const copy = comp.instance(); + ctx.board.appendChild(copy); + comp.remove(); + await ctx.penpot.waitForLayoutUpdate(); + expect(() => comp.duplicate()).toThrow('deleted component'); + expect(() => { + comp.annotation = 'Usage'; + }).toThrow('deleted component'); + expect(() => copy.updateMain()).toThrow('main component'); + }); + + test('updateMain rejects a copy on an inactive page and inside another copy', async (ctx) => { + const main = nestedComponentMain(ctx); + const outerCopy = main.component()?.instance() as Board; + ctx.board.appendChild(outerCopy); + expect(() => outerCopy.children[0].updateMain()).toThrow( + 'inside another component copy', + ); + + const original = ctx.penpot.currentPage; + expect(original).not.toBeNull(); + if (!original) return; + const page = ctx.penpot.createPage(); + try { + await ctx.penpot.openPage(page); + expect(() => outerCopy.updateMain()).toThrow('not currently active'); + } finally { + await ctx.penpot.openPage(original); + page.remove(); + } + }); + + test('component assets can be edited without activating their main instance page', async (ctx) => { + const original = ctx.penpot.currentPage; + expect(original).not.toBeNull(); + if (!original) return; + const comp = makeComponent(ctx); + const mainId = comp.mainInstance().id; + const page = ctx.penpot.createPage(); + try { + await ctx.penpot.openPage(page); + const selected = ctx.penpot.createRectangle(); + (page.root as Board).appendChild(selected); + ctx.penpot.selection = [selected]; + const zoom = ctx.penpot.viewport.zoom; + const center = ctx.penpot.viewport.center; + + comp.annotation = 'Edited from another page'; + const duplicate = comp.duplicate(); + await ctx.penpot.waitForLayoutUpdate(); + expect(original.getShapeById(duplicate.mainInstance().id)).not.toBeNull(); + expect(comp.annotation).toBe('Edited from another page'); + duplicate.remove(); + comp.remove(); + await ctx.penpot.waitForLayoutUpdate(); + expect(original.getShapeById(mainId)).toBeNull(); + expect(ctx.penpot.currentPage?.id).toBe(page.id); + expect(ctx.penpot.selection.map((shape) => shape.id)).toEqual([ + selected.id, + ]); + expect(ctx.penpot.viewport.zoom).toBeCloseTo(zoom); + expect(ctx.penpot.viewport.center.x).toBeCloseTo(center.x); + expect(ctx.penpot.viewport.center.y).toBeCloseTo(center.y); + } finally { + await ctx.penpot.openPage(original); + page.remove(); + } + }); +}); diff --git a/plugins/libs/plugin-types/index.d.ts b/plugins/libs/plugin-types/index.d.ts index f1e1d68e53..6369de533c 100644 --- a/plugins/libs/plugin-types/index.d.ts +++ b/plugins/libs/plugin-types/index.d.ts @@ -2937,6 +2937,12 @@ export interface Library extends PluginData { */ readonly components: LibraryComponent[]; + /** + * Deleted components, including individual deleted variants. + * Use a component's `restore()` method to restore it. + */ + readonly deletedComponents: LibraryComponent[]; + /** * A catalog of Design Tokens in the library. * @@ -3032,6 +3038,39 @@ export interface LibraryColor extends LibraryElement { * This interface extends `LibraryElement` and includes properties specific to component elements. */ export interface LibraryComponent extends LibraryElement { + /** + * The component annotation, or null when none is set. + * Set null to delete it. Text can contain at most 300 characters. + * Writing requires `library:write` permission and a live component in the + * current file's library. + */ + annotation: string | null; + + /** + * Duplicates this component and its main instance in the current file's library. + * Requires `library:write` permission. The component must not be deleted. + */ + duplicate(): LibraryComponent; + + /** + * Deletes this component and its main instance from the current file's library. + * Requires `library:write` permission. Does nothing if already deleted. + * Surviving copies keep their link so the component can be restored later. + */ + remove(): void; + + /** + * Restores this component if it has been deleted, including its main instance. + * Leaves the current page, selection, zoom, and viewport unchanged. + * Requires `library:write` permission. Does nothing if already restored. + * + * @example + * ```js + * penpot.selection[0].component()?.restore(); + * ``` + */ + restore(): void; + /** * Creates an instance of the component. * @return Returns a `Shape` object representing the instance of the component. @@ -4249,6 +4288,15 @@ export interface ShapeBase extends PluginData { */ resetOverrides(): void; + /** + * Applies this copy's overrides to its main component and synchronizes copies. + * Similar to the editor's "update main component" action. + * Requires `content:write` and `library:write` permissions. + * The shape must be a component copy head on the active page of the current file, + * outside any other component copy, with a main component that is not deleted. + */ + updateMain(): void; + /** * Switch a VariantComponent copy to the nearest one that has the specified property value * @param pos The position of the property to update diff --git a/scripts/check-commit b/scripts/check-commit index d0e9f59899..df4672ea59 100755 --- a/scripts/check-commit +++ b/scripts/check-commit @@ -5,7 +5,7 @@ Check commit messages against Penpot's commit guidelines. Validates commit messages using the rules defined in: - .github/workflows/commit-checker.yml (regex pattern) - CONTRIBUTING.md (formatting rules, subject length, DCO) - - .serena/memories/workflow/creating-commits.md (body wrapped at 76 chars) + - .serena/memories/workflow/creating-commits.md (body wrapped at 80 chars) By default, checks HEAD. Use --commit to specify a different commit. @@ -39,12 +39,17 @@ COMMIT_PATTERN = re.compile( MERGE_PATTERN = re.compile(r"^(Merge|Revert|Reapply).+[^.]$") +# ── Subject line length ────────────────────────────────────────────────────── +# Subject lines must fit in 80 characters. Merge/Revert/Reapply commits are +# exempt: git prepends "Merge ..."/"Revert ..." to an existing subject, so +# enforcing the limit would fail on text the author did not write. +MAX_SUBJECT_LINE = 80 + # ── Body line wrapping ─────────────────────────────────────────────────────── -# Commit bodies must wrap at 76 characters (see -# .serena/memories/workflow/creating-commits.md). That leaves room for the -# four-space indent git log adds, fitting an 80-column terminal. Trailers and +# Commit bodies must wrap at 80 characters (see +# .serena/memories/workflow/creating-commits.md). Trailers and # URLs are exempt: they cannot be wrapped without losing meaning. -MAX_BODY_LINE = 76 +MAX_BODY_LINE = 80 TRAILER_PATTERN = re.compile( r"^(Signed-off-by|Co-developed-by|Reviewed-by|" @@ -110,11 +115,13 @@ def check_regex(message): def check_subject_length(message): - """Subject line must be ≤ 70 characters.""" + """Subject line must be ≤ 80 characters (Merge/Revert/Reapply exempt).""" first_line = message.split("\n")[0] - if len(first_line) > 70: + if MERGE_PATTERN.match(first_line): + return True, None + if len(first_line) > MAX_SUBJECT_LINE: return False, ( - f"Subject line exceeds 70 characters ({len(first_line)} chars):\n" + f"Subject line exceeds {MAX_SUBJECT_LINE} characters ({len(first_line)} chars):\n" f" {first_line}" ) return True, None @@ -166,7 +173,7 @@ def check_body_blank_line(message): def check_body_line_length(message): - """Body lines must wrap at 76 characters or fewer. + """Body lines must wrap at 80 characters or fewer. The subject (first line) has its own length rule. Trailers (e.g. Signed-off-by) and lines carrying a URL are exempt, since wrapping them @@ -250,11 +257,11 @@ def main(): validators = [ ("Regex pattern", check_regex), - ("Subject ≤ 70 chars", check_subject_length), + ("Subject ≤ 80 chars", check_subject_length), ("No trailing period in subject", check_subject_no_trailing_dot), ("Subject capitalized", check_subject_capitalized), ("Blank line after subject", check_body_blank_line), - ("Body wrapped at 76 chars", check_body_line_length), + ("Body wrapped at 80 chars", check_body_line_length), ("No Co-authored-by trailer", check_no_co_authored_by), ] diff --git a/scripts/test_check_commit.py b/scripts/test_check_commit.py index a3a7a1c2b9..037bc8fd56 100644 --- a/scripts/test_check_commit.py +++ b/scripts/test_check_commit.py @@ -55,20 +55,20 @@ class BodyLineLengthTests(unittest.TestCase): self.assert_ok(message) def test_line_at_limit_passes(self): - line = "x " * 38 # 76 chars, breakable - self.assertEqual(len(line), 76) + line = "x " * 40 # 80 chars, breakable + self.assertEqual(len(line), 80) self.assert_ok(":bug: Fix crash\n\n" + line + "\n") def test_line_one_over_limit_fails(self): - line = "x " * 38 + "x" # 77 chars, breakable - self.assertEqual(len(line), 77) + line = "x " * 40 + "x" # 81 chars, breakable + self.assertEqual(len(line), 81) error = self.assert_fail(":bug: Fix crash\n\n" + line + "\n") - self.assertIn("76", error) + self.assertIn("80", error) def test_long_body_line_fails(self): long_line = "word " * 20 # 100 chars, breakable error = self.assert_fail(":bug: Fix crash\n\n" + long_line + "\n") - self.assertIn("76", error) + self.assertIn("80", error) self.assertIn("line 3", error) def test_subject_is_not_checked(self): @@ -150,15 +150,34 @@ class SubjectRulesRegressionTests(unittest.TestCase): ok, _ = check_commit.check_subject_no_trailing_dot(":bug: Fix crash.") self.assertFalse(ok) - def test_subject_at_70_chars_passes(self): - # ":bug: " is 6 chars, so 64 chars of text reach exactly 70. - ok, error = check_commit.check_subject_length(":bug: " + "S" * 64) + def test_subject_at_80_chars_passes(self): + # ":bug: " is 6 chars, so 74 chars of text reach exactly 80. + ok, error = check_commit.check_subject_length(":bug: " + "S" * 74) self.assertTrue(ok, error) - def test_subject_over_70_chars_fails(self): - ok, error = check_commit.check_subject_length(":bug: " + "S" * 65) + def test_subject_over_80_chars_fails(self): + ok, error = check_commit.check_subject_length(":bug: " + "S" * 75) self.assertFalse(ok) - self.assertIn("70", error) + self.assertIn("80", error) + + def test_merge_subject_skips_length_check(self): + # Git generates Merge subjects from branch names; the limit is skipped. + subject = "Merge branch 'some-very-long-branch-name-that-keeps-going' into develop with extra text here" + self.assertGreater(len(subject), 80) + ok, error = check_commit.check_subject_length(subject) + self.assertTrue(ok, error) + + def test_revert_subject_skips_length_check(self): + subject = 'Revert ":bug: Fix some earlier commit with a fairly long subject line already here" extra' + self.assertGreater(len(subject), 80) + ok, error = check_commit.check_subject_length(subject) + self.assertTrue(ok, error) + + def test_reapply_subject_skips_length_check(self): + subject = 'Reapply ":bug: Fix some earlier commit with a fairly long subject line already here" extra' + self.assertGreater(len(subject), 80) + ok, error = check_commit.check_subject_length(subject) + self.assertTrue(ok, error) if __name__ == "__main__":