From d1e0f80265e5b46b244b78df2d7d408480e6a6a3 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Fri, 23 Oct 2026 20:45:13 +0200 Subject: [PATCH] :bug: Keep update-main and deferred text sync in one undo group Use the same explicit group for inverse sync and every copy sync. Text reflow can finish after the update transaction closes; one undo must still restore the main, its copies and the source overrides together (F152). The five-operation text regression fails before this change. All 12 frontend generated replays pass: 865 assertions. Common and frontend lint and format pass. Shared benchmark ratios: mains 0.47/0.53/0.55; levels 0.51; chain 0.64; library 0.64; many shapes 0.96; switch 0.68, no flags. AI-assisted-by: GPT-6 --- .../frontend/workspace-state-persistence-subtleties.md | 3 +++ frontend/src/app/main/data/workspace/libraries.cljs | 10 ++++++---- .../composable_tests/generated_test.cljs | 5 +++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index 373f28aa26..b0394052a5 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -51,6 +51,9 @@ - `leave-signals` (document `visibilitychange`→hidden, window `blur`) emits `::force-persist` when edits are buffered, so leaving the tab/window sends the 3 s batch at once; tests mock `dps/leave-signals` BEFORE `initialize-persistence` runs. pagehide/beforeunload are too late for an async save. - Exports and the `.penpot` download call `force-persist-files-and-wait file-ids timeout` (waits only for those files, ends on `:error` or timeout, never raises) and then `unsaved-changes? state file-ids` to warn; it reads `refs/persistence`, so tests mock it with `(l/derived :persistence store)`. - `persistence-failed` emits a `save-failed` audit event (`:cause-type :code :retrying :queued-commits`) whenever the user is notified (`notify-failure?`, same rule as the toast), including delegated auth/not-found failures. +- Updating a component main passes one explicit undo group through the + 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. - 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. diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 28c37118b6..1a2d70e01e 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -845,7 +845,7 @@ NOTE: It's possible that the component to update is defined in an external library file, so this function may cause to modify a file different of that the one we are currently editing." - [id] + [id & {:keys [undo-group]}] (dm/assert! (uuid? id)) (ptk/reify ::update-component ptk/WatchEvent @@ -863,6 +863,7 @@ (when (ctk/instance-head? shape) (let [[local-changes nonlocal-changes] (-> (pcb/empty-changes it) + (cond-> undo-group (pcb/set-undo-group undo-group)) (pcb/with-container container) (cll/generate-sync-shape-inverse libraries container id) (cll/split-local-changes))] @@ -922,17 +923,18 @@ (let [page (dsh/lookup-page state) shape (ctn/get-shape page shape-id) component-id (:component-id shape) - undo-id (js/Symbol)] + undo-id (js/Symbol) + undo-group (uuid/next)] (apply rx/of (concat [(dwu/start-undo-transaction undo-id) - (update-component shape-id) + (update-component shape-id :undo-group undo-group) ;; These calls are necessary for properly sync thumbnails ;; when a main component does not live in the same page. (update-component-thumbnail-sync state component-id file-id "frame") (update-component-thumbnail-sync state component-id file-id "component")] - (component-sync-events state component-id file-id nil) + (component-sync-events state component-id file-id undo-group) [(dwu/commit-undo-transaction undo-id)])))))) (defn update-component-thumbnail diff --git a/frontend/test/frontend_tests/composable_tests/generated_test.cljs b/frontend/test/frontend_tests/composable_tests/generated_test.cljs index a64bca9ee6..94a23ab82b 100644 --- a/frontend/test/frontend_tests/composable_tests/generated_test.cljs +++ b/frontend/test/frontend_tests/composable_tests/generated_test.cljs @@ -89,3 +89,8 @@ (t/deftest ^:async nesting-restores-default-constraints-on-undo (await (replay {:world [2 0] :ops [[:nest 0]]}))) + +(t/deftest ^:async update-main-after-nesting-a-rotated-text-copy-undoes-together + (await (replay {:world [2 0 {:text true}] + :ops [[:duplicate 0] [:nest 0] [:rotate 0 0 0] + [:edit 0 0 1 1] [:update-main 0]]})))