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

🐛 Keep delayed copy measurements with their undo group

Browse Source 
Route non-stacked derived commits to their applied group when a later
edit or transaction intervenes. Copy remeasurement must undo with the
font action rather than becoming a separate step after an unrelated edit.
Keep adjacent non-stacked entries and exclude the redo tail from routing.

Extend the font-boundary regression to component mains and copies, and
cover both stacking modes during a later transaction. The regression
fails before the fix; all 1,563 frontend tests and 10,880 assertions pass.
Live WASM history passes exact undo/redo and full-sync consistency.

The sync core is unchanged by this undo fix. Its preceding benchmark
ratios remain 0.48-0.76 of the unchanged baseline, with no regression flag.

AI-assisted-by: gpt-6
This commit is contained in:
alonso.torres 2026-10-27 10:16:15 +01:00
parent 92228bd55c
commit a62a15206d
4 changed files with 101 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
.serena/memories/frontend/workspace-state-persistence-subtleties.md
View File

@ -55,12 +55,12 @@
inverse sync and all copy syncs. Deferred text reflow may commit after
the transaction closes; its changes must still join the user action (F152).
- Undo transactions can stay open only temporarily; timed-out pending transactions are force-committed after about 20s. Undo entries are capped at 50.
- Stacked derived changes first look for their own group among applied undo
entries (F214). A delayed font resize must not join a later unrelated
edit or transaction. Preserve an older entry's selection; never search
its redo tail. Without a matching group, stacking keeps its current-entry
behavior. Non-stacked delayed copy measurement is tracked separately as
F222 while its async regression and routing correction are pending.
- Derived changes first look for their own group among applied undo entries
(F214, F222). Delayed font and copy measurements extend that group across
later edits or transactions, even without `:stack-undo?`. Preserve an
older entry's selection; never search its redo tail. Adjacent non-stacked
changes keep separate entries; without a matching group, stacking keeps
its current-entry behavior.
- Undo/redo are ignored while a normal editor/drawing interaction is active, except grid-layout edition handles undo through this path.
- After local commits and when render-wasm is active, text shapes get derived `:position-data` recomputed in a separate commit tagged `#{:position-data}`; that tag is excluded from the position-data watcher to avoid loops.
- WASM component thumbnails cancel the entire upload/render stream on

11
frontend/src/app/main/data/workspace/undo.cljs
View File

