From 7391a4086af2cd22d23182191ec81be9560e98fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Fri, 10 Mar 2023 15:43:26 +0100 Subject: [PATCH] :wrench: Refactor delete/restore components --- backend/src/app/rpc/commands/files.clj | 16 +- backend/src/app/tasks/file_gc.clj | 34 ++--- common/src/app/common/data.cljc | 16 ++ common/src/app/common/pages/changes.cljc | 39 +---- .../src/app/common/pages/changes_builder.cljc | 34 ++--- common/src/app/common/pages/helpers.cljc | 15 +- common/src/app/common/types/component.cljc | 7 +- .../src/app/common/types/components_list.cljc | 29 +++- common/src/app/common/types/container.cljc | 6 +- common/src/app/common/types/file.cljc | 100 +++++-------- common/src/app/common/types/modifiers.cljc | 2 +- common/src/app/common/types/pages_list.cljc | 6 +- common/src/app/common/types/shape_tree.cljc | 47 +++++- common/test/common_tests/data_test.cljc | 38 +++++ .../test/common_tests/pages_helpers_test.cljc | 44 +----- frontend/src/app/libs/file_builder.cljs | 3 +- .../src/app/main/data/workspace/changes.cljs | 3 +- .../app/main/data/workspace/libraries.cljs | 54 ++----- .../data/workspace/libraries_helpers.cljs | 4 +- .../src/app/main/data/workspace/shapes.cljs | 2 +- .../main/data/workspace/state_helpers.cljs | 4 - .../src/app/main/ui/workspace/libraries.cljs | 3 +- .../app/main/ui/workspace/sidebar/assets.cljs | 50 ++++--- frontend/src/app/render.cljs | 3 +- frontend/src/app/worker/export.cljs | 7 +- .../frontend_tests/helpers/libraries.cljs | 2 - .../frontend_tests/state_components_test.cljs | 138 ++++++++++++++---- 27 files changed, 378 insertions(+), 328 deletions(-) diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index 258ac59fc..5bc4fa07b 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -13,6 +13,7 @@ [app.common.pages.helpers :as cph] [app.common.pages.migrations :as pmg] [app.common.spec :as us] + [app.common.types.components-list :as ctkl] [app.common.types.file :as ctf] [app.common.types.shape-tree :as ctt] [app.config :as cf] @@ -485,16 +486,19 @@ (defn get-team-shared-files [conn team-id] (letfn [(assets-sample [assets limit] - (let [sorted-assets (->> (vals assets) - (sort-by #(str/lower (:name %))))] - {:count (count sorted-assets) - :sample (into [] (take limit sorted-assets))})) + (let [sorted-assets (->> (vals assets) + (sort-by #(str/lower (:name %))))] + {:count (count sorted-assets) + :sample (into [] (take limit sorted-assets))})) (library-summary [{:keys [id data] :as file}] (binding [pmap/*load-fn* (partial load-pointer conn id)] - (let [components-sample (-> (assets-sample (:components data) 4) + (let [load-objects (fn [component] + (binding [pmap/*load-fn* (partial load-pointer conn id)] + (ctf/load-component-objects data component))) + components-sample (-> (assets-sample (ctkl/components data) 4) (update :sample - #(map (partial ctf/load-component-objects data) %)))] + #(map load-objects %)))] {:components components-sample :media (assets-sample (:media data) 3) :colors (assets-sample (:colors data) 3) diff --git a/backend/src/app/tasks/file_gc.clj b/backend/src/app/tasks/file_gc.clj index f673213b3..81464453f 100644 --- a/backend/src/app/tasks/file_gc.clj +++ b/backend/src/app/tasks/file_gc.clj @@ -13,6 +13,7 @@ [app.common.data :as d] [app.common.logging :as l] [app.common.pages.migrations :as pmg] + [app.common.types.components-list :as ctkl] [app.common.types.file :as ctf] [app.common.types.shape-tree :as ctt] [app.config :as cf] @@ -204,36 +205,31 @@ (filter #(ctf/used-in? file-data library-id % :component)) components)) - find-used-components + find-unused-components (fn [components files-data] - ; Find what components are used in any of the files. + ; Find what components are NOT used in any of the files. (loop [files-data files-data - components components - used-components #{}] + components components] (let [file-data (first files-data)] (if (or (nil? file-data) (empty? components)) - used-components + components (let [used-components-file (find-used-components-file components file-data)] (recur (rest files-data) - (into #{} (remove used-components-file) components) - (into used-components used-components-file))))))) + (into #{} (remove used-components-file) components))))))) - deleted-components (set (vals (:deleted-components library-data))) - saved-components (find-used-components deleted-components - (cons library-data - (retrieve-client-files conn library-id))) - new-deleted-components (d/index-by :id (vec saved-components)) - - total (- (count deleted-components) - (count saved-components))] + deleted-components (set (ctkl/deleted-components-seq library-data)) + unused-components (find-unused-components deleted-components + (cons library-data + (retrieve-client-files conn library-id))) + total (count unused-components)] (when-not (zero? total) (l/debug :hint "clean deleted components" :total total) - (let [new-data (-> library-data - (assoc :deleted-components new-deleted-components) - (blob/encode))] + (let [new-data (reduce #(ctkl/delete-component %1 (:id %2)) + library-data + unused-components)] (db/update! conn :file - {:data new-data} + {:data (blob/encode new-data)} {:id library-id}))))) (def ^:private sql:get-unused-fragments diff --git a/common/src/app/common/data.cljc b/common/src/app/common/data.cljc index bf2ea3ced..6007e5a6d 100644 --- a/common/src/app/common/data.cljc +++ b/common/src/app/common/data.cljc @@ -255,6 +255,11 @@ (subvec v 0 index) (subvec v (inc index)))) +(defn without-obj + "Clear collection from specified obj and without nil values." + [coll o] + (into [] (filter #(not= % o)) coll)) + (defn zip [col1 col2] (map vector col1 col2)) @@ -477,6 +482,17 @@ (->> (apply c/iteration args) (concat-all))) +(defn insert-at-index + "Insert a list of elements at the given index of a previous list. + Replace all existing elems." + [elems index new-elems] + (let [[before after] (split-at index elems) + p? (set new-elems)] + (concat-vec [] + (remove p? before) + new-elems + (remove p? after)))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Data Parsing / Conversion diff --git a/common/src/app/common/pages/changes.cljc b/common/src/app/common/pages/changes.cljc index 2e58f5440..77ad253fb 100644 --- a/common/src/app/common/pages/changes.cljc +++ b/common/src/app/common/pages/changes.cljc @@ -26,15 +26,6 @@ [app.common.types.shape-tree :as ctst] [app.common.types.typographies-list :as ctyl])) -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; -;; Specific helpers -;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; - -(defn- without-obj - "Clear collection from specified obj and without nil values." - [coll o] - (into [] (filter #(not= % o)) coll)) - ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Page Transformation Changes ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -111,27 +102,9 @@ (defmethod process-change :del-obj [data {:keys [page-id component-id id ignore-touched]}] - (letfn [(delete-from-parent [parent] - (let [parent (update parent :shapes without-obj id)] - (cond-> parent - (and (:shape-ref parent) - (not ignore-touched)) - (-> (update :touched cph/set-touched-group :shapes-group) - (dissoc :remote-synced?))))) - - (delete-from-objects [objects] - (if-let [target (get objects id)] - (let [parent-id (or (:parent-id target) - (:frame-id target)) - children (cph/get-children id objects)] - (-> (reduce dissoc objects children) - (dissoc id) - (d/update-when parent-id delete-from-parent))) - objects))] - - (if page-id - (d/update-in-when data [:pages-index page-id :objects] delete-from-objects) - (d/update-in-when data [:components component-id :objects] delete-from-objects)))) + (if page-id + (d/update-in-when data [:pages-index page-id] ctst/delete-shape id ignore-touched) + (d/update-in-when data [:components component-id] ctst/delete-shape id ignore-touched))) ;; reg-objects operation "regenerates" the geometry and selrect of the parent groups (defmethod process-change :reg-objects @@ -197,7 +170,7 @@ (insert-items [prev-shapes index shapes] (let [prev-shapes (or prev-shapes [])] (if index - (cph/insert-at-index prev-shapes index shapes) + (d/insert-at-index prev-shapes index shapes) (cph/append-at-the-end prev-shapes shapes)))) (add-to-parent [parent index shapes] @@ -226,7 +199,7 @@ (not ignore-touched))] (-> objects - (d/update-in-when [pid :shapes] without-obj sid) + (d/update-in-when [pid :shapes] d/without-obj sid) (d/update-in-when [pid :shapes] d/vec-without-nils) (cond-> component? (d/update-when pid #(-> % (update :touched cph/set-touched-group :shapes-group) @@ -301,7 +274,7 @@ (defmethod process-change :mov-page [data {:keys [id index]}] - (update data :pages cph/insert-at-index index [id])) + (update data :pages d/insert-at-index index [id])) (defmethod process-change :add-color [data {:keys [color]}] diff --git a/common/src/app/common/pages/changes_builder.cljc b/common/src/app/common/pages/changes_builder.cljc index 158e5044a..1c60fb09b 100644 --- a/common/src/app/common/pages/changes_builder.cljc +++ b/common/src/app/common/pages/changes_builder.cljc @@ -15,7 +15,6 @@ [app.common.math :as mth] [app.common.pages :as cp] [app.common.pages.helpers :as cph] - [app.common.types.components-list :as ctkl] [app.common.types.file :as ctf] [app.common.uuid :as uuid])) @@ -612,7 +611,8 @@ (fn [undo-changes] (-> undo-changes (d/preconj {:type :del-component - :id id}) + :id id + :skip-undelete? true}) (into (comp (map :id) (map lookupf) (map mk-change)) @@ -631,7 +631,7 @@ :id id :name (:name new-component) :path (:path new-component) - :objects (:objects new-component)}) + :objects (:objects new-component)}) ;; this won't exist in components-v2 (update :undo-changes d/preconj {:type :mod-component :id id :name (:name prev-component) @@ -640,29 +640,13 @@ changes))) (defn delete-component - [changes id components-v2] + [changes id] (assert-library changes) - (let [library-data (::library-data (meta changes)) - component (ctkl/get-component library-data id) - shapes (ctf/get-component-shapes library-data component)] - (-> changes - (update :redo-changes conj {:type :del-component - :id id}) - (update :undo-changes - (fn [undo-changes] - (cond-> undo-changes - components-v2 - (d/preconj {:type :purge-component - :id id}) - - :always - (d/preconj {:type :add-component - :id id - :name (:name component) - :path (:path component) - :main-instance-id (:main-instance-id component) - :main-instance-page (:main-instance-page component) - :shapes shapes}))))))) + (-> changes + (update :redo-changes conj {:type :del-component + :id id}) + (update :undo-changes d/preconj {:type :restore-component + :id id}))) (defn restore-component [changes id] diff --git a/common/src/app/common/pages/helpers.cljc b/common/src/app/common/pages/helpers.cljc index d28d4d044..7f90b3277 100644 --- a/common/src/app/common/pages/helpers.cljc +++ b/common/src/app/common/pages/helpers.cljc @@ -9,6 +9,8 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.spec :as us] + [app.common.types.components-list :as ctkl] + [app.common.types.pages-list :as ctpl] [app.common.types.shape.layout :as ctl] [app.common.uuid :as uuid] [cuerdas.core :as str])) @@ -300,8 +302,8 @@ (us/assert uuid? id) (-> (if (= type :page) - (get-in file [:pages-index id]) - (get-in file [:components id])) + (ctpl/get-page file id) + (ctkl/get-component file id)) (assoc :type type))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -321,15 +323,6 @@ (update page :objects #(into % (d/index-by :id objects-list)))) -(defn insert-at-index - [objects index ids] - (let [[before after] (split-at index objects) - p? (set ids)] - (d/concat-vec [] - (remove p? before) - ids - (remove p? after)))) - (defn append-at-the-end [prev-ids ids] (reduce (fn [acc id] diff --git a/common/src/app/common/types/component.cljc b/common/src/app/common/types/component.cljc index fee289292..d963f441b 100644 --- a/common/src/app/common/types/component.cljc +++ b/common/src/app/common/types/component.cljc @@ -34,10 +34,11 @@ (and (= shape-id (:main-instance-id component)) (= page-id (:main-instance-page component)))) -;; Obsolete in components-v2 (defn get-component-root [component] - (get-in component [:objects (:id component)])) + (if (some? (:main-instance-id component)) + (get-in component [:objects (:main-instance-id component)]) + (get-in component [:objects (:id component)]))) (defn uses-library-components? "Check if the shape uses any component in the given library." @@ -67,5 +68,3 @@ :remote-synced? :shape-ref :touched)) - - diff --git a/common/src/app/common/types/components_list.cljc b/common/src/app/common/types/components_list.cljc index 7bbb5ab0b..1a9b40f0b 100644 --- a/common/src/app/common/types/components_list.cljc +++ b/common/src/app/common/types/components_list.cljc @@ -10,9 +10,18 @@ [app.common.data.macros :as dm] [app.common.files.features :as feat])) +(defn components + [file-data] + (d/removem (fn [[_ component]] (:deleted component)) + (:components file-data))) + (defn components-seq [file-data] - (vals (:components file-data))) + (remove :deleted (vals (:components file-data)))) + +(defn deleted-components-seq + [file-data] + (filter :deleted (vals (:components file-data)))) (defn add-component [file-data {:keys [id name path main-instance-id main-instance-page shapes]}] @@ -53,7 +62,15 @@ (defn get-component [file-data component-id] - (get-in file-data [:components component-id])) + (let [component (get-in file-data [:components component-id])] + (when-not (:deleted component) + component))) + +(defn get-deleted-component + [file-data component-id] + (let [component (get-in file-data [:components component-id])] + (when (:deleted component) + component))) (defn update-component [file-data component-id f] @@ -62,3 +79,11 @@ (defn delete-component [file-data component-id] (update file-data :components dissoc component-id)) + +(defn mark-component-deleted + [file-data component-id] + (assoc-in file-data [:components component-id :deleted] true)) + +(defn mark-component-undeleted + [file-data component-id] + (d/dissoc-in file-data [:components component-id :deleted])) diff --git a/common/src/app/common/types/container.cljc b/common/src/app/common/types/container.cljc index c23bd1475..9afd74327 100644 --- a/common/src/app/common/types/container.cljc +++ b/common/src/app/common/types/container.cljc @@ -6,11 +6,11 @@ (ns app.common.types.container (:require - [app.common.data.macros :as dm] [app.common.geom.point :as gpt] [app.common.geom.shapes :as gsh] [app.common.pages.common :as common] [app.common.spec :as us] + [app.common.types.components-list :as ctkl] [app.common.types.pages-list :as ctpl] [app.common.types.shape-tree :as ctst] [clojure.spec.alpha :as s])) @@ -43,8 +43,8 @@ (us/assert uuid? id) (-> (if (= type :page) - (dm/get-in file [:pages-index id]) - (dm/get-in file [:components id])) + (ctpl/get-page file id) + (ctkl/get-component file id)) (assoc :type type))) (defn get-shape diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc index 6dfa41450..4045043a0 100644 --- a/common/src/app/common/types/file.cljc +++ b/common/src/app/common/types/file.cljc @@ -131,7 +131,7 @@ or the component itself in v1)" [file-data component] (let [components-v2 (dm/get-in file-data [:options :components-v2])] - (if components-v2 + (if (and components-v2 (not (:deleted component))) (let [component-page (get-component-page file-data component)] (cph/make-container component-page :page)) (cph/make-container component :component)))) @@ -140,26 +140,17 @@ "Retrieve the root shape of the component." [file-data component] (let [components-v2 (dm/get-in file-data [:options :components-v2])] - (if components-v2 + (if (and components-v2 (not (:deleted component))) (-> file-data (get-component-page component) (ctn/get-shape (:main-instance-id component))) (ctk/get-component-root component)))) -(defn get-component-shapes - "Retrieve all shapes of the component" - [file-data component] - (let [components-v2 (dm/get-in file-data [:options :components-v2])] - (if components-v2 - (let [instance-page (get-component-page file-data component)] - (cph/get-children-with-self (:objects instance-page) (:main-instance-id component))) - (vals (:objects component))))) - (defn get-component-shape - "Retrieve one shape in the component." + "Retrieve one shape in the component by id." [file-data component shape-id] (let [components-v2 (dm/get-in file-data [:options :components-v2])] - (if components-v2 + (if (and components-v2 (not (:deleted component))) (let [component-page (get-component-page file-data component)] (ctn/get-shape component-page shape-id)) (dm/get-in component [:objects shape-id])))) @@ -171,73 +162,54 @@ (when (:shape-ref shape) (get-component-shape file-data component (:shape-ref shape)))) +(defn get-component-shapes + "Retrieve all shapes of the component" + [file-data component] + (let [components-v2 (dm/get-in file-data [:options :components-v2])] + (if components-v2 + (let [instance-page (get-component-page file-data component)] + (cph/get-children-with-self (:objects instance-page) (:main-instance-id component))) + (vals (:objects component))))) + (defn load-component-objects "Add an :objects property to the component, with only the shapes that belong to it" [file-data component] (let [components-v2 (dm/get-in file-data [:options :components-v2])] - (if components-v2 + (if (and components-v2 component (nil? (:objects component))) ;; This operation may be called twice, e.g. in an idempotent change (let [component-page (get-component-page file-data component) - page-objects (:objects component-page) - objects (->> (cons (:main-instance-id component) - (cph/get-children-ids page-objects (:main-instance-id component))) - (map #(get page-objects %)) - (d/index-by :id))] + page-objects (:objects component-page) + objects (->> (cons (:main-instance-id component) + (cph/get-children-ids page-objects (:main-instance-id component))) + (map #(get page-objects %)) + (d/index-by :id))] (assoc component :objects objects)) component))) (defn delete-component - "Delete a component and store it to be able to be recovered later. - - Remember also the position of the main instance." + "Mark a component as deleted and store the main instance shapes iside it, to + be able to be recovered later." ([file-data component-id] (delete-component file-data component-id false)) - ([file-data component-id _skip-undelete?] - (let [_components-v2 (dm/get-in file-data [:options :components-v2]) - - ;; TODO: replace :deleted-components with a :deleted? flag in normal shapes - ;; see task https://tree.taiga.io/project/penpot/task/4998 - ;; - ;; add-to-deleted-components - ;; (fn [file-data] - ;; (let [component (ctkl/get-component file-data component-id)] - ;; (if (some? component) - ;; (let [main-instance (get-component-root file-data component) - ;; component (assoc component - ;; :main-instance-x (:x main-instance) ; An instance root is always a group, - ;; :main-instance-y (:y main-instance))] ; or a frame, so it will have :x and :y - ;; (when (nil? main-instance) - ;; (throw (ex-info "Cannot delete the main instance before the component" {:component-id component-id}))) - ;; (assoc-in file-data [:deleted-components component-id] component)) - ;; file-data))) - ] - - (cond-> file-data - ;; (and components-v2 (not skip-undelete?)) - ;; (add-to-deleted-components) - - :always - (ctkl/delete-component component-id))))) - -(defn get-deleted-component - "Retrieve a component that has been deleted but still is in the safe store." - [file-data component-id] - (dm/get-in file-data [:deleted-components component-id])) + ([file-data component-id skip-undelete?] + (let [components-v2 (dm/get-in file-data [:options :components-v2])] + (if (or (not components-v2) skip-undelete?) + (ctkl/delete-component file-data component-id) + (-> file-data + (ctkl/update-component component-id (partial load-component-objects file-data)) + (ctkl/mark-component-deleted component-id)))))) (defn restore-component - "Recover a deleted component and put it again in place." + "Recover a deleted component and all its shapes and put all this again in place." [file-data component-id] - (let [component (-> (dm/get-in file-data [:deleted-components component-id]) - (dissoc :main-instance-x :main-instance-y))] - (cond-> file-data - (some? component) - (-> (assoc-in [:components component-id] component) - (d/dissoc-in [:deleted-components component-id]))))) + (-> file-data + (ctkl/update-component component-id #(dissoc % :objects)) + (ctkl/mark-component-undeleted component-id))) (defn purge-component "Remove permanently a component." [file-data component-id] - (d/dissoc-in file-data [:deleted-components component-id])) + (ctkl/delete-component file-data component-id)) (defmulti uses-asset? "Checks if a shape uses the given asset." @@ -555,7 +527,7 @@ ([file-data page-id libraries show-ids show-touched] (let [page (ctpl/get-page file-data page-id) objects (:objects page) - components (:components file-data) + components (ctkl/components file-data) root (d/seek #(nil? (:parent-id %)) (vals objects))] (letfn [(show-shape [shape-id level objects] @@ -590,7 +562,7 @@ component-file (when component-file-id (get libraries component-file-id nil)) component (when component-id (if component-file - (dm/get-in component-file [:data :components component-id]) + (ctkl/get-component (:data component-file) component-id) (get components component-id))) component-shape (when component (if component-file @@ -611,7 +583,7 @@ component-file-id (:component-file shape) component-file (when component-file-id (get libraries component-file-id nil)) component (if component-file - (dm/get-in component-file [:data :components component-id]) + (ctkl/get-component (:data component-file) component-id) (get components component-id))] (str/format " (%s%s)" (when component-file (str/format "<%s> " (:name component-file))) diff --git a/common/src/app/common/types/modifiers.cljc b/common/src/app/common/types/modifiers.cljc index 6c3b3acde..5333f4488 100644 --- a/common/src/app/common/types/modifiers.cljc +++ b/common/src/app/common/types/modifiers.cljc @@ -709,7 +709,7 @@ (update shape :shapes (fn [shapes] (if (vector? shapes) - (cph/insert-at-index shapes index value) + (d/insert-at-index shapes index value) (d/concat-vec shapes value)))) (update shape :shapes d/concat-vec value))) diff --git a/common/src/app/common/types/pages_list.cljc b/common/src/app/common/types/pages_list.cljc index 777b6c072..4821d098e 100644 --- a/common/src/app/common/types/pages_list.cljc +++ b/common/src/app/common/types/pages_list.cljc @@ -6,8 +6,8 @@ (ns app.common.types.pages-list (:require - [app.common.data.macros :as dm] - [app.common.pages.helpers :as cph])) + [app.common.data :as d] + [app.common.data.macros :as dm])) (defn get-page [file-data id] @@ -23,7 +23,7 @@ (cond exists? pages (nil? index) (conj pages id) - :else (cph/insert-at-index pages index [id]))))) + :else (d/insert-at-index pages index [id]))))) (update :pages-index assoc id (dissoc page :index)))) (defn pages-seq diff --git a/common/src/app/common/types/shape_tree.cljc b/common/src/app/common/types/shape_tree.cljc index b6ab8f735..4d2414325 100644 --- a/common/src/app/common/types/shape_tree.cljc +++ b/common/src/app/common/types/shape_tree.cljc @@ -36,7 +36,7 @@ (conj shapes id) :else - (cph/insert-at-index shapes index [id])))) + (d/insert-at-index shapes index [id])))) update-parent (fn [parent] @@ -49,28 +49,63 @@ (-> (update :touched cph/set-touched-group :shapes-group) (dissoc :remote-synced?))))) - ;; TODO: this looks wrong, why we allow nil values? update-objects (fn [objects parent-id] - (if (and (or (nil? parent-id) (contains? objects parent-id)) - (or (nil? frame-id) (contains? objects frame-id))) + (let [parent-id (if (contains? objects parent-id) + parent-id + uuid/zero) + frame-id (if (contains? objects frame-id) + frame-id + uuid/zero)] (-> objects (assoc id (-> shape (assoc :frame-id frame-id) (assoc :parent-id parent-id) (assoc :id id))) - (update parent-id update-parent)) - objects)) + (update parent-id update-parent)))) parent-id (or parent-id frame-id)] (update container :objects update-objects parent-id))) +(defn get-shape + "Get a shape identified by id" + [container id] + (-> container :objects (get id))) + (defn set-shape "Replace a shape in the tree with a new one" [container shape] (assoc-in container [:objects (:id shape)] shape)) +(defn delete-shape + "Remove a shape and all its children from the tree. + + Remove it also from its parent, and marks it as touched + if needed, unless ignore-touched is true." + ([container shape-id] + (delete-shape container shape-id false)) + + ([container shape-id ignore-touched] + (letfn [(delete-from-parent [parent] + (let [parent (update parent :shapes d/without-obj shape-id)] + (cond-> parent + (and (:shape-ref parent) (not ignore-touched)) + (-> (update :touched cph/set-touched-group :shapes-group) + (dissoc :remote-synced?))))) + + (delete-from-objects [objects] + (if-let [target (get objects shape-id)] + (let [parent-id (or (:parent-id target) + (:frame-id target)) + children-ids (cph/get-children-ids objects shape-id)] + (-> (reduce dissoc objects children-ids) + (dissoc shape-id) + (d/update-when parent-id delete-from-parent))) + objects))] + + (update container :objects delete-from-objects)))) + (defn get-frames "Retrieves all frame objects as vector" [objects] diff --git a/common/test/common_tests/data_test.cljc b/common/test/common_tests/data_test.cljc index 0c0521003..10a3f830f 100644 --- a/common/test/common_tests/data_test.cljc +++ b/common/test/common_tests/data_test.cljc @@ -65,3 +65,41 @@ (t/is (= 0 (d/check-num [] 0))) (t/is (= 0 (d/check-num :foo 0))) (t/is (= 0 (d/check-num nil 0)))) + +(t/deftest insert-at-index + ;; insert different object + (t/is (= (d/insert-at-index [:a :b] 1 [:c :d]) + [:a :c :d :b])) + + ;; insert on the start + (t/is (= (d/insert-at-index [:a :b] 0 [:c]) + [:c :a :b])) + + ;; insert on the end 1 + (t/is (= (d/insert-at-index [:a :b] 2 [:c]) + [:a :b :c])) + + ;; insert on the end with not existing index + (t/is (= (d/insert-at-index [:a :b] 10 [:c]) + [:a :b :c])) + + ;; insert existing in a contiguous index + (t/is (= (d/insert-at-index [:a :b] 1 [:a]) + [:a :b])) + + ;; insert existing in the same index + (t/is (= (d/insert-at-index [:a :b] 0 [:a]) + [:a :b])) + + ;; insert existing in other index case 1 + (t/is (= (d/insert-at-index [:a :b :c] 2 [:a]) + [:b :a :c])) + + ;; insert existing in other index case 2 + (t/is (= (d/insert-at-index [:a :b :c :d] 0 [:d]) + [:d :a :b :c])) + + ;; insert existing in other index case 3 + (t/is (= (d/insert-at-index [:a :b :c :d] 1 [:a]) + [:a :b :c :d]))) + diff --git a/common/test/common_tests/pages_helpers_test.cljc b/common/test/common_tests/pages_helpers_test.cljc index 6079b0b84..76a4f4a22 100644 --- a/common/test/common_tests/pages_helpers_test.cljc +++ b/common/test/common_tests/pages_helpers_test.cljc @@ -10,50 +10,10 @@ [clojure.pprint :refer [pprint]] [app.common.pages.helpers :as cph])) -(t/deftest insert-at-index - ;; insert different object - (t/is (= (cph/insert-at-index [:a :b] 1 [:c :d]) - [:a :c :d :b])) - - ;; insert on the start - (t/is (= (cph/insert-at-index [:a :b] 0 [:c]) - [:c :a :b])) - - ;; insert on the end 1 - (t/is (= (cph/insert-at-index [:a :b] 2 [:c]) - [:a :b :c])) - - ;; insert on the end with not existing index - (t/is (= (cph/insert-at-index [:a :b] 10 [:c]) - [:a :b :c])) - - ;; insert existing in a contiguous index - (t/is (= (cph/insert-at-index [:a :b] 1 [:a]) - [:a :b])) - - ;; insert existing in the same index - (t/is (= (cph/insert-at-index [:a :b] 0 [:a]) - [:a :b])) - - ;; insert existing in other index case 1 - (t/is (= (cph/insert-at-index [:a :b :c] 2 [:a]) - [:b :a :c])) - - ;; insert existing in other index case 2 - (t/is (= (cph/insert-at-index [:a :b :c :d] 0 [:d]) - [:d :a :b :c])) - - ;; insert existing in other index case 3 - (t/is (= (cph/insert-at-index [:a :b :c :d] 1 [:a]) - [:a :b :c :d])) - - ) - - (t/deftest parse-path-name (t/is (= ["foo" "bar"] (cph/parse-path-name "foo/bar"))) (t/is (= ["" "foo"] (cph/parse-path-name "foo"))) (t/is (= ["" "foo"] (cph/parse-path-name "/foo"))) (t/is (= ["" ""] (cph/parse-path-name ""))) - (t/is (= ["" ""] (cph/parse-path-name nil))) - ) + (t/is (= ["" ""] (cph/parse-path-name nil)))) + diff --git a/frontend/src/app/libs/file_builder.cljs b/frontend/src/app/libs/file_builder.cljs index f2793f42a..f30014832 100644 --- a/frontend/src/app/libs/file_builder.cljs +++ b/frontend/src/app/libs/file_builder.cljs @@ -9,6 +9,7 @@ [app.common.data :as d] [app.common.file-builder :as fb] [app.common.media :as cm] + [app.common.types.components-list :as ctkl] [app.common.uuid :as uuid] [app.util.dom :as dom] [app.util.json :as json] @@ -108,7 +109,7 @@ components-stream (->> files-stream (rx/flat-map vals) - (rx/filter #(d/not-empty? (get-in % [:data :components]))) + (rx/filter #(d/not-empty? (ctkl/components-seq (:data %)))) (rx/flat-map e/parse-library-components)) pages-stream diff --git a/frontend/src/app/main/data/workspace/changes.cljs b/frontend/src/app/main/data/workspace/changes.cljs index 771b873ce..e501e3652 100644 --- a/frontend/src/app/main/data/workspace/changes.cljs +++ b/frontend/src/app/main/data/workspace/changes.cljs @@ -34,7 +34,6 @@ (declare commit-changes) - (defn- add-group-id [changes state] (let [undo (:workspace-undo state) @@ -215,7 +214,7 @@ add-page-id (fn [{:keys [id type page] :as change}] (cond-> change - (page-change? type) + (and (page-change? type) (nil? (:page-id change))) (assoc :page-id (or id (:id page))))) changes-by-pages diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 726cd60e5..289c25c8a 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -22,7 +22,6 @@ [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.common.types.file.media-object :as ctfm] - [app.common.types.pages-list :as ctpl] [app.common.types.typography :as ctt] [app.common.uuid :as uuid] [app.main.data.dashboard :as dd] @@ -421,12 +420,12 @@ (let [data (get state :workspace-data)] (if (features/active-feature? state :components-v2) (let [component (ctkl/get-component data id) - page (ctpl/get-page data (:main-instance-page component)) - shape (ctn/get-shape page (:main-instance-id component))] - (rx/of (dwsh/delete-shapes (:id page) #{(:id shape)}))) + page (ctf/get-component-page data component) + shape (ctf/get-component-root data component)] + (rx/of (dwsh/delete-shapes (:id page) #{(:id shape)}))) ;; Deleting main root triggers component delete (let [changes (-> (pcb/empty-changes it) (pcb/with-library-data data) - (pcb/delete-component id false))] + (pcb/delete-component id))] (rx/of (dch/commit-changes changes)))))))) (defn restore-component @@ -437,39 +436,18 @@ (ptk/reify ::restore-component ptk/WatchEvent (watch [it state _] - (assert "Restore component not implemented") ; until we allow a :deleted flag in shapes - (let [file-data (wsh/get-file state library-id) - component (ctf/get-deleted-component file-data component-id) - page (ctpl/get-page file-data (:main-instance-page component)) - - components-v2 - (features/active-feature? state :components-v2) - - ; Make a new main instance, with the same id of the original - [_main-instance shapes] - (ctn/make-component-instance page - component - file-data - (gpt/point (:main-instance-x component) - (:main-instance-y component)) - components-v2 - {:main-instance? true - :force-id (:main-instance-id component)}) - - changes (-> (pcb/empty-changes it) - (pcb/with-library-data file-data) - (pcb/with-page page)) - - changes (reduce #(pcb/add-object %1 %2 {:ignore-touched true}) - changes - shapes) - - ; restore-component change needs to be done after add main instance - ; because when undo changes, the orden is inverse - changes (pcb/restore-component changes component-id)] - - (rx/of (dch/commit-changes (assoc changes :file-id library-id))))))) - + (let [library-data (wsh/get-file state library-id) + component (ctkl/get-deleted-component library-data component-id) + page (ctf/get-component-page library-data component) + shapes (cph/get-children-with-self (:objects component) (:main-instance-id component)) + changes (-> (pcb/empty-changes it) + (pcb/with-library-data library-data) + (pcb/with-page page)) + changes (reduce #(pcb/add-object %1 %2 {:ignore-touched true}) + changes + shapes) + changes (pcb/restore-component changes component-id)] + (rx/of (dch/commit-changes changes)))))) (defn instantiate-component "Create a new shape in the current page, from the component with the given id diff --git a/frontend/src/app/main/data/workspace/libraries_helpers.cljs b/frontend/src/app/main/data/workspace/libraries_helpers.cljs index 5b56bd116..b3f2a1622 100644 --- a/frontend/src/app/main/data/workspace/libraries_helpers.cljs +++ b/frontend/src/app/main/data/workspace/libraries_helpers.cljs @@ -241,7 +241,7 @@ (let [file (wsh/get-file state file-id) components-v2 (get-in file [:options :components-v2])] - (loop [local-components (vals (get file :components)) + (loop [local-components (ctkl/components-seq file) changes (pcb/empty-changes it)] (if-let [local-component (first local-components)] (recur (next local-components) @@ -480,7 +480,7 @@ (let [library (dm/get-in libraries [(:component-file shape-inst) :data]) component (or (ctkl/get-component library (:component-id shape-inst)) (and reset? - (ctf/get-deleted-component library (:component-id shape-inst)))) + (ctkl/get-deleted-component library (:component-id shape-inst)))) shape-main (when component (ctf/get-ref-shape library component shape-inst)) diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index 5e57a888a..2aed1ff76 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -292,7 +292,7 @@ changes (reduce (fn [changes component-id] ;; It's important to delete the component before the main instance, because we ;; need to store the instance position if we want to restore it later. - (pcb/delete-component changes component-id components-v2)) + (pcb/delete-component changes component-id)) changes components-to-delete) diff --git a/frontend/src/app/main/data/workspace/state_helpers.cljs b/frontend/src/app/main/data/workspace/state_helpers.cljs index dd01688d6..9a569fbd1 100644 --- a/frontend/src/app/main/data/workspace/state_helpers.cljs +++ b/frontend/src/app/main/data/workspace/state_helpers.cljs @@ -40,10 +40,6 @@ ([state page-id] (dm/get-in state [:workspace-data :pages-index page-id :options]))) -(defn lookup-component-objects - ([state component-id] - (dm/get-in state [:workspace-data :components component-id :objects]))) - (defn lookup-local-components ([state] (dm/get-in state [:workspace-data :components]))) diff --git a/frontend/src/app/main/ui/workspace/libraries.cljs b/frontend/src/app/main/ui/workspace/libraries.cljs index b2ba31971..fc18741a3 100644 --- a/frontend/src/app/main/ui/workspace/libraries.cljs +++ b/frontend/src/app/main/ui/workspace/libraries.cljs @@ -7,6 +7,7 @@ (ns app.main.ui.workspace.libraries (:require [app.common.data :as d] + [app.common.types.components-list :as ctkl] [app.main.data.dashboard :as dd] [app.main.data.modal :as modal] [app.main.data.workspace.libraries :as dwl] @@ -44,7 +45,7 @@ (defn local-library-str [library] - (let [components-count (count (get-in library [:data :components] [])) + (let [components-count (count (or (ctkl/components-seq (:data library)) [])) graphics-count (count (get-in library [:data :media] [])) colors-count (count (get-in library [:data :colors] [])) typography-count (count (get-in library [:data :typographies] []))] diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets.cljs index fb2e4b594..db8877310 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets.cljs @@ -12,6 +12,7 @@ [app.common.pages.helpers :as cph] [app.common.spec :as us] [app.common.text :as txt] + [app.common.types.components-list :as ctkl] [app.common.types.file :as ctf] [app.config :as cf] [app.main.data.events :as ev] @@ -374,8 +375,11 @@ components-v2 (mf/use-ctx ctx/components-v2) - file (or (:data file) file) - + file (or (:data file) file) + root-shape (ctf/get-component-root file component) + component-container (if components-v2 + (ctf/get-component-page file component) + component) unselect-all (mf/use-fn (fn [] @@ -444,26 +448,26 @@ :on-drag-over on-drag-over :on-drop on-drop} - [:& component-svg {:root-shape (ctf/get-component-root file component) - :objects (:objects (if components-v2 - (ctf/get-component-page file component) - component))}] - (let [renaming? (= renaming (:id component))] + (when (and (some? root-shape) (some? component-container)) [:* - [:& editable-label - {:class-name (dom/classnames - :cell-name listing-thumbs? - :item-name (not listing-thumbs?) - :editing renaming?) - :value (cph/merge-path-item (:path component) (:name component)) - :tooltip (cph/merge-path-item (:path component) (:name component)) - :display-value (:name component) - :editing? renaming? - :disable-dbl-click? true - :on-change do-rename - :on-cancel cancel-rename}] - (when @dragging? - [:div.dragging])])])) + [:& component-svg {:root-shape root-shape + :objects (:objects component-container)}] + (let [renaming? (= renaming (:id component))] + [:* + [:& editable-label + {:class-name (dom/classnames + :cell-name listing-thumbs? + :item-name (not listing-thumbs?) + :editing renaming?) + :value (cph/merge-path-item (:path component) (:name component)) + :tooltip (cph/merge-path-item (:path component) (:name component)) + :display-value (:name component) + :editing? renaming? + :disable-dbl-click? true + :on-change do-rename + :on-cancel cancel-rename}] + (when @dragging? + [:div.dragging])])])])) (mf/defc components-group [{:keys [file prefix groups open-groups renaming listing-thumbs? selected-components on-asset-click @@ -1938,8 +1942,8 @@ (l/derived (fn [state] (let [wfile (:workspace-data state)] (if (= (:id wfile) id) - (vals (get wfile :components)) - (vals (get-in state [:workspace-libraries id :data :components]))))) + (ctkl/components-seq wfile) + (ctkl/components-seq (get-in state [:workspace-libraries id :data]))))) st/state =)) (defn file-typography-ref diff --git a/frontend/src/app/render.cljs b/frontend/src/app/render.cljs index 69ea2ecd2..96273a902 100644 --- a/frontend/src/app/render.cljs +++ b/frontend/src/app/render.cljs @@ -11,6 +11,7 @@ [app.common.logging :as l] [app.common.math :as mth] [app.common.spec :as us] + [app.common.types.components-list :as ctkl] [app.common.uri :as u] [app.main.data.fonts :as df] [app.main.features :as features] @@ -247,7 +248,7 @@ :border "0px solid black"}]]])] [:ul.nav - (for [[id data] (get-in file [:data :components])] + (for [[id data] (ctkl/components (:data file))] (let [on-click (fn [event] (dom/prevent-default event) (swap! state assoc :component-id id))] diff --git a/frontend/src/app/worker/export.cljs b/frontend/src/app/worker/export.cljs index 325d19c14..08f15742b 100644 --- a/frontend/src/app/worker/export.cljs +++ b/frontend/src/app/worker/export.cljs @@ -9,6 +9,7 @@ [app.common.data :as d] [app.common.media :as cm] [app.common.text :as ct] + [app.common.types.components-list :as ctkl] [app.config :as cfg] [app.main.render :as r] [app.main.repo :as rp] @@ -51,7 +52,7 @@ :version current-version :libraries (->> (:libraries file) (into #{}) (mapv str)) :exportType (d/name export-type) - :hasComponents (d/not-empty? (get-in file [:data :components])) + :hasComponents (d/not-empty? (ctkl/components-seq (:data file))) :hasDeletedComponents (d/not-empty? (get-in file [:data :deleted-components])) :hasMedia (d/not-empty? (get-in file [:data :media])) :hasColors (d/not-empty? (get-in file [:data :colors])) @@ -329,7 +330,7 @@ (select-keys (get external-refs (:id file)))) media (-> (get-in file [:data :media]) (select-keys (get external-refs (:id file)))) - components (-> (get-in file [:data :components]) + components (-> (ctkl/components (:data file)) (select-keys (get external-refs (:id file))))] (cond-> target (d/not-empty? colors) @@ -434,7 +435,7 @@ components-stream (->> files-stream (rx/flat-map vals) - (rx/filter #(d/not-empty? (get-in % [:data :components]))) + (rx/filter #(d/not-empty? (ctkl/components-seq (:data %)))) (rx/flat-map parse-library-components)) deleted-components-stream diff --git a/frontend/test/frontend_tests/helpers/libraries.cljs b/frontend/test/frontend_tests/helpers/libraries.cljs index b7a24f505..359667a31 100644 --- a/frontend/test/frontend_tests/helpers/libraries.cljs +++ b/frontend/test/frontend_tests/helpers/libraries.cljs @@ -7,11 +7,9 @@ (ns frontend-tests.helpers.libraries (:require [app.common.pages.helpers :as cph] - [app.common.types.component :as ctk] [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.main.data.workspace.state-helpers :as wsh] - [cljs.pprint :refer [pprint]] [cljs.test :as t :include-macros true] [frontend-tests.helpers.pages :as thp])) diff --git a/frontend/test/frontend_tests/state_components_test.cljs b/frontend/test/frontend_tests/state_components_test.cljs index 333b3823f..5315f79d5 100644 --- a/frontend/test/frontend_tests/state_components_test.cljs +++ b/frontend/test/frontend_tests/state_components_test.cljs @@ -1,6 +1,7 @@ (ns frontend-tests.state-components-test (:require [app.common.geom.point :as gpt] + [app.common.types.components-list :as ctkl] [app.common.types.container :as ctn] [app.common.types.file :as ctf] [app.main.data.workspace :as dw] @@ -303,14 +304,14 @@ (filter #(not= % component-id)) (first)) - [[instance1 shape1] - [c-instance1 c-shape1] - component1] + [[_instance1 _shape1] + [_c-instance1 _c-shape1] + _component1] (thl/resolve-instance-and-main new-state (:id main1)) - [[c-component2 c-shape2] + [[_c-component2 _c-shape2] component2] (thl/resolve-component new-state @@ -331,39 +332,114 @@ (thp/sample-shape :shape1 :rect {:name "Rect 1"}) (thp/make-component :main1 :component1 - [(thp/id :shape1)])) - - file (wsh/get-local-file state) - - main1 (thp/get-shape state :main1) - component-id (:component-id main1) + [(thp/id :shape1)]) + (thp/instantiate-component :instance1 + (thp/id :component1))) store (the/prepare-store state done - (fn [new-state] - ; Expected shape tree: - ; - ; [Page] - ; Root Frame - ; - (let [[main1 shape1] - (thl/resolve-noninstance - new-state - (:id main1)) + (fn [new-state] + ; Expected shape tree: + ; + ; [Page] + ; Root Frame + ; Rect 1 #--> ? + ; Rect 1 ---> ? + ; + (let [[main1 shape1] + (thl/resolve-noninstance + new-state + (thp/id :main1)) - libs (wsh/get-libraries new-state) - component (ctf/get-component libs - (:component-file main1) - (:component-id main1))] + [[instance1 shape2] [c-instance1 c-shape2] component1] + (thl/resolve-instance-and-main-allow-dangling + new-state + (thp/id :instance1)) - (t/is (nil? main1)) - (t/is (nil? shape1)) - (t/is (nil? component)))))] + file (wsh/get-local-file new-state) + component2 (ctkl/get-component file (thp/id :component1)) + component3 (ctkl/get-deleted-component file (thp/id :component1)) + + saved-objects (:objects component3) + saved-main1 (get saved-objects (:shape-ref instance1)) + saved-shape2 (get saved-objects (:shape-ref shape2))] + + (t/is (nil? main1)) + (t/is (nil? shape1)) + + (t/is (= (:name instance1) "Rect 1")) + (t/is (= (:touched instance1) nil)) + (t/is (not= (:shape-ref instance1) nil)) + (t/is (= (:name shape2) "Rect 1")) + (t/is (= (:touched shape2) nil)) + (t/is (not= (:shape-ref shape2) nil)) + (t/is (nil? c-instance1)) + (t/is (nil? c-shape2)) + (t/is (nil? component1)) + + (t/is (nil? component2)) + + (t/is (= (:name component3) "Rect 1")) + (t/is (= (:deleted component3) true)) + (t/is (some? (:objects component3))) + + (t/is (= (:name saved-main1) "Rect 1")) + (t/is (= (:name saved-shape2) "Rect 1")))))] (ptk/emit! - store - (dwl/delete-component {:id component-id}) - (dwl/sync-file (:id file) (:id file) :components component-id) - :the/end)))) + store + (dwl/delete-component {:id (thp/id :component1)}) + :the/end)))) + +(t/deftest test-restore-component + (t/async + done + (let [state (-> thp/initial-state + (thp/sample-page) + (thp/sample-shape :shape1 :rect + {:name "Rect 1"}) + (thp/make-component :main1 :component1 + [(thp/id :shape1)]) + (thp/instantiate-component :instance1 + (thp/id :component1))) + + store (the/prepare-store state done + (fn [new-state] + ; Expected shape tree: + ; + ; [Page] + ; Root Frame + ; Rect 1 #--> Rect 1 + ; Rect 1 ---> Rect 1 + ; Rect 1 + ; Rect 1 + ; + ; [Rect 1] + ; page1 / Rect 1 + ; + (let [[[instance1 shape2] [c-instance1 c-shape2] component1] + (thl/resolve-instance-and-main + new-state + (thp/id :instance1)) + + file (wsh/get-local-file new-state) + component2 (ctkl/get-component file (thp/id :component1)) + + saved-objects (:objects component2)] + + (t/is (= (:name instance1) "Rect 1")) + (t/is (= (:name shape2) "Rect 1")) + (t/is (= (:name c-instance1) "Rect 1")) + (t/is (= (:name c-shape2) "Rect 1")) + + (t/is (some? component1)) + (t/is (some? component2)) + (t/is (nil? saved-objects)))))] + + (ptk/emit! + store + (dwl/delete-component {:id (thp/id :component1)}) + (dwl/restore-component thp/current-file-id (thp/id :component1)) + :the/end)))) (t/deftest test-instantiate-component (t/async