github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 14:00:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Sync a reordered copy child into its main on update main

Browse Source 
In an inverse sync, when a copy child paired with a main child further
along the main's children, compare-children asked whether the copy
child was the main of the main child, with the arguments of
ctk/is-main-of? reversed. The answer was always no, so the pair took
the swapped callback, which does nothing in an inverse sync, and only
the move ran: the overrides of that copy child never reached the main.
The UI does not let a copy reorder its children, so it only showed
when the order differed for another reason, such as the undo of a
sync reorder.

The inverse branch now asks synced-pair? with the main child first,
as the direct branch does.

Benchmark (lower quantile, ms; baseline / now / ratio):
  deepest-edit-through-three-levels    35.2 /  35.5 / 1.01
  edit-through-a-chain-of-dependents   68.9 /  66.1 / 0.96
  library-update-into-copies           83.1 /  77.0 / 0.93
  main-edit-into-10-copies              1.8 /   1.6 / 0.92
  main-edit-into-100-copies            15.4 /  16.2 / 1.05
  main-edit-into-1000-copies          153.1 / 156.4 / 1.02
  variant-switch-into-copies           32.1 /  28.3 / 0.88

AI-assisted-by: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
alonso.torres 2026-10-13 14:14:45 +02:00
parent 78ef2dffe6
commit e7a65b4479
2 changed files with 45 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
common/src/app/common/logic/libraries.cljc
View File

@ -1256,13 +1256,13 @@
:else
(if inverse?
(let [is-main? (ctk/is-main-of? child-inst child-main')]
(let [synced? (synced-pair? child-main' child-inst)]
(recur (next children-inst)
(remove #(= (:id %) (:id child-main')) children-main)
(cond-> changes
is-main?
synced?
(both-cb child-inst child-main')
(not is-main?)
(not synced?)
(swapped-cb child-inst child-main')
:always
(moved-cb child-inst child-main'))))

42
common/test/common_tests/logic/comp_sync_test.cljc
View File

@ -18,6 +18,7 @@
[app.common.test-helpers.ids-map :as thi]
[app.common.test-helpers.shapes :as ths]
[app.common.types.component :as ctk]
[app.common.types.container :as ctn]
[app.common.types.shape-tree :as ctst]
[clojure.test :as t]))
@ -539,3 +540,44 @@
;; A position-only change in the main component must not propagate to copies
;; and therefore must produce no redo-changes.
(t/is (empty? (:redo-changes sync-changes)))))
(t/deftest test-update-main-from-a-copy-with-reordered-children
(let [;; ==== Setup
file (-> (thf/sample-file :file1)
(tho/add-component-with-many-children-and-copy :component1
:main-root
[:main-child1 :main-child2]
:copy-root
:copy-root-params {:children-labels [:copy-child1 :copy-child2]}))
page (thf/current-page file)
;; The UI never reorders the children of a copy, so the order is
;; set directly: the copy holds them in the reverse order of the main.
file (update-in file [:data :pages-index (:id page) :objects (thi/id :copy-root) :shapes]
(comp vec reverse))
changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page))
#{(thi/id :copy-child2)}
#(assoc % :hidden true)
(:objects page)
{})
file (thf/apply-changes file changes)
;; ==== Action
page (thf/current-page file)
container (ctn/make-container page :page)
changes (-> (pcb/empty-changes)
(pcb/with-container container)
(cll/generate-sync-shape-inverse (:data file)
{(:id file) file}
container
(thi/id :copy-root)))
file' (thf/apply-changes file changes)
;; ==== Get
main-root' (ths/get-shape file' :main-root)
main-child2' (ths/get-shape file' :main-child2)]
;; ==== Check
(t/is (true? (:hidden main-child2')))
(t/is (= [(thi/id :main-child2) (thi/id :main-child1)] (:shapes main-root')))))