@ -161,9 +161,9 @@
(assoc-in [:workspace-undo :transaction :tags] tags)))
(defn append-undo
"Add changes to undo history. Stacked changes extend their applied undo
group when present, otherwise the current entry. An open transaction
accumulates changes unless they stack into another applied group."
"Add changes to undo history. Delayed changes extend their applied undo
group across later edits or transactions, even without stacking.
Stacked changes without a matching group extend the current entry."
[entry stack?]
(assert (check-undo-entry entry))
@ -176,7 +176,7 @@
entry (assoc entry :selected-after selected-after)
index (get-in state [:workspace-undo :index] -1)
transaction (get-in state [:workspace-undo :transaction])
group-index (when (and stack? (:undo-group entry)
group-index (when (and (:undo-group entry)
(not= (:undo-group entry) (:undo-group transaction)))
(last (keep-indexed
(fn [index item]
@ -185,7 +185,8 @@
(cond
;; A delayed derived change still belongs to its primary action,
;; even after another edit or transaction has started.
(some? group-index)
(and (some? group-index)
(or stack? (< group-index index) transaction))
(stack-undo-entry state entry group-index)
(and (get-in state [:workspace-undo :transaction])

108
frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs
View File

@ -182,57 +182,67 @@
(st/emit! #(assoc % :features features))))))
(t/deftest ^:async a-delayed-font-resize-stays-with-its-font-edit
(let [features (:features @st/state)
ready (atom true)
face {:font-id "gfont-roboto" :weight 400 :style 0 :emoji? false}]
(thw/setup-wasm-mocks!)
(try
(await
(mock/with-mocks*
{wasm/text-editor-has-focus? (mock/stub (constantly false))
wasm.fonts/get-content-fonts (mock/stub (constantly [face]))
wasm.fonts/make-font-data (mock/stub identity)
wasm.fonts/font-ready? (mock/stub (fn [_] @ready))
wasm/get-text-dimensions (mock/stub (fn [] {:x 0 :y 0 :width 100 :max-width 100
:height (if @ready 40 20)}))
dwthw/render-thumbnail (mock/stub (fn [& _] (ptk/data-event ::thumbnail-skipped)))}
(let [file (-> (thf/sample-file :file1)
(doseq [component? [false true]]
(let [features (:features @st/state)
ready (atom true)
face {:font-id "gfont-roboto" :weight 400 :style 0 :emoji? false}
measuring (atom nil)]
(thw/setup-wasm-mocks!)
(try
(await
(mock/with-mocks*
{wasm/text-editor-has-focus? (mock/stub (constantly false))
wasm.fonts/get-content-fonts (mock/stub (constantly [face]))
wasm.fonts/make-font-data (mock/stub identity)
wasm.fonts/font-ready? (mock/stub (fn [_] @ready))
wasm/set-shape-text-content (mock/stub (fn [id _] (reset! measuring id)))
wasm/get-text-dimensions (mock/stub (fn [] {:x 0 :y 0 :width 100 :max-width 100
:height (if @ready
(if (= @measuring (thi/id :copy-child)) 50 40)
20)}))
dwthw/render-thumbnail (mock/stub (fn [& _] (ptk/data-event ::thumbnail-skipped)))}
(let [file (cond-> (thf/sample-file :file1)
component?
(thtx/add-text-component-with-copy [[["Read the guide"]]])
(not component?)
(ths/add-sample-shape :text :type :text :width 100 :height 20
:grow-type :auto-height
:content (thtx/content [[["Read the guide"]]]))
(ths/add-sample-shape :rect :type :rect :opacity 1))
_ (ftm/install! (tm/make-situation file))
before (ftm/snapshot)]
(st/emit! #(update % :features (fnil conj #{}) "render-wasm/v1"))
(reset! ready false)
(st/emit! (dwt/update-all-attrs [(thi/id :text)]
{:font-id "gfont-roboto" :font-family "Roboto"
:font-variant-id "regular"}))
(await (ha/wait-for #(some (comp seq :font) (vals (wrf/pending)))
"font edit is pending"))
(st/emit! (dwsh/update-shapes [(thi/id :rect)] #(assoc % :opacity 0.4)))
(await (ha/wait-for #(= 0.4 (:opacity (ths/get-shape (ftm/current-file) :rect)))
"unrelated edit commits before the font arrives"))
(let [text (ths/get-shape (ftm/current-file) :text)
faces (map (comp wasm.fonts/font-data-key wasm.fonts/make-font-data)
(wasm.fonts/get-content-fonts (:content text)))]
(t/is (seq faces) "the font wait has a face to release")
(reset! ready true)
(doseq [face faces] (rx/push! wasm.fonts/font-stored-stream face)))
(t/is (= :settled (await (ftm/await-step []))))
(t/is (mth/close? 40 (:height (ths/get-shape (ftm/current-file) :text))))
(let [after (ftm/snapshot)]
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :rect))))
(t/is (mth/close? 40 (:height (ths/get-shape (ftm/current-file) :text)))
"undoing the unrelated edit keeps the font's measured height")
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (uc/same-state? before (ftm/snapshot)))
(t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo]))))
(t/is (uc/same-state? after (ftm/snapshot)))))))
(finally
(thw/teardown-wasm-mocks!)
(st/emit! #(assoc % :features features))))))
:content (thtx/content [[["Read the guide"]]])))
text-id (thi/id (if component? :main-child :text))
file (-> file
(ths/add-sample-shape :rect :type :rect :opacity 1))
_ (ftm/install! (tm/make-situation file))
before (ftm/snapshot)]
(st/emit! #(update % :features (fnil conj #{}) "render-wasm/v1"))
(reset! ready false)
(st/emit! (dwt/update-all-attrs [text-id]
{:font-id "gfont-roboto" :font-family "Roboto"
:font-variant-id "regular"}))
(await (ha/wait-for #(some (comp seq :font) (vals (wrf/pending)))
"font edit is pending"))
(st/emit! (dwsh/update-shapes [(thi/id :rect)] #(assoc % :opacity 0.4)))
(await (ha/wait-for #(= 0.4 (:opacity (ths/get-shape (ftm/current-file) :rect)))
"unrelated edit commits before the font arrives"))
(let [text (ths/get-shape-by-id (ftm/current-file) text-id)
faces (map (comp wasm.fonts/font-data-key wasm.fonts/make-font-data)
(wasm.fonts/get-content-fonts (:content text)))]
(t/is (seq faces) "the font wait has a face to release")
(reset! ready true)
(doseq [face faces] (rx/push! wasm.fonts/font-stored-stream face)))
(t/is (= :settled (await (ftm/await-step []))))
(t/is (mth/close? 40 (:height (ths/get-shape-by-id (ftm/current-file) text-id))))
(let [after (ftm/snapshot)]
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :rect))))
(t/is (mth/close? 40 (:height (ths/get-shape-by-id (ftm/current-file) text-id)))
"undoing the unrelated edit keeps the font's measured height")
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (uc/same-state? before (ftm/snapshot)))
(t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo]))))
(t/is (uc/same-state? after (ftm/snapshot)))))))
(finally
(thw/teardown-wasm-mocks!)
(st/emit! #(assoc % :features features)))))))
(t/deftest ^:async rotation-nodes-compose-successive-turns
(let [initial (ftm/start-generated (setup/simple-component-with-labeled-copy))

59
frontend/test/frontend_tests/data/workspace_undo_test.cljs
View File

@ -60,35 +60,36 @@
(get-in @st/state [:workspace-undo :index]))
(t/deftest ^:async delayed-derived-changes-do-not-join-an-unrelated-transaction
(let [file (-> (thf/sample-file :file)
(ths/add-sample-shape :first :type :rect :opacity 1)
(ths/add-sample-shape :second :type :rect :opacity 1))
_ (ftm/install! (tm/make-situation file))
before (ftm/snapshot)
first-group (uuid/next)
second-group (uuid/next)
transaction (js/Symbol)
edit (fn [label attr value opts]
(let [page (thf/current-page (ftm/current-file))
changes (cls/generate-update-shapes
(pcb/empty-changes nil (:id page)) [(thi/id label)]
#(assoc % attr value) (:objects page) {})]
(dch/commit-changes (merge changes opts))))]
(t/is (= :settled (await (ftm/await-step [(edit :first :opacity 0.4 {:undo-group first-group})]))))
(t/is (= :settled
(await (ftm/await-step [(dwu/start-undo-transaction transaction)
(edit :second :opacity 0.5 {:undo-group second-group})
(edit :first :r1 8 {:undo-group first-group :stack-undo? true})
(dwu/commit-undo-transaction transaction)]))))
(let [after (ftm/snapshot)]
(t/is (= [first-group second-group] (mapv :undo-group (get-in @st/state [:workspace-undo :items]))))
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :second))))
(t/is (= 8 (:r1 (ths/get-shape (ftm/current-file) :first))))
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (uc/same-state? before (ftm/snapshot)))
(t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo]))))
(t/is (uc/same-state? after (ftm/snapshot))))))
(doseq [stack? [true false]]
(let [file (-> (thf/sample-file :file)
(ths/add-sample-shape :first :type :rect :opacity 1)
(ths/add-sample-shape :second :type :rect :opacity 1))
_ (ftm/install! (tm/make-situation file))
before (ftm/snapshot)
first-group (uuid/next)
second-group (uuid/next)
transaction (js/Symbol)
edit (fn [label attr value opts]
(let [page (thf/current-page (ftm/current-file))
changes (cls/generate-update-shapes
(pcb/empty-changes nil (:id page)) [(thi/id label)]
#(assoc % attr value) (:objects page) {})]
(dch/commit-changes (merge changes opts))))]
(t/is (= :settled (await (ftm/await-step [(edit :first :opacity 0.4 {:undo-group first-group})]))))
(t/is (= :settled
(await (ftm/await-step [(dwu/start-undo-transaction transaction)
(edit :second :opacity 0.5 {:undo-group second-group})
(edit :first :r1 8 {:undo-group first-group :stack-undo? stack?})
(dwu/commit-undo-transaction transaction)]))))
(let [after (ftm/snapshot)]
(t/is (= [first-group second-group] (mapv :undo-group (get-in @st/state [:workspace-undo :items]))))
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (= 1 (:opacity (ths/get-shape (ftm/current-file) :second))))
(t/is (= 8 (:r1 (ths/get-shape (ftm/current-file) :first))))
(t/is (= :settled (await (ftm/await-step [dwu/undo]))))
(t/is (uc/same-state? before (ftm/snapshot)))
(t/is (= :settled (await (ftm/await-step [dwu/redo dwu/redo]))))
(t/is (uc/same-state? after (ftm/snapshot)))))))
(t/deftest ^:async undo-and-redo-an-entry-without-undo-group
(ftm/install! (setup/simple-component-with-labeled-copy))