diff --git a/.serena/memories/common/file-change-validation-migration-subtleties.md b/.serena/memories/common/file-change-validation-migration-subtleties.md index d013415730..7333faadc3 100644 --- a/.serena/memories/common/file-change-validation-migration-subtleties.md +++ b/.serena/memories/common/file-change-validation-migration-subtleties.md @@ -21,6 +21,7 @@ - Full referential/semantic validation currently runs only when file features contain `"components/v2"`. - Validation starts at root plus orphan shapes, then validates component records. `validate-file!` raises `:validation :referential-integrity` with collected details. - `repair-file` does not mutate data directly; it reduces validation errors into redo changes using `changes-builder`. Callers must apply or persist those changes. +- `repair-file-until-valid` stops on: no errors; unchanged `:data` after apply (backend page/`PointerMap` `=` is pointer identity); stable error fingerprint `(frequencies (map #(dissoc % :shape) errors))` vs previous pass (check fingerprint before `repair-file`/`process-changes`); or `max-iterations`. Each `repair-error` builds from the original `file-data`, so N `:set`-based errors on one shape take N passes. - `:missing-slot` fires only for a REAL swap: a copy sub-head whose `shape-ref` is no longer a child of the near main parent. A pure positional mismatch (ref still a sibling elsewhere) is a reorder — valid, realigned by the async component sync; do not "repair" it by assigning swap slots (a slot freezes the child out of normal sync). `fix-missing-swap-slots` (migration 0019) follows the same membership rule. ## Migrations diff --git a/backend/src/app/binfile/common.clj b/backend/src/app/binfile/common.clj index 170a514704..746dd3e55a 100644 --- a/backend/src/app/binfile/common.clj +++ b/backend/src/app/binfile/common.clj @@ -683,7 +683,8 @@ "Make the file its own tokens source when its tokens source is not one of the given library-ids, as happens when the library was deleted. The tokens lib of the source is copied while the library still exists, even - if deleted; otherwise the tokens source and status are removed." + if deleted; otherwise the tokens source is removed and the file gets back + the tokens status kept aside for its own tokens lib, if any." [cfg data library-ids] (let [source-id (cfo/get-tokens-source data)] (if (and (some? source-id) @@ -692,7 +693,7 @@ :include-deleted? true :throw-if-not-exists? false)] (cfo/absorb-tokens-lib data (:data library)) - (dissoc data :tokens-source :tokens-status)) + (cfo/detach-tokens-source data)) data))) (defn embed-assets diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 2f119a769f..f8d856fb76 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -77,6 +77,12 @@ ::mdef/buckets mtx/default-request-histogram-buckets ::mdef/type :histogram} + :update-file-repair + {::mdef/name "penpot_rpc_update_file_repair_total" + ::mdef/help "update-file auto-repair attempts by outcome (repaired or unrepaired)." + ::mdef/labels ["outcome"] + ::mdef/type :counter} + :rpc-main-timing {::mdef/name "penpot_rpc_main_timing" ::mdef/help "RPC command method call timing for main" diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index d538563ad6..c014c4dda5 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -1009,6 +1009,7 @@ :file-id (str file-id)) (bfc/update-file! cfg {:id file-id + :features (:features file) :migrations (:migrations file) :revn (inc (:revn file)) :data (:data file) diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 85a0a58b1f..5c47dbf61d 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -12,6 +12,7 @@ [app.common.features :as cfeat] [app.common.files.changes :as cpc] [app.common.files.migrations :as fmg] + [app.common.files.repair :as cfr] [app.common.files.validate :as val] [app.common.logging :as l] [app.common.schema :as sm] @@ -41,6 +42,7 @@ (declare ^:private get-lagged-changes) (declare ^:private send-notifications!) +(declare ^:private send-repair-notifications) (declare ^:private update-file) (declare ^:private update-file*) (declare ^:private process-changes-and-validate) @@ -124,6 +126,27 @@ (contains? token-change-types type) (contains? file-change-types type))) +;; Not a real editor session: file-change messages with this id are +;; delivered to every connected session, including the saver. +(def repair-session-id + #uuid "a11c0de5-0000-4000-8000-000000000001") + +(defn- referential-integrity-error? + [cause] + (= :referential-integrity (:code (ex-data cause)))) + +(defn- record-repair-outcome + [cfg outcome] + (when-let [metrics (::mtx/metrics cfg)] + (when (mtx/metrics? metrics) + (mtx/run! metrics {:id :update-file-repair + :labels [(name outcome)] + :inc 1})))) + +(defn- error-codes + [errors] + (into [] (keep :code) errors)) + ;; If features are specified from params and the final feature ;; set is different than the persisted one, update it on the ;; database. @@ -225,7 +248,9 @@ :soft-validate-affected-ms (get timings :soft-validate-affected-ms) :soft-validate-schema-ms (get timings :soft-validate-schema-ms) :validate-affected-ms (get timings :validate-affected-ms) - :validate-schema-ms (get timings :validate-schema-ms))))) + :validate-schema-ms (get timings :validate-schema-ms) + :repair-ms (get timings :repair-ms) + :reprocess-changes-ms (get timings :reprocess-changes-ms))))) (catch Throwable cause ;; NOTE: `with-defer` hooks only run on the success path ;; (`handle-response`), so timings collected before a hard @@ -241,7 +266,9 @@ :soft-validate-affected-ms (get timings :soft-validate-affected-ms) :soft-validate-schema-ms (get timings :soft-validate-schema-ms) :validate-affected-ms (get timings :validate-affected-ms) - :validate-schema-ms (get timings :validate-schema-ms))) + :validate-schema-ms (get timings :validate-schema-ms) + :repair-ms (get timings :repair-ms) + :reprocess-changes-ms (get timings :reprocess-changes-ms))) (throw cause)))))) (defn- update-file* @@ -275,27 +302,65 @@ changes skip-validate)) deleted-at - (ct/plus timestamp (ct/duration {:hours 1}))] + (ct/plus timestamp (ct/duration {:hours 1})) - (when-let [file (::snapshot file)] - (let [deleted-at (ct/plus timestamp (ldel/get-deletion-delay team)) - label (str "internal/snapshot/" revn)] + repair + (::repair file) - (fsnap/create! cfg file - {:label label + snapshot-deleted-at + (ct/plus timestamp (ldel/get-deletion-delay team)) + + change-created-at + (cond-> timestamp + repair + (ct/plus (ct/duration {:millis 1})))] + + (if repair + (let [snapshot-file (or (::snapshot file) + (fdata/realize cfg (:original repair)))] + (fsnap/create! cfg snapshot-file + {:label (str "internal/repair/" revn) :created-by "system" - :deleted-at deleted-at + :deleted-at snapshot-deleted-at + :profile-id profile-id + :session-id repair-session-id})) + (when-let [file (::snapshot file)] + (fsnap/create! cfg file + {:label (str "internal/snapshot/" revn) + :created-by "system" + :deleted-at snapshot-deleted-at :profile-id profile-id :session-id session-id}))) + (when repair + (db/insert! conn :file-change + {:id (uuid/next) + :session-id repair-session-id + :profile-id profile-id + :created-at timestamp + :updated-at timestamp + :deleted-at deleted-at + :file-id (:id file) + :revn (:revn repair) + :version (:version file) + :features (into-array (:features file)) + :changes (blob/encode (:changes repair))} + {::db/return-keys false}) + (l/wrn :hint "file auto-repaired" + :file-id (str (:id file)) + :error-codes (error-codes (:errors repair)) + :error-count (count (:errors repair)) + :change-count (count (:changes repair))) + (record-repair-outcome cfg :repaired)) + ;; Insert change (xlog) with deleted_at in a future data for ;; make them automatically eleggible for GC once they expires (db/insert! conn :file-change {:id (uuid/next) :session-id session-id :profile-id profile-id - :created-at timestamp - :updated-at timestamp + :created-at change-created-at + :updated-at change-created-at :deleted-at deleted-at :file-id (:id file) :revn (:revn file) @@ -304,12 +369,14 @@ :changes (blob/encode changes)} {::db/return-keys false}) - (persist-file! cfg file) + (persist-file! cfg (dissoc file ::repair)) (when (contains? cf/flags :redis-cache) (invalidate-caches! cfg file)) ;; Send asynchronous notifications + (when repair + (send-repair-notifications cfg params file repair)) (send-notifications! cfg params file) (with-meta {:revn revn :lagged (get-lagged-changes conn params)} @@ -434,9 +501,105 @@ (l/error :hint "file validation error" :cause cause)))) +(defn- apply-changes + [cfg file changes] + (let [state (atom {}) + file (binding [cpc/*state* state] + (-> file + (update :data cpc/process-changes changes) + (update :data d/without-nils)))] + (if-let [media-refs (-> @state :media-refs not-empty)] + (bfc/update-media-references! cfg file media-refs) + file))) + +(defn- fail-repair + [cfg file errors cause] + (l/wrn :hint "file auto-repair did not recover the file" + :file-id (str (:id file)) + :error-codes (error-codes errors) + :error-count (count errors)) + (record-repair-outcome cfg :unrepaired) + (throw cause)) + +(defn- repair-preexisting-damage + "Repair damage that was already in `original` on the pages/components + the client's `changes` touch, then re-apply those changes. Throws + `cause` when the client's changes are at fault or the repair cannot + fix the file." + [cfg original libs changes tracked tracked-0 cause] + (let [libs (or libs {})] + (try + (let [pre-errors + (binding [pmap/*tracked* nil] + (val/validate-file-affected original libs changes))] + + (when (empty? pre-errors) + (throw cause)) + + (when tracked + (reset! tracked tracked-0)) + + (let [trepair (ct/tpoint) + + result + (try + (binding [pmap/*tracked* tracked] + (cfr/repair-file-until-valid + original libs + (fn [file applied] + (binding [pmap/*tracked* nil] + (val/validate-file-affected file libs (into applied changes)))) + :initial-errors pre-errors)) + (finally + (record-validation-timing cfg :repair-ms "repair" (inst-ms (trepair))))) + + repair-changes (:changes result)] + + (when (seq (:errors result)) + (fail-repair cfg original (:errors result) cause)) + + (let [treprocess (ct/tpoint) + repaired (try + (binding [pmap/*tracked* tracked] + (apply-changes cfg (:file result) changes)) + (finally + (record-validation-timing cfg :reprocess-changes-ms "reprocess-changes" + (inst-ms (treprocess))))) + scope (into repair-changes changes)] + + (binding [pmap/*tracked* nil] + (val/validate-file-affected! repaired libs scope) + + (when (contains? cf/flags :file-schema-validation) + (let [t (ct/tpoint)] + (try + (val/validate-file-schema! repaired) + (finally + (record-validation-timing cfg :validate-schema-ms "validate-schema" (inst-ms (t)))))))) + + (-> repaired + (assoc :revn (+ (:revn original) 2)) + (assoc ::repair {:changes repair-changes + :errors pre-errors + :revn (inc (:revn original)) + :original (dissoc original ::snapshot)}))))) + (catch Throwable inner + (if (identical? inner cause) + (throw cause) + (do + (l/wrn :hint "file auto-repair failed" + :file-id (str (:id original)) + :cause inner) + (record-repair-outcome cfg :unrepaired) + (throw cause))))))) + (defn- process-changes-and-validate [cfg file changes skip-validate] - (let [should-resolve-libs? + (let [original file + tracked pmap/*tracked* + tracked-0 (some-> tracked deref) + + should-resolve-libs? (and (or (contains? cf/flags :file-validation) (contains? cf/flags :soft-file-validation)) (not skip-validate)) @@ -452,33 +615,20 @@ (when should-resolve-libs? (record-validation-timing cfg :resolve-libs-ms "resolve-libs" (inst-ms (tlibs)))))) - ;; The main purpose of this atom is provide a contextual state - ;; for the changes subsystem where optionally some hints can - ;; be provided for the changes processing. Right now we are - ;; using it for notify about the existence of media refs when - ;; a new shape is added. - state - (atom {}) - tproc (ct/tpoint) file (try - (let [file - (binding [cpc/*state* state] - (-> (files/check-version! file) - (update :revn inc) - (update :data cpc/process-changes changes) - (update :data d/without-nils))) - - file - (if-let [media-refs (-> @state :media-refs not-empty)] - (bfc/update-media-references! cfg file media-refs) - file)] - file) + (apply-changes cfg + (-> (files/check-version! file) + (update :revn inc)) + changes) (finally - (record-validation-timing cfg :process-changes-ms "process-changes" (inst-ms (tproc)))))] + (record-validation-timing cfg :process-changes-ms "process-changes" (inst-ms (tproc))))) + + pending-repair + (atom nil)] (binding [pmap/*tracked* nil] (when (contains? cf/flags :soft-file-validation) @@ -500,18 +650,25 @@ (let [t (ct/tpoint)] (try (val/validate-file-affected! file libs changes) + (catch Throwable cause + (reset! pending-repair cause) + (when-not (referential-integrity-error? cause) + (throw cause))) (finally (record-validation-timing cfg :validate-affected-ms "validate-affected" (inst-ms (t))))))) (when (and (contains? cf/flags :file-schema-validation) - (not skip-validate)) + (not skip-validate) + (nil? @pending-repair)) (let [t (ct/tpoint)] (try (val/validate-file-schema! file) (finally (record-validation-timing cfg :validate-schema-ms "validate-schema" (inst-ms (t)))))))) - file)) + (if-let [cause @pending-repair] + (repair-preexisting-damage cfg original libs changes tracked tracked-0 cause) + file))) (defn- take-snapshot? "Defines the rule when file `data` snapshot should be saved." @@ -541,6 +698,34 @@ (mapv (fn [row] (update row :changes blob/decode))))) +(defn- send-repair-notifications + [cfg {:keys [team session-id] :as params} file repair] + (let [changes (:changes repair) + lchanges (filter library-change? changes) + msgbus (::mbus/msgbus cfg)] + + (mbus/pub! msgbus + :topic (:id file) + :message {:type :file-change + :profile-id (:profile-id params) + :file-id (:id file) + :session-id repair-session-id + :revn (:revn repair) + :vern (:vern file) + :changes changes + :repaired true}) + + (when (and (:is-shared file) (seq lchanges)) + (mbus/pub! msgbus + :topic (:id team) + :message {:type :library-change + :profile-id (:profile-id params) + :file-id (:id file) + :session-id session-id + :revn (:revn repair) + :modified-at (ct/now) + :changes lchanges})))) + (defn- send-notifications! [cfg {:keys [team changes session-id] :as params} file] (let [lchanges (filter library-change? changes) diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index 51075ecda0..a0bab89055 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -170,6 +170,89 @@ (db/get-update-count) (pos?))))))) +(defn update-profile-email! + "Support helper: change a profile's email address. + + Looks up the profile by its current email. Rejects with + `:email-already-exists` when the new address is already used by + another non-deleted profile. Does not clear bounce/complaint + reports; call `clear-email-complaint-reports!` for that." + [& {:keys [email new-email]}] + (assert (string? email) "expected email") + (assert (string? new-email) "expected new-email") + + (let [email (profile/clean-email email) + new-email (profile/clean-email new-email)] + (db/tx-run! sys/system + (fn [{:keys [::db/conn] :as system}] + (let [profile (profile/get-profile-by-email conn email)] + (when-not profile + (ex/raise :type :not-found + :code :profile-not-found + :hint "profile not found for email" + :email email)) + + (if (= email new-email) + :unchanged + (do + (when (profile/get-profile-by-email conn new-email) + (ex/raise :type :validation + :code :email-already-exists)) + + (db/update! conn :profile + {:email new-email} + {:id (:id profile)} + {::db/return-keys false}) + + (audit/insert system + {:name "update-profile-email" + :type "action" + :props {:id (:id profile) + :email email + :new-email new-email} + :context {:triggered-by "srepl" + :cause "explicit call to update-profile-email!"}}) + :updated))))))) + +(defn clear-email-complaint-reports! + "Support helper: delete bounce and complaint reports for an email + from `global_complaint_report` and, when a profile owns that email, + from `profile_complaint_report`. Also unmutes that profile so mail + can be sent again after a false-positive bounce or complaint." + [email] + (assert (string? email) "expected email") + + (let [email (profile/clean-email email)] + (db/tx-run! sys/system + (fn [{:keys [::db/conn] :as system}] + (let [profile (db/get* conn :profile {:email email}) + global-deleted + (-> (db/delete! conn :global-complaint-report {:email email}) + (db/get-update-count)) + profile-deleted + (if profile + (-> (db/delete! conn :profile-complaint-report + {:profile-id (:id profile)}) + (db/get-update-count)) + 0)] + (when profile + (db/update! conn :profile + {:is-muted false} + {:id (:id profile)} + {::db/return-keys false})) + + (when (or profile (pos? global-deleted)) + (audit/insert system + {:name "clear-email-complaint-reports" + :type "action" + :props (cond-> {:email email + :global-deleted global-deleted + :profile-deleted profile-deleted} + profile (assoc :id (:id profile))) + :context {:triggered-by "srepl" + :cause "explicit call to clear-email-complaint-reports!"}}) + :cleared)))))) + (defn parse-emails "Parse the emails into a seq of cleaned emails. Accepts a single email, a comma separated list of emails or a coll of emails. diff --git a/backend/src/app/srepl/procs/file_repair.clj b/backend/src/app/srepl/procs/file_repair.clj index f863f250cf..c671f2d736 100644 --- a/backend/src/app/srepl/procs/file_repair.clj +++ b/backend/src/app/srepl/procs/file_repair.clj @@ -6,7 +6,6 @@ (ns app.srepl.procs.file-repair (:require - [app.common.files.changes :as cfc] [app.common.files.repair :as cfr] [app.common.files.validate :as cfv] [app.common.logging :as l])) @@ -17,44 +16,28 @@ (defn repair-file "Internal helper for validate and repair the file. The operation is - applied multiple times untile file is fixed or max iteration counter + applied multiple times until file is fixed or max iteration counter is reached (default 10). - This function should not be used directly, it is used throught the + This function should not be used directly, it is used through the app.srepl.main/repair-file! helper. In practical terms this function is private and implementation detail." [file libs & {:keys [max-iterations] :or {max-iterations 10}}] - (let [validate-and-repair - (fn [file libs iteration] - (when-let [errors (not-empty (cfv/validate-file file libs))] - (l/trc :hint "repairing file" - :file-id (str (:id file)) - :iteration iteration - :errors (count errors)) - (let [changes (cfr/repair-file file libs errors)] - (-> file - (update :revn inc) - (update :data cfc/process-changes changes))))) + (let [orig-revn (:revn file) + result (cfr/repair-file-until-valid + file libs + (fn [file _] + (cfv/validate-file file libs)) + :max-iterations max-iterations) + file' (update (:file result) :revn (fnil + 0) (:iterations result))] - process-file - (fn [file libs] - (loop [file file - iteration 0] - (if (< iteration max-iterations) - (if-let [file (validate-and-repair file libs iteration)] - (recur file (inc iteration)) - file) - (do - (l/wrn :hint "max retry num reached on repairing file" - :file-id (str (:id file)) - :iteration iteration) - file)))) + (when (>= (:iterations result) max-iterations) + (l/wrn :hint "max retry num reached on repairing file" + :file-id (str (:id file)) + :iteration (:iterations result))) - file' - (process-file file libs)] - - (when (not= (:revn file) (:revn file')) + (when (not= orig-revn (:revn file')) (l/trc :hint "file repaired" :file-id (str (:id file)))) file')) diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 2d222f7add..3acb1cba58 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -2346,6 +2346,61 @@ (t/is (nil? (:error out))) (check-tokens-absorbed prepared))) +(t/deftest unpublishing-a-library-keeps-its-consumers-readable + ;; The test helpers enable all supported features, so these files use + ;; fdata/pointer-map and the absorbed page is stored as a fragment + (let [profile (th/create-profile* 1 {:is-active true}) + library (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared true}) + consumer (th/create-file* 2 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared false}) + _ (th/link-file-to-library* {:file-id (:id consumer) :library-id (:id library)}) + color-id (uuid/random) + page-id (first (get-in consumer [:data :pages])) + shape-id (uuid/random)] + + (update-file! :file-id (:id library) + :profile-id (:id profile) + :revn 0 + :changes [{:type :add-color + :color {:id color-id :name "Brand" :color "#abcdef" :opacity 1}}]) + (update-file! + :file-id (:id consumer) + :profile-id (:id profile) + :revn 0 + :changes + [{:type :add-obj + :page-id page-id + :id shape-id + :parent-id uuid/zero + :frame-id uuid/zero + :components-v2 true + :obj (cts/setup-shape {:id shape-id :name "Rect" :frame-id uuid/zero + :parent-id uuid/zero :type :rect + :fills [{:fill-color "#abcdef" + :fill-opacity 1 + :fill-color-ref-id color-id + :fill-color-ref-file (:id library)}]})}]) + + (let [out (th/command! {::th/type :set-file-shared + ::rpc/profile-id (:id profile) + :id (:id library) + :is-shared false})] + (t/is (nil? (:error out)))) + + ;; The absorption changed the page of the shape: the consumer reads + ;; it back, with the fill now linked to its own color + (let [out (th/command! {::th/type :get-file + ::rpc/profile-id (:id profile) + :id (:id consumer)}) + data (-> out :result :data) + fill (get-in data [:pages-index page-id :objects shape-id :fills 0])] + (t/is (nil? (:error out))) + (t/is (contains? (:colors data) color-id)) + (t/is (= (:id consumer) (:fill-color-ref-file fill)))))) + (t/deftest deleted-files-permanently-delete (let [prof (th/create-profile* 1 {:is-active true}) team-id (:default-team-id prof) diff --git a/backend/test/backend_tests/rpc_file_update_repair_test.clj b/backend/test/backend_tests/rpc_file_update_repair_test.clj new file mode 100644 index 0000000000..e681d559ff --- /dev/null +++ b/backend/test/backend_tests/rpc_file_update_repair_test.clj @@ -0,0 +1,433 @@ +;; 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 SUBSIDIARY SL + +(ns backend-tests.rpc-file-update-repair-test + (:require + [app.binfile.common :as bfc] + [app.common.features :as cfeat] + [app.common.files.repair :as cfr] + [app.common.files.validate :as cfv] + [app.common.test-helpers.compositions :as tho] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as ths] + [app.common.types.shape :as cts] + [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] + [app.features.file-snapshots :as fsnap] + [app.metrics :as mtx] + [app.msgbus :as mbus] + [app.rpc :as-alias rpc] + [app.rpc.commands.files-update :as files-update] + [app.srepl.procs.file-repair :as file-repair] + [app.util.pointer-map :as pmap] + [backend-tests.helpers :as th] + [clojure.test :as t]) + (:import + io.prometheus.client.CollectorRegistry)) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(defn- with-repair-flags + [f] + (with-redefs [cf/flags (conj cf/flags :file-validation)] + (f))) + +(defn- with-full-repair-flags + [f] + (with-redefs [cf/flags (conj cf/flags :file-validation :file-schema-validation)] + (f))) + +(defn- repair-count + [outcome] + (or (.getSampleValue ^CollectorRegistry + (mtx/get-registry (::mtx/metrics th/*system*)) + "penpot_rpc_update_file_repair_total" + (into-array String ["outcome"]) + (into-array String [(name outcome)])) + 0.0)) + +(defn- add-rect + [page-id shape-id] + {:type :add-obj + :page-id page-id + :id shape-id + :parent-id uuid/zero + :frame-id uuid/zero + :components-v2 true + :obj (cts/setup-shape + {:id shape-id + :name "rect" + :frame-id uuid/zero + :parent-id uuid/zero + :type :rect})}) + +(defn- set-attr + [page-id shape-id attr val] + {:type :mod-obj + :id shape-id + :page-id page-id + :operations [{:type :set :attr attr :val val}]}) + +(defn- update-file + [profile-id file-id revn changes & {:keys [skip-validate session-id] + :or {session-id (uuid/random)}}] + (th/command! (cond-> {::th/type :update-file + ::rpc/profile-id profile-id + :id file-id + :session-id session-id + :revn revn + :vern 0 + :features cfeat/supported-features + :changes changes} + skip-validate (assoc :skip-validate true)))) + +(defn- stored-file + [file-id] + (bfc/get-file th/*system* file-id :realize? true)) + +(defn- change-rows + [file-id] + (->> (th/db-query :file-change {:file-id file-id} {::db/remove-deleted false}) + (filter (fn [row] + (and (some? (:changes row)) + (nil? (:label row))))) + (sort-by :created-at) + (vec))) + +(defn- label-rows + [file-id] + (->> (th/db-query :file-change {:file-id file-id} {::db/remove-deleted false}) + (filter :label) + (vec))) + +(defn- pointer-map-in? + [x] + (cond + (pmap/pointer-map? x) true + (map? x) (boolean (some pointer-map-in? (vals x))) + (sequential? x) (boolean (some pointer-map-in? x)) + :else false)) + +(defn- setup-file + [& {:keys [is-shared] :or {is-shared false}}] + (let [profile (th/create-profile* 1 {:is-active true}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:default-project-id profile) + :is-shared is-shared}) + page-id (first (get-in file [:data :pages])) + shape-a (uuid/next) + shape-b (uuid/next) + out (update-file (:id profile) (:id file) 0 + [(add-rect page-id shape-a) + (add-rect page-id shape-b)])] + (t/is (th/success? out)) + {:profile profile + :file-id (:id file) + :page-id page-id + :shape-a shape-a + :shape-b shape-b + :revn (:revn (stored-file (:id file)))})) + +(defn- break-frame + [{:keys [profile file-id page-id shape-a revn]}] + (let [out (update-file (:id profile) file-id revn + [(set-attr page-id shape-a :frame-id (uuid/next))] + :skip-validate true)] + (t/is (th/success? out)) + (stored-file file-id))) + +(t/deftest repairs-preexisting-frame-not-found + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-a shape-b] :as ctx} (setup-file) + broken (break-frame ctx) + before (repair-count :repaired) + out (update-file (:id profile) file-id (:revn broken) + [(set-attr page-id shape-b :name "renamed")]) + stored (stored-file file-id) + shape (get-in stored [:data :pages-index page-id :objects shape-a]) + frame (get-in stored [:data :pages-index page-id :objects (:frame-id shape)])] + (t/is (th/success? out)) + (t/is (nil? (cfv/validate-file stored {}))) + (t/is (some? frame)) + (t/is (= "renamed" + (get-in stored [:data :pages-index page-id :objects shape-b :name]))) + (t/is (= 1.0 (- (repair-count :repaired) before))))))) + +(t/deftest repairs-preexisting-frame-not-found-with-schema-validation + (with-full-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-a shape-b] :as ctx} (setup-file) + broken (break-frame ctx) + n (:revn broken) + before-repaired (repair-count :repaired) + before-unrepaired (repair-count :unrepaired) + out (update-file (:id profile) file-id n + [(set-attr page-id shape-b :name "renamed")]) + stored (stored-file file-id) + shape (get-in stored [:data :pages-index page-id :objects shape-a]) + frame (get-in stored [:data :pages-index page-id :objects (:frame-id shape)]) + labels (label-rows file-id) + repair (filter #(= (str "internal/repair/" n) (:label %)) labels) + snap (filter #(= (str "internal/snapshot/" n) (:label %)) labels)] + (t/is (th/success? out)) + (t/is (nil? (cfv/validate-file stored {}))) + (t/is (some? frame)) + (t/is (= "renamed" + (get-in stored [:data :pages-index page-id :objects shape-b :name]))) + (t/is (= (+ n 2) (:revn stored))) + (t/is (= 1 (count repair))) + (t/is (zero? (count snap))) + (t/is (= 1.0 (- (repair-count :repaired) before-repaired))) + (t/is (= 0.0 (- (repair-count :unrepaired) before-unrepaired))))))) + +(t/deftest rejects-client-caused-break + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-a revn]} (setup-file) + before (stored-file file-id) + out (update-file (:id profile) file-id revn + [(set-attr page-id shape-a :frame-id (uuid/next))]) + after (stored-file file-id)] + (t/is (not (th/success? out))) + (t/is (= :referential-integrity (:code (ex-data (:error out))))) + (t/is (= (:revn before) (:revn after))))))) + +(t/deftest skip-validate-does-not-repair + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-a shape-b] :as ctx} (setup-file) + broken (break-frame ctx) + out (update-file (:id profile) file-id (:revn broken) + [(set-attr page-id shape-b :name "renamed")] + :skip-validate true) + stored (stored-file file-id) + shape (get-in stored [:data :pages-index page-id :objects shape-a])] + (t/is (th/success? out)) + (t/is (nil? (get-in stored [:data :pages-index page-id :objects (:frame-id shape)]))))))) + +(t/deftest damage-on-untouched-page-is-left + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-a revn]} (setup-file) + page-2 (uuid/next) + shape-c (uuid/next) + _ (t/is (th/success? + (update-file (:id profile) file-id revn + [{:type :add-page :id page-2 :name "page-2"} + (add-rect page-2 shape-c)]))) + file (stored-file file-id) + _ (t/is (th/success? + (update-file (:id profile) file-id (:revn file) + [(set-attr page-2 shape-c :frame-id (uuid/next))] + :skip-validate true))) + file (stored-file file-id) + out (update-file (:id profile) file-id (:revn file) + [(set-attr page-id shape-a :name "on-page-1")]) + stored (stored-file file-id) + shape (get-in stored [:data :pages-index page-2 :objects shape-c])] + (t/is (th/success? out)) + (t/is (nil? (get-in stored [:data :pages-index page-2 :objects (:frame-id shape)]))))))) + +(t/deftest growing-scope-repairs-component-touched-by-repair + (thi/reset-idmap!) + (with-repair-flags + (fn [] + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (assoc :revn 0) + (tho/add-simple-component :component1 :root1 :child1) + (ths/add-sample-shape :other) + (update-in [:data :components (thi/id :component1)] + assoc :main-instance-id (uuid/next) + :objects {(uuid/next) {:id (uuid/next)}})) + page-id (thi/id :page1) + changes [(set-attr page-id (thi/id :other) :name "renamed")] + cfg {::files-update/validation-timings (atom {})} + result (with-redefs [bfc/get-resolved-file-libraries (fn [_ _] {})] + (@#'files-update/process-changes-and-validate cfg file changes nil)) + stored result] + (t/is (some? (::files-update/repair stored))) + (t/is (nil? (cfv/validate-file stored {}))) + (t/is (nil? (get-in stored [:data :components (thi/id :component1) :objects]))))))) + +(t/deftest repair-records-timings-with-schema-validation + (thi/reset-idmap!) + (with-full-repair-flags + (fn [] + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (assoc :revn 0) + (ths/add-sample-shape :shape1) + (ths/update-shape :shape1 :frame-id (uuid/next))) + changes [(set-attr (thi/id :page1) (thi/id :shape1) :name "renamed")] + timings (atom {}) + cfg {::files-update/validation-timings timings} + result (with-redefs [bfc/get-resolved-file-libraries (fn [_ _] {})] + (@#'files-update/process-changes-and-validate cfg file changes nil))] + (t/is (some? (::files-update/repair result))) + (t/is (number? (:repair-ms @timings))) + (t/is (number? (:reprocess-changes-ms @timings))) + (t/is (number? (:validate-schema-ms @timings))))))) + +(t/deftest unrepairable-rejects-and-calls-repair-once + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file) + broken (break-frame ctx) + calls (atom 0) + before (repair-count :unrepaired) + out (with-redefs [cfr/repair-file (fn [_ _ _] + (swap! calls inc) + [])] + (update-file (:id profile) file-id (:revn broken) + [(set-attr page-id shape-b :name "renamed")]))] + (t/is (not (th/success? out))) + (t/is (= :referential-integrity (:code (ex-data (:error out))))) + (t/is (= 1 @calls)) + (t/is (= 1.0 (- (repair-count :unrepaired) before))))))) + +(t/deftest repair-writes-change-rows-and-reuses-snapshot + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file) + broken (break-frame ctx) + before (count (change-rows file-id)) + session (uuid/next) + calls (atom []) + n (:revn broken) + out (with-redefs [mbus/pub! (fn [_cfg & {:keys [topic message]}] + (swap! calls conj {:topic topic :message message}))] + (update-file (:id profile) file-id n + [(set-attr page-id shape-b :name "renamed")] + :session-id session)) + rows (change-rows file-id) + new (subvec rows before) + labels (label-rows file-id) + repair (filter #(= (str "internal/repair/" n) (:label %)) labels) + snap (filter #(= (str "internal/snapshot/" n) (:label %)) labels) + msgs (filter #(= :file-change (:type (:message %))) @calls) + stored (stored-file file-id)] + (t/is (th/success? out)) + (t/is (= "renamed" + (get-in stored [:data :pages-index page-id :objects shape-b :name]))) + (t/is (= 2 (count new))) + (t/is (= (inc n) (:revn (first new)))) + (t/is (= files-update/repair-session-id (:session-id (first new)))) + (t/is (some? (:deleted-at (first new)))) + (t/is (= (:created-at (first new)) (:updated-at (first new)))) + (t/is (= (+ n 2) (:revn (second new)))) + (t/is (= session (:session-id (second new)))) + (t/is (some? (:deleted-at (second new)))) + (t/is (= (:created-at (second new)) (:updated-at (second new)))) + (t/is (not (pos? (compare (:created-at (first new)) + (:created-at (second new)))))) + (t/is (= 1 (count repair))) + (t/is (zero? (count snap))) + (t/is (>= (count msgs) 2)) + (t/is (true? (get-in (first msgs) [:message :repaired]))) + (t/is (= files-update/repair-session-id + (get-in (first msgs) [:message :session-id]))) + (t/is (= session (get-in (second msgs) [:message :session-id]))) + (t/is (not (true? (get-in (second msgs) [:message :repaired])))) + (t/is (= (+ n 2) (get-in (second msgs) [:message :revn]))))))) + +(t/deftest repair-library-change-keeps-saver-session + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file :is-shared true) + broken (break-frame ctx) + session (uuid/next) + calls (atom []) + out (with-redefs [mbus/pub! (fn [_cfg & {:keys [topic message]}] + (swap! calls conj {:topic topic :message message}))] + (update-file (:id profile) file-id (:revn broken) + [(set-attr page-id shape-b :name "renamed")] + :session-id session)) + lib-msgs (filter #(= :library-change (:type (:message %))) @calls)] + (t/is (th/success? out)) + (t/is (seq lib-msgs)) + (doseq [msg lib-msgs] + (t/is (= session (get-in msg [:message :session-id]))) + (t/is (not= files-update/repair-session-id + (get-in msg [:message :session-id])))))))) + +(t/deftest repair-snapshot-without-auto-snapshot + (with-redefs [cf/flags (-> cf/flags + (conj :file-validation) + (disj :auto-file-snapshot))] + (let [{:keys [profile file-id page-id shape-b] :as ctx} (setup-file) + broken (break-frame ctx) + n (:revn broken) + out (update-file (:id profile) file-id n + [(set-attr page-id shape-b :name "renamed")]) + labels (label-rows file-id) + repair (filter #(= (str "internal/repair/" n) (:label %)) labels) + snaps (filter #(re-find #"^internal/snapshot/" (str (:label %))) labels) + row (first repair) + snap (fsnap/get-snapshot th/*system* file-id (:id row)) + sfile {:id file-id + :features (:features (stored-file file-id)) + :data (:data snap)}] + (t/is (th/success? out)) + (t/is (= 1 (count repair))) + (t/is (zero? (count snaps))) + (t/is (false? (pointer-map-in? (:data snap)))) + (t/is (some? (cfv/validate-file sfile {})))))) + +(t/deftest srepl-repair-grows-revn-by-iterations + (thi/reset-idmap!) + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (assoc :revn 3) + (ths/add-sample-shape :shape1) + (ths/update-shape :shape1 :frame-id (uuid/next))) + loop (cfr/repair-file-until-valid + file {} + (fn [file _] (cfv/validate-file file {}))) + wrapped (file-repair/repair-file file {})] + (t/is (pos? (:iterations loop))) + (t/is (= (+ (:revn file) (:iterations loop)) (:revn wrapped))) + (t/is (nil? (cfv/validate-file wrapped {}))))) + +(t/deftest healthy-save-writes-one-change-row + (with-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-b revn]} (setup-file) + before (count (change-rows file-id)) + out (update-file (:id profile) file-id revn + [(set-attr page-id shape-b :name "ok")]) + labels (label-rows file-id)] + (t/is (th/success? out)) + (t/is (= 1 (- (count (change-rows file-id)) before))) + (t/is (empty? (filter #(re-find #"^internal/repair/" (str (:label %))) labels))))))) + +(t/deftest repairs-several-missing-children-on-root + (with-full-repair-flags + (fn [] + (let [{:keys [profile file-id page-id shape-a shape-b revn]} (setup-file) + r1 (uuid/next) + r2 (uuid/next) + r3 (uuid/next) + _ (t/is (th/success? + (update-file (:id profile) file-id revn + [(set-attr page-id uuid/zero :shapes + [shape-a shape-b r1 r2 r3])] + :skip-validate true))) + broken (stored-file file-id) + errors (cfv/validate-file broken {}) + out (update-file (:id profile) file-id (:revn broken) + [(set-attr page-id shape-b :name "renamed")]) + stored (stored-file file-id) + root (get-in stored [:data :pages-index page-id :objects uuid/zero])] + (t/is (= [:child-not-found :child-not-found :child-not-found] + (mapv :code errors))) + (t/is (= #{uuid/zero} (into #{} (map :shape-id) errors))) + (t/is (th/success? out)) + (t/is (= [shape-a shape-b] (:shapes root))) + (t/is (nil? (cfv/validate-file stored {}))) + (t/is (= "renamed" + (get-in stored [:data :pages-index page-id :objects shape-b :name]))))))) diff --git a/backend/test/backend_tests/srepl_main_test.clj b/backend/test/backend_tests/srepl_main_test.clj index 1d131d1fe3..9cbf5a39af 100644 --- a/backend/test/backend_tests/srepl_main_test.clj +++ b/backend/test/backend_tests/srepl_main_test.clj @@ -205,3 +205,104 @@ ;; to the zero one. (t/is (nil? (:profile-id event))) (t/is (= (:id profile) (get-in event [:props :id]))))))))) + +(t/deftest update-profile-email-changes-email + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil}] + (with-redefs [sys/system th/*system*] + (let [profile (th/create-profile* 1 {:is-active true}) + result (srepl/update-profile-email! + :email (:email profile) + :new-email "corrected@example.com")] + (t/is (= :updated result)) + (t/is (= "corrected@example.com" + (:email (th/db-get :profile {:id (:id profile)})))) + (let [event (->> (:call-args-list @audit-mock) + (map second) + first)] + (t/is (= "update-profile-email" (:name event))) + (t/is (= "action" (:type event))) + (t/is (= (:id profile) (get-in event [:props :id]))) + (t/is (= (:email profile) (get-in event [:props :email]))) + (t/is (= "corrected@example.com" + (get-in event [:props :new-email])))))))) + +(t/deftest update-profile-email-rejects-collision + (with-redefs [sys/system th/*system*] + (let [profile1 (th/create-profile* 1 {:is-active true}) + profile2 (th/create-profile* 2 {:is-active true}) + error (try + (srepl/update-profile-email! + :email (:email profile1) + :new-email (:email profile2)) + (catch Throwable e e))] + (t/is (th/ex-of-code? error :email-already-exists)) + (t/is (= (:email profile1) + (:email (th/db-get :profile {:id (:id profile1)}))))))) + +(t/deftest update-profile-email-unchanged-when-same + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil}] + (with-redefs [sys/system th/*system*] + (let [profile (th/create-profile* 1 {:is-active true}) + result (srepl/update-profile-email! + :email (:email profile) + :new-email (:email profile))] + (t/is (= :unchanged result)) + (t/is (empty? (:call-args-list @audit-mock))))))) + +(t/deftest update-profile-email-not-found + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil}] + (with-redefs [sys/system th/*system*] + (let [error (try + (srepl/update-profile-email! + :email "missing@example.com" + :new-email "corrected@example.com") + (catch Throwable e e))] + (t/is (th/ex-of-code? error :profile-not-found)) + (t/is (= :not-found (:type (ex-data error)))) + (t/is (empty? (:call-args-list @audit-mock))))))) + +(t/deftest clear-email-complaint-reports-deletes-and-unmutes + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil}] + (with-redefs [sys/system th/*system*] + (let [profile (th/create-profile* 1 {:is-active true :is-muted true + :email "bounced@example.com"}) + _ (th/create-complaint-for th/*system* + {:id (:id profile) :type :bounce}) + _ (th/create-complaint-for th/*system* + {:id (:id profile) :type :complaint}) + _ (th/create-global-complaint-for th/*system* + {:email (:email profile) + :type :bounce}) + _ (th/create-global-complaint-for th/*system* + {:email (:email profile) + :type :complaint}) + result (srepl/clear-email-complaint-reports! (:email profile))] + (t/is (= :cleared result)) + (t/is (empty? (th/db-query :global-complaint-report + {:email (:email profile)}))) + (t/is (empty? (th/db-query :profile-complaint-report + {:profile-id (:id profile)}))) + (t/is (false? (:is-muted (th/db-get :profile {:id (:id profile)})))) + (let [event (->> (:call-args-list @audit-mock) + (map second) + first)] + (t/is (= "clear-email-complaint-reports" (:name event))) + (t/is (= "action" (:type event))) + (t/is (= (:id profile) (get-in event [:props :id]))) + (t/is (= (:email profile) (get-in event [:props :email])))))))) + +(t/deftest clear-email-complaint-reports-without-profile + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil}] + (with-redefs [sys/system th/*system*] + (let [email "orphan-bounce@example.com" + _ (th/create-global-complaint-for th/*system* + {:email email :type :bounce}) + result (srepl/clear-email-complaint-reports! email)] + (t/is (= :cleared result)) + (t/is (empty? (th/db-query :global-complaint-report {:email email}))) + (let [event (->> (:call-args-list @audit-mock) + (map second) + first)] + (t/is (= "clear-email-complaint-reports" (:name event))) + (t/is (= email (get-in event [:props :email]))) + (t/is (nil? (get-in event [:props :id])))))))) diff --git a/common/src/app/common/files/changes.cljc b/common/src/app/common/files/changes.cljc index ce45edd485..8e4bbb67d9 100644 --- a/common/src/app/common/files/changes.cljc +++ b/common/src/app/common/files/changes.cljc @@ -407,6 +407,12 @@ [:theme-ids [:set ::sm/uuid]] [:set-ids [:set ::sm/uuid]]]] + [:set-own-tokens-status + [:map {:title "SetOwnTokensStatus"} + [:type [:= :set-own-tokens-status]] + [:theme-ids [:maybe [:set ::sm/uuid]]] + [:set-ids [:maybe [:set ::sm/uuid]]]]] + [:rename-token-set-group [:map {:title "RenameTokenSetGroup"} [:type [:= :rename-token-set-group]] @@ -1067,6 +1073,13 @@ (-> (cfo/ensure-tokens-lib data) (cfo/update-tokens-status ctos/set-tokens-status theme-ids set-ids))) +(defmethod process-change :set-own-tokens-status + [data {:keys [theme-ids set-ids]}] + (cfo/set-own-tokens-status data + (when (or (some? theme-ids) (some? set-ids)) + (ctos/make-tokens-status :active-theme-ids theme-ids + :active-set-ids set-ids)))) + (defmethod process-change :rename-token-set-group [data {:keys [set-group-path set-group-fname]}] (-> (cfo/ensure-tokens-lib data) diff --git a/common/src/app/common/files/changes_builder.cljc b/common/src/app/common/files/changes_builder.cljc index 3317d91653..94f50fccef 100644 --- a/common/src/app/common/files/changes_builder.cljc +++ b/common/src/app/common/files/changes_builder.cljc @@ -1083,6 +1083,24 @@ (update :undo-changes conj {:type :set-tokens-status :theme-ids prev-theme-ids :set-ids prev-set-ids}) (apply-changes-local))))) +(defn set-own-tokens-status + "Keep aside the tokens status of the file's own tokens-lib, or remove it + when `tokens-status` is nil." + [changes tokens-status] + (assert-library! changes) + (assert (or (ctos/tokens-status? tokens-status) + (nil? tokens-status))) + (let [library-data (::library-data (meta changes)) + prev-tokens-status (cfo/get-own-tokens-status library-data) + ->change (fn [tokens-status] + {:type :set-own-tokens-status + :theme-ids (some-> tokens-status ctos/get-active-theme-ids) + :set-ids (some-> tokens-status ctos/get-active-set-ids)})] + (-> changes + (update :redo-changes conj (->change tokens-status)) + (update :undo-changes conj (->change prev-tokens-status)) + (apply-changes-local)))) + (defn rename-token-set-group [changes set-group-path set-group-fname] (let [undo-path (ctob/replace-last-path-name set-group-path set-group-fname) diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc index 4756f8a758..f0a52b788c 100644 --- a/common/src/app/common/files/repair.cljc +++ b/common/src/app/common/files/repair.cljc @@ -8,6 +8,7 @@ (:require [app.common.data :as d] [app.common.data.macros :as dm] + [app.common.files.changes :as cfc] [app.common.files.changes-builder :as pcb] [app.common.files.helpers :as cfh] [app.common.files.tokens :as cfo] @@ -22,8 +23,6 @@ [app.common.types.variant :as ctv] [app.common.uuid :as uuid])) -(log/set-level! :debug) - (defmulti repair-error (fn [code _error _file-data _libraries] code)) @@ -867,3 +866,58 @@ (pcb/empty-changes nil) errors)] redo-changes)) + +(defn- error-fingerprint + [errors] + (frequencies (map #(dissoc % :shape) errors))) + +(defn repair-file-until-valid + "Repeatedly validate and repair `file` until `errors-fn` returns nil, + a repair pass leaves `:data` unchanged, or the error fingerprint + (errors without `:shape`) is unchanged from the previous pass. + + `errors-fn` is `(fn [file applied-changes] errors-or-nil)`. Never + changes `:revn`. + + Returns `{:file file' :changes [...] :iterations n :errors remaining}`." + [file libraries errors-fn & {:keys [max-iterations initial-errors] + :or {max-iterations 10}}] + (loop [file file + applied [] + iteration 0 + prev-fp nil + seed-errors initial-errors] + (if (>= iteration max-iterations) + {:file file + :changes applied + :iterations iteration + :errors (not-empty (errors-fn file applied))} + (let [errors (if (and (zero? iteration) (some? seed-errors)) + seed-errors + (errors-fn file applied))] + (if (empty? errors) + {:file file + :changes applied + :iterations iteration + :errors nil} + (let [fp (error-fingerprint errors)] + (cond + (= fp prev-fp) + {:file file + :changes applied + :iterations (inc iteration) + :errors (not-empty errors)} + + :else + (let [batch (repair-file file libraries errors) + data' (cfc/process-changes (:data file) batch)] + (if (= data' (:data file)) + {:file file + :changes applied + :iterations (inc iteration) + :errors (not-empty errors)} + (recur (assoc file :data data') + (into applied batch) + (inc iteration) + fp + nil)))))))))) diff --git a/common/src/app/common/files/tokens.cljc b/common/src/app/common/files/tokens.cljc index e8a28e7ca2..f8fb4e32aa 100644 --- a/common/src/app/common/files/tokens.cljc +++ b/common/src/app/common/files/tokens.cljc @@ -549,6 +549,37 @@ [file-data f & args] (d/update-when file-data :tokens-status #(apply f % args))) +(defn get-own-tokens-status + "The tokens status of the file's own tokens-lib, kept aside while the + tokens source is an external library." + [file-data] + (:own-tokens-status file-data)) + +(defn set-own-tokens-status + "Keep aside the tokens status of the file's own tokens-lib. Setting it to + nil removes the attribute." + [file-data tokens-status] + (assert (or (nil? tokens-status) (ctos/tokens-status? tokens-status)) + "expected nil or valid tokens-status") + (if (nil? tokens-status) + (dissoc file-data :own-tokens-status) + (assoc file-data :own-tokens-status tokens-status))) + +(defn detach-tokens-source + "Make the file its own tokens source again, restoring the tokens status + kept aside for its own tokens-lib. If none was kept, the tokens status + is removed." + [file-data] + (let [own-tokens-status (get-own-tokens-status file-data)] + (if (some? own-tokens-status) + (-> file-data + (set-tokens-source nil) + (set-own-tokens-status nil) + (assoc :tokens-status own-tokens-status)) + (-> file-data + (set-tokens-source nil) + (dissoc :tokens-status))))) + (defn absorb-tokens-lib "Copy the tokens-lib of the library into the file, replacing the previous one if any, and unset the tokens-source, so the file becomes @@ -558,19 +589,26 @@ one of the library is copied along with the tokens-lib. If the library has no tokens-lib, only the tokens-source is unset, and - the file keeps its own tokens-lib and tokens-status. If there is no - library-data, there is nothing to absorb and the file is returned - unchanged." + the file keeps its own tokens-lib, with the tokens status kept aside for + it if any. If there is no library-data, there is nothing to absorb and + the file is returned unchanged." [file-data library-data] (if (nil? library-data) file-data (let [tokens-lib (get-tokens-lib library-data)] (if (nil? tokens-lib) - (set-tokens-source file-data nil) + (let [own-tokens-status (get-own-tokens-status file-data)] + (cond-> (-> file-data + (set-tokens-source nil) + (set-own-tokens-status nil)) + (some? own-tokens-status) + (assoc :tokens-status own-tokens-status))) (let [tokens-status (or (get-tokens-status file-data) (get-tokens-status library-data))] (cond-> (-> file-data (set-tokens-source nil) + ;; The kept status refers to the replaced tokens-lib + (set-own-tokens-status nil) (assoc :tokens-lib tokens-lib)) (some? tokens-status) (assoc :tokens-status tokens-status))))))) diff --git a/common/src/app/common/logic/tokens.cljc b/common/src/app/common/logic/tokens.cljc index 53d2ae5396..c1f948e8c7 100644 --- a/common/src/app/common/logic/tokens.cljc +++ b/common/src/app/common/logic/tokens.cljc @@ -16,36 +16,58 @@ ;; Tokens source (defn- own-tokens-status - "The current tokens status of `file-data`, keeping only the themes and sets - that exist in its own tokens lib." - [file-data] - (let [tokens-status (cfo/get-tokens-status file-data) - tokens-lib (cfo/get-tokens-lib file-data)] + "`tokens-status` keeping only the themes and sets that exist in the own + tokens lib of `file-data`." + [file-data tokens-status] + (let [tokens-lib (cfo/get-tokens-lib file-data)] (if (and tokens-status tokens-lib) (cfo/sync-tokens-status-with-lib tokens-status tokens-lib) (ctos/make-tokens-status)))) (defn generate-set-tokens-source "Create changes for setting the tokens source of a file to `library`, - copying the library's tokens status. If the library is the file itself, - the themes and sets that don't exist in its own tokens lib are deactivated. - If the library is nil, the tokens source is removed and the tokens status - is cleared." + copying the library's tokens status. + + While an external library is the source, the tokens status of the file's + own tokens lib is kept aside, and it is restored when the file goes back + to being its own source (the library is the file itself, or nil). With + nothing kept aside, going back to the file itself deactivates the themes + and sets that don't exist in its own tokens lib, and a nil library clears + the tokens status." [changes library] - (let [library-id (:id library) - file-data (pcb/get-library-data changes) - library-tokens-status (cond - (nil? library) - (ctos/make-tokens-status) + (let [file-data (pcb/get-library-data changes) + file-id (:id file-data) + library-id (:id library) + to-own? (or (nil? library) (= library-id file-id)) + from-own? (cfo/effective-tokens-source? file-data file-id) + current-status (cfo/get-tokens-status file-data) + kept-status (cfo/get-own-tokens-status file-data) - (= library-id (:id file-data)) - (own-tokens-status file-data) + tokens-status + (cond + (and to-own? (some? kept-status)) + (own-tokens-status file-data kept-status) - :else - (-> library ctf/file-data cfo/get-tokens-status))] - (-> changes - (pcb/set-tokens-source library-id) - (pcb/set-tokens-status library-tokens-status)))) + (nil? library) + (ctos/make-tokens-status) + + to-own? + (own-tokens-status file-data current-status) + + :else + (-> library ctf/file-data cfo/get-tokens-status)) + + kept-status' + (cond + to-own? nil + from-own? (own-tokens-status file-data current-status) + :else kept-status)] + + (cond-> (-> changes + (pcb/set-tokens-source library-id) + (pcb/set-tokens-status tokens-status)) + (not (identical? kept-status kept-status')) + (pcb/set-own-tokens-status kept-status')))) ;; Tokens lib diff --git a/common/src/app/common/test_helpers/tokens.cljc b/common/src/app/common/test_helpers/tokens.cljc index 23a0550a4a..88923c691b 100644 --- a/common/src/app/common/test_helpers/tokens.cljc +++ b/common/src/app/common/test_helpers/tokens.cljc @@ -30,6 +30,10 @@ [file] (-> file (ctf/file-data) (cfo/get-tokens-status))) +(defn get-own-tokens-status + [file] + (-> file (ctf/file-data) (cfo/get-own-tokens-status))) + (defn add-tokens-lib "Ensure the file has a tokens-lib and a tokens-status in its data, creating empty ones if not" [file] diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index df4733c25a..444484b68f 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -90,7 +90,8 @@ [:plugin-data {:optional true} schema:plugin-data] [:tokens-source {:optional true} ::sm/uuid] ;; The tokens-lib may be in this file or in an external library [:tokens-lib {:optional true} ctob/schema:tokens-lib] - [:tokens-status {:optional true} ctos/schema:tokens-status]]) + [:tokens-status {:optional true} ctos/schema:tokens-status] + [:own-tokens-status {:optional true} ctos/schema:tokens-status]]) ;; Kept aside while the tokens source is an external library (def schema:file-metadata [:map {:title "Metadata"} @@ -534,12 +535,13 @@ shape-meta (meta ref-shape) ref-file (:file shape-meta) ref-container (:container shape-meta)] - (when ref-shape + (if ref-shape (if-let [swap-slot (ctk/get-swap-slot ref-shape)] swap-slot (if (ctk/main-instance? ref-shape) (:id shape) - (find-swap-slot ref-shape ref-container ref-file libraries (conj viewed-ids (:id shape))))))))))) + (find-swap-slot ref-shape ref-container ref-file libraries (conj viewed-ids (:id shape))))) + (:id shape))))))) ;; If cannot locate ref-shape (e.g. in an unlinked external library), use what we have (defn match-swap-slot? [shape-main shape-inst container-inst container-main file libraries] @@ -681,7 +683,8 @@ (defn find-asset-type-usages "Find all usages of an asset in a file (may be in pages or in the components - of the local library). + of the local library, deleted ones included, since they keep their shapes + to be restored and to resolve their remaining copies). Returns a list ((asset ((container shapes) (container shapes)...))...)" [file-data library-data asset-type] @@ -697,12 +700,17 @@ (when (d/not-empty? instances) [[container instances]]))) + containers + (concat (containers-seq file-data) + (map #(ctn/make-container % :component) + (ctkl/deleted-components-seq file-data))) + find-asset-usages - (fn [file-data asset] - (mapcat #(find-usages-in-container % asset) (containers-seq file-data)))] + (fn [asset] + (mapcat #(find-usages-in-container % asset) containers))] (mapcat (fn [asset] - (let [instances (find-asset-usages file-data asset)] + (let [instances (find-asset-usages asset)] (when (d/not-empty? instances) [[asset instances]]))) assets))) @@ -1241,7 +1249,7 @@ (cond-> (update-in file [:data :pages-index] detach-pages) (not (cfo/effective-tokens-source? (:data file) file-id)) - (update :data dissoc :tokens-source :tokens-status)))) + (update :data cfo/detach-tokens-source)))) ;; Base font size diff --git a/common/test/common_tests/files/repair_test.cljc b/common/test/common_tests/files/repair_test.cljc index 27f6964de2..f6f24821ce 100644 --- a/common/test/common_tests/files/repair_test.cljc +++ b/common/test/common_tests/files/repair_test.cljc @@ -378,3 +378,171 @@ (t/is (= uuid/zero (:parent-id shape1'))) (t/is (= uuid/zero (:parent-id shape2'))) (t/is (= #{(thi/id :shape1) (thi/id :shape2)} (set (:shapes root'))))))) + +(defn- broken-frame-file + [] + (-> (thf/sample-file :file1 :page-label :page1) + (assoc :revn 3) + (ths/add-sample-shape :shape1) + (ths/update-shape :shape1 :frame-id (uuid/next)))) + +(t/deftest repair-file-until-valid-fixes-frame-not-found + (t/testing "repairs a :frame-not-found shape and leaves :revn untouched" + (let [file (broken-frame-file) + errors (cfv/validate-file file {}) + result (cfr/repair-file-until-valid + file {} + (fn [file _] (cfv/validate-file file {}))) + file' (:file result) + errors' (cfv/validate-file file' {}) + shape' (ths/get-shape file' :shape1 :page-label :page1)] + + (t/is (= #{:frame-not-found} (into #{} (map :code) errors))) + (t/is (pos? (count (:changes result)))) + (t/is (nil? (:errors result))) + (t/is (nil? errors')) + (t/is (= 3 (:revn file'))) + (t/is (some? (get-in file' [:data :pages-index (thi/id :page1) :objects (:frame-id shape')])))))) + +(t/deftest repair-file-until-valid-healthy-file + (t/testing "a valid file is returned unchanged" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (assoc :revn 2) + (ths/add-sample-shape :shape1)) + result (cfr/repair-file-until-valid + file {} + (fn [file _] (cfv/validate-file file {})))] + (t/is (identical? file (:file result))) + (t/is (= [] (:changes result))) + (t/is (zero? (:iterations result))) + (t/is (nil? (:errors result))) + (t/is (= 2 (:revn (:file result))))))) + +(t/deftest repair-file-until-valid-passes-applied-changes + (t/testing "errors-fn receives the changes applied so far" + (let [calls (atom []) + file (broken-frame-file) + result (cfr/repair-file-until-valid + file {} + (fn [file applied] + (swap! calls conj applied) + (cfv/validate-file file {})))] + (t/is (nil? (:errors result))) + (t/is (>= (count @calls) 2)) + (t/is (= [] (first @calls))) + (t/is (pos? (count (last @calls))))))) + +(t/deftest repair-file-until-valid-stops-on-noop-repair + (t/testing "an empty repair batch stops after one iteration" + (let [file (broken-frame-file) + errors (cfv/validate-file file {})] + (with-redefs [cfr/repair-file (fn [_ _ _] [])] + (let [result (cfr/repair-file-until-valid + file {} + (fn [file _] (cfv/validate-file file {})))] + (t/is (= 1 (:iterations result))) + (t/is (= [] (:changes result))) + (t/is (= (mapv :code errors) (mapv :code (:errors result))))))))) + +(t/deftest repair-file-until-valid-repairs-several-errors-on-one-shape + (t/testing "three missing children of one frame are all removed although + every pass reports the same [code shape-id]" + (let [file (-> (thf/sample-file :file1 :page-label :page1) + (ths/add-sample-shape :frame1 :type :frame) + (ths/update-shape :frame1 :shapes [(uuid/next) (uuid/next) (uuid/next)])) + errors (cfv/validate-file file {}) + result (cfr/repair-file-until-valid + file {} + (fn [file _] (cfv/validate-file file {}))) + frame1' (ths/get-shape (:file result) :frame1 :page-label :page1)] + (t/is (= [:child-not-found :child-not-found :child-not-found] (mapv :code errors))) + (t/is (= #{(thi/id :frame1)} (into #{} (map :shape-id) errors))) + (t/is (nil? (:errors result))) + (t/is (nil? (cfv/validate-file (:file result) {}))) + (t/is (= [] (:shapes frame1')))))) + +(t/deftest repair-file-until-valid-stops-when-repair-changes-nothing + (t/testing "a non-empty repair that leaves data equal stops after one iteration" + (let [file (broken-frame-file) + shape-id (thi/id :shape1) + page-id (thi/id :page1) + cur-name (:name (ths/get-shape file :shape1 :page-label :page1))] + (with-redefs [cfr/repair-file + (fn [_ _ _] + [{:type :mod-obj + :id shape-id + :page-id page-id + :operations [{:type :set :attr :name :val cur-name}]}])] + (let [result (cfr/repair-file-until-valid + file {} + (fn [_ _] [{:code :frame-not-found :shape-id shape-id}]) + :max-iterations 10)] + (t/is (= 1 (:iterations result))) + (t/is (= [] (:changes result))) + (t/is (= [:frame-not-found] (mapv :code (:errors result))))))))) + +(t/deftest repair-file-until-valid-stops-when-error-fingerprint-stable + (t/testing "a mutating repair with an unchanged error report stops after two iterations" + (let [file (broken-frame-file) + shape-id (thi/id :shape1) + page-id (thi/id :page1) + pass (atom 0) + sticky {:code :frame-not-found + :shape-id shape-id + :hint "stuck" + :args {:x 1}}] + (with-redefs [cfr/repair-file + (fn [_ _ _] + [{:type :mod-obj + :id shape-id + :page-id page-id + :operations [{:type :set + :attr :name + :val (str "pass-" (swap! pass inc))}]}])] + (let [result (cfr/repair-file-until-valid + file {} + (fn [_ _] [sticky]) + :max-iterations 10)] + (t/is (= 2 (:iterations result))) + (t/is (= 1 (count (:changes result)))) + (t/is (= [:frame-not-found] (mapv :code (:errors result))))))))) + +(t/deftest repair-file-until-valid-stops-at-max-iterations + (t/testing "a repair that never empties errors stops at max-iterations" + (let [file (broken-frame-file) + pass (atom 0) + shape-id (thi/id :shape1) + page-id (thi/id :page1)] + (with-redefs [cfr/repair-file + (fn [_ _ _] + [{:type :mod-obj + :id shape-id + :page-id page-id + :operations [{:type :set + :attr :name + :val (str "pass-" (swap! pass inc))}]}])] + (let [result (cfr/repair-file-until-valid + file {} + (fn [_ _] + [{:code :frame-not-found + :shape-id (uuid/next)}]) + :max-iterations 3)] + (t/is (= 3 (:iterations result))) + (t/is (= 3 (count (:changes result)))) + (t/is (some? (:errors result)))))))) + +(t/deftest repair-file-until-valid-uses-initial-errors + (t/testing "errors-fn is not called on the first iteration when :initial-errors is given" + (let [calls (atom 0) + file (broken-frame-file) + seed (cfv/validate-file file {}) + result (cfr/repair-file-until-valid + file {} + (fn [file applied] + (swap! calls inc) + (cfv/validate-file file {})) + :initial-errors seed)] + (t/is (nil? (:errors result))) + ;; First iteration used the seed; errors-fn runs only after the repair. + (t/is (= 1 @calls))))) + diff --git a/common/test/common_tests/logic/swap_missing_library_test.cljc b/common/test/common_tests/logic/swap_missing_library_test.cljc new file mode 100644 index 0000000000..64853a1a74 --- /dev/null +++ b/common/test/common_tests/logic/swap_missing_library_test.cljc @@ -0,0 +1,79 @@ +;; 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 SUBSIDIARY SL + +(ns common-tests.logic.swap-missing-library-test + (:require + [app.common.files.validate :as cfv] + [app.common.test-helpers.components :as thc] + [app.common.test-helpers.compositions :as tho] + [app.common.test-helpers.files :as thf] + [app.common.test-helpers.ids-map :as thi] + [app.common.test-helpers.shapes :as ths] + [clojure.test :as t])) + +(t/use-fixtures :each thi/test-fixture) + +(defn- setup-library [] + ;; {:frame-a} [:name Frame1] # [Component :component-a] + ;; :child-a [:name Rect1] + (-> (thf/sample-file :library) + (tho/add-simple-component :component-a :frame-a :child-a + :root-params {:name "component-a"} + :child-params {:name "child-a" + :type :rect + :fills (ths/sample-fills-color :fill-color "#111111")}))) + +(defn- setup-file [library] + ;; {:frame-b} [:name Frame1] # [Component :component-b] + ;; :copy-a [:name component-a] @--> [Component :component-a] (in library) :frame-a + ;; [:name child-a] ---> :child-a + ;; + ;; :copy-b [:name Frame1] #--> [Component :component-b] :frame-b + ;; [:name component-a] @--> [Component :component-a] (in library) :copy-a + ;; [:name child-a] ---> + ;; + ;; {:frame-c} [:name Frame1] # [Component :component-c] + ;; :child-c [:name Rect1] + (-> (thf/sample-file :file) + (tho/add-frame :frame-b) + (thc/instantiate-component :component-a :copy-a + :parent-label :frame-b + :library library) + (thc/make-component :component-b :frame-b) + (thc/instantiate-component :component-b :copy-b) + (tho/add-simple-component :component-c :frame-c :child-c + :root-params {:name "component-c"} + :child-params {:name "child-c" + :type :rect + :fills (ths/sample-fills-color :fill-color "#333333")}))) + +(defn- propagate-all-component-changes [file] + (-> file + (tho/propagate-component-changes :component-c) + (tho/propagate-component-changes :component-b))) + +(defn- fill-colors [file] + [(tho/bottom-fill-color file :frame-b) + (tho/bottom-fill-color file :copy-b)]) + +(t/deftest test-swap-nested-copy-with-main-in-missing-library + (let [library (setup-library) + file (setup-file library) + + ;; The library is not passed to the swap, so the main of A is not + ;; available. + file' (tho/swap-component-in-first-child file :copy-b :component-c)] + + (cfv/validate-file! file' {(:id library) library}) + + (t/testing "after the swap, B and its copy contain the new copy of C" + (t/is (= ["#111111" "#333333"] (fill-colors file')))) + + (t/testing "changes in the main of C propagate to B and its copy" + (let [file'' (tho/update-bottom-color file' :frame-c "#444444" + :propagate-fn propagate-all-component-changes)] + + (t/is (= ["#111111" "#444444"] (fill-colors file''))))))) diff --git a/common/test/common_tests/logic/token_test.cljc b/common/test/common_tests/logic/token_test.cljc index 799d895f44..f9b11e2fb3 100644 --- a/common/test/common_tests/logic/token_test.cljc +++ b/common/test/common_tests/logic/token_test.cljc @@ -115,6 +115,54 @@ (t/is (= (ctos/get-active-set-ids file-tokens-status) (ctos/get-active-set-ids undo-tokens-status))))) + (t/testing "the file's own tokens status is kept aside while a library is the source, and restored when going back" + (let [own-set-id (uuid/next) + lib-set-id (uuid/next) + lib2-set-id (uuid/next) + library-file (tht/sample-file-with-tokens + :file-id :library + :lib-fn #(ctob/add-set % (ctob/make-token-set :id lib-set-id :name "library set")) + :status-fn #(ctos/set-tokens-status % #{} #{lib-set-id})) + library2-file (tht/sample-file-with-tokens + :file-id :library2 + :lib-fn #(ctob/add-set % (ctob/make-token-set :id lib2-set-id :name "library2 set")) + :status-fn #(ctos/set-tokens-status % #{} #{lib2-set-id})) + file (tht/sample-file-with-tokens + :file-id :file1 + :lib-fn #(ctob/add-set % (ctob/make-token-set :id own-set-id :name "own set")) + :status-fn #(ctos/set-tokens-status % #{} #{own-set-id})) + set-source (fn [file library] + (let [changes (-> (pcb/empty-changes) + (pcb/with-library-data (ctf/file-data file)) + (clt/generate-set-tokens-source library))] + [(thf/apply-changes file changes) changes])) + active-sets #(some-> % ctos/get-active-set-ids) + + [to-library to-library-changes] (set-source file library-file) + [to-library2 _] (set-source to-library library2-file) + [back-to-own back-changes] (set-source to-library2 to-library2) + undo-to-library (thf/apply-undo-changes to-library to-library-changes) + undo-back-to-own (thf/apply-undo-changes back-to-own back-changes)] + + ;; A library as source: its status is used and the own status is kept aside + (t/is (= #{lib-set-id} (active-sets (tht/get-tokens-status to-library)))) + (t/is (= #{own-set-id} (active-sets (tht/get-own-tokens-status to-library)))) + + ;; Another library as source: the own status is still kept aside + (t/is (= #{lib2-set-id} (active-sets (tht/get-tokens-status to-library2)))) + (t/is (= #{own-set-id} (active-sets (tht/get-own-tokens-status to-library2)))) + + ;; Back to the file itself: the own status is restored, nothing is kept aside + (t/is (nil? (tht/get-tokens-source back-to-own))) + (t/is (= #{own-set-id} (active-sets (tht/get-tokens-status back-to-own)))) + (t/is (nil? (tht/get-own-tokens-status back-to-own))) + + ;; Undo works both ways + (t/is (= #{own-set-id} (active-sets (tht/get-tokens-status undo-to-library)))) + (t/is (nil? (tht/get-own-tokens-status undo-to-library))) + (t/is (= #{lib2-set-id} (active-sets (tht/get-tokens-status undo-back-to-own)))) + (t/is (= #{own-set-id} (active-sets (tht/get-own-tokens-status undo-back-to-own)))))) + (t/testing "if the library is nil, the tokens source is cleared and the tokens status is reset to empty" (let [set-id (uuid/next) file (tht/sample-file-with-tokens diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index e78fbf3da9..884c9a7356 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -72,6 +72,7 @@ [common-tests.logic.swap-and-reset-test] [common-tests.logic.swap-as-override-test] [common-tests.logic.swap-keeps-id-test] + [common-tests.logic.swap-missing-library-test] [common-tests.logic.text-sync-test] [common-tests.logic.text-touched-test] [common-tests.logic.token-apply-test] @@ -178,6 +179,7 @@ 'common-tests.logic.swap-and-reset-test 'common-tests.logic.swap-as-override-test 'common-tests.logic.swap-keeps-id-test + 'common-tests.logic.swap-missing-library-test 'common-tests.logic.text-sync-test 'common-tests.logic.text-touched-test 'common-tests.logic.token-apply-test diff --git a/common/test/common_tests/types/absorb_assets_test.cljc b/common/test/common_tests/types/absorb_assets_test.cljc index 8e8ee2ee36..6d2e46e0db 100644 --- a/common/test/common_tests/types/absorb_assets_test.cljc +++ b/common/test/common_tests/types/absorb_assets_test.cljc @@ -7,6 +7,9 @@ (ns common-tests.types.absorb-assets-test (:require [app.common.data :as d] + [app.common.files.changes-builder :as pcb] + [app.common.files.validate :as cfv] + [app.common.logic.shapes :as cls] [app.common.test-helpers.components :as thc] [app.common.test-helpers.compositions :as tho] [app.common.test-helpers.files :as thf] @@ -57,6 +60,50 @@ (t/is (ctk/is-main-of? main-root' copy-root')) (t/is (ctk/main-instance-of? (:id main-root') (:id (second pages')) component')))) +(t/deftest absorb-components-used-in-deleted-components + ;; A deleted component keeps its shapes in `:objects`, to be able to + ;; restore it and to resolve its remaining copies. When those shapes + ;; use library components, they must also be remapped to the local + ;; file, or they will not match the copies in the pages. + (let [;; Setup + library (-> (thf/sample-file :library :is-shared true) + (tho/add-simple-component :icon :icon-main :icon-rect)) + + file (-> (thf/sample-file :file) + (tho/add-frame :card-main :name "Card") + (thc/instantiate-component :icon :card-icon + :library library + :parent-label :card-main) + (thc/make-component :card :card-main) + (thc/instantiate-component :card :card-copy)) + + page (thf/current-page file) + changes (-> (pcb/empty-changes nil (:id page)) + (cls/generate-delete-shapes file page (:objects page) + #{(thi/id :card-main)} {}) + (second)) + file (thf/apply-changes file changes :validate? false) + errors (cfv/validate-file file {(:id library) library}) + + ;; Action + file' (ctf/update-file-data + file + #(ctf/absorb-assets % (:data library))) + + ;; Get + data' (ctf/file-data file') + card' (ctkl/get-deleted-component data' (thi/id :card)) + shapes' (concat (mapcat (comp vals :objects) (ctpl/pages-seq data')) + (mapcat (comp vals :objects) (vals (:components data')))) + library-refs' (filter #(= (thi/id :library) (:component-file %)) shapes') + errors' (cfv/validate-file file' {})] + + ;; Check + (t/is (nil? errors) "the file is valid before absorbing the library") + (t/is (some? (:objects card')) "the deleted component keeps its shapes") + (t/is (empty? library-refs') "no shape points to the library") + (t/is (nil? errors')))) + (t/deftest absorb-colors (let [;; Setup library (-> (thf/sample-file :library :is-shared true) diff --git a/docs/user-guide/first-steps/shortcuts.njk b/docs/user-guide/first-steps/shortcuts.njk index ab8279486a..e80d59f025 100644 --- a/docs/user-guide/first-steps/shortcuts.njk +++ b/docs/user-guide/first-steps/shortcuts.njk @@ -35,9 +35,9 @@ desc: Get quickstart tips, shortcuts, and tutorials for Penpot! Learn interface

