mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 05:50:35 +00:00
🐛 Keep delayed font resizes in their undo group
Stack a delayed derived change into its applied undo group when that group exists, instead of an unrelated later entry or transaction. Preserve an older entry's selection and exclude the redo tail. The delayed-font regression fails before the fix and passes afterward. Async transaction controls pass 18 assertions; the completion suite passes 17 tests / 646 assertions. Full frontend passes 1,563 tests / 10,860 assertions, including unchanged goldens. Lint and formatting pass. A standalone-text WASM history passes 9 checks, with exact undo/redo, full-sync consistency, the original page restored and no pending work. Benchmark ratios against the unchanged Phase 1e baseline: main 10/100/1,000 copies 0.62/0.59/0.62; three levels 0.68; dependency chain 0.54; library update 0.54; many shapes 0.48; variant switch 0.76. All worlds pass without regression flags. The separate late copy-measurement path remains tracked as F222. AI-assisted-by: gpt-6
This commit is contained in:
parent
00b743da0b
commit
92228bd55c
@ -55,6 +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.
|
||||
- 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
|
||||
|
||||
@ -132,20 +132,20 @@
|
||||
state))
|
||||
|
||||
(defn- stack-undo-entry
|
||||
"Extends the current undo entry in the workspace with new changes if it
|
||||
"Extends the selected undo entry with new changes if it
|
||||
exists, or creates a new entry if it doesn't. When stacking onto an
|
||||
existing entry, the entry's original timestamp is preserved so the
|
||||
history panel keeps showing when the action originated. Issue #7660."
|
||||
[state {:keys [undo-changes redo-changes selected-after] :as entry}]
|
||||
(let [index (get-in state [:workspace-undo :index] -1)]
|
||||
(if (>= index 0)
|
||||
(update-in state [:workspace-undo :items index]
|
||||
(fn [item]
|
||||
(-> item
|
||||
(update :undo-changes #(into undo-changes %))
|
||||
(update :redo-changes #(into % redo-changes))
|
||||
(assoc :selected-after selected-after))))
|
||||
(add-undo-entry state entry))))
|
||||
[state {:keys [undo-changes redo-changes selected-after] :as entry} index]
|
||||
(if (>= index 0)
|
||||
(update-in state [:workspace-undo :items index]
|
||||
(fn [item]
|
||||
(cond-> (-> item
|
||||
(update :undo-changes #(into undo-changes %))
|
||||
(update :redo-changes #(into % redo-changes)))
|
||||
(= index (get-in state [:workspace-undo :index]))
|
||||
(assoc :selected-after selected-after))))
|
||||
(add-undo-entry state entry)))
|
||||
|
||||
(defn- accumulate-undo-entry
|
||||
"Extends the current undo transaction with new changes."
|
||||
@ -161,11 +161,9 @@
|
||||
(assoc-in [:workspace-undo :transaction :tags] tags)))
|
||||
|
||||
(defn append-undo
|
||||
"UpdateEvent to add an entry to the undo stack, or extend the current undo transaction
|
||||
or last undo entry.
|
||||
- If `stack?` is true, it will stack the entry on top of the current undo entry.
|
||||
- If `stack?` is false, it will add a new entry to the undo stack.
|
||||
- If there is an open transaction, it will accumulate the changes in that transaction."
|
||||
"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."
|
||||
[entry stack?]
|
||||
|
||||
(assert (check-undo-entry entry))
|
||||
@ -175,8 +173,21 @@
|
||||
ptk/UpdateEvent
|
||||
(update [_ state]
|
||||
(let [selected-after (dm/get-in state [:workspace-local :selected])
|
||||
entry (assoc entry :selected-after selected-after)]
|
||||
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)
|
||||
(not= (:undo-group entry) (:undo-group transaction)))
|
||||
(last (keep-indexed
|
||||
(fn [index item]
|
||||
(when (= (:undo-group entry) (:undo-group item)) index))
|
||||
(take (inc index) (get-in state [:workspace-undo :items])))))]
|
||||
(cond
|
||||
;; A delayed derived change still belongs to its primary action,
|
||||
;; even after another edit or transaction has started.
|
||||
(some? group-index)
|
||||
(stack-undo-entry state entry group-index)
|
||||
|
||||
(and (get-in state [:workspace-undo :transaction])
|
||||
(or (not stack?)
|
||||
(d/not-empty? (get-in state [:workspace-undo :transaction :undo-changes]))
|
||||
@ -184,7 +195,7 @@
|
||||
(accumulate-undo-entry state entry)
|
||||
|
||||
stack?
|
||||
(stack-undo-entry state entry)
|
||||
(stack-undo-entry state entry index)
|
||||
|
||||
:else
|
||||
(add-undo-entry state entry))))))
|
||||
|
||||
@ -37,6 +37,7 @@
|
||||
[app.main.data.workspace.wasm-text :as dwwt]
|
||||
[app.main.store :as st]
|
||||
[app.render-wasm.api :as wasm]
|
||||
[app.render-wasm.api.fonts :as wasm.fonts]
|
||||
[app.util.text-editor :as ted]
|
||||
[beicon.v2.core :as rx]
|
||||
[cljs.test :as t :include-macros true]
|
||||
@ -180,6 +181,59 @@
|
||||
(thw/teardown-wasm-mocks!)
|
||||
(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)
|
||||
(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))))))
|
||||
|
||||
(t/deftest ^:async rotation-nodes-compose-successive-turns
|
||||
(let [initial (ftm/start-generated (setup/simple-component-with-labeled-copy))
|
||||
first-turn (await (ftm/run-generated-step initial (n/rotate :copy-root 45)))
|
||||
|
||||
@ -13,9 +13,12 @@
|
||||
[app.common.files.changes-builder :as pcb]
|
||||
[app.common.logic.shapes :as cls]
|
||||
[app.common.test-helpers.composable.comp.setups :as setup]
|
||||
[app.common.test-helpers.composable.comp.undo-check :as uc]
|
||||
[app.common.test-helpers.composable.core :as tm]
|
||||
[app.common.test-helpers.files :as thf]
|
||||
[app.common.test-helpers.ids-map :as thi]
|
||||
[app.common.test-helpers.shapes :as ths]
|
||||
[app.common.uuid :as uuid]
|
||||
[app.main.data.changes :as dch]
|
||||
[app.main.data.workspace.undo :as dwu]
|
||||
[app.main.store :as st]
|
||||
@ -56,6 +59,37 @@
|
||||
[]
|
||||
(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))))))
|
||||
|
||||
(t/deftest ^:async undo-and-redo-an-entry-without-undo-group
|
||||
(ftm/install! (setup/simple-component-with-labeled-copy))
|
||||
(let [changes (fill-main-child-changes red)]
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user