mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 05:50:35 +00:00
🐛 Fix loss of swap slot when copying to clipboard (#12434)
* 🐛 Fix loss of swap slot when copying to clipboard * 🔧 Add test and make test helpers run same code as real functions * 🔧 Changes after review
This commit is contained in:
Andrés Moya
GitHub
parent
028547e19b
commit
3863e92993
@ -1259,23 +1259,22 @@
|
||||
(defn get-ref-chain-until-target-ref
|
||||
"Returns a vector with the shape ref chain until target-ref, including itself"
|
||||
[container libraries shape target-ref]
|
||||
(loop [chain [shape]
|
||||
current shape]
|
||||
(if (= current target-ref)
|
||||
(loop [chain [shape]
|
||||
current shape
|
||||
container container]
|
||||
(if (= (:id current) target-ref)
|
||||
chain
|
||||
(if-let [ref (find-ref-shape nil container libraries current :with-context? true)]
|
||||
(recur (conj chain ref) ref)
|
||||
(recur (conj chain ref) ref (:container (meta ref)))
|
||||
chain))))
|
||||
|
||||
(defn get-touched-from-ref-chain-until-target-ref
|
||||
"Returns a set with the :touched of all the items on the shape
|
||||
ref chain until target-ref, including itself"
|
||||
[container libraries shape target-ref]
|
||||
(let [chain (get-ref-chain-until-target-ref container libraries shape target-ref)
|
||||
more-touched (->> chain
|
||||
(map :touched)
|
||||
(remove nil?)
|
||||
(apply set/union)
|
||||
(remove ctk/swap-slot?)
|
||||
set)]
|
||||
(set/union (or (:touched shape) #{}) more-touched)))
|
||||
[container libraries shape target-ref & {:keys [remove-slots?] :or {remove-slots? true}}]
|
||||
(let [chain (get-ref-chain-until-target-ref container libraries shape target-ref)
|
||||
more-touched (cond->> chain
|
||||
:always (map :touched)
|
||||
:always (reduce set/union #{})
|
||||
remove-slots? (remove ctk/swap-slot?))]
|
||||
(set/union (or (:touched shape) #{}) (set more-touched))))
|
||||
@ -67,6 +67,43 @@
|
||||
[potok.v2.core :as ptk]
|
||||
[promesa.core :as p]))
|
||||
|
||||
(defn- advance-shape
|
||||
[file libraries page level-delta head-id objects shape]
|
||||
(let [new-shape-ref (ctf/advance-shape-ref file page libraries shape level-delta {:include-deleted? true})
|
||||
container (ctn/make-container page :page)
|
||||
;; The head becomes a root copy, which cannot carry a swap slot, so
|
||||
;; strip inherited slots only for it. Nested sub-heads keep them so
|
||||
;; they still pair with the children of the new near main.
|
||||
head? (= (:id shape) head-id)
|
||||
new-touched (ctf/get-touched-from-ref-chain-until-target-ref
|
||||
container libraries shape new-shape-ref :remove-slots? head?)]
|
||||
(cond-> objects
|
||||
(and (some? new-shape-ref) (not= new-shape-ref (:shape-ref shape)))
|
||||
(update (:id shape) assoc :shape-ref new-shape-ref :touched new-touched))))
|
||||
|
||||
(defn- advance-copy
|
||||
[file libraries page objects shape]
|
||||
(if (and (ctk/instance-head? shape) (not (ctk/main-instance? shape)))
|
||||
(let [level-delta (if (nil? (ctk/get-swap-slot shape))
|
||||
(ctn/get-nesting-level-delta (:objects page) shape uuid/zero)
|
||||
0)]
|
||||
(if (pos? level-delta)
|
||||
(reduce (partial advance-shape file libraries page level-delta (:id shape))
|
||||
objects
|
||||
(cfh/get-children-with-self objects (:id shape)))
|
||||
objects))
|
||||
objects))
|
||||
|
||||
(defn advance-copies
|
||||
"When copying an instance that is nested inside another one, we need to
|
||||
advance the shape refs to one or more levels of remote mains."
|
||||
[file libraries page selected data]
|
||||
(let [heads (mapcat #(ctn/get-child-heads (:objects data) %) selected)]
|
||||
(update data :objects
|
||||
#(reduce (partial advance-copy file libraries page)
|
||||
%
|
||||
heads))))
|
||||
|
||||
(defn copy-selected
|
||||
[]
|
||||
(letfn [(sort-selected [state data]
|
||||
@ -148,39 +185,13 @@
|
||||
(let [frame (get objects parent-frame-id)]
|
||||
(gsh/translate-to-frame shape frame))))
|
||||
|
||||
;; When copying an instance that is nested inside another one, we need to
|
||||
;; advance the shape refs to one or more levels of remote mains.
|
||||
(advance-copies [state selected data]
|
||||
(let [file (dsh/lookup-file state)
|
||||
libraries (:files state)
|
||||
;; FIXME
|
||||
page (dsh/lookup-page state)
|
||||
heads (mapcat #(ctn/get-child-heads (:objects data) %) selected)]
|
||||
(update data :objects
|
||||
#(reduce (partial advance-copy file libraries page)
|
||||
%
|
||||
heads))))
|
||||
|
||||
(advance-copy [file libraries page objects shape]
|
||||
(if (and (ctk/instance-head? shape) (not (ctk/main-instance? shape)))
|
||||
(let [level-delta (if (nil? (ctk/get-swap-slot shape))
|
||||
(ctn/get-nesting-level-delta (:objects page) shape uuid/zero)
|
||||
0)]
|
||||
(if (pos? level-delta)
|
||||
(reduce (partial advance-shape file libraries page level-delta)
|
||||
objects
|
||||
(cfh/get-children-with-self objects (:id shape)))
|
||||
objects))
|
||||
objects))
|
||||
|
||||
(advance-shape [file libraries page level-delta objects shape]
|
||||
(let [new-shape-ref (ctf/advance-shape-ref file page libraries shape level-delta {:include-deleted? true})
|
||||
container (ctn/make-container page :page)
|
||||
new-touched (ctf/get-touched-from-ref-chain-until-target-ref container libraries shape new-shape-ref)]
|
||||
(cond-> objects
|
||||
(and (some? new-shape-ref) (not= new-shape-ref (:shape-ref shape)))
|
||||
(-> (assoc-in [(:id shape) :shape-ref] new-shape-ref)
|
||||
(assoc-in [(:id shape) :touched] new-touched)))))
|
||||
(advance-copies-from-state [state selected data]
|
||||
(advance-copies (dsh/lookup-file state)
|
||||
(:files state)
|
||||
;; FIXME
|
||||
(dsh/lookup-page state)
|
||||
selected
|
||||
data))
|
||||
|
||||
(on-copy-error [error]
|
||||
(js/console.error "clipboard blocked:" error)
|
||||
@ -232,7 +243,7 @@
|
||||
(rx/merge-map (partial prepare-object objects frame-id))
|
||||
(rx/reduce (partial collect-data state) initial)
|
||||
(rx/map (partial sort-selected state))
|
||||
(rx/map (partial advance-copies state selected))
|
||||
(rx/map (partial advance-copies-from-state state selected))
|
||||
(rx/map #(t/encode-str % {:type :json-verbose}))
|
||||
(rx/map #(wapi/create-blob % "text/plain"))
|
||||
(rx/subs! resolve reject))))]
|
||||
@ -247,7 +258,7 @@
|
||||
(rx/merge-map (partial prepare-object objects frame-id))
|
||||
(rx/reduce (partial collect-data state) initial)
|
||||
(rx/map (partial sort-selected state))
|
||||
(rx/map (partial advance-copies state selected))
|
||||
(rx/map (partial advance-copies-from-state state selected))
|
||||
(rx/map #(t/encode-str % {:type :json-verbose}))
|
||||
(rx/map clipboard/to-clipboard)
|
||||
(rx/catch on-copy-error)
|
||||
|
||||
@ -14,13 +14,11 @@
|
||||
[app.common.geom.point :as gpt]
|
||||
[app.common.geom.shapes :as gsh]
|
||||
[app.common.logic.libraries :as cll]
|
||||
[app.common.types.component :as ctk]
|
||||
[app.common.types.container :as ctn]
|
||||
[app.common.types.file :as ctf]
|
||||
[app.common.types.shape :as cts]
|
||||
[app.common.types.shape-tree :as ctst]
|
||||
[app.common.uuid :as uuid]
|
||||
[app.main.data.helpers :as dsh]
|
||||
[app.main.data.workspace.clipboard :as dwc]
|
||||
[app.main.data.workspace.groups :as dwg]
|
||||
[app.main.data.workspace.layout :as layout]))
|
||||
|
||||
@ -225,34 +223,7 @@
|
||||
(if (= parent-frame-id uuid/zero)
|
||||
shape
|
||||
(let [frame (get objects parent-frame-id)]
|
||||
(gsh/translate-to-frame shape frame))))
|
||||
|
||||
;; When copying an instance that is nested inside another one, we need to
|
||||
;; advance the shape refs to one or more levels of remote mains.
|
||||
(advance-copies [data]
|
||||
(let [heads (mapcat #(ctn/get-child-heads (:objects data) %) selected)]
|
||||
(update data :objects
|
||||
#(reduce (partial advance-copy file libraries page)
|
||||
%
|
||||
heads))))
|
||||
|
||||
(advance-copy [file libraries page objects shape]
|
||||
(if (and (ctk/instance-head? shape) (not (ctk/main-instance? shape)))
|
||||
(let [level-delta (if (nil? (ctk/get-swap-slot shape))
|
||||
(ctn/get-nesting-level-delta (:objects page) shape uuid/zero)
|
||||
0)]
|
||||
(if (pos? level-delta)
|
||||
(reduce (partial advance-shape file libraries page level-delta)
|
||||
objects
|
||||
(cfh/get-children-with-self objects (:id shape)))
|
||||
objects))
|
||||
objects))
|
||||
|
||||
(advance-shape [file libraries page level-delta objects shape]
|
||||
(let [new-shape-ref (ctf/advance-shape-ref file page libraries shape level-delta {:include-deleted? true})]
|
||||
(cond-> objects
|
||||
(and (some? new-shape-ref) (not= new-shape-ref (:shape-ref shape)))
|
||||
(assoc-in [(:id shape) :shape-ref] new-shape-ref))))]
|
||||
(gsh/translate-to-frame shape frame))))]
|
||||
|
||||
(let [file-id (:id file)
|
||||
frame-id (cfh/common-parent-frame objects selected)
|
||||
@ -273,4 +244,4 @@
|
||||
(map (partial prepare-object objects frame-id))
|
||||
(reduce collect-data initial)
|
||||
sort-selected
|
||||
advance-copies))))
|
||||
(dwc/advance-copies file libraries page selected)))))
|
||||
|
||||
@ -5,6 +5,7 @@
|
||||
;; Copyright (c) KALEIDOS SUBSIDIARY SL
|
||||
(ns frontend-tests.logic.comp-remove-swap-slots-test
|
||||
(:require
|
||||
[app.common.files.validate :as cfv]
|
||||
[app.common.test-helpers.components :as cthc]
|
||||
[app.common.test-helpers.compositions :as ctho]
|
||||
[app.common.test-helpers.files :as cthf]
|
||||
@ -870,3 +871,149 @@
|
||||
(t/is (some? copied-blue1'))
|
||||
(t/is (nil? (ctk/get-swap-slot copied-blue1')))))))))
|
||||
|
||||
|
||||
(t/deftest test-keep-inherited-swap-slot-copy-paste-nested-copy-to-root
|
||||
(t/async
|
||||
done
|
||||
(let [;; ==== Setup
|
||||
;; {:frame-red} [:name frame-red] # [Component :red]
|
||||
;; {:frame-blue} [:name frame-blue] # [Component :blue]
|
||||
;; {:frame-icon} [:name frame-icon] # [Component :icon]
|
||||
;; :red-copy-icon [:name frame-red] @--> frame-red
|
||||
;; {:frame-title} [:name frame-title] # [Component :title]
|
||||
;; :icon-copy-title [:name frame-icon] @--> frame-icon
|
||||
;; :blue-copy-title [:name frame-blue, :swap-slot-label :red-copy-icon] @--> frame-blue
|
||||
;; :title-copy [:name frame-title] #--> frame-title
|
||||
;; :icon-copy-in-title-copy [:name frame-icon] @--> :icon-copy-title
|
||||
;; :blue-copy-in-title-copy [:name frame-blue] @--> :blue-copy-title
|
||||
file (-> (cthf/sample-file :file1)
|
||||
(ctho/add-frame :frame-red :name "frame-red")
|
||||
(cthc/make-component :red :frame-red)
|
||||
(ctho/add-frame :frame-blue :name "frame-blue")
|
||||
(cthc/make-component :blue :frame-blue)
|
||||
(ctho/add-frame :frame-icon :name "frame-icon")
|
||||
(cthc/make-component :icon :frame-icon)
|
||||
(cthc/instantiate-component :red :red-copy-icon :parent-label :frame-icon)
|
||||
(ctho/add-frame :frame-title :name "frame-title")
|
||||
(cthc/make-component :title :frame-title)
|
||||
(cthc/instantiate-component :icon :icon-copy-title :parent-label :frame-title
|
||||
:children-labels [:red-copy-in-icon-copy-title])
|
||||
(cthc/component-swap :red-copy-in-icon-copy-title :blue :blue-copy-title)
|
||||
(cthc/instantiate-component :title :title-copy
|
||||
:children-labels [:icon-copy-in-title-copy
|
||||
:blue-copy-in-title-copy]))
|
||||
store (ths/setup-store file)
|
||||
|
||||
;; ==== Action
|
||||
page (cthf/current-page file)
|
||||
icon (cths/get-shape file :icon-copy-in-title-copy)
|
||||
features #{"components/v2"}
|
||||
version 47
|
||||
|
||||
pdata (thp/simulate-copy-shape #{(:id icon)} (:objects page) {(:id file) file} page file features version)
|
||||
|
||||
events
|
||||
[(dws/select-shape uuid/zero)
|
||||
(dw/paste-shapes pdata)]]
|
||||
|
||||
(ths/run-store
|
||||
store done events
|
||||
(fn [new-state]
|
||||
(let [;; ==== Get
|
||||
file' (ths/get-file-from-state new-state)
|
||||
page' (cthf/current-page file')
|
||||
icon' (cths/get-shape file' :icon-copy-in-title-copy)
|
||||
blue' (cths/get-shape file' :blue-copy-in-title-copy)
|
||||
copied-icon' (find-copied-shape icon' page' uuid/zero)
|
||||
copied-blue' (find-copied-shape blue' page' (:id copied-icon'))
|
||||
errors (try
|
||||
(cfv/validate-file! file' {(:id file') file'})
|
||||
nil
|
||||
(catch :default cause
|
||||
(ex-data cause)))]
|
||||
|
||||
;; ==== Check
|
||||
;; the copied icon now refers to the icon main, where blue
|
||||
;; does not exist, so the copied blue must keep the swap slot
|
||||
;; inherited from :blue-copy-title
|
||||
(t/is (some? copied-icon'))
|
||||
(t/is (= (:shape-ref copied-icon') (cthi/id :frame-icon)))
|
||||
(t/is (some? copied-blue'))
|
||||
(t/is (= (ctk/get-swap-slot copied-blue') (cthi/id :red-copy-icon)))
|
||||
|
||||
;; the file must be valid after the paste
|
||||
(t/is (nil? errors))))))))
|
||||
|
||||
(t/deftest test-keep-touched-copy-paste-nested-copy-to-root
|
||||
(t/async
|
||||
done
|
||||
(let [;; ==== Setup
|
||||
;; Add a nested copy inside a copy of :green, and override its fill
|
||||
;; so that the copy has a touched attribute.
|
||||
file (-> (setup-file)
|
||||
(cthc/instantiate-component :green :green-copy-2
|
||||
:parent-label :frame-b2
|
||||
:children-labels [:red-copy-in-green-copy-2])
|
||||
(cths/update-shape :red-copy-in-green-copy-2
|
||||
:fills (cths/sample-fills-color :fill-color "#ff0000")))
|
||||
store (ths/setup-store file)
|
||||
|
||||
;; ==== Action
|
||||
page (cthf/current-page file)
|
||||
red (cths/get-shape file :red-copy-in-green-copy-2)
|
||||
features #{"components/v2"}
|
||||
version 47
|
||||
|
||||
pdata (thp/simulate-copy-shape #{(:id red)} (:objects page) {(:id file) file} page file features version)
|
||||
|
||||
events
|
||||
[(dws/select-shape uuid/zero)
|
||||
(dw/paste-shapes pdata)]]
|
||||
|
||||
(ths/run-store
|
||||
store done events
|
||||
(fn [new-state]
|
||||
(let [;; ==== Get
|
||||
file' (ths/get-file-from-state new-state)
|
||||
page' (cthf/current-page file')
|
||||
red' (cths/get-shape file' :red-copy-in-green-copy-2)
|
||||
copied-red' (find-copied-shape red' page' uuid/zero)]
|
||||
|
||||
;; ==== Check
|
||||
;; the touched fill override must be preserved in the pasted copy
|
||||
(t/is (some? copied-red'))
|
||||
(t/is (contains? (:touched copied-red') :fill-group))
|
||||
(t/is (= (:touched red') (:touched copied-red')))))))))
|
||||
|
||||
(t/deftest test-remove-swap-slot-copy-paste-nested-swapped-copy-to-root
|
||||
(t/async
|
||||
done
|
||||
(let [;; ==== Setup (sample file as is)
|
||||
file (setup-file)
|
||||
store (ths/setup-store file)
|
||||
|
||||
;; ==== Action
|
||||
page (cthf/current-page file)
|
||||
blue (cths/get-shape file :blue-copy-in-green-copy)
|
||||
features #{"components/v2"}
|
||||
version 47
|
||||
|
||||
pdata (thp/simulate-copy-shape #{(:id blue)} (:objects page) {(:id file) file} page file features version)
|
||||
|
||||
events
|
||||
[(dws/select-shape uuid/zero)
|
||||
(dw/paste-shapes pdata)]]
|
||||
|
||||
(ths/run-store
|
||||
store done events
|
||||
(fn [new-state]
|
||||
(let [;; ==== Get
|
||||
file' (ths/get-file-from-state new-state)
|
||||
page' (cthf/current-page file')
|
||||
blue' (cths/get-shape file' :blue-copy-in-green-copy)
|
||||
copied-blue' (find-copied-shape blue' page' uuid/zero)]
|
||||
|
||||
;; ==== Check
|
||||
;; the copied blue has no swap-id
|
||||
(t/is (some? copied-blue'))
|
||||
(t/is (nil? (ctk/get-swap-slot copied-blue')))))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user