mirror of
https://github.com/penpot/penpot.git
synced 2026-09-06 20:18:39 +00:00
🐛 Fix nested components becomes flat after reset (#9841)
This commit is contained in:
parent
7843bb1208
commit
aa996c5118
@ -825,20 +825,30 @@
|
|||||||
changes)))
|
changes)))
|
||||||
|
|
||||||
(defn- find-main-container
|
(defn- find-main-container
|
||||||
"Find the container that has the main shape."
|
"Find the container that has the main shape.
|
||||||
[container-inst shape-inst shape-main library component]
|
|
||||||
|
When walking up through nested component copies, each intermediate component
|
||||||
|
may live in a different library/file. We resolve the right file-data for each
|
||||||
|
component via `ctf/find-component-file` so we can locate components that span
|
||||||
|
multiple libraries."
|
||||||
|
[container-inst shape-inst shape-main library file libraries component]
|
||||||
(loop [shape-inst' shape-inst
|
(loop [shape-inst' shape-inst
|
||||||
component' component]
|
library' library
|
||||||
(let [container (ctf/get-component-container library component')] ; TODO: this won't work if some intermediate component is in a different library
|
component' component]
|
||||||
(if (some? (ctn/get-shape container (:id shape-main))) ; for this to work we need to have access to the libraries list here
|
(let [container (ctf/get-component-container library' component')]
|
||||||
|
(if (some? (ctn/get-shape container (:id shape-main)))
|
||||||
container
|
container
|
||||||
(let [parent (ctn/get-shape container-inst (:parent-id shape-inst'))
|
(let [parent (ctn/get-shape container-inst (:parent-id shape-inst'))
|
||||||
shape-inst' (ctn/get-head-shape (:objects container-inst) parent)
|
shape-inst' (ctn/get-head-shape (:objects container-inst) parent)
|
||||||
component' (or (ctkl/get-component library (:component-id shape-inst'))
|
next-file (some-> shape-inst'
|
||||||
(ctkl/get-deleted-component library (:component-id shape-inst')))]
|
:component-file
|
||||||
|
(->> (ctf/find-component-file file libraries)))
|
||||||
|
next-data (some-> next-file :data)
|
||||||
|
component' (when next-data
|
||||||
|
(or (ctkl/get-component next-data (:component-id shape-inst'))
|
||||||
|
(ctkl/get-deleted-component next-data (:component-id shape-inst'))))]
|
||||||
(if (some? component')
|
(if (some? component')
|
||||||
(recur shape-inst'
|
(recur shape-inst' next-data component')
|
||||||
component')
|
|
||||||
nil))))))
|
nil))))))
|
||||||
|
|
||||||
(defn- generate-sync-shape-direct-recursive
|
(defn- generate-sync-shape-direct-recursive
|
||||||
@ -884,7 +894,7 @@
|
|||||||
set-remote-synced?
|
set-remote-synced?
|
||||||
(change-remote-synced shape-inst container true))
|
(change-remote-synced shape-inst container true))
|
||||||
|
|
||||||
component-container (find-main-container container shape-inst shape-main library component)
|
component-container (find-main-container container shape-inst shape-main library file libraries component)
|
||||||
|
|
||||||
children-inst (vec (ctn/get-direct-children container shape-inst))
|
children-inst (vec (ctn/get-direct-children container shape-inst))
|
||||||
children-main (vec (ctn/get-direct-children component-container shape-main))
|
children-main (vec (ctn/get-direct-children component-container shape-main))
|
||||||
|
|||||||
@ -14,6 +14,7 @@
|
|||||||
[app.common.test-helpers.files :as thf]
|
[app.common.test-helpers.files :as thf]
|
||||||
[app.common.test-helpers.ids-map :as thi]
|
[app.common.test-helpers.ids-map :as thi]
|
||||||
[app.common.test-helpers.shapes :as ths]
|
[app.common.test-helpers.shapes :as ths]
|
||||||
|
[app.common.types.container :as ctn]
|
||||||
[clojure.test :as t]))
|
[clojure.test :as t]))
|
||||||
|
|
||||||
(t/use-fixtures :each thi/test-fixture)
|
(t/use-fixtures :each thi/test-fixture)
|
||||||
@ -386,6 +387,85 @@
|
|||||||
;; After reset + propagation the copy should mirror the canonical color
|
;; After reset + propagation the copy should mirror the canonical color
|
||||||
(t/is (= copy2-bottom-color "#aabbcc"))))
|
(t/is (= copy2-bottom-color "#aabbcc"))))
|
||||||
|
|
||||||
|
(t/deftest test-reset-nested-component-from-different-library-preserves-children
|
||||||
|
;; Regression test for the cross-library find-main-container bug:
|
||||||
|
;; avatar-img lives in library1, avatar (which nests avatar-img) lives in library2,
|
||||||
|
;; and card (which nests avatar) lives in the current file.
|
||||||
|
;; Resetting overrides on avatar used to delete avatar-img because find-main-container
|
||||||
|
;; only searched within the single library argument (library2) and could not find
|
||||||
|
;; the avatar-img component which belongs to library1.
|
||||||
|
(let [;; ==== Setup
|
||||||
|
;; library1: avatar-img-component (simple component with a child rect)
|
||||||
|
library1
|
||||||
|
(-> (thf/sample-file :library1 :is-shared true)
|
||||||
|
(tho/add-simple-component :avatar-img-component
|
||||||
|
:avatar-img-main
|
||||||
|
:avatar-img-child
|
||||||
|
:child-params {:fills (ths/sample-fills-color :fill-color "#abcdef")}))
|
||||||
|
|
||||||
|
;; library2: avatar-component (nests avatar-img from library1)
|
||||||
|
library2
|
||||||
|
(-> (thf/sample-file :library2 :is-shared true)
|
||||||
|
(tho/add-frame :avatar-main)
|
||||||
|
(thc/instantiate-component :avatar-img-component
|
||||||
|
:avatar-img-nested-head
|
||||||
|
:library library1
|
||||||
|
:parent-label :avatar-main
|
||||||
|
:children-labels [:avatar-img-nested-child])
|
||||||
|
(thc/make-component :avatar-component :avatar-main))
|
||||||
|
|
||||||
|
;; file: card-component (nests avatar from library2) + a copy of card
|
||||||
|
file
|
||||||
|
(-> (thf/sample-file :file)
|
||||||
|
(tho/add-frame :card-main)
|
||||||
|
(thc/instantiate-component :avatar-component
|
||||||
|
:avatar-in-card-main
|
||||||
|
:library library2
|
||||||
|
:parent-label :card-main
|
||||||
|
:children-labels [:avatar-img-in-card-main])
|
||||||
|
(thc/make-component :card-component :card-main)
|
||||||
|
(thc/instantiate-component :card-component
|
||||||
|
:card-copy
|
||||||
|
:children-labels [:avatar-in-card-copy]))
|
||||||
|
|
||||||
|
page (thf/current-page file)
|
||||||
|
avatar-in-card-copy (ths/get-shape file :avatar-in-card-copy)
|
||||||
|
|
||||||
|
;; ==== Action: override a fill on the avatar copy, then reset
|
||||||
|
changes
|
||||||
|
(cls/generate-update-shapes (pcb/empty-changes nil (:id page))
|
||||||
|
#{(:id avatar-in-card-copy)}
|
||||||
|
(fn [shape]
|
||||||
|
(assoc shape :fills (ths/sample-fills-color :fill-color "#fabada")))
|
||||||
|
(:objects page)
|
||||||
|
{})
|
||||||
|
|
||||||
|
file-mdf (thf/apply-changes file changes)
|
||||||
|
container-mdf (ctn/get-container (:data file-mdf) :page (:id page))
|
||||||
|
|
||||||
|
changes
|
||||||
|
(cll/generate-reset-component (pcb/empty-changes)
|
||||||
|
file-mdf
|
||||||
|
{(:id file-mdf) file-mdf
|
||||||
|
(:id library1) library1
|
||||||
|
(:id library2) library2}
|
||||||
|
container-mdf
|
||||||
|
(:id avatar-in-card-copy))
|
||||||
|
|
||||||
|
file' (thf/apply-changes file-mdf changes :validate? false)
|
||||||
|
|
||||||
|
;; ==== Get
|
||||||
|
avatar-in-card-copy' (ths/get-shape file' :avatar-in-card-copy)
|
||||||
|
;; avatar-img nested head must still exist inside avatar-in-card-copy after reset
|
||||||
|
avatar-img-copy-id (first (:shapes avatar-in-card-copy'))
|
||||||
|
avatar-img-copy' (when avatar-img-copy-id
|
||||||
|
(ths/get-shape-by-id file' avatar-img-copy-id))]
|
||||||
|
|
||||||
|
;; ==== Check
|
||||||
|
(t/is (some? avatar-in-card-copy') "avatar copy must still exist after reset")
|
||||||
|
(t/is (some? avatar-img-copy-id) "avatar-img copy must not be deleted from avatar's children")
|
||||||
|
(t/is (some? avatar-img-copy') "avatar-img copy shape must be present in objects")))
|
||||||
|
|
||||||
(t/deftest test-reset-without-propagation-does-not-update-copies
|
(t/deftest test-reset-without-propagation-does-not-update-copies
|
||||||
;; This is the regression test for the misplaced-parenthesis bug: when
|
;; This is the regression test for the misplaced-parenthesis bug: when
|
||||||
;; propagate-fn is NOT passed to reset-overrides the copies of the component
|
;; propagate-fn is NOT passed to reset-overrides the copies of the component
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user