🔧 Refactor delete/restore components

This commit is contained in:
Andrés Moya 2023-03-10 15:43:26 +01:00
parent b91f1959b4
commit 7391a4086a
27 changed files with 378 additions and 328 deletions

View file

@ -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

View file

@ -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]}]

View file

@ -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]

View file

@ -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]

View file

@ -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))

View file

@ -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]))

View file

@ -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

View file

@ -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)))

View file

@ -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)))

View file

@ -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

View file

@ -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]

View file

@ -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])))

View file

@ -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))))