diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index b0394052a5..9d0551783d 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -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 diff --git a/frontend/src/app/main/data/workspace/undo.cljs b/frontend/src/app/main/data/workspace/undo.cljs index 26c267561e..60d1bd340a 100644 --- a/frontend/src/app/main/data/workspace/undo.cljs +++ b/frontend/src/app/main/data/workspace/undo.cljs @@ -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)))))) diff --git a/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs b/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs index e99c2e1e79..7720cf63d3 100644 --- a/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs +++ b/frontend/test/frontend_tests/composable_tests/completion_contract_test.cljs @@ -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))) diff --git a/frontend/test/frontend_tests/data/workspace_undo_test.cljs b/frontend/test/frontend_tests/data/workspace_undo_test.cljs index dd99aa1825..a687ba5ec2 100644 --- a/frontend/test/frontend_tests/data/workspace_undo_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_undo_test.cljs @@ -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)]