Paste cannot be customized.

Keyboard layouts

-

Letters are recorded by the character they type, so they follow your layout (on AZERTY, the A key records a). With Shift, digit keys record the digit itself (Shift0, not the shifted symbol), on every layout and in every browser.

-

Punctuation keys (such as -, [, / or ') are recorded by their physical position on the keyboard, so the same key works on AZERTY, QWERTZ and other layouts — but only in browsers that expose the keyboard layout (Chromium). There, Shift with the + key records Shift+. In browsers without that support, such as Firefox or Safari, punctuation falls back to the typed character (Shift* instead): those shortcuts still work in the browser where you recorded them, but Shift combinations with punctuation may stop working if you switch browsers. If that affects you, record those shortcuts in a Chromium browser.

-

Two known limitations: number keys set opacity (0–9) only where digits are typed without Shift, so AZERTY layouts need the numeric keypad for them; and \ needs AltGr on some layouts (Spanish), where it is recorded with Alt and its exact form depends on the browser.

+

Letters are recorded by the character they type, so they follow your layout (on AZERTY, the A key records a). With Shift, Alt or another modifier, letter and digit keys record the key itself, not the character the modifier makes (Shift0, not Shift); OptionM on macOS, not µ).

+

Punctuation keys (such as -, [, / or ') are recorded by their physical position on the keyboard, so Shift with the + key records Shift+ on a Spanish layout. In browsers that do not expose the keyboard layout, such as Firefox or Safari, Penpot records the character you type instead; the shortcut still works there. Keys typed with AltGr record the character they type (\ on a Spanish layout).

+

One known limitation: number keys set opacity (0–9) only where digits are typed without Shift, so AZERTY layouts need the numeric keypad for them.

Reset to default

When you have custom shortcuts, a Reset all button appears next to the search field. It lists every shortcut that will go back to its default before you confirm.

@@ -617,11 +617,6 @@ desc: Get quickstart tips, shortcuts, and tutorials for Penpot! Learn interface - - Add node - Shift+ - ⇧+ - Delete node, segment or handle Delete or Backspace diff --git a/frontend/packages/mousetrap/index.js b/frontend/packages/mousetrap/index.js index e3864396a4..3166b24404 100644 --- a/frontend/packages/mousetrap/index.js +++ b/frontend/packages/mousetrap/index.js @@ -94,8 +94,8 @@ const _SPECIAL_ALIASES = { /** * Physical codes treated positionally. Letters and digits resolve by * glyph (`event.key`), but punctuation resolves by position (`event.code`) - * through the layout map, so the same physical key matches on every - * layout with and without `Shift`. Without a map the typed glyph applies. + * through the layout map, or `_US_PUNCTUATION` without one, so the same + * physical key matches with and without `Shift`. See `_candidates`. * * @type {Set} */ @@ -113,6 +113,45 @@ const _PUNCTUATION_CODES = new Set([ "Slash", ]); +/** + * Primary character of each punctuation key on a US layout. Used when + * the browser has no layout map (Firefox, Safari), so punctuation still + * resolves by position and every browser names a key the same way. + * + * @type {Object} + */ +const _US_PUNCTUATION = { + Minus: "-", + Equal: "=", + BracketLeft: "[", + BracketRight: "]", + Backslash: "\\", + Semicolon: ";", + Quote: "'", + Backquote: "`", + Comma: ",", + Period: ".", + Slash: "/", +}; + +/** + * Letter or digit named by `keyCode`. With a modifier held, `event.key` + * carries the glyph the modifier makes (`!` for Shift+1, `µ` for + * Option+M on macOS, or `Dead`), while `keyCode` still names the key. + * Unlike `event.code` it follows the layout, so Ctrl+Z stays `z` on + * AZERTY. + * + * @param {Event} e + * @return {string|null} + */ +function _letterOrDigitFromKeyCode(e) { + const keyCode = e.keyCode; + if ((keyCode >= 48 && keyCode <= 57) || (keyCode >= 65 && keyCode <= 90)) { + return String.fromCharCode(keyCode).toLowerCase(); + } + return null; +} + /** * Resolves an event to its canonical key. Implemented as * `Mousetrap#_characterFromEvent` so punctuation can consult the @@ -130,27 +169,38 @@ function _modifiersMatch(modifiers1, modifiers2) { return modifiers1.sort().join(",") === modifiers2.sort().join(","); } +/** + * whether the `AltGraph` modifier is active for the event + * + * @param {Event} e + * @returns {boolean} + */ +function _isAltGraph(e) { + return ( + typeof e.getModifierState === "function" && e.getModifierState("AltGraph") + ); +} + /** * takes a key event and figures out what the modifiers are * * On Windows `AltGr` reports `ctrlKey + altKey` as well. When the - * `AltGraph` modifier is active the phantom `ctrl` is dropped so typing - * (e.g. `]` on Spanish keyboards) does not fire `ctrl+alt` shortcuts. + * `AltGraph` modifier is active both phantom modifiers are dropped, so + * typing (e.g. `]` or `\` on Spanish keyboards) neither fires `ctrl+alt` + * shortcuts nor hides the plain glyph binding. * * @param {Event} e * @returns {Array} */ function _eventModifiers(e) { const modifiers = []; - const altGraph = - typeof e.getModifierState === "function" && - e.getModifierState("AltGraph"); + const altGraph = _isAltGraph(e); if (e.shiftKey) { modifiers.push("shift"); } - if (e.altKey) { + if (e.altKey && !altGraph) { modifiers.push("alt"); } @@ -383,22 +433,209 @@ class Mousetrap { } /** - * takes the event and returns the canonical key + * drops the layout map when it does not describe the active layout * - * Letters, digits and named keys resolve by glyph: single characters - * lowercase so `Shift+Z` matches a `ctrl+shift+z` bind, `" "` maps to - * `space`, and named keys map through `_KEY_NAMES`. The exception is a - * digit row key held with `Shift`, which resolves to the digit itself - * (`Digit0` types ")" on US layouts but "0" on AZERTY, so `shift+0` - * must mean the physical digit key everywhere). Punctuation - * resolves positionally via the layout map (see `setLayoutMap`), - * falling back to the glyph when no map is set. Numpad digits stay - * positional (`num0`..`num9`) so they keep their own bindings. + * Chrome on Linux returns the map of the first configured layout, not + * the active one: with Spanish first and US active, the map names the + * `=` key `¡`. A punctuation key pressed without modifiers types the + * primary character of its position, so when that differs from the + * map, punctuation goes back to the glyph and US-table fallback for the + * rest of the session. + * + * @param {Event} e + * @returns void + */ + _checkLayoutMap(e) { + if ( + !this._layoutMap || + e.type !== "keydown" || + !e.code || + !_PUNCTUATION_CODES.has(e.code) || + typeof e.key !== "string" || + e.key.length !== 1 || + e.shiftKey || + e.ctrlKey || + e.altKey || + e.metaKey || + _isAltGraph(e) + ) { + return; + } + + let mapped = null; + try { + mapped = this._layoutMap.get(e.code); + } catch { + mapped = null; + } + + if ( + typeof mapped === "string" && + mapped !== "" && + mapped.toLowerCase() !== e.key.toLowerCase() + ) { + this._layoutMap = null; + } + } + + /** + * takes the event and returns the canonical key: the first candidate + * of `_candidates`, which is also what the shortcut recorder stores + * (see `keyForRecording`). * * @param {Event} e * @return {string|null} */ _characterFromEvent(e) { + const candidates = this._candidates(e); + return candidates.length ? candidates[0][0] : null; + } + + /** + * public entry point for the shortcut recorder: the key token to store + * for this event, always one of `_candidates`, so this instance matches + * the recorded shortcut. Returns `null` for modifiers alone. + * + * Letters and digits with a modifier use `keyCode`. Punctuation uses + * its position when there is a layout map, since the map names the + * printed key. Without a map (Firefox, Safari) the typed glyph is a + * better label than the US key at that position, so the Spanish `+` + * records `+`, not `]`. With AltGr the typed glyph wins too: AltGr+º + * on a Spanish layout records `\`, the character the user typed. + * + * @param {Event} e + * @return {string|null} + */ + keyForRecording(e) { + this._checkLayoutMap(e); + + const modifiers = _eventModifiers(e); + const numpad = e.code && e.code.indexOf("Numpad") === 0; + const keyCodeKey = + modifiers.length && !numpad ? _letterOrDigitFromKeyCode(e) : null; + const position = this._positionFromEvent(e); + const glyph = this._glyphFromEvent(e); + + let key; + if (keyCodeKey) { + key = keyCodeKey; + } else if (this._layoutMap && !_isAltGraph(e)) { + key = position || glyph; + } else { + key = glyph || position; + } + + return key && !_isModifier(key) ? key : null; + } + + /** + * the `[key, modifiers]` pairs an event may match, most specific + * first. `_handleKeyEvent` dispatches the first pair that has a + * binding: + * + * 1. With a modifier held, letters and digits by `keyCode`, so + * `Shift+1` is `shift+1` and `Option+M` is `alt+m`. + * 2. Punctuation by position: the primary character from the layout + * map (see `setLayoutMap`), or from a US table when the browser has + * no map, so `Shift+*` on a Spanish layout is `shift++`. + * 3. The typed glyph (`event.key`), so `+` on a US layout (typed with + * Shift) matches a `shift++` binding. + * 4. The typed glyph without the Shift it needed, so `?` and `+` + * bindings fire. + * + * With AltGr, 3 comes before 2 (see `keyForRecording`). + * + * @param {Event} e + * @return {Array} + */ + _candidates(e) { + const modifiers = _eventModifiers(e); + const candidates = []; + const add = (key, mods) => { + if (key) { + candidates.push([key, mods]); + } + }; + + const numpad = e.code && e.code.indexOf("Numpad") === 0; + if (modifiers.length && !numpad) { + add(_letterOrDigitFromKeyCode(e), modifiers); + } + + // With AltGr the typed glyph comes before the position: AltGr+º + // types `\` on a Spanish layout, and the key at that position is `º`. + const altGraph = _isAltGraph(e); + const position = this._positionFromEvent(e); + const glyph = this._glyphFromEvent(e); + if (altGraph) { + add(glyph, modifiers); + add(position, modifiers); + } else { + add(position, modifiers); + add(glyph, modifiers); + } + + if ( + glyph && + glyph.length === 1 && + modifiers.includes("shift") && + !/[a-z0-9]/.test(glyph) + ) { + add( + glyph, + modifiers.filter((m) => m !== "shift"), + ); + } + + // On Windows Ctrl+Alt is AltGr: Ctrl+Alt+E types `€` on a Spanish + // layout. When nothing is bound to the glyph, fall back to the + // `ctrl+alt` binding of the key. + if (altGraph && e.ctrlKey && e.altKey && !numpad) { + add(_letterOrDigitFromKeyCode(e), [...modifiers, "alt", "ctrl"]); + } + + return candidates; + } + + /** + * primary character of a punctuation key by its position, or `null` + * + * @param {Event} e + * @return {string|null} + */ + _positionFromEvent(e) { + if (!e.code || !_PUNCTUATION_CODES.has(e.code)) { + return null; + } + + let mapped = null; + if (this._layoutMap) { + try { + mapped = this._layoutMap.get(e.code); + } catch { + mapped = null; + } + } + + if (typeof mapped !== "string" || mapped === "") { + mapped = _US_PUNCTUATION[e.code]; + } + + return mapped ? mapped.toLowerCase() : null; + } + + /** + * the typed glyph of the event as a canonical key + * + * Single characters lowercase so `Shift+Z` matches a `ctrl+shift+z` + * bind, `" "` maps to `space`, and named keys map through + * `_KEY_NAMES`. Numpad digits stay positional (`num0`..`num9`) so they + * keep their own bindings. + * + * @param {Event} e + * @return {string|null} + */ + _glyphFromEvent(e) { // Numpad digits as "num0".."num9" — keeps them separate from main-row bindings across NumLock states and event types. if (e.code && e.code.indexOf("Numpad") === 0) { const suffix = e.code.substring(6); @@ -407,35 +644,11 @@ class Mousetrap { } } - // Punctuation is positional: with a layout map the primary - // (unshifted) character of the physical key wins, so `Shift+*` on a - // Spanish layout (code `BracketRight`) resolves to `+` like the - // recorder stores. Without a map the typed glyph applies. - if (e.code && _PUNCTUATION_CODES.has(e.code) && this._layoutMap) { - let mapped = null; - try { - mapped = this._layoutMap.get(e.code); - } catch { - mapped = null; - } - if (typeof mapped === "string" && mapped !== "") { - return mapped.toLowerCase(); - } - } - const key = e.key; if (typeof key !== "string" || key === "") { return null; } - // Shifted digits resolve to the digit itself: with `Shift` held, - // `Digit0` types ")" on US layouts but "0" on AZERTY, so the glyph - // can't be the token. `shift+0` means the physical digit key - // everywhere, like `shift+z` means `z` regardless of `Shift`. - if (e.shiftKey && e.code && /^Digit[0-9]$/.test(e.code) && key.length === 1) { - return e.code.charAt(5); - } - if (_KEY_NAMES[key]) { return _KEY_NAMES[key]; } @@ -862,18 +1075,35 @@ class Mousetrap { * @returns void */ _handleKeyEvent(e) { - const character = this._characterFromEvent(e); + this._checkLayoutMap(e); + + const candidates = this._candidates(e); // no character found then stop - if (!character) { + if (!candidates.length) { return; } + // dispatch the first candidate that has a binding, or the first one + // so sequences still reset on unbound keys + let picked = candidates[0]; + for (const candidate of candidates) { + if ( + this._callbacks[candidate[0]] && + this._getMatches(candidate[0], candidate[1], e).length + ) { + picked = candidate; + break; + } + } + + const [character, modifiers] = picked; + if (e.type == "keyup" && this._ignoreNextKeyup === character) { this._ignoreNextKeyup = false; return; } - this.handleKey(character, _eventModifiers(e), e); + this.handleKey(character, modifiers, e); } } diff --git a/frontend/packages/mousetrap/test/bind-dispatch.test.js b/frontend/packages/mousetrap/test/bind-dispatch.test.js index 7512151126..6ae0eb45a2 100644 --- a/frontend/packages/mousetrap/test/bind-dispatch.test.js +++ b/frontend/packages/mousetrap/test/bind-dispatch.test.js @@ -36,11 +36,15 @@ describe("bind and dispatch", () => { shiftPlus++; }); fire(target, makeKeyEvent({ key: "+", code: "Equal" })); + // Without a layout map Shift + the Spanish "+" key types "*" and + // nothing says the key is "+", so "shift++" does not fire. fire(target, makeKeyEvent({ key: "*", code: "BracketRight", shiftKey: true })); - // "*" normalizes by glyph, so "shift++" (shift + "+" key) does not fire; - // this pins the layout-independent behavior for the "+" physical key. assert.equal(plus, 1); assert.equal(shiftPlus, 0); + // A layout map names the key, so it does. + mousetrap.setLayoutMap({ get: (code) => (code === "BracketRight" ? "+" : undefined) }); + fire(target, makeKeyEvent({ key: "*", code: "BracketRight", shiftKey: true })); + assert.equal(shiftPlus, 1); }); it("resolves bind-string aliases to canonical tokens", () => { diff --git a/frontend/packages/mousetrap/test/layout-map.test.js b/frontend/packages/mousetrap/test/layout-map.test.js index 3cb34eb722..e3b914932d 100644 --- a/frontend/packages/mousetrap/test/layout-map.test.js +++ b/frontend/packages/mousetrap/test/layout-map.test.js @@ -23,7 +23,16 @@ describe("layout map", () => { it("resolves punctuation by position, so Shift+* fires a shift++ bind", () => { const event = { key: "*", code: "BracketRight", shiftKey: true }; assert.equal(firedWithMap(spanishMap, "shift++", event), 1); - assert.equal(firedWithMap(spanishMap, "shift+*", event), 0); + }); + + it("prefers the position over the glyph when both are bound", () => { + const { mousetrap, target } = freshInstance(); + mousetrap.setLayoutMap(spanishMap); + const calls = []; + mousetrap.bind("shift++", () => calls.push("shift++")); + mousetrap.bind("shift+*", () => calls.push("shift+*")); + fire(target, makeKeyEvent({ key: "*", code: "BracketRight", shiftKey: true })); + assert.deepEqual(calls, ["shift++"]); }); it("matches the unshifted primary on the same position", () => { @@ -92,4 +101,74 @@ describe("layout map", () => { fire(target, makeKeyEvent({ key: "a", code: "KeyQ" })); assert.equal(calls, 1); }); + + // Spanish layout as Chrome reports it: the "'" key sits on Minus and + // "º" on Backquote. + const fullSpanishMap = { + get(code) { + return { BracketRight: "+", Minus: "'", Backquote: "º", Equal: "¡" }[code]; + }, + }; + + const altGraph = (over) => ({ + getModifierState: (name) => name === "AltGraph", + ...over, + }); + + it("fires ? on Spanish, where it is Shift + the ' key", () => { + const event = { key: "?", code: "Minus", shiftKey: true }; + assert.equal(firedWithMap(fullSpanishMap, "?", event), 1); + }); + + it("matches the AltGr glyph before the key position (Linux)", () => { + // AltGr+º types "\" with no ctrl or alt on Linux. + const { mousetrap, target } = freshInstance(); + mousetrap.setLayoutMap(fullSpanishMap); + const calls = []; + mousetrap.bind("\\", () => calls.push("\\")); + mousetrap.bind("º", () => calls.push("º")); + fire(target, makeKeyEvent(altGraph({ key: "\\", code: "Backquote" }))); + assert.deepEqual(calls, ["\\"]); + }); + + it("matches the AltGr glyph when AltGr reports ctrl+alt (Windows)", () => { + const event = altGraph({ key: "\\", code: "Backquote", ctrlKey: true, altKey: true }); + assert.equal(firedWithMap(fullSpanishMap, "\\", event), 1); + }); + + it("records the AltGr glyph, not the key position", () => { + const { mousetrap } = freshInstance(); + mousetrap.setLayoutMap(fullSpanishMap); + const event = makeKeyEvent(altGraph({ key: "\\", code: "Backquote" })); + assert.equal(mousetrap.keyForRecording(event), "\\"); + }); + + it("drops a map that disagrees with an unmodified punctuation key", () => { + // Chrome on Linux keeps the map of the first layout (Spanish) while + // US is active: the "=" key types "=" but the map says "¡". + const { mousetrap, target } = freshInstance(); + mousetrap.setLayoutMap(fullSpanishMap); + const calls = []; + mousetrap.bind("=", () => calls.push("=")); + mousetrap.bind("-", () => calls.push("-")); + fire(target, makeKeyEvent({ key: "=", code: "Equal" })); + fire(target, makeKeyEvent({ key: "-", code: "Minus" })); + assert.deepEqual(calls, ["=", "-"]); + assert.equal(mousetrap._layoutMap, null); + }); + + it("drops a disagreeing map before recording", () => { + const { mousetrap } = freshInstance(); + mousetrap.setLayoutMap(fullSpanishMap); + assert.equal(mousetrap.keyForRecording(makeKeyEvent({ key: "=", code: "Equal" })), "="); + }); + + it("keeps a map that agrees with the typed key", () => { + const { mousetrap, target } = freshInstance(); + mousetrap.setLayoutMap(fullSpanishMap); + fire(target, makeKeyEvent({ key: "+", code: "BracketRight" })); + fire(target, makeKeyEvent({ key: "*", code: "BracketRight", shiftKey: true })); + fire(target, makeKeyEvent({ key: "Dead", code: "Quote" })); + assert.equal(mousetrap._layoutMap, fullSpanishMap); + }); }); diff --git a/frontend/packages/mousetrap/test/modifiers.test.js b/frontend/packages/mousetrap/test/modifiers.test.js index 748b71feeb..a12453821c 100644 --- a/frontend/packages/mousetrap/test/modifiers.test.js +++ b/frontend/packages/mousetrap/test/modifiers.test.js @@ -31,7 +31,7 @@ describe("modifiers", () => { assert.equal(fired("shift+ctrl+alt+q", event), 1); }); - it("drops the phantom ctrl while AltGraph is active", () => { + it("drops the phantom ctrl and alt while AltGraph is active", () => { const altgr = { key: "]", code: "BracketRight", @@ -40,7 +40,38 @@ describe("modifiers", () => { getModifierState: (mod) => mod === "AltGraph", }; assert.equal(fired("ctrl+alt+]", altgr), 0); - assert.equal(fired("alt+]", altgr), 1); + assert.equal(fired("alt+]", altgr), 0); + assert.equal(fired("]", altgr), 1); + }); + + it("falls back to ctrl+alt for a letter that AltGr turns into a glyph", () => { + // Spanish layout on Windows: Ctrl+Alt+E types "€". + const event = { + key: "€", + code: "KeyE", + keyCode: 69, + ctrlKey: true, + altKey: true, + getModifierState: (mod) => mod === "AltGraph", + }; + assert.equal(fired("ctrl+alt+e", event), 1); + assert.equal(fired("€", event), 1); + }); + + it("matches letters and digits by keyCode when a modifier changes the glyph", () => { + // Shift+1 types "!" on US and Spanish layouts. + assert.equal(fired("shift+1", { key: "!", code: "Digit1", keyCode: 49, shiftKey: true }), 1); + // Spanish layout: Shift+0 types "=". + assert.equal(fired("shift+0", { key: "=", code: "Digit0", keyCode: 48, shiftKey: true }), 1); + // macOS: Option+M types "µ" and Option+E is a dead key. + assert.equal(fired("alt+m", { key: "µ", code: "KeyM", keyCode: 77, altKey: true }), 1); + assert.equal(fired("alt+e", { key: "Dead", code: "KeyE", keyCode: 69, altKey: true }), 1); + assert.equal(fired("alt+shift+5", { key: "%", code: "Digit5", keyCode: 53, altKey: true, shiftKey: true }), 1); + }); + + it("keeps AZERTY letters by layout, not by position", () => { + // AZERTY: the key at the US "W" position types "z" and has keyCode 90. + assert.equal(fired("ctrl+z", { key: "z", code: "KeyW", keyCode: 90, ctrlKey: true }), 1); }); it("keeps a real ctrl+alt without AltGraph", () => { diff --git a/frontend/packages/mousetrap/test/normalize.test.js b/frontend/packages/mousetrap/test/normalize.test.js index b91b5ff40d..3219a08458 100644 --- a/frontend/packages/mousetrap/test/normalize.test.js +++ b/frontend/packages/mousetrap/test/normalize.test.js @@ -54,30 +54,54 @@ describe("canonical key normalization", () => { it("resolves a shifted digit row key to the digit itself", () => { // US Shift+0 types ")", AZERTY Shift+0 types "0": shift+0 must mean - // the physical digit key on every layout (production keyCode behavior). - assert.equal(fired("shift+0", { key: ")", code: "Digit0", shiftKey: true }), 1); - assert.equal(fired("shift+1", { key: "!", code: "Digit1", shiftKey: true }), 1); - assert.equal(fired("shift+2", { key: "@", code: "Digit2", shiftKey: true }), 1); - assert.equal(fired("shift+8", { key: "*", code: "Digit8", shiftKey: true }), 1); - // ...so the shifted glyphs are not bindable tokens for digit codes. - assert.equal(fired("shift+)", { key: ")", code: "Digit0", shiftKey: true }), 0); - assert.equal(fired("shift+*", { key: "*", code: "Digit8", shiftKey: true }), 0); + // the physical digit key on every layout, named by keyCode. + assert.equal(fired("shift+0", { key: ")", code: "Digit0", keyCode: 48, shiftKey: true }), 1); + assert.equal(fired("shift+1", { key: "!", code: "Digit1", keyCode: 49, shiftKey: true }), 1); + assert.equal(fired("shift+2", { key: "@", code: "Digit2", keyCode: 50, shiftKey: true }), 1); + assert.equal(fired("shift+8", { key: "*", code: "Digit8", keyCode: 56, shiftKey: true }), 1); // AZERTY shifted digits already type the digit: unchanged. - assert.equal(fired("shift+0", { key: "0", code: "Digit0", shiftKey: true }), 1); + assert.equal(fired("shift+0", { key: "0", code: "Digit0", keyCode: 48, shiftKey: true }), 1); // Unshifted digits are untouched. - assert.equal(fired("0", { key: "0", code: "Digit0" }), 1); - assert.equal(fired("&", { key: "&", code: "Digit1" }), 1); + assert.equal(fired("0", { key: "0", code: "Digit0", keyCode: 48 }), 1); }); - it("without a layout map, punctuation resolves by glyph", () => { + it("prefers the digit over the shifted glyph when both are bound", () => { + const { mousetrap, target } = freshInstance(); + const calls = []; + mousetrap.bind("shift+0", () => calls.push("shift+0")); + mousetrap.bind("shift+)", () => calls.push("shift+)")); + fire(target, makeKeyEvent({ key: ")", code: "Digit0", keyCode: 48, shiftKey: true })); + assert.deepEqual(calls, ["shift+0"]); + }); + + it("without a layout map, punctuation resolves by US position or by glyph", () => { // Spanish layout: Shift + the physical "+" key (BracketRight) types "*". + assert.equal(fired("shift+]", { key: "*", code: "BracketRight", shiftKey: true }), 1); assert.equal(fired("shift+*", { key: "*", code: "BracketRight", shiftKey: true }), 1); - assert.equal(fired("shift+]", { key: "*", code: "BracketRight", shiftKey: true }), 0); assert.equal(fired("+", { key: "+", code: "BracketRight" }), 1); - // US layout: Shift + BracketRight types "}". + // US layout: Shift + BracketRight types "}"; the glyph still matches. assert.equal(fired("shift+}", { key: "}", code: "BracketRight", shiftKey: true }), 1); - assert.equal(fired("shift+]", { key: "}", code: "BracketRight", shiftKey: true }), 0); assert.equal(fired("?", { key: "?", code: "Slash" }), 1); + // US layout: Shift + Comma types "<". + assert.equal(fired("shift+,", { key: "<", code: "Comma", shiftKey: true }), 1); + assert.equal(fired("ctrl+shift+.", { key: ">", code: "Period", ctrlKey: true, shiftKey: true }), 1); + }); + + it("matches a glyph binding without the Shift the glyph needs", () => { + // US layout: "?" is Shift+Slash and "+" is Shift+Equal. + assert.equal(fired("?", { key: "?", code: "Slash", shiftKey: true }), 1); + assert.equal(fired("+", { key: "+", code: "Equal", shiftKey: true }), 1); + // Spanish layout: "?" is Shift+Minus. + assert.equal(fired("?", { key: "?", code: "Minus", shiftKey: true }), 1); + }); + + it("dispatches only the most specific match", () => { + const { mousetrap, target } = freshInstance(); + const calls = []; + mousetrap.bind("shift+/", () => calls.push("shift+/")); + mousetrap.bind("?", () => calls.push("?")); + fire(target, makeKeyEvent({ key: "?", code: "Slash", shiftKey: true })); + assert.deepEqual(calls, ["shift+/"]); }); it("lowercases function keys", () => { @@ -107,3 +131,60 @@ describe("canonical key normalization", () => { assert.equal(fired("ñ", { key: "Ñ", code: "Semicolon" }), 1); }); }); + +describe("keyForRecording", () => { + const record = (eventOver, layoutMap = null) => { + const { mousetrap } = freshInstance(); + mousetrap.setLayoutMap(layoutMap); + return mousetrap.keyForRecording(makeKeyEvent(eventOver)); + }; + const spanishMap = { get: (code) => ({ BracketRight: "+", Minus: "'" })[code] }; + + it("records letters and digits with a modifier by keyCode", () => { + assert.equal(record({ key: "!", code: "Digit1", keyCode: 49, shiftKey: true }), "1"); + assert.equal(record({ key: "µ", code: "KeyM", keyCode: 77, altKey: true }), "m"); + assert.equal(record({ key: "Dead", code: "KeyE", keyCode: 69, altKey: true }), "e"); + }); + + it("records punctuation by position when there is a layout map", () => { + assert.equal(record({ key: "*", code: "BracketRight", shiftKey: true }, spanishMap), "+"); + assert.equal(record({ key: "+", code: "BracketRight" }, spanishMap), "+"); + }); + + it("records the typed glyph when there is no layout map", () => { + assert.equal(record({ key: "+", code: "BracketRight" }), "+"); + assert.equal(record({ key: "*", code: "BracketRight", shiftKey: true }), "*"); + }); + + it("falls back to the US position for a dead key without a map", () => { + assert.equal(record({ key: "Dead", code: "Quote" }), "'"); + }); + + it("returns null for a modifier pressed alone", () => { + assert.equal(record({ key: "Shift", code: "ShiftLeft", shiftKey: true }), null); + }); + + it("records a key the dispatcher then matches", () => { + const cases = [ + [{ key: "!", code: "Digit1", keyCode: 49, shiftKey: true }, null], + [{ key: "µ", code: "KeyM", keyCode: 77, altKey: true }, null], + [{ key: "*", code: "BracketRight", shiftKey: true }, null], + [{ key: "*", code: "BracketRight", shiftKey: true }, spanishMap], + [{ key: "?", code: "Minus", shiftKey: true, ctrlKey: true }, null], + [{ key: "?", code: "Minus", shiftKey: true, ctrlKey: true }, spanishMap], + ]; + for (const [eventOver, map] of cases) { + const { mousetrap, target } = freshInstance(); + mousetrap.setLayoutMap(map); + const event = makeKeyEvent(eventOver); + const mods = ["ctrl", "alt", "shift"].filter((m) => event[m + "Key"]); + const combo = [...mods, mousetrap.keyForRecording(event)].join("+"); + let calls = 0; + mousetrap.bind(combo.replace(/\+\+$/, "++"), () => { + calls++; + }); + fire(target, event); + assert.equal(calls, 1, `${combo} should fire`); + } + }); +}); diff --git a/frontend/resources/images/features/2.19-paths.jpg b/frontend/resources/images/features/2.19-paths.jpg new file mode 100644 index 0000000000..72ed1c9e43 Binary files /dev/null and b/frontend/resources/images/features/2.19-paths.jpg differ diff --git a/frontend/resources/images/features/2.19-per-side-strokes.jpg b/frontend/resources/images/features/2.19-per-side-strokes.jpg new file mode 100644 index 0000000000..aebb4bb263 Binary files /dev/null and b/frontend/resources/images/features/2.19-per-side-strokes.jpg differ diff --git a/frontend/resources/images/features/2.19-performance.jpg b/frontend/resources/images/features/2.19-performance.jpg new file mode 100644 index 0000000000..dbdcad2aa6 Binary files /dev/null and b/frontend/resources/images/features/2.19-performance.jpg differ diff --git a/frontend/resources/images/features/2.19-shortcuts.jpg b/frontend/resources/images/features/2.19-shortcuts.jpg new file mode 100644 index 0000000000..39e20afca4 Binary files /dev/null and b/frontend/resources/images/features/2.19-shortcuts.jpg differ diff --git a/frontend/resources/images/features/2.19-slide-0.jpg b/frontend/resources/images/features/2.19-slide-0.jpg new file mode 100644 index 0000000000..3937dcbd66 Binary files /dev/null and b/frontend/resources/images/features/2.19-slide-0.jpg differ diff --git a/frontend/resources/images/features/2.19-tokens.jpg b/frontend/resources/images/features/2.19-tokens.jpg new file mode 100644 index 0000000000..6650b335c0 Binary files /dev/null and b/frontend/resources/images/features/2.19-tokens.jpg differ diff --git a/frontend/scripts/check-translations/words.es.txt b/frontend/scripts/check-translations/words.es.txt index 6b9622c5bc..89cfcb7dc0 100644 --- a/frontend/scripts/check-translations/words.es.txt +++ b/frontend/scripts/check-translations/words.es.txt @@ -281,7 +281,6 @@ lectores letras manifiesto menos -navegadores nosotros otras pero diff --git a/frontend/src/app/main/data/shortcuts.cljs b/frontend/src/app/main/data/shortcuts.cljs index 9a045d4995..be6d97068b 100644 --- a/frontend/src/app/main/data/shortcuts.cljs +++ b/frontend/src/app/main/data/shortcuts.cljs @@ -17,36 +17,18 @@ (log/set-level! :warn) -;; Keyboard layout map shared by the shortcut recorder -;; (`app.main.ui.shortcuts`) and the Mousetrap dispatcher: punctuation -;; resolves by physical position through this map, so recording and -;; dispatch use the same token. `nil` until `getLayoutMap` resolves (or -;; when the browser has no keyboard API); both sides then fall back to -;; the typed glyph, which keeps them consistent with and without a map. -(defonce layout-map (atom nil)) - -(defn layout-map-supported? - "Synchronous keyboard-layout API probe: true when the browser exposes - navigator.keyboard.getLayoutMap (Chromium). Punctuation then resolves - by physical position; without it both sides fall back to the typed - glyph, so Shift+punctuation shortcuts may not fire." - ([] (layout-map-supported? - (when (exists? js/navigator) js/navigator))) - ([navigator] - (let [keyboard (when (some? navigator) (.-keyboard ^js navigator))] - (boolean - (and (some? keyboard) - (exists? (.-getLayoutMap ^js keyboard))))))) - -(let [navigator (when (exists? js/navigator) js/navigator)] - (when (layout-map-supported? navigator) - (-> (.getLayoutMap ^js (.-keyboard ^js navigator)) - (.then (fn [m] - (cljs.core/reset! layout-map m) - (.setLayoutMap mousetrap m)) - (fn [_] - (cljs.core/reset! layout-map nil) - (.setLayoutMap mousetrap nil)))))) +;; Keyboard layout map for Mousetrap: punctuation resolves by physical +;; position through it. The shortcut recorder (`app.main.ui.shortcuts`) +;; asks Mousetrap for the key it stores, so both use the same map. Absent +;; until `getLayoutMap` resolves, and in browsers without the keyboard API +;; (Firefox, Safari). +(let [navigator (when (exists? js/navigator) js/navigator) + keyboard (when (some? navigator) (.-keyboard ^js navigator))] + (when (and (some? keyboard) + (exists? (.-getLayoutMap ^js keyboard))) + (-> (.getLayoutMap ^js keyboard) + (.then (fn [m] (.setLayoutMap mousetrap m)) + (fn [_] (.setLayoutMap mousetrap nil)))))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Helpers diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index b84faf4356..3044a05f6c 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1462,6 +1462,12 @@ (wrf/finish! task) (swap! pending-sync-barriers* disj task))) + ;; Each batch entry is [[event task] old-data]. + finish-batch-barriers! + (fn [batch] + (doseq [[[_ task] _] batch] + (finish-sync-barrier! task))) + commits-s (->> stream (rx/filter dch/commit?) @@ -1474,6 +1480,9 @@ (rx/filter (complement :skip-component-sync?)) ;; Keep waits pending while component changes are checked. (rx/map start-sync-barrier) + ;; Paired as it arrives, so later commits in the same batch + ;; cannot hide the shapes this one moved or deleted. + (rx/with-latest-from workspace-buffer-s) (rx/share)) ;; Buffers commits until a timer turn passes with no new ones. The @@ -1499,32 +1508,33 @@ (log/info :hint "detected component changes" :ids (map str changed-components) :undo-group undo-group) - (map #(vector ::component-changed % undo-group) + (map #(vector ::component-changed % (:id old-data) undo-group) changed-components)) :else ;; Undos only bump :modified-at. (map #(vector ::touch-component %) changed-components)))) - ;; One event per distinct component change in the batch + ;; One event per distinct component change in the batch, each + ;; commit checked against the file data right before it get-component-events - (fn [batch old-data] + (fn [batch] (->> batch - (mapcat #(get-component-changes old-data (first %))) + (mapcat (fn [[[event _] old-data]] + (get-component-changes old-data event))) (distinct) - (map (fn [[type component-id undo-group]] + (map (fn [[type component-id file-id undo-group]] (if (= type ::component-changed) - (component-changed component-id (:id old-data) undo-group) + (component-changed component-id file-id undo-group) (touch-component component-id)))) (rx/from))) component-events-s (->> commit-batches-s - (rx/with-latest-from workspace-buffer-s) (rx/mapcat - (fn [[batch old-data]] - (->> (get-component-events batch old-data) - (rx/finalize #(run! (comp finish-sync-barrier! second) batch))))) + (fn [batch] + (->> (get-component-events batch) + (rx/finalize #(finish-batch-barriers! batch))))) ;; Close barriers left behind when the page shuts down. (rx/finalize #(wrf/finish-tasks! @pending-sync-barriers*)) (rx/share)) diff --git a/frontend/src/app/main/data/workspace/notifications.cljs b/frontend/src/app/main/data/workspace/notifications.cljs index b3eead45ef..6945fb00a9 100644 --- a/frontend/src/app/main/data/workspace/notifications.cljs +++ b/frontend/src/app/main/data/workspace/notifications.cljs @@ -16,6 +16,7 @@ [app.main.data.common :as dc] [app.main.data.helpers :as dsh] [app.main.data.modal :as modal] + [app.main.data.notifications :as ntf] [app.main.data.plugins :as dpl] [app.main.data.websocket :as dws] [app.main.data.workspace :as-alias dw] @@ -26,6 +27,7 @@ [app.main.data.workspace.texts :as dwt] [app.main.router :as rt] [app.util.globals :refer [global]] + [app.util.i18n :refer [tr]] [app.util.mouse :as mse] [app.util.object :as obj] [app.util.rxops :as rxs] @@ -245,7 +247,8 @@ [:session-id ::sm/uuid] [:revn :int] [:vern :int] - [:changes cpc/schema:changes]]) + [:changes cpc/schema:changes] + [:repaired {:optional true} :boolean]]) (def ^:private check-file-change-params! (sm/check-fn schema:handle-file-change)) @@ -267,13 +270,20 @@ ;; and update the persistence internal state with the updated ;; file-revn - (rx/of (dch/commit {:file-id file-id - :file-revn revn - :file-vern vern - :save-undo? false - :source :remote - :redo-changes (vec changes) - :undo-changes []}))))) + (rx/concat + (rx/of (dch/commit {:file-id file-id + :file-revn revn + :file-vern vern + :save-undo? false + :source :remote + :redo-changes (vec changes) + :undo-changes []})) + (if (:repaired msg) + (rx/of (ntf/show {:content (tr "workspace.notifications.file-repaired") + :type :toast + :level :info + :timeout nil})) + (rx/empty)))))) (defn handle-file-deleted [{:keys [file-id] :as msg}] diff --git a/frontend/src/app/main/data/workspace/path.cljs b/frontend/src/app/main/data/workspace/path.cljs index de8decc1ba..636d253a2f 100644 --- a/frontend/src/app/main/data/workspace/path.cljs +++ b/frontend/src/app/main/data/workspace/path.cljs @@ -55,7 +55,6 @@ ;; Path tools (dm/export tools/make-curve) (dm/export tools/make-corner) -(dm/export tools/add-node) (dm/export tools/remove-node) (dm/export tools/delete-selected) (dm/export tools/delete-selected-with-segments) diff --git a/frontend/src/app/main/data/workspace/path/shortcuts.cljs b/frontend/src/app/main/data/workspace/path/shortcuts.cljs index 21f1c87960..fbe1483de6 100644 --- a/frontend/src/app/main/data/workspace/path/shortcuts.cljs +++ b/frontend/src/app/main/data/workspace/path/shortcuts.cljs @@ -57,14 +57,6 @@ :overwrite true :fn #(st/emit! (drp/change-edit-mode :draw))} - :add-node {:tooltip (ds/shift "+") - :label (fn [] (tr "shortcuts.add-node")) - :command ["shift++" "shift+="] - :show-command "shift++" - :subsections [:path-editor] - :section [:workspace] - :fn #(st/emit! (drp/add-node))} - :delete-node {:tooltip (ds/supr) :label (fn [] (tr "shortcuts.delete-node")) :command ["del" "backspace"] diff --git a/frontend/src/app/main/data/workspace/path/tools.cljs b/frontend/src/app/main/data/workspace/path/tools.cljs index fe25a9e13e..54fa954410 100644 --- a/frontend/src/app/main/data/workspace/path/tools.cljs +++ b/frontend/src/app/main/data/workspace/path/tools.cljs @@ -121,9 +121,6 @@ (fn [ht] (reduce #(assoc %1 %2 type) (or ht {}) nodes))))) state))))) -(defn add-node [] - (process-path-tool (fn [content points] (path/split-segments content points 0.5)))) - (defn remove-node "Removes nodes and heals the gap with a fitted curve." ([] diff --git a/frontend/src/app/main/data/workspace/shortcuts.cljs b/frontend/src/app/main/data/workspace/shortcuts.cljs index b31fdd95d3..9b8eb6bd09 100644 --- a/frontend/src/app/main/data/workspace/shortcuts.cljs +++ b/frontend/src/app/main/data/workspace/shortcuts.cljs @@ -681,8 +681,7 @@ :hide-ui {:tooltip "\\" :label (fn [] (tr "shortcuts.hide-ui")) - :command ["\\" "alt+\\" "alt+º"] - :show-command "\\" + :command "\\" :subsections [:panels :basics] :section [:workspace :basics] :fn #(st/emit! (toggle-layout-flag :hide-ui))} @@ -719,8 +718,7 @@ :zoom-selected {:tooltip (ds/shift "2") :label (fn [] (tr "shortcuts.zoom-selected")) - :command ["shift+2" "shift+num2" "shift+'" "shift+\""] - :show-command "shift+2" + :command ["shift+2" "shift+num2"] :subsections [:zoom-workspace] :section [:workspace] :fn #(st/emit! dw/zoom-to-selected-shape)} diff --git a/frontend/src/app/main/data/workspace/tokens/library_edit.cljs b/frontend/src/app/main/data/workspace/tokens/library_edit.cljs index 6b22169719..cb0f4ea573 100644 --- a/frontend/src/app/main/data/workspace/tokens/library_edit.cljs +++ b/frontend/src/app/main/data/workspace/tokens/library_edit.cljs @@ -510,7 +510,8 @@ (pcb/with-library-data data) (clo/generate-set-tokens-source library) (pcb/set-tokens-source library-id))] - (rx/of (dch/commit-changes changes)))))) + (rx/of (dch/commit-changes changes) + (dwtp/propagate-workspace-tokens)))))) (defn delete-token-set [id] diff --git a/frontend/src/app/main/ui/releases.cljs b/frontend/src/app/main/ui/releases.cljs index 8252d42849..86289fe4ed 100644 --- a/frontend/src/app/main/ui/releases.cljs +++ b/frontend/src/app/main/ui/releases.cljs @@ -37,6 +37,7 @@ [app.main.ui.releases.v2-16] [app.main.ui.releases.v2-17] [app.main.ui.releases.v2-18] + [app.main.ui.releases.v2-19] [app.main.ui.releases.v2-2] [app.main.ui.releases.v2-3] [app.main.ui.releases.v2-4] @@ -108,4 +109,4 @@ (defmethod rc/render-release-notes "0.0" [params] - (rc/render-release-notes (assoc params :version "2.18"))) + (rc/render-release-notes (assoc params :version "2.19"))) diff --git a/frontend/src/app/main/ui/releases/v2_19.cljs b/frontend/src/app/main/ui/releases/v2_19.cljs new file mode 100644 index 0000000000..241be4cc05 --- /dev/null +++ b/frontend/src/app/main/ui/releases/v2_19.cljs @@ -0,0 +1,208 @@ +;; 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 SUBSIDIARY SL + +(ns app.main.ui.releases.v2-19 + (:require-macros [app.main.style :as stl]) + (:require + [app.common.data.macros :as dm] + [app.main.ui.ds.buttons.button :refer [button*]] + [app.main.ui.releases.common :as c] + [rumext.v2 :as mf])) + +(defmethod c/render-release-notes "2.19" + [{:keys [slide klass next finish navigate version]}] + (mf/html + (case slide + :start + [:div {:class (stl/css-case :modal-overlay true)} + [:div.animated {:class klass} + [:div {:class (stl/css :modal-container)} + [:img {:src "images/features/2.19-slide-0.jpg" + :class (stl/css :start-image) + :border "0" + :alt "Penpot 2.19 is here!"}] + + [:div {:class (stl/css :modal-content)} + [:div {:class (stl/css :modal-header)} + [:h1 {:class (stl/css :modal-title)} + "What’s new in Penpot?"] + + [:div {:class (stl/css :version-tag)} + (dm/str "Version " version)]] + + [:div {:class (stl/css :features-block)} + + [:p {:class (stl/css :feature-content)} + "Per-side strokes, your own shortcuts, tokens shared in libraries, and a big step for WebGL rendering."] + + [:p {:class (stl/css :feature-content)} + "2.19 is a packed release. You can now set stroke width per side, customize your keyboard shortcuts, and draw and edit paths with far more control. Design tokens can now live in a shared library and stay in sync across files, and WebGL rendering gets better text, smoother canvases, and much faster exports. On top of that come performance improvements, a wide round of bug fixes, and plenty of quality-of-life polish."]] + + [:div {:class (stl/css :navigation)} + [:> button* {:class (stl/css :next-btn) + :on-click next + :variant "primary"} + "Continue"]]]]]] + + 0 + [:div {:class (stl/css-case :modal-overlay true)} + [:div.animated {:class klass} + [:div {:class (stl/css :modal-container)} + [:img {:src "images/features/2.19-per-side-strokes.jpg" + :class (stl/css :start-image) + :border "0" + :alt "Per-side strokes. Yes, finally!"}] + + [:div {:class (stl/css :modal-content)} + [:div {:class (stl/css :modal-header)} + [:h1 {:class (stl/css :modal-title)} + "Per-side strokes. Yes, finally!"]] + + [:div {:class (stl/css :feature)} + [:p {:class (stl/css :feature-content)} + "We know, this one kept you waiting. Rectangles and boards can now have a different stroke width on each side (top, right, bottom, left). Switch from uniform to per-side in the Stroke section, just like independent border radius. Need only a bottom divider or a left accent? No more extra shapes or masking tricks."] + + [:p {:class (stl/css :feature-content)} + "It works with design tokens too: apply a token to all sides or a different one to each. Inspect shows the value or token applied to every side, matching how CSS borders behave in code."]] + + [:div {:class (stl/css :navigation)} + [:> c/navigation-bullets* + {:slide slide + :navigate navigate + :total 5}] + + [:> button* {:class (stl/css :next-btn) + :on-click next + :variant "primary"} + "Continue"]]]]]] + + 1 + [:div {:class (stl/css-case :modal-overlay true)} + [:div.animated {:class klass} + [:div {:class (stl/css :modal-container)} + [:img {:src "images/features/2.19-shortcuts.jpg" + :class (stl/css :start-image) + :border "0" + :alt "Shortcuts that fit your hands"}] + + [:div {:class (stl/css :modal-content)} + [:div {:class (stl/css :modal-header)} + [:h1 {:class (stl/css :modal-title)} + "Shortcuts that fit your hands"]] + + [:div {:class (stl/css :feature)} + [:p {:class (stl/css :feature-content)} + "You can now reassign any keyboard shortcut in Penpot. Search by action or by key combination, record a new binding, and Penpot warns you if it is already taken. Reset one shortcut or all of them whenever you want."] + + [:p {:class (stl/css :feature-content)} + "Great news if you type on AZERTY, QWERTZ, or any other keyboard layout, and for anyone who wants Penpot to match their muscle memory. You can also export and import your setup to take it with you."]] + + [:div {:class (stl/css :navigation)} + [:> c/navigation-bullets* + {:slide slide + :navigate navigate + :total 5}] + + [:> button* {:class (stl/css :next-btn) + :on-click next + :variant "primary"} + "Continue"]]]]]] + + 2 + [:div {:class (stl/css-case :modal-overlay true)} + [:div.animated {:class klass} + [:div {:class (stl/css :modal-container)} + [:img {:src "images/features/2.19-paths.jpg" + :class (stl/css :start-image) + :border "0" + :alt "Drawing and path editing, refreshed"}] + + [:div {:class (stl/css :modal-content)} + [:div {:class (stl/css :modal-header)} + [:h1 {:class (stl/css :modal-title)} + "Drawing and path editing, refreshed"]] + + [:div {:class (stl/css :feature)} + [:p {:class (stl/css :feature-content)} + "The path editor gets a full refresh: smoother freehand curves that keep sharp corners sharp, the path toolbar available while you draw, node snapping on by default, and the ability to continue drawing from existing nodes and segments. Hold Shift to snap angles in 15° steps."] + + [:p {:class (stl/css :feature-content)} + "The result: everything you need to draw your own icons and bring any illustration you can imagine to life, without leaving Penpot."]] + + [:div {:class (stl/css :navigation)} + [:> c/navigation-bullets* + {:slide slide + :navigate navigate + :total 5}] + + [:> button* {:class (stl/css :next-btn) + :on-click next + :variant "primary"} + "Continue"]]]]]] + + 3 + [:div {:class (stl/css-case :modal-overlay true)} + [:div.animated {:class klass} + [:div {:class (stl/css :modal-container)} + [:img {:src "images/features/2.19-tokens.jpg" + :class (stl/css :start-image) + :border "0" + :alt "Design tokens, shared from a library"}] + + [:div {:class (stl/css :modal-content)} + [:div {:class (stl/css :modal-header)} + [:h1 {:class (stl/css :modal-title)} + "Design tokens, shared from a library"]] + + [:div {:class (stl/css :feature)} + [:p {:class (stl/css :feature-content)} + "Keep your tokens in one library and use them in every file that links it. Tokens stay in sync when the library changes, just like components and colors. In the consuming file you can apply tokens and switch themes, while edits happen in the source library, so your design system stays the single source of truth."] + + [:p {:class (stl/css :feature-content)} + "The Assets tab shows where your tokens come from."]] + + [:div {:class (stl/css :navigation)} + [:> c/navigation-bullets* + {:slide slide + :navigate navigate + :total 5}] + + [:> button* {:class (stl/css :next-btn) + :on-click next + :variant "primary"} + "Continue"]]]]]] + + 4 + [:div {:class (stl/css-case :modal-overlay true)} + [:div.animated {:class klass} + [:div {:class (stl/css :modal-container)} + [:img {:src "images/features/2.19-performance.jpg" + :class (stl/css :start-image) + :border "0" + :alt "WebGL rendering takes a big step"}] + + [:div {:class (stl/css :modal-content)} + [:div {:class (stl/css :modal-header)} + [:h1 {:class (stl/css :modal-title)} + "WebGL rendering takes a big step"]] + + [:div {:class (stl/css :feature)} + [:p {:class (stl/css :feature-content)} + "WebGL rendering gets one of its biggest upgrades yet. Improved tile rendering keeps large files smooth while you move around the canvas, and the new text editor built for WebGL is more precise and much faster. Exports now use the same engine, so they are radically faster and higher quality."] + + [:p {:class (stl/css :feature-content)} + "If you have not tried it yet, enable it in Your account → Settings. Your feedback keeps shaping it."]] + + [:div {:class (stl/css :navigation)} + [:> c/navigation-bullets* + {:slide slide + :navigate navigate + :total 5}] + + [:> button* {:class (stl/css :next-btn) + :on-click finish + :variant "primary"} + "Let's go"]]]]]]))) diff --git a/frontend/src/app/main/ui/releases/v2_19.scss b/frontend/src/app/main/ui/releases/v2_19.scss new file mode 100644 index 0000000000..ca0969b1a1 --- /dev/null +++ b/frontend/src/app/main/ui/releases/v2_19.scss @@ -0,0 +1,7 @@ +// 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 SUBSIDIARY SL + +@use "base"; diff --git a/frontend/src/app/main/ui/settings/shortcuts.cljs b/frontend/src/app/main/ui/settings/shortcuts.cljs index 841841ac5b..e1e9b884c0 100644 --- a/frontend/src/app/main/ui/settings/shortcuts.cljs +++ b/frontend/src/app/main/ui/settings/shortcuts.cljs @@ -23,7 +23,6 @@ [app.main.ui.ds.foundations.typography.heading :refer [heading*]] [app.main.ui.ds.foundations.typography.text :refer [text*]] [app.main.ui.ds.layout.tab-switcher :refer [tab-switcher*]] - [app.main.ui.ds.notifications.context-notification :refer [context-notification*]] [app.main.ui.ds.product.empty-state :refer [empty-state*]] [app.main.ui.settings.import-shortcuts-diff-modal] [app.main.ui.settings.restore-shortcuts-modal] @@ -529,10 +528,6 @@ [:> icon* {:icon-id i/info :class (stl/css :shortcuts-description-icon)}] (tr "shortcuts.reload-hint")] - (when-not (ds/layout-map-supported?) - [:> context-notification* {:level :warning - :class (stl/css :layout-map-warning)} - (tr "shortcuts.layout-map-missing")]) [:> (mf/provider ctx/shortcuts-ctx) {:value shortcuts-ctx-value} [:div {:class (stl/css :shortcuts-content)} diff --git a/frontend/src/app/main/ui/settings/shortcuts.scss b/frontend/src/app/main/ui/settings/shortcuts.scss index c86ca879b5..522147f61e 100644 --- a/frontend/src/app/main/ui/settings/shortcuts.scss +++ b/frontend/src/app/main/ui/settings/shortcuts.scss @@ -314,7 +314,3 @@ padding: var(--sp-s) 0; margin-block-end: var(--sp-l); } - -.layout-map-warning { - margin-block-end: var(--sp-m); -} diff --git a/frontend/src/app/main/ui/shortcuts.cljs b/frontend/src/app/main/ui/shortcuts.cljs index 30aed17a2c..4166314a37 100644 --- a/frontend/src/app/main/ui/shortcuts.cljs +++ b/frontend/src/app/main/ui/shortcuts.cljs @@ -7,6 +7,7 @@ (ns app.main.ui.shortcuts (:require-macros [app.main.style :as stl]) (:require + ["@penpot/mousetrap$default" :as mousetrap] [app.common.data :as d] [app.common.data.macros :as dm] [app.config :as cf] @@ -34,10 +35,6 @@ (def ^:private modifier-keys #{"Control" "Shift" "Alt" "Meta"}) -(def ^:private punctuation-codes - #{"Minus" "Equal" "BracketLeft" "BracketRight" "Backslash" - "Semicolon" "Quote" "Backquote" "Comma" "Period" "Slash"}) - (def ^:private key-name-map {"ArrowUp" "up" "Up" "up" @@ -202,71 +199,16 @@ (fn? (.-getModifierState native))) (.getModifierState native "AltGraph")))))) -(defn- numpad-code? - [code] - (and (string? code) - (str/starts-with? code "Numpad") - (= 1 (count (subs code 6))))) - -(defn- numpad-alias - [code] - (let [suffix (subs code 6)] - (when (re-matches #"[0-9]" suffix) - (str "num" suffix)))) - -(defn- punctuation-alias - "Resolves punctuation by physical position through the shared layout - map (`app.main.data.shortcuts/layout-map`, also pushed into Mousetrap), - so recording and dispatch use the same token. Without a map the typed - glyph applies, which keeps both sides consistent." - [code key] - (let [mapped (when (some? @ds/layout-map) - (.get ^js @ds/layout-map code))] - (if (string? mapped) - (str/lower mapped) - (str/lower key)))) - -(defn- digit-code? - [code] - (and (string? code) - (boolean (re-matches #"Digit[0-9]" code)))) - -(defn- digit-alias - "Shifted digits resolve to the digit itself: with Shift held, Digit0 - types ) on US layouts but 0 on AZERTY, so shift+0 must mean the - physical digit key everywhere, like shift+z means z." - [^js event code key] - (when (and (.-shiftKey ^js event) - (digit-code? code) - (= 1 (count key))) - (subs code 5))) - (defn- normalize-key - "Normalizes `event.key` to the lowercase canonical token mousetrap - matches on. Letters and unshifted digits resolve by glyph (`a`, not - `q` on AZERTY); a digit row key held with Shift resolves to the digit - itself (`shift+0`, never `shift+)`); punctuation resolves by physical - position through the layout map, so `Shift+*` on a Spanish layout - records `shift++`, the primary key the user sees." + "Returns the key token Mousetrap matches for `event`, asking Mousetrap + itself (`keyForRecording`) so a recorded shortcut always fires: letters + and digits with a modifier by keyCode (`shift+1`, `alt+m`), punctuation + by position when the browser has a layout map, the typed glyph + otherwise." [^js event] - (let [native (event->native event) - key (.-key ^js event) - code (.-code ^js native)] + (let [key (.-key event)] (when (and (string? key) (not (contains? modifier-keys key))) - (cond - ;; Numpad digits keep their own `numN` bindings, like mousetrap. - (numpad-code? code) - (numpad-alias code) - - ;; Punctuation: primary key of the physical position. - (contains? punctuation-codes code) - (punctuation-alias code key) - - :else - (or (get key-name-map key) - ;; Shifted digit row: the digit itself, on every layout. - (digit-alias event code key) - (str/lower key)))))) + (.keyForRecording ^js mousetrap (event->native event))))) (defn- event->modifier-parts [^js event] @@ -281,7 +223,8 @@ (cf/check-platform? :macos)) (conj "command") - (.-altKey event) + (and (.-altKey event) + (not alt-graph?)) (conj "alt") (.-shiftKey event) diff --git a/frontend/src/app/plugins/library.cljs b/frontend/src/app/plugins/library.cljs index ed3c12c906..8fd581e353 100644 --- a/frontend/src/app/plugins/library.cljs +++ b/frontend/src/app/plugins/library.cljs @@ -917,6 +917,9 @@ (not (r/check-permission plugin-id "library:write")) (u/not-valid plugin-id :restore "Plugin doesn't have 'library:write' permission") + (not= file-id (:current-file-id @st/state)) + (u/not-valid plugin-id :restore "Can only restore components from the current file's library") + (nil? component) (u/not-valid plugin-id :restore "Component does not exist") diff --git a/frontend/test/frontend_tests/data/workspace_notifications_test.cljs b/frontend/test/frontend_tests/data/workspace_notifications_test.cljs new file mode 100644 index 0000000000..aceafe6bed --- /dev/null +++ b/frontend/test/frontend_tests/data/workspace_notifications_test.cljs @@ -0,0 +1,52 @@ +;; 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 SUBSIDIARY SL + +(ns frontend-tests.data.workspace-notifications-test + (:require + [app.common.data :as d] + [app.common.uuid :as uuid] + [app.main.data.changes :as dch] + [app.main.data.notifications :as ntf] + [app.main.data.workspace.notifications :as dwntf] + [app.util.i18n :as i18n] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [potok.v2.core :as ptk])) + +(defn- file-change-msg + [extra] + (merge {:type :file-change + :profile-id (uuid/next) + :file-id (uuid/next) + :session-id (uuid/next) + :revn 1 + :vern 0 + :changes []} + extra)) + +(defn- ^:async watch-file-change + [msg] + (let [events (atom [])] + (await (async/observe (ptk/watch (dwntf/handle-file-change msg) {} (rx/empty)) + :on-next #(swap! events conj %))) + @events)) + +(t/deftest ^:async repaired-file-change-applies-and-notifies + (let [events (await (watch-file-change (file-change-msg {:repaired true}))) + types (mapv ptk/type events) + show (d/seek #(= ::ntf/show (ptk/type %)) events) + state (ptk/update show {})] + (t/is (= [::dch/commit ::ntf/show] types)) + (t/is (= :info (get-in state [:notification :level]))) + (t/is (nil? (get-in state [:notification :timeout])) + "the toast stays until the user closes it") + (t/is (= (i18n/tr "workspace.notifications.file-repaired") + (get-in state [:notification :content]))))) + +(t/deftest ^:async ordinary-file-change-does-not-notify + (let [events (await (watch-file-change (file-change-msg {})))] + (t/is (= [::dch/commit] (mapv ptk/type events))))) diff --git a/frontend/test/frontend_tests/logic/comp_watcher_test.cljs b/frontend/test/frontend_tests/logic/comp_watcher_test.cljs new file mode 100644 index 0000000000..f8d3bc8783 --- /dev/null +++ b/frontend/test/frontend_tests/logic/comp_watcher_test.cljs @@ -0,0 +1,100 @@ +;; 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 SUBSIDIARY SL +(ns frontend-tests.logic.comp-watcher-test + "Automatic propagation of main edits to copies, driven by the real + `dwl/watch-component-changes` on an isolated store." + (:require + [app.common.test-helpers.compositions :as ctho] + [app.common.test-helpers.files :as cthf] + [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.main.data.helpers :as dsh] + [app.main.data.workspace.libraries :as dwl] + [app.main.data.workspace.pages :as dwpg] + [app.main.data.workspace.shapes :as dwsh] + [app.main.data.workspace.thumbnails-wasm :as dwt.wasm] + [app.main.refs :as refs] + [app.main.store :as st] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [frontend-tests.helpers.mock :as mock] + [frontend-tests.helpers.pages :as thp] + [frontend-tests.helpers.state :as ths] + [frontend-tests.helpers.wasm :as thw] + [okulary.core :as l] + [potok.v2.core :as ptk])) + +(t/use-fixtures :each + {:before (fn [] + (thp/reset-idmap!) + (thw/setup-wasm-mocks!)) + :after thw/teardown-wasm-mocks!}) + +(defn- setup-file + [] + ;; {:main-root} [:name Frame1] # [Component :component1] + ;; :main-child [:name Rect1] + ;; + ;; :copy-root [:name Frame1] #--> [Component :component1] :main-root + ;; :copy-child [:name Rect1] ---> :main-child + ;; + ;; :outside [:name Frame1] + (-> (cthf/sample-file :file1) + (ctho/add-simple-component-with-copy :component1 + :main-root + :main-child + :copy-root + :copy-root-params {:children-labels [:copy-child]}) + (ctho/add-frame :outside))) + +(defn- noop-render-thumbnail + [& _] + (ptk/reify ::noop-render-thumbnail + ptk/WatchEvent + (watch [_ _ _] (rx/empty)))) + +(defn- copy-children + [store] + (let [file (ths/get-file-from-state @store)] + (:shapes (cths/get-shape file :copy-root)))) + +(t/deftest ^:async child-moved-out-of-main-leaves-copy-when-another-edit-follows + ;; The move and the fill change commit in the same tick, so the watcher + ;; inspects both in one batch. The move must be checked against the file + ;; data right before it, where the rect still sits in the main. + (let [file (setup-file) + store (ths/setup-store file)] + (await + (mock/with-mocks* + {;; The watcher reads the file data through this lens. + refs/workspace-data (l/derived dsh/lookup-file-data store) + ;; Thumbnail renders read the global store. + st/state store + dwt.wasm/render-thumbnail noop-render-thumbnail} + + (ptk/emit! store (dwl/watch-component-changes)) + (t/is (= 1 (count (copy-children store))) + "the copy starts with its rect") + + (ptk/emit! store + (dwsh/relocate-shapes #{(cthi/id :main-child)} (cthi/id :outside) 0) + (dwsh/update-shapes [(cthi/id :outside)] + #(assoc % :fills [{:fill-color "#ff0000" + :fill-opacity 1}]))) + + (await (async/wait-for #(empty? (copy-children store)) + "the copy to lose its rect")) + + (let [file' (ths/get-file-from-state @store)] + (t/is (= (cthi/id :outside) + (:parent-id (cths/get-shape file' :main-child))) + "the rect moved out of the main") + (t/is (empty? (copy-children store)) + "the copy lost its rect")) + + (ptk/emit! store (dwpg/finalize-page (:id file) (cthf/current-page-id file))) + (await (async/settle)))))) diff --git a/frontend/test/frontend_tests/plugins/library_test.cljs b/frontend/test/frontend_tests/plugins/library_test.cljs index 8edd8a0e30..9180376715 100644 --- a/frontend/test/frontend_tests/plugins/library_test.cljs +++ b/frontend/test/frontend_tests/plugins/library_test.cljs @@ -55,13 +55,10 @@ (cthf/apply-changes file changes))) (t/deftest ^:async component-restore-preserves-the-users-view - (doseq [scenario [:same-page :other-page :linked-library]] + (doseq [scenario [:same-page :other-page]] (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})) + store (ths/setup-store file {:renderer :svg}) view {:selected #{(thi/id :copy)} :zoom 2 :vport {:x 100 :y 200 :width 800 :height 600} @@ -108,6 +105,23 @@ (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-linked-libraries + (let [library (file-with-deleted-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))] + (t/is (thrown-with-msg? js/Error #"current file" (.restore component))) + (await (async/settle)) + (t/is (empty? @events)) + (t/is (= (:files before) (:files @store)))))))) + (t/deftest ^:async component-restore-rejects-a-missing-component (let [file (cthf/sample-file :restore-file) store (ths/setup-store file {:renderer :svg})] diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 5e18876e88..343c2e2cc2 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -47,6 +47,7 @@ [frontend-tests.helpers-shapes-test] [frontend-tests.logic.bool-move-test] [frontend-tests.logic.comp-remove-swap-slots-test] + [frontend-tests.logic.comp-watcher-test] [frontend-tests.logic.components-and-tokens] [frontend-tests.logic.copy-paste-typography-test] [frontend-tests.logic.copying-and-duplicating-test] @@ -105,6 +106,7 @@ [frontend-tests.tokens.logic.token-actions-test] [frontend-tests.tokens.logic.token-data-test] [frontend-tests.tokens.logic.token-remapping-test] + [frontend-tests.tokens.logic.tokens-source-test] [frontend-tests.tokens.logic.tokens-status-test] [frontend-tests.tokens.referenced-token-value-test] [frontend-tests.tokens.style-dictionary-test] @@ -212,6 +214,7 @@ 'frontend-tests.helpers-shapes-test 'frontend-tests.logic.bool-move-test 'frontend-tests.logic.comp-remove-swap-slots-test + 'frontend-tests.logic.comp-watcher-test 'frontend-tests.logic.components-and-tokens 'frontend-tests.logic.copy-paste-typography-test 'frontend-tests.logic.copying-and-duplicating-test @@ -274,6 +277,7 @@ 'frontend-tests.tokens.token-errors-test 'frontend-tests.tokens.token-path-test 'frontend-tests.tokens.tokenscript-test + 'frontend-tests.tokens.logic.tokens-source-test 'frontend-tests.tokens.logic.tokens-status-test 'frontend-tests.tokens.workspace-tokens-remap-test 'frontend-tests.ui.check-updates-test diff --git a/frontend/test/frontend_tests/tokens/logic/tokens_source_test.cljs b/frontend/test/frontend_tests/tokens/logic/tokens_source_test.cljs new file mode 100644 index 0000000000..6813fd1da4 --- /dev/null +++ b/frontend/test/frontend_tests/tokens/logic/tokens_source_test.cljs @@ -0,0 +1,136 @@ +;; 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 SUBSIDIARY SL + +(ns frontend-tests.tokens.logic.tokens-source-test + (:require + [app.common.test-helpers.compositions :as ctho] + [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] + [app.common.test-helpers.tokens :as ctht] + [app.common.types.tokens-lib :as ctob] + [app.common.types.tokens-status :as ctos] + [app.main.data.workspace.libraries :as dwl] + [app.main.data.workspace.tokens.library-edit :as dwtl] + [app.main.repo :as rp] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [frontend-tests.helpers.mock :as mock] + [frontend-tests.helpers.pages :as thp] + [frontend-tests.helpers.state :as ths] + [frontend-tests.helpers.wasm :as thw] + [potok.v2.core :as ptk])) + +(t/use-fixtures :each + {:before (fn [] + (thp/reset-idmap!) + (thw/setup-wasm-mocks!)) + :after thw/teardown-wasm-mocks!}) + +(defn- setup-library + "A library whose active set defines `red` as green." + [] + (ctht/sample-file-with-tokens + :file-id :library + :lib-fn #(-> % + (ctob/add-set (ctob/make-token-set :id (cthi/new-id! :library-set) + :name "library-set")) + (ctob/add-token (cthi/id :library-set) + (ctob/make-token :id (cthi/new-id! :library-red) + :name "red" + :type :color + :value "#00ff00"))) + :status-fn #(ctos/set-tokens-status % #{} #{(cthi/id :library-set)}))) + +(defn- setup-file + "A file whose own active set defines `red` as dark red, applied to the + fill of a rectangle." + [] + (-> (ctht/sample-file-with-tokens + :file-id :local + :lib-fn #(-> % + (ctob/add-set (ctob/make-token-set :id (cthi/new-id! :local-set) + :name "local-set")) + (ctob/add-token (cthi/id :local-set) + (ctob/make-token :id (cthi/new-id! :local-red) + :name "red" + :type :color + :value "rgb(123, 34, 34)"))) + :status-fn #(ctos/set-tokens-status % #{} #{(cthi/id :local-set)})) + (ctho/add-rect :rect1) + (ctht/apply-token-to-shape :rect1 "red" [:fill] [:fill] "#7b2222"))) + +(defn- fill-color + [store] + (let [file (ths/get-file-from-state @store)] + (-> (cths/get-shape file :rect1) :fills first :fill-color))) + +(defn- active-set-ids + [store] + (-> (ths/get-file-from-state @store) ctht/get-tokens-status ctos/get-active-set-ids)) + +(defn- tokens-source + [store] + (-> (ths/get-file-from-state @store) ctht/get-tokens-source)) + +(defn- ^:async use-library-tokens + "Set `library` as tokens source and wait for the canvas to use it." + [store library] + (ptk/emit! store (dwtl/set-tokens-source (:id library))) + (await (async/wait-for #(= "#00ff00" (fill-color store)) + "the fill uses the library value of `red`" + {:max-ticks 200}))) + +(t/deftest ^:async switching-tokens-source-updates-the-canvas + (let [library (setup-library) + file (setup-file) + store (ths/setup-store file {:libraries [library]})] + + (t/is (= "#7b2222" (fill-color store))) + + (ptk/emit! store (dwtl/set-tokens-source (:id library))) + (await (async/wait-for #(= "#00ff00" (fill-color store)) + "the fill uses the library value of `red`" + {:max-ticks 200})) + (t/is (= "#00ff00" (fill-color store)) + "Setting the library as tokens source resolves `red` from the library") + + (ptk/emit! store (dwtl/set-tokens-source (:id file))) + (await (async/settle)) + (t/is (contains? (active-set-ids store) (cthi/id :local-set)) + "Going back to the local tokens keeps the local set active") + (await (async/wait-for #(= "#7b2222" (fill-color store)) + "the fill uses the local value of `red` again" + {:max-ticks 200})) + (t/is (= "#7b2222" (fill-color store)) + "Going back to the local tokens resolves `red` from the file again") + + (rx/dispose! store))) + +(t/deftest ^:async unlinking-the-tokens-source-library-restores-the-local-tokens + (let [library (setup-library) + file (setup-file) + store (ths/setup-store file {:libraries [library]})] + (await (use-library-tokens store library)) + + (await + (mock/with-mocks* + {rp/cmd! (mock/stub (fn [_ _] (rx/observe-on :async (rx/of []))))} + ;; The same events the libraries dialog emits on unlink + (ptk/emit! store + (dwl/unlink-file-from-library (:id file) (:id library)) + (dwtl/set-tokens-source (:id file))) + (await (async/wait-for #(= "#7b2222" (fill-color store)) + "the fill uses the local value of `red` again" + {:max-ticks 200})))) + + (t/is (nil? (tokens-source store))) + (t/is (contains? (active-set-ids store) (cthi/id :local-set)) + "Unlinking the library keeps the local set active") + (t/is (= "#7b2222" (fill-color store))) + + (rx/dispose! store))) + diff --git a/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs b/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs index 3ed7e035c9..f2df889ef0 100644 --- a/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs +++ b/frontend/test/frontend_tests/ui/settings_shortcuts_test.cljs @@ -1,5 +1,6 @@ (ns frontend-tests.ui.settings-shortcuts-test (:require + ["@penpot/mousetrap$default" :as mousetrap] [app.config :as cf] [app.main.data.dashboard.shortcuts :as dsc] [app.main.data.profile :as du] @@ -495,17 +496,25 @@ (defn- mock-wrapped-keydown "Simulates a goog.events.BrowserEvent-wrapped keydown: `key` and modifiers - are forwarded directly, and `code` is only reachable via `getBrowserEvent`. - `key` is passed through as-is (not defaulted) so callers can simulate a - missing `event.key` with `:key nil` - substituting \"\" would defeat that, - since an empty string is truthy in ClojureScript." - [{:keys [key code shift ctrl alt meta]}] - #js {:key key - :shiftKey (boolean shift) - :ctrlKey (boolean ctrl) - :altKey (boolean alt) - :metaKey (boolean meta) - :getBrowserEvent (fn [] #js {:code code})}) + are forwarded directly, and the native event behind `getBrowserEvent` + carries every field (`code` and `keyCode` only live there). `key` is + passed through as-is (not defaulted) so callers can simulate a missing + `event.key` with `:key nil` - substituting \"\" would defeat that, since + an empty string is truthy in ClojureScript." + [{:keys [key code key-code shift ctrl alt meta]}] + (let [native #js {:key key + :code code + :keyCode key-code + :shiftKey (boolean shift) + :ctrlKey (boolean ctrl) + :altKey (boolean alt) + :metaKey (boolean meta)}] + #js {:key key + :shiftKey (boolean shift) + :ctrlKey (boolean ctrl) + :altKey (boolean alt) + :metaKey (boolean meta) + :getBrowserEvent (fn [] native)})) (defn- mock-native-keydown "Simulates a plain, unwrapped native KeyboardEvent (no getBrowserEvent), @@ -531,7 +540,7 @@ even when the typed glyph differs" (with-redefs [cf/check-platform? (constantly false)] (try - (reset! ds/layout-map #js {:get (fn [code] (when (= code "Period") "."))}) + (.setLayoutMap mousetrap #js {:get (fn [code] (when (= code "Period") "."))}) (t/is (= "shift+." (#'ui-shortcuts/keyboard-event->mousetrap (mock-wrapped-keydown {:key ">" :code "Period" :shift true})))) @@ -539,7 +548,7 @@ (#'ui-shortcuts/keyboard-event->mousetrap (mock-wrapped-keydown {:key ":" :code "Period" :shift true})))) (finally - (reset! ds/layout-map nil)))))) + (.setLayoutMap mousetrap nil)))))) (t/deftest mousetrap-shift-period-on-spanish-iso-layout (t/testing "Spanish (ISO) layout without a layout map: Shift + Period @@ -633,28 +642,27 @@ (t/deftest mousetrap-shifted-digit-resolves-to-digit (t/testing "a digit row key held with Shift records the digit itself: US Shift+0 types \")\" but records \"shift+0\", so the physical - digit key means the same on every layout (production keyCode - behavior)" + digit key means the same on every layout (named by keyCode)" (with-redefs [cf/check-platform? (constantly false)] (t/is (= "shift+0" (#'ui-shortcuts/keyboard-event->mousetrap - (mock-wrapped-keydown {:key ")" :code "Digit0" :shift true})))) + (mock-wrapped-keydown {:key ")" :code "Digit0" :key-code 48 :shift true})))) (t/is (= "shift+1" (#'ui-shortcuts/keyboard-event->mousetrap - (mock-wrapped-keydown {:key "!" :code "Digit1" :shift true})))) + (mock-wrapped-keydown {:key "!" :code "Digit1" :key-code 49 :shift true})))) (t/is (= "shift+2" (#'ui-shortcuts/keyboard-event->mousetrap - (mock-wrapped-keydown {:key "@" :code "Digit2" :shift true})))) + (mock-wrapped-keydown {:key "@" :code "Digit2" :key-code 50 :shift true})))) (t/is (= "shift+8" (#'ui-shortcuts/keyboard-event->mousetrap - (mock-wrapped-keydown {:key "*" :code "Digit8" :shift true}))))))) + (mock-wrapped-keydown {:key "*" :code "Digit8" :key-code 56 :shift true}))))))) (t/deftest mousetrap-shifted-digit-already-digit-unchanged (t/testing "AZERTY Shift+Digit0 already types \"0\": unchanged" (with-redefs [cf/check-platform? (constantly false)] (t/is (= "shift+0" (#'ui-shortcuts/keyboard-event->mousetrap - (mock-wrapped-keydown {:key "0" :code "Digit0" :shift true}))))))) + (mock-wrapped-keydown {:key "0" :code "Digit0" :key-code 48 :shift true}))))))) (t/deftest mousetrap-space-and-named-keys (t/testing "\" \" records as space and named keys stay lowercase" @@ -672,15 +680,15 @@ (#'ui-shortcuts/keyboard-event->mousetrap (mock-wrapped-keydown {:key "Delete" :code "Delete"}))))))) -(t/deftest mousetrap-altgr-drops-phantom-ctrl - (t/testing "Spanish AltGr++] types \"]\" with ctrl+alt flags, but the - phantom ctrl is dropped so no ctrl+alt shortcut fires" +(t/deftest mousetrap-altgr-drops-phantom-modifiers + (t/testing "Spanish AltGr+] types \"]\" with ctrl+alt flags, but both + phantom modifiers are dropped and it records the glyph" (with-redefs [cf/check-platform? (constantly false)] (let [event #js {:key "]" :shiftKey false :ctrlKey true :altKey true :metaKey false :getModifierState (fn [m] (= m "AltGraph")) :getBrowserEvent (fn [] nil)}] - (t/is (= "alt+]" + (t/is (= "]" (#'ui-shortcuts/keyboard-event->mousetrap event))))))) (t/deftest mousetrap-real-ctrl-alt-keeps-both @@ -699,7 +707,7 @@ what the user sees and what Mousetrap matches" (with-redefs [cf/check-platform? (constantly false)] (try - (reset! ds/layout-map #js {:get (fn [code] (when (= code "BracketRight") "+"))}) + (.setLayoutMap mousetrap #js {:get (fn [code] (when (= code "BracketRight") "+"))}) (t/is (= "shift++" (#'ui-shortcuts/keyboard-event->mousetrap (mock-wrapped-keydown {:key "*" :code "BracketRight" :shift true})))) @@ -707,7 +715,7 @@ (#'ui-shortcuts/keyboard-event->mousetrap (mock-wrapped-keydown {:key "+" :code "BracketRight"})))) (finally - (reset! ds/layout-map nil)))))) + (.setLayoutMap mousetrap nil)))))) (t/deftest mousetrap-spanish-shift-plus-without-layout-map-records-glyph (t/testing "Spanish layout without a layout map: Shift + the physical @@ -725,11 +733,24 @@ (#'ui-shortcuts/keyboard-event->mousetrap (mock-wrapped-keydown {:key "+" :code "BracketRight"}))))))) -(t/deftest layout-map-supported-matches-keyboard-api - (t/testing "false without a navigator or without a keyboard API" - (t/is (false? (ds/layout-map-supported? nil))) - (t/is (false? (ds/layout-map-supported? #js {}))) - (t/is (false? (ds/layout-map-supported? #js {:keyboard #js {}})))) - (t/testing "true when getLayoutMap exists (Chromium)" - (t/is (true? (ds/layout-map-supported? - #js {:keyboard #js {:getLayoutMap (fn [] nil)}}))))) +(t/deftest mousetrap-shift-digit-records-the-digit + (t/testing "Shift+1 types \"!\" but records \"shift+1\", the key the user + sees and the default zoom shortcuts use" + (with-redefs [cf/check-platform? (constantly false)] + (t/is (= "shift+1" + (#'ui-shortcuts/keyboard-event->mousetrap + (mock-wrapped-keydown {:key "!" :code "Digit1" :key-code 49 :shift true})))) + (t/is (= "shift+0" + (#'ui-shortcuts/keyboard-event->mousetrap + (mock-wrapped-keydown {:key "=" :code "Digit0" :key-code 48 :shift true}))))))) + +(t/deftest mousetrap-option-letter-records-the-letter + (t/testing "macOS Option+M types \"µ\" and Option+E is a dead key, but + both record the letter" + (with-redefs [cf/check-platform? (constantly false)] + (t/is (= "alt+m" + (#'ui-shortcuts/keyboard-event->mousetrap + (mock-wrapped-keydown {:key "µ" :code "KeyM" :key-code 77 :alt true})))) + (t/is (= "alt+e" + (#'ui-shortcuts/keyboard-event->mousetrap + (mock-wrapped-keydown {:key "Dead" :code "KeyE" :key-code 69 :alt true}))))))) diff --git a/frontend/translations/ar.po b/frontend/translations/ar.po index a027c7f3dd..0b07c80ca7 100644 --- a/frontend/translations/ar.po +++ b/frontend/translations/ar.po @@ -1727,10 +1727,6 @@ msgstr "مُختلط" msgid "shortcuts.add-comment" msgstr "تعليقات" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "إضافة عقدة" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "محاذاة لأسفل" diff --git a/frontend/translations/ca.po b/frontend/translations/ca.po index 900e2048d1..005dc3b908 100644 --- a/frontend/translations/ca.po +++ b/frontend/translations/ca.po @@ -4745,10 +4745,6 @@ msgstr "Selecciona els elements que utilitzen aquest estil" msgid "shortcuts.add-comment" msgstr "Comentaris" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Afegeix node" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Alinea a baix" diff --git a/frontend/translations/cs.po b/frontend/translations/cs.po index 6807d634eb..5760d638f1 100644 --- a/frontend/translations/cs.po +++ b/frontend/translations/cs.po @@ -2597,10 +2597,6 @@ msgstr "Vyberte položky pomocí tohoto stylu" msgid "shortcuts.add-comment" msgstr "Komentáře" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Přidat uzel" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Zarovnat dolů" diff --git a/frontend/translations/de.po b/frontend/translations/de.po index b535d2d859..b04b506455 100644 --- a/frontend/translations/de.po +++ b/frontend/translations/de.po @@ -3507,10 +3507,6 @@ msgstr "Elemente mit diesem Stil auswählen" msgid "shortcuts.add-comment" msgstr "Kommentare" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Punkt hinzufügen" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Unten ausrichten" diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 57b5569932..3fa9a763f3 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -4809,10 +4809,6 @@ msgstr "Select items using this style" msgid "shortcuts.add-comment" msgstr "Comments" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Add node" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Align bottom" @@ -5089,13 +5085,6 @@ msgstr "Join nodes" msgid "shortcuts.key-combo" msgstr "Press the new keys" -#: src/app/main/ui/settings/shortcuts.cljs:484 -msgid "shortcuts.layout-map-missing" -msgstr "" -"This browser does not expose the keyboard layout, so shortcuts that " -"combine Shift with punctuation (for example Shift+,) may not work here. " -"They keep working in Chromium browsers." - #: src/app/main/data/workspace/text/shortcuts.cljs:249 msgid "shortcuts.line-through" msgstr "Toggle line through" @@ -7057,6 +7046,12 @@ msgstr "UPDATES" msgid "workspace.notification-pill.detail" msgstr "Details" +#: src/app/main/data/workspace/notifications.cljs:282 +msgid "workspace.notifications.file-repaired" +msgstr "" +"We fixed some issues in this file. You can find a copy of the original version in " +"the version history." + #: src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs:784 msgid "workspace.options.add-interaction" msgstr "Click the + button to add interactions." diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 6d20e94fe9..431886f0d1 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -4873,10 +4873,6 @@ msgstr "Seleccionar elementos que usan este estilo" msgid "shortcuts.add-comment" msgstr "Añadir comentarios" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Añadir nodo" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Alinear abajo" @@ -5153,13 +5149,6 @@ msgstr "Unir nodos" msgid "shortcuts.key-combo" msgstr "Pulsa las nuevas teclas" -#: src/app/main/ui/settings/shortcuts.cljs:484 -msgid "shortcuts.layout-map-missing" -msgstr "" -"Este navegador no expone el mapa de teclado, así que los atajos que " -"combinan Shift con puntuación (por ejemplo Shift+,) pueden no funcionar " -"aquí. Siguen funcionando en navegadores Chromium." - #: src/app/main/data/workspace/text/shortcuts.cljs:249 msgid "shortcuts.line-through" msgstr "Alternar tachado" @@ -7141,6 +7130,12 @@ msgstr "ACTUALIZACIONES" msgid "workspace.notification-pill.detail" msgstr "Detalles" +#: src/app/main/data/workspace/notifications.cljs:282 +msgid "workspace.notifications.file-repaired" +msgstr "" +"Hemos corregido algunos problemas de este archivo. Hay una copia de la " +"versión original en el historial de versiones." + #: src/app/main/ui/workspace/sidebar/options/menus/interactions.cljs:784 msgid "workspace.options.add-interaction" msgstr "Pulsa el botón + para añadir interacciones." diff --git a/frontend/translations/eu.po b/frontend/translations/eu.po index 78a7089a5e..4594f82431 100644 --- a/frontend/translations/eu.po +++ b/frontend/translations/eu.po @@ -1862,10 +1862,6 @@ msgstr "Batzuk" msgid "shortcuts.add-comment" msgstr "Gehitu iruzkinak" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Gehitu nodoa" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Lerrokatu behean" diff --git a/frontend/translations/fa.po b/frontend/translations/fa.po index bf01868ef6..a8c6460fba 100644 --- a/frontend/translations/fa.po +++ b/frontend/translations/fa.po @@ -4678,10 +4678,6 @@ msgstr "انتخاب موارد با استفاده از این سبک" msgid "shortcuts.add-comment" msgstr "نظرات" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "افزودن گره" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "تراز به پایین" diff --git a/frontend/translations/fr.po b/frontend/translations/fr.po index 5686f05331..63c56a4fc0 100644 --- a/frontend/translations/fr.po +++ b/frontend/translations/fr.po @@ -4785,10 +4785,6 @@ msgstr "Sélectionnez les éléments qui utilisent ce style" msgid "shortcuts.add-comment" msgstr "Commentaires" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Ajouter un nœud" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Aligner en bas" diff --git a/frontend/translations/fr_CA.po b/frontend/translations/fr_CA.po index 79fb1a0323..e5b34259e6 100644 --- a/frontend/translations/fr_CA.po +++ b/frontend/translations/fr_CA.po @@ -4509,10 +4509,6 @@ msgstr "Sélectionner les éléments avec ce style" msgid "shortcuts.add-comment" msgstr "Commentaires" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Ajouter un nœud" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Aligner en bas" diff --git a/frontend/translations/ha.po b/frontend/translations/ha.po index dbc38704ca..6382d0fa45 100644 --- a/frontend/translations/ha.po +++ b/frontend/translations/ha.po @@ -1957,10 +1957,6 @@ msgstr "gauraya" msgid "shortcuts.add-comment" msgstr "Bayyana Ra'ayi" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "kara Girma" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "tsarin kasa" diff --git a/frontend/translations/he.po b/frontend/translations/he.po index 0e9eabca97..38e7e83532 100644 --- a/frontend/translations/he.po +++ b/frontend/translations/he.po @@ -3967,10 +3967,6 @@ msgstr "בחירת פריטים שמשתמשים בסגנון הזה" msgid "shortcuts.add-comment" msgstr "הערות" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "הוספת מפרק" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "יישור לתחתית" diff --git a/frontend/translations/hi.po b/frontend/translations/hi.po index b55d4239cd..7d0d01d99d 100644 --- a/frontend/translations/hi.po +++ b/frontend/translations/hi.po @@ -3362,10 +3362,6 @@ msgstr "इस शैली का उपयोग करके आइटम च msgid "shortcuts.add-comment" msgstr "टिप्पणियाँ" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "नोड जोड़ें" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "नीचे संरेखित करें" diff --git a/frontend/translations/hr.po b/frontend/translations/hr.po index b3c94b2d6a..93081b3c64 100644 --- a/frontend/translations/hr.po +++ b/frontend/translations/hr.po @@ -2596,10 +2596,6 @@ msgstr "Odaberite stavke koristeći ovaj stil" msgid "shortcuts.add-comment" msgstr "Komentari" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Dodaj čvor" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Poravnaj dno" diff --git a/frontend/translations/id.po b/frontend/translations/id.po index 03058d1683..250ff26b5f 100644 --- a/frontend/translations/id.po +++ b/frontend/translations/id.po @@ -2805,10 +2805,6 @@ msgstr "Pilih item menggunakan gaya ini" msgid "shortcuts.add-comment" msgstr "Komentar" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Tambahkan simpul" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Sesuaikan ke bawah" diff --git a/frontend/translations/it.po b/frontend/translations/it.po index 3512c5d007..3fa5b6d7b9 100644 --- a/frontend/translations/it.po +++ b/frontend/translations/it.po @@ -3875,10 +3875,6 @@ msgstr "Seleziona gli elementi che utilizzano questo stile" msgid "shortcuts.add-comment" msgstr "Commenti" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Aggiungi nodo" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Allinea in basso" diff --git a/frontend/translations/jpn_JP.po b/frontend/translations/jpn_JP.po index 7b0a3d78b9..65ee25f35b 100644 --- a/frontend/translations/jpn_JP.po +++ b/frontend/translations/jpn_JP.po @@ -4521,10 +4521,6 @@ msgstr "このスタイルを使用している要素を選択" msgid "shortcuts.add-comment" msgstr "コメント" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "ノードを追加" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "下揃え" diff --git a/frontend/translations/ko.po b/frontend/translations/ko.po index fdb131f37d..e3beca6dcc 100644 --- a/frontend/translations/ko.po +++ b/frontend/translations/ko.po @@ -3200,10 +3200,6 @@ msgstr "이 스타일을 사용하는 항목 선택" msgid "shortcuts.add-comment" msgstr "댓글" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "노드 추가" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "아래 정렬" diff --git a/frontend/translations/lv.po b/frontend/translations/lv.po index 6f2f7b95a9..33ab93be32 100644 --- a/frontend/translations/lv.po +++ b/frontend/translations/lv.po @@ -3106,10 +3106,6 @@ msgstr "Atlasīt vienumus, kas izmanto šo stilu" msgid "shortcuts.add-comment" msgstr "Komentāri" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Pievienot mezglu" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Līdzināt pie apakšas" diff --git a/frontend/translations/ms.po b/frontend/translations/ms.po index 4a858ea689..42d2623a31 100644 --- a/frontend/translations/ms.po +++ b/frontend/translations/ms.po @@ -2019,10 +2019,6 @@ msgstr "Bercampur" msgid "shortcuts.add-comment" msgstr "Komen" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Tambah nota" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Sejajarkan bahagian bawah" diff --git a/frontend/translations/nl.po b/frontend/translations/nl.po index 3d55474017..678e93c364 100644 --- a/frontend/translations/nl.po +++ b/frontend/translations/nl.po @@ -4755,10 +4755,6 @@ msgstr "Selecteer items in deze stijl" msgid "shortcuts.add-comment" msgstr "Commentaar" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Knooppunt toevoegen" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Onderaan uitlijnen" diff --git a/frontend/translations/pl.po b/frontend/translations/pl.po index c3afe44ee7..352c7e9eec 100644 --- a/frontend/translations/pl.po +++ b/frontend/translations/pl.po @@ -1856,10 +1856,6 @@ msgstr "Mieszane" msgid "shortcuts.add-comment" msgstr "Komentarze" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Dodaj węzeł" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Wyrównaj do dołu" diff --git a/frontend/translations/pt_BR.po b/frontend/translations/pt_BR.po index 3c27a3205f..0847bf55bf 100644 --- a/frontend/translations/pt_BR.po +++ b/frontend/translations/pt_BR.po @@ -2511,10 +2511,6 @@ msgstr "Misto" msgid "shortcuts.add-comment" msgstr "Comentários" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Adicionar ponto" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Alinhar a base" diff --git a/frontend/translations/pt_PT.po b/frontend/translations/pt_PT.po index 3140b7a1a6..57f7cbd423 100644 --- a/frontend/translations/pt_PT.po +++ b/frontend/translations/pt_PT.po @@ -2657,10 +2657,6 @@ msgstr "Selecionar itens que usem este estilo" msgid "shortcuts.add-comment" msgstr "Comentários" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Adicionar nó" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Alinhar abaixo" diff --git a/frontend/translations/ro.po b/frontend/translations/ro.po index d4b4fc6751..2581de553f 100644 --- a/frontend/translations/ro.po +++ b/frontend/translations/ro.po @@ -3189,10 +3189,6 @@ msgstr "Alege elemente folosind acest stil" msgid "shortcuts.add-comment" msgstr "Comentarii" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Adaugă nod" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Aliniază jos" diff --git a/frontend/translations/ru.po b/frontend/translations/ru.po index ccf5d11a4c..af06c34ea8 100644 --- a/frontend/translations/ru.po +++ b/frontend/translations/ru.po @@ -4722,10 +4722,6 @@ msgstr "Выбрать элементы, использующие этот ст msgid "shortcuts.add-comment" msgstr "Комментарии" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Добавить узел" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Выровнять по нижнему краю" diff --git a/frontend/translations/sr.po b/frontend/translations/sr.po index 589b4d6c88..75ebabcdde 100644 --- a/frontend/translations/sr.po +++ b/frontend/translations/sr.po @@ -2200,10 +2200,6 @@ msgstr "Изаберите ставке користећи овај стил" msgid "shortcuts.add-comment" msgstr "Коментари" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Додај чвор" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Поравнај на дно" diff --git a/frontend/translations/sv.po b/frontend/translations/sv.po index 293b654895..c749f67233 100644 --- a/frontend/translations/sv.po +++ b/frontend/translations/sv.po @@ -4706,10 +4706,6 @@ msgstr "Välj objekt med denna stil" msgid "shortcuts.add-comment" msgstr "Kommentarer" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Lägg till nod" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Justera botten" diff --git a/frontend/translations/tr.po b/frontend/translations/tr.po index adbc51754c..853e76ab7a 100644 --- a/frontend/translations/tr.po +++ b/frontend/translations/tr.po @@ -4702,10 +4702,6 @@ msgstr "Bu biçimi kullanan ögeleri seç" msgid "shortcuts.add-comment" msgstr "Yorumlar" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Düğüm ekle" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Alta hizala" diff --git a/frontend/translations/ukr_UA.po b/frontend/translations/ukr_UA.po index e716947918..519ce16bf8 100644 --- a/frontend/translations/ukr_UA.po +++ b/frontend/translations/ukr_UA.po @@ -3581,10 +3581,6 @@ msgstr "Оберіть елементи, використовуючи цей с msgid "shortcuts.add-comment" msgstr "Коментарі" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Додати вузол" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Вирівняти по низу" diff --git a/frontend/translations/yo.po b/frontend/translations/yo.po index 1f8e5ab1b4..23d4d7dfc4 100644 --- a/frontend/translations/yo.po +++ b/frontend/translations/yo.po @@ -1844,10 +1844,6 @@ msgstr "Dàárú" msgid "shortcuts.add-comment" msgstr "Ọ́rọ̀íwòye" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "Fí ípàdé kùn" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "Sópọ sí ísàle" diff --git a/frontend/translations/zh_CN.po b/frontend/translations/zh_CN.po index 94376eaabc..f17366656f 100644 --- a/frontend/translations/zh_CN.po +++ b/frontend/translations/zh_CN.po @@ -3948,10 +3948,6 @@ msgstr "选择使用此样式的项目" msgid "shortcuts.add-comment" msgstr "评论" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "添加节点" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "底部对齐" diff --git a/frontend/translations/zh_Hant.po b/frontend/translations/zh_Hant.po index ebb8577137..56f41a99d3 100644 --- a/frontend/translations/zh_Hant.po +++ b/frontend/translations/zh_Hant.po @@ -4443,10 +4443,6 @@ msgstr "選取使用此樣式的項目" msgid "shortcuts.add-comment" msgstr "留言" -#: src/app/main/data/workspace/path/shortcuts.cljs:61 -msgid "shortcuts.add-node" -msgstr "新增節點" - #: src/app/main/data/workspace/shortcuts.cljs:532 msgid "shortcuts.align-bottom" msgstr "靠下對齊" diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index f74e320625..08b8e2d58e 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -4,7 +4,7 @@ - **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 `LibraryComponent.restore()` to restore a deleted component and its main instance without changing the current page, selection, zoom, or viewport. Requires `library:write` permission and only applies to the current file's library. - **plugin-types:** Added `Shape.updateMain()`, `LibraryComponent.duplicate()`, writable component `annotation`, and `Library.deletedComponents`. Declared the existing `LibraryComponent.remove()` method. - **plugin-types:** Added `LibraryContext.tokensSource` to read or set the current file's token source to a connected library, or restore local tokens. - **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. diff --git a/plugins/apps/composable-test-suite/src/composable-tests/cases.ts b/plugins/apps/composable-test-suite/src/composable-tests/cases.ts index e9150ec16c..793903e15d 100644 --- a/plugins/apps/composable-test-suite/src/composable-tests/cases.ts +++ b/plugins/apps/composable-test-suite/src/composable-tests/cases.ts @@ -5,6 +5,7 @@ import { createTestCaseRemoteMainCopySyncNested } from "./cases/caseRemoteMainCo import { createTestCaseVariantSwitchPropagates } from "./cases/caseVariantSwitchPropagates.ts"; import { createTestCaseCopySubheadDeletePreservesSlots } from "./cases/caseCopySubheadDeletePreservesSlots.ts"; import { createTestCaseMainReorderKeepsCopySlots } from "./cases/caseMainReorderKeepsCopySlots.ts"; +import { createTestCaseMainChildMovedOutLeavesCopy } from "./cases/caseMainChildMovedOutLeavesCopy.ts"; /** * All composable test cases currently defined. A factory (not a constant): each @@ -19,5 +20,6 @@ export function allCases(): readonly TestCase[] { createTestCaseVariantSwitchPropagates(), createTestCaseCopySubheadDeletePreservesSlots(), createTestCaseMainReorderKeepsCopySlots(), + createTestCaseMainChildMovedOutLeavesCopy(), ]; } diff --git a/plugins/apps/composable-test-suite/src/composable-tests/cases/caseMainChildMovedOutLeavesCopy.ts b/plugins/apps/composable-test-suite/src/composable-tests/cases/caseMainChildMovedOutLeavesCopy.ts new file mode 100644 index 0000000000..04df9632fd --- /dev/null +++ b/plugins/apps/composable-test-suite/src/composable-tests/cases/caseMainChildMovedOutLeavesCopy.ts @@ -0,0 +1,59 @@ +import { Board } from "@penpot/plugin-types"; +import { TestCase } from "../test-suite/TestCase.ts"; +import { Color } from "../model/Color"; +import { ShapePropFillColor } from "../model/ShapeProp.ts"; +import { Assert } from "../util/Assert.ts"; +import { OpAssert } from "../operations/OpAssert"; +import { OpSequence } from "../operations/OpSequence.ts"; +import { OpOptional } from "../operations/OpOptional.ts"; +import { OpChangeProperty } from "../operations/OpChangeProperty"; +import { OpCreateBoard } from "../operations/OpCreateBoard"; +import { OpCreateSimpleComponentWithCopy } from "../operations/OpCreateSimpleComponentWithCopy"; +import { OpMoveShape } from "../operations/OpMoveShape"; + +const BASELINE = new Color("#aaaaaa"); +const EDIT_COLOR = new Color("#ff0000"); + +/** + * A shape moved out of a main leaves its copies, also when another edit lands + * right after the move, before propagation has run. + * + * The baseline assertion waits for the setup to settle; the setup's own commits + * would otherwise sync the copy in the same pass and hide the result. The move + * and the optional edit then run back to back, so their commits arrive together; + * without the edit, the move is the last commit. + */ +export function createTestCaseMainChildMovedOutLeavesCopy(): TestCase { + const foundation = new OpCreateSimpleComponentWithCopy(BASELINE); + const { mainChild, copyRoot } = foundation.roles; + const outside = new OpCreateBoard("Outside"); + const outsideBoard = outside.roles.board; + + const move = new OpMoveShape(mainChild, outsideBoard, "main rect", "outside board"); + + return new TestCase( + "MainChildMovedOutLeavesCopy", + "A component containing a single rectangle is created, plus a copy of it and a board " + + "outside the component. The main's rectangle is moved into that board, optionally " + + "followed at once by a fill change on the board. The copy must lose its rectangle " + + "either way.", + new OpSequence( + foundation, + outside, + new OpAssert("the copy starts with its rectangle", (s) => { + const copyChildren = (s.get(copyRoot) as Board).children; + Assert.that(copyChildren.length === 1, `the copy should have one child, found ${copyChildren.length}`); + }), + move, + new OpOptional(new OpChangeProperty(outsideBoard, new ShapePropFillColor(), EDIT_COLOR, "outside board")), + new OpAssert("the rectangle left the main and the copy", (s) => { + move.assertHasMoved(s, mainChild); + const copyChildren = (s.get(copyRoot) as Board).children; + Assert.that( + copyChildren.length === 0, + `the copy should have no children, found ${copyChildren.map((c) => c.name).join(", ")}` + ); + }) + ) + ); +} diff --git a/plugins/apps/composable-test-suite/src/composable-tests/operations/OpCreateBoard.ts b/plugins/apps/composable-test-suite/src/composable-tests/operations/OpCreateBoard.ts new file mode 100644 index 0000000000..eeb7bb64ff --- /dev/null +++ b/plugins/apps/composable-test-suite/src/composable-tests/operations/OpCreateBoard.ts @@ -0,0 +1,39 @@ +import { Board } from "@penpot/plugin-types"; +import { Operation } from "../core/Operation"; +import { Role } from "../core/Role"; +import { RoleBundle } from "../core/RoleBundle"; +import { Situation } from "../core/Situation"; + +/** The roles exposed by {@link OpCreateBoard}. */ +class RolesBoard extends RoleBundle { + /** The free board the operation created. */ + readonly board = new Role("board"); +} + +/** + * Creates a free board on the page, outside any component, and binds it to its + * `board` role. Gives a case somewhere to move shapes to, or to edit, apart from + * the configuration under test. + */ +export class OpCreateBoard extends Operation { + readonly roles = new RolesBoard(); + + /** + * @param boardName - the name given to the new board + */ + constructor(private readonly boardName: string) { + super(); + } + + async applyTo(situation: Situation): Promise { + const board = penpot.createBoard(); + board.name = this.boardName; + board.resize(100, 100); + situation.applyPosAdvanceX(board); + situation.bind(this.roles.board, board); + } + + toString(): string { + return `create board ${this.boardName}`; + } +} diff --git a/plugins/apps/composable-test-suite/src/composable-tests/operations/OpMoveShape.ts b/plugins/apps/composable-test-suite/src/composable-tests/operations/OpMoveShape.ts new file mode 100644 index 0000000000..6795dc121b --- /dev/null +++ b/plugins/apps/composable-test-suite/src/composable-tests/operations/OpMoveShape.ts @@ -0,0 +1,53 @@ +import { Board } from "@penpot/plugin-types"; +import { Operation } from "../core/Operation"; +import { Situation } from "../core/Situation"; +import { ShapeTarget, resolveTarget } from "../core/ShapeTarget"; +import { Assert } from "../util/Assert"; + +/** + * Moves the shape a target resolves to into another parent, via the Plugin API's + * `appendChild`. Used to move a shape out of the main of a component, which must + * remove the corresponding shape from its copies. + */ +export class OpMoveShape extends Operation { + /** + * @param target - resolves the shape to move (role or situation-derived) + * @param newParent - resolves the board that receives the shape + * @param targetLabel - a human-readable name for the moved shape, for the log + * @param parentLabel - a human-readable name for the new parent, for the log + */ + constructor( + private readonly target: ShapeTarget, + private readonly newParent: ShapeTarget, + private readonly targetLabel: string = String(target), + private readonly parentLabel: string = String(newParent) + ) { + super(); + } + + async applyTo(situation: Situation): Promise { + const shape = resolveTarget(this.target, situation); + const parent = resolveTarget(this.newParent, situation) as Board; + parent.appendChild(shape); + } + + /** + * Asserts that the shape `target` resolves to now sits in this operation's + * new parent. + * + * @param situation - the situation to resolve the targets against + * @param target - resolves the shape expected to have moved + */ + assertHasMoved(situation: Situation, target: ShapeTarget): void { + const shape = resolveTarget(target, situation); + const parent = resolveTarget(this.newParent, situation); + Assert.that( + shape.parent?.id === parent.id, + `expected "${shape.name}" inside "${parent.name}" but its parent is "${shape.parent?.name}"` + ); + } + + toString(): string { + return `move ${this.targetLabel} into ${this.parentLabel}`; + } +} diff --git a/plugins/libs/plugin-types/index.d.ts b/plugins/libs/plugin-types/index.d.ts index 5d6c27e156..94858bdffb 100644 --- a/plugins/libs/plugin-types/index.d.ts +++ b/plugins/libs/plugin-types/index.d.ts @@ -3060,7 +3060,8 @@ export interface LibraryComponent extends LibraryElement { remove(): void; /** - * Restores this component if it has been deleted, including its main instance. + * Restores this component if it has been deleted, including its main instance, + * in the current file's library. * Leaves the current page, selection, zoom, and viewport unchanged. * Requires `library:write` permission. Does nothing if already restored. *