diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index 0cb9e7bbc5..7e8196080f 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -7,6 +7,7 @@ - `ongoing-tasks` controls `window.onbeforeunload`: any non-empty set blocks tab unload. - `app.main.streams/wasm-modifiers` and `workspace-selrect` are behavior subjects used for high-frequency interactive preview state that bypasses normal store updates and lenses. - Keyboard modifier streams merge a window blur signal so stuck modifier-key state is cleared after focus loss. +- Focus mode (`dws/toggle-focus-mode`) keeps a watcher on page objects that adds new shapes to `:workspace-focus-selected` and drops removed ones; when the set empties (delete, rollback) `update-focus-shapes` leaves focus mode (`::focus-mode-left` stops the watcher) and both exits restore the pre-focus selection without removed shapes. Without it the next drawn shape re-armed focus mode alone. ## Repo calls @@ -22,7 +23,7 @@ - Applying a remote commit first rolls back the pending local commits of its own file (`pcm/pending-commits`: queued entries, then `[:persistence :buffer]`; other files' commits never touch its data), applies the remote changes, then replays pending local redo changes. Index updates are emitted for undo, remote redo, and replayed redo paths. Leaving the buffer out makes a remote change to the same attr win locally but lose on the server. - Local commits are independently consumed by undo, persistence, WASM model updates, thumbnail/library watchers, and text position-data recalculation. - Persistence buffers local commits in the store (`buffer-commit` → `[:persistence :buffer]`, stamped with `:remote-seq`): status becomes pending after about 200ms; the 3s debounce or `::force-persist` emits `flush-buffer`, which merges the buffer per file into `append-commit`s, and each append removes its own commits from the buffer (a commit arriving mid-flush stays). `initialize-persistence-state` flushes a leftover buffer. Queue entries (`pcm/queue-entry`) hold the entry keys of the first commit (id, created-at, origin, features, file-id/revn/vern) and `:commits` (id, origin, undo-group, redo/undo changes, remote-seq of each merged commit), no merged change lists; every reader goes through `:commits`. The save sends `:changes-with-metadata`, one group per commit with `:commit-id`, never flat `:changes`. -- A save answered with `:rejected` (backend kept the commits before the first failing one, see `mem:backend/subtleties`) is not a failure: `rollback-rejected` runs before `commit-persisted`. The unit is the undo group (one user action, which can commit to several files, e.g. `update-component` writes the copy's file and the library, and `component-changed` syncs inherit the edit's group; multi-file actions must share a non-nil group): the pure bookkeeping lives in `app.main.data.persistence.commits` (`pcm`): `pcm/rollback-plan` reverts, in the saved file, the pending commits from the first rejected one on, and in any file the pending commits from the first one of a reverted group on, iterating until no new group appears; other files' unrelated commits stay. `pcm/without-rolled-back` drops pending reverted commits from queue entries (partial entries trimmed, empty ones leave the queue) and the buffer; saved commits of a reverted group (the head's kept ones, and `:saved`, which `keep-saved-commits` fills on `discard-commit` with saved commits whose group a pending commit still shares) are undone by a new `save-undo? false` commit (`compensate-commits`, flushed like any edit); for a file whose data is not loaded the undo goes straight to the queue as an entry, nothing applied locally. Per file, `dch/rebase-commits` undoes every pending commit newest first, re-applies the remote changes kept since the oldest pending commit, and redoes the pending commits that stay. Undo changes hold absolute values from commit time, so undoing only the rejected commits would overwrite a peer's later change to the same attr. Remote changes are numbered (`record-remote-changes`, `[:persistence :remote-seq]`) and kept per file under `:remote-changes` only while the file has pending commits; `prune-remote-changes` (discard, release) forgets those at or below the oldest pending stamp, since re-applying them would undo saved edits. Then `dwu/forget-undo-groups groups undo-index` (cut history before the first entry whose `:undo-groups` — every group the entry holds, filled by `append-undo`, transactions and stacking — meets a reverted group; no match keeps the history; the open transaction is emptied only when cut or holding one, keeping its group for later commits; a reverted Ctrl+Z/Ctrl+Shift+Z commit, which carries real `:undo-changes` and `:undo-index` (the index it moved from, kept by `pcm` commit-keys), puts the index back to the plan's `:undo-index`, the oldest one's; background commits with `:undo-changes []` (position-data, component `modified-at`, deleted-font fixes) stay on screen after a rollback by design), selection pruned and edition mode left when the edited shape is gone, or when it is a path being edited that the rollback changed: `discard-path-edit` drops `[:workspace-local :edit-path id]` first, so `finalize-path-content` sees no `:old-content` and the editor's stale copy is never saved (`prune-removed-shapes` takes the edited object from before the rebase to compare). Text editions stay open: the v3 WASM editor already shows the reverted text, and the v2 editor resets its DOM root from the shape on `::rolled-back` when the text differs, since its unmount `on-blur` would otherwise save the stale DOM content (`prune-removed-shapes`; `v2-update-text-shape-content` also skips a missing shape, so the editor's exit commits nothing), `errors/flash` `:unhandled` toast tagged `:persistence-rejected`, hidden after `rejected-timeout-ms` (10 s). It ends with `(ptk/data-event ::rolled-back {:file-ids})`: `dwl/sync-file` waits for it or `wait-persisted` and marks the library synced only on a landed save (on rollback it restores `:synced-at` and re-checks updates after the rollback's own save). `update-status` ignores `:pending` when buffer and queue are empty (`nothing-to-save?`): a dropped commit's 200 ms pending debounce would otherwise leave the status stuck and `wait-persisted-or-error` waiting forever. `prune-removed-shapes` also closes a `:colorpicker` modal. `rebase-commits` is not a commit (not saved/broadcast/undoable, commit watchers such as thumbnails do not see it) and falls back to `::dw/reload-current-file` if applying throws. `debug.commitInvalidChange({kind, before, after})` forces such a rejection live. +- A save answered with `:rejected` (backend kept the commits before the first failing one, see `mem:backend/subtleties`) is not a failure: `rollback-rejected` runs before `commit-persisted`. The unit is the undo group (one user action, which can commit to several files, e.g. `update-component` writes the copy's file and the library, and `component-changed` syncs inherit the edit's group; multi-file actions must share a non-nil group): the pure bookkeeping lives in `app.main.data.persistence.commits` (`pcm`): `pcm/rollback-plan` reverts, in the saved file, the pending commits from the first rejected one on, and in any file the pending commits from the first one of a reverted group on, iterating until no new group appears; other files' unrelated commits stay. `pcm/without-rolled-back` drops pending reverted commits from queue entries (partial entries trimmed, empty ones leave the queue) and the buffer; saved commits of a reverted group (the head's kept ones, and `:saved`, which `keep-saved-commits` fills on `discard-commit` with saved commits whose group a pending commit still shares) are undone by a new `save-undo? false` commit (`compensate-commits`, flushed like any edit); for a file whose data is not loaded the undo goes straight to the queue as an entry, nothing applied locally. Per file, `dch/rebase-commits` undoes every pending commit newest first, re-applies the remote changes kept since the oldest pending commit, and redoes the pending commits that stay. Undo changes hold absolute values from commit time, so undoing only the rejected commits would overwrite a peer's later change to the same attr. Remote changes are numbered (`record-remote-changes`, `[:persistence :remote-seq]`; it also keeps the highest received revn per file in `:remote-revn`) and kept per file under `:remote-changes` only while the file has pending commits; `prune-remote-changes` (discard, release) forgets those at or below the oldest pending stamp, since re-applying them would undo saved edits. Then `dwu/forget-undo-groups groups undo-index` (cut history before the first entry whose `:undo-groups` — every group the entry holds, filled by `append-undo`, transactions and stacking — meets a reverted group; no match keeps the history; the open transaction is emptied only when cut or holding one, keeping its group for later commits; a reverted Ctrl+Z/Ctrl+Shift+Z commit, which carries real `:undo-changes` and `:undo-index` (the index it moved from, kept by `pcm` commit-keys), puts the index back to the plan's `:undo-index`, the oldest one's; background commits with `:undo-changes []` (position-data, component `modified-at`, deleted-font fixes) stay on screen after a rollback by design), selection pruned and edition mode left when the edited shape is gone, or when it is a path being edited that the rollback changed: `discard-path-edit` drops `[:workspace-local :edit-path id]` first, so `finalize-path-content` sees no `:old-content` and the editor's stale copy is never saved (`prune-removed-shapes` takes the edited object from before the rebase to compare). Text editions stay open: the v3 WASM editor already shows the reverted text, and the v2 editor resets its DOM root from the shape on `::rolled-back` when the text differs (and, under WASM, emits `v2-sync-wasm-text-layout` so the canvas and size follow), since its unmount `on-blur` would otherwise save the stale DOM content (`prune-removed-shapes`; `v2-update-text-shape-content` also skips a missing shape, so the editor's exit commits nothing), `errors/flash` `:unhandled` toast, text/tag/timeout from `rejection-notice`: a file other than `:current-file-id` is named (`errors.save-rejected-changes-undone-in-file`, tag `:persistence-rejected-other-file`; generic text when `:files` has no name); a `:preexisting-damage` answer says the page data is damaged (`errors.save-rejected-damaged-data`); otherwise `errors.save-rejected-changes-undone` (says the latest changes were undone: edits made while the save was in flight go too), tag `dps/rejected-tag`, hidden after `rejected-timeout-ms` (10 s). Single-toast store: the backend repair notice (`dps/notify-repaired`, emitted by `handle-file-change` on `:repaired`, sticky, tag `dps/file-repaired-tag`) and a rejection notice of the open file fold into one sticky warning (`workspace.notifications.file-repaired-changes-undone`, tag `dps/repaired-rejected-tag`) whichever comes second, keeping the rejection's report link, so neither is lost. It ends with `(ptk/data-event ::rolled-back (pcm/rollback-targets plan))` (`:file-ids :page-ids :shape-ids :comment-thread-ids`), emitted by `notify-rolled-back` once the reverted data is in the state: when a rebase failed (its deref has `:failed true`) it waits for the reload's `::dw/bundle-fetched` (`::dw/all-libraries-resolved` when a library failed) and is dropped on `finalize-workspace`; the reload replaces the whole file, so shapes outside the listed ids may change too. Comment positions are saved by their own RPC, not by `update-file`, so `dwcm/watch-comment-thread-position-changes` reacts to it: threads of reverted `:set-comment-thread-position` changes get the restored page-data position in `:comment-threads` and on the server (`update-comment-thread-position`), and threads left on a frame the rollback removed move to `uuid/zero` (`update-comment-thread-frame`, as `detach-comment-thread` on delete). `dwl/sync-file` also waits for it or `wait-persisted` and marks the library synced only on a landed save (on rollback it restores `:synced-at` and re-checks updates after the rollback's own save; on a failed save it restores `:synced-at` and does the same re-check on the file's next `::rolled-back`, i.e. the role-change release, until `finalize-workspace`). `update-status` ignores `:pending` when buffer and queue are empty (`nothing-to-save?`): a dropped commit's 200 ms pending debounce would otherwise leave the status stuck and `wait-persisted-or-error` waiting forever. `prune-removed-shapes` also closes a `:colorpicker` modal. `rebase-commits` is not a commit (not saved/broadcast/undoable, commit watchers miss it) and falls back to `::dw/reload-current-file` if applying throws; once it ran, its deref gives `{:file-id :changes :failed}`. Thumbnails read that: `dwl/watch-component-changes` checks each rebase against the data before it (`ch/components-changed`) and re-renders and uploads the thumbnails of the mains it changed (`update-component-thumbnail`), and the SVG `dwth/watch-state-changes` takes rebases as commits (clears and re-renders frame thumbnails). The WASM `render-thumbnail :persist? true` skips the upload when the main's subtree changed during the render (that change brings its own render), so a render of rejected data that finishes after the rollback is not stored. `debug.commitInvalidChange({kind, before, after})` forces such a rejection live. - Persistence sends revn as the max of the commit revn and locally tracked latest revn; remote commits update that revn tracker. - Save answer `{:revn (pre-update) :lagged [rows revn > sent revn, created-at order, own row last] :rejected?}`. `settle-saved-commit` replaces a direct `commit-persisted`: for the current file, when the last `:lagged` row is this session's (`:session-id state`) and a row of another session (a peer, or the backend repair session, whose `:file-change` `:repaired` message also reaches the saver) has revn > `:remote-revn`, the head entry gets `::awaiting` (attempt-state `:in-flight`, no resend) and stays pending until a remote commit with that revn arrives, so the normal remote rebase puts that row under the saved commits, as the server ordered them; then `commit-persisted`. Bounded: `unseen-changes-timeout-ms` (10 s, `rx/timer`) acknowledges and emits `::dw/reload-current-file`; `finalize-workspace` acknowledges at once. Repair changes are never applied from the HTTP answer; ws messages per file arrive in publish order, so max received revn is the seen test. - A reload to another version (peer restore; `pending-file-changes` already skips other-vern edits): `bundle-fetched` calls `dps/forget-other-versions` → `pcm/without-other-versions`: other-vern buffered and `:saved` commits dropped, other-vern queue entries kept but emptied (`:commits []`), remote changes pruned, a `:pending` status with nothing left turns `:saved`. So remote rebases and rollbacks never replay them on the reloaded data. `stale-commit?` (entry/commit `:file-vern` ≠ loaded file `:vern`, never when the file has no `:vern`): attempt-state `:stale` → acknowledged unsent; `rollback-rejected` of a stale entry does nothing (no rebase, toast or `::rolled-back`); `save-attempt-failed` drops a stale commit's failure (e.g. `:vern-conflict`) instead of halting. `merge-commit` groups the buffer by file and vern (one save carries one vern). diff --git a/frontend/src/app/main/data/persistence/commits.cljs b/frontend/src/app/main/data/persistence/commits.cljs index 5ddd2af826..a1f5e3d253 100644 --- a/frontend/src/app/main/data/persistence/commits.cljs +++ b/frontend/src/app/main/data/persistence/commits.cljs @@ -200,14 +200,18 @@ (:shapes change)))) (defn rollback-targets - "The files of the commits `plan` reverts, and the pages and page shapes - their changes touch." + "The files of the commits `plan` reverts, and the pages, page shapes and + comment thread positions their changes touch." [{:keys [files compensate]}] (let [commits (into (vec compensate) (mapcat (comp :reverted val)) files) changes (into [] (mapcat :redo-changes) commits)] - {:file-ids (into (set (keys files)) (map :file-id) compensate) - :page-ids (into #{} (keep change-page-id) changes) - :shape-ids (into #{} (mapcat change-shape-ids) changes)})) + {:file-ids (into (set (keys files)) (map :file-id) compensate) + :page-ids (into #{} (keep change-page-id) changes) + :shape-ids (into #{} (mapcat change-shape-ids) changes) + :comment-thread-ids (into #{} + (comp (filter #(= :set-comment-thread-position (:type %))) + (map :comment-thread-id)) + changes)})) (defn- drop-commits "Removes the commits in `ids` from the queue and the buffer; an entry left diff --git a/frontend/src/app/main/data/workspace/comments.cljs b/frontend/src/app/main/data/workspace/comments.cljs index 79d804c50b..656fdaa651 100644 --- a/frontend/src/app/main/data/workspace/comments.cljs +++ b/frontend/src/app/main/data/workspace/comments.cljs @@ -20,6 +20,7 @@ [app.main.data.event :as ev] [app.main.data.helpers :as dsh] [app.main.data.notifications :as ntf] + [app.main.data.persistence :as-alias dps] [app.main.data.workspace.common :as dwco] [app.main.data.workspace.drawing :as dwd] [app.main.data.workspace.edition :as dwe] @@ -219,21 +220,62 @@ (rx/ignore)) (rx/empty))))) +(defn- restore-rolled-back-threads + "After a rollback, puts the threads in `comment-thread-ids` back where the + restored page data has them, in the rendered threads and the backend. + Threads left on a frame the rollback removed (in `shape-ids`) move to the + page root, as deleting the frame does." + [{:keys [comment-thread-ids shape-ids]}] + (ptk/reify ::restore-rolled-back-threads + ptk/WatchEvent + (watch [_ state _] + (let [threads (:comment-threads state) + + restored + (into {} + (keep (fn [id] + (when-let [{:keys [page-id]} (get threads id)] + (when-let [{:keys [position frame-id]} + (-> (dsh/lookup-page state page-id) + (dm/get-in [:comment-thread-positions id]))] + [id {:comment-thread-id id + :position position + :frame-id frame-id}])))) + comment-thread-ids) + + orphaned? + (fn [{:keys [id page-id] :as thread}] + (let [frame-id (or (dm/get-in restored [id :frame-id]) (:frame-id thread))] + (and (contains? shape-ids frame-id) + (nil? (get (dsh/lookup-page-objects state page-id) frame-id)))))] + + (rx/concat + (rx/from (map sync-comment-thread-position (vals restored))) + (->> (vals threads) + (filter orphaned?) + (map dcmt/update-comment-thread-frame) + (rx/from))))))) + (defn watch-comment-thread-position-changes - "Syncs rendered threads and the backend when an undo/redo changes a comment position." + "Syncs rendered threads and the backend when an undo/redo changes a comment + position, and when a rollback reverts a change of positions or frames." [stopper] (ptk/reify ::watch-comment-thread-position-changes ptk/WatchEvent (watch [_ _ stream] - (->> stream - (rx/filter dwc/commit?) - (rx/map deref) - (rx/filter #(contains? undo-origins (:origin %))) - (rx/mapcat (fn [commit] - (->> (:redo-changes commit) - (filter #(= :set-comment-thread-position (:type %))) - (rx/from)))) - (rx/map sync-comment-thread-position) + (->> (rx/merge + (->> stream + (rx/filter dwc/commit?) + (rx/map deref) + (rx/filter #(contains? undo-origins (:origin %))) + (rx/mapcat (fn [commit] + (->> (:redo-changes commit) + (filter #(= :set-comment-thread-position (:type %))) + (rx/from)))) + (rx/map sync-comment-thread-position)) + (->> stream + (rx/filter (ptk/type? ::dps/rolled-back)) + (rx/map (comp restore-rolled-back-threads deref)))) (rx/take-until stopper))))) (defn frame-pin-transform diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 5a6758fc92..6081572ea6 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1300,16 +1300,19 @@ (wrf/with-pending :sync-file pending-ids frontend-sync) (when (not= file-id library-id) - (let [stopper-s (rx/filter (ptk/type? ::dw/finalize-workspace) stream) - restore (if (= ::unchanged @synced-at) - (rx/empty) - (rx/of (restore-library-synced-at library-id @synced-at)))] + (let [stopper-s (rx/filter (ptk/type? ::dw/finalize-workspace) stream) + rolled-back-s (->> stream + (rx/filter (ptk/type? ::dps/rolled-back)) + (rx/filter #(contains? (:file-ids (deref %)) file-id))) + restore (if (= ::unchanged @synced-at) + (rx/empty) + (rx/of (restore-library-synced-at library-id @synced-at))) + ;; Checked once the rollback has undone the sync locally + notify #(->> (dps/wait-persisted) + (rx/map (fn [_] (notify-sync-file))))] (->> (rx/merge ;; The server rejected the sync: its commits were undone - (->> stream - (rx/filter (ptk/type? ::dps/rolled-back)) - (rx/filter #(contains? (:file-ids (deref %)) file-id)) - (rx/map (constantly ::rolled-back))) + (rx/map (constantly ::rolled-back) rolled-back-s) ;; The flush of the sync commits starts within 3 s; a save ;; that fails for good ends the wait with no value (->> (rx/timer 3000) @@ -1324,15 +1327,17 @@ (mark-library-synced file-id library-id) ;; Never saved: the update notice can show again + ;; once the release of the failed save undoes the sync ::failed - restore - - ::rolled-back (rx/concat restore - ;; Checked once the rollback has undone the sync locally - (->> (dps/wait-persisted) - (rx/map (fn [_] (notify-sync-file))))))))))))))))))) + (->> rolled-back-s + (rx/take 1) + (rx/take-until stopper-s) + (rx/mapcat (fn [_] (notify))))) + + ::rolled-back + (rx/concat restore (notify)))))))))))))))) ;; FIXME: the data should be set on the backend for clock consistency @@ -1598,7 +1603,31 @@ notifier-s (->> component-events-s (rx/debounce 5000) - (rx/tap #(log/trc :hint "buffer initialized")))] + (rx/tap #(log/trc :hint "buffer initialized"))) + + ;; A rollback is a rebase, not a commit: no component change + ;; is detected, so the thumbnails of the mains it reverted are + ;; rendered and uploaded here, each rebase checked against the + ;; file data right before it + rolled-back-thumbnails-s + (->> stream + (rx/filter (ptk/type? ::dch/rebase-commits)) + (rx/map deref) + (rx/filter (complement :failed)) + ;; Its own data pairing: `workspace-data-s` is shared and + ;; does not replay the current data to a late subscriber + (rx/with-latest-from + (->> (rx/from-atom refs/workspace-data {:emit-current-value? true}) + (rx/concat (rx/of nil)) + (rx/buffer 2 1) + (rx/map first))) + (rx/mapcat + (fn [[{:keys [file-id changes]} old-data]] + (if (= file-id (:id old-data)) + (->> (into #{} (mapcat (partial ch/components-changed old-data)) changes) + (rx/from) + (rx/map #(update-component-thumbnail % file-id))) + (rx/empty)))))] (when (or (contains? cf/flags :component-thumbnails) (features/active-feature? state "render-wasm/v1")) @@ -1630,7 +1659,9 @@ (->> component-events-s (rx/filter (ptk/type? ::touch-component)) (rx/map deref) - (rx/map render-component-thumbnail-event))) + (rx/map render-component-thumbnail-event)) + + rolled-back-thumbnails-s) (rx/take-until stopper-s))))))) diff --git a/frontend/src/app/main/data/workspace/selection.cljs b/frontend/src/app/main/data/workspace/selection.cljs index aaab6e6802..ae0831f5f6 100644 --- a/frontend/src/app/main/data/workspace/selection.cljs +++ b/frontend/src/app/main/data/workspace/selection.cljs @@ -580,21 +580,57 @@ (let [hover-value (if value #{id} #{})] (assoc-in state [:workspace-local :hover] hover-value))))) +(defn- leave-focus-mode + "Leaves focus mode, bringing back the viewport and the selection from + before it. Shapes removed since then are left out of the selection." + [state] + (let [objects (dsh/lookup-page-objects state) + local (:workspace-pre-focus state) + selected (:selected local)] + (-> state + (assoc :workspace-local + (cond-> local + (some? selected) + (assoc :selected (into (empty selected) (filter #(contains? objects %)) selected)))) + (dissoc :workspace-focus-selected) + (dissoc :workspace-pre-focus)))) + (defn update-focus-shapes + "Adds the `added` shapes to the focus set and takes the `removed` ones + out. Focus mode ends when no focused shape is left, so a later shape + never brings it back on its own." [added removed] - (ptk/reify ::update-focus-shapes - ptk/UpdateEvent - (update [_ state] + (let [left? (volatile! false)] + (ptk/reify ::update-focus-shapes + ptk/UpdateEvent + (update [_ state] + (let [objects (dsh/lookup-page-objects state) + current (:workspace-focus-selected state) + focus (-> current + (set/union added) + (set/difference removed)) + focus (cfh/clean-loops objects focus)] + (cond + (empty? current) + state - (let [objects (dsh/lookup-page-objects state) + (empty? focus) + (do (vreset! left? true) + (leave-focus-mode state)) - focus (-> (:workspace-focus-selected state) - (set/union added) - (set/difference removed)) - focus (cfh/clean-loops objects focus)] + :else + (assoc state :workspace-focus-selected focus)))) - (-> state - (assoc :workspace-focus-selected focus)))))) + ptk/WatchEvent + (watch [_ _ _] + (if @left? + (rx/of (ptk/data-event ::focus-mode-left)) + (rx/empty))) + + ptk/EffectEvent + (effect [_ state _] + (when @left? + (dwvw/maybe-sync-workspace-local-viewport! state)))))) (defn toggle-focus-mode "Zoom in on and center viewport on selection; @@ -615,10 +651,7 @@ (if in-mode? ;; Exit focus, restoring previous viewport, selection, etc - (-> state - (assoc :workspace-local (:workspace-pre-focus state)) - (dissoc :workspace-focus-selected) - (dissoc :workspace-pre-focus)) + (leave-focus-mode state) (if have-selection? ;; Enter focus and save viewport, selection, etc (-> state @@ -633,6 +666,7 @@ ptk/WatchEvent (watch [_ state stream] (let [stopper (rx/filter #(or (= ::toggle-focus-mode (ptk/type %)) + (= ::focus-mode-left (ptk/type %)) (= ::dwpg/finalize-page (ptk/type %))) stream)] (when (d/not-empty? (:workspace-focus-selected state)) (->> (rx/merge diff --git a/frontend/src/app/main/data/workspace/thumbnails.cljs b/frontend/src/app/main/data/workspace/thumbnails.cljs index b83d09842a..baf65f0ccc 100644 --- a/frontend/src/app/main/data/workspace/thumbnails.cljs +++ b/frontend/src/app/main/data/workspace/thumbnails.cljs @@ -361,10 +361,12 @@ (rx/buffer 2 1) (rx/share)) - ;; All commits stream, indepentendly of the source of the commit + ;; All commits stream, indepentendly of the source of the commit. + ;; A rollback's rebase is not a commit but changes the data too. all-commits-s (->> stream - (rx/filter dch/commit?) + (rx/filter #(or (dch/commit? %) + (ptk/type? ::dch/rebase-commits %))) (rx/map deref) (rx/observe-on :async) (rx/with-latest-from workspace-data-s) diff --git a/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs b/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs index 4bcfe857f6..b5b78b92c5 100644 --- a/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs +++ b/frontend/src/app/main/data/workspace/thumbnails_wasm.cljs @@ -122,16 +122,25 @@ (catch :default err (rx/error! subs err))))))) + (get-subtree + [state] + (-> (dsh/lookup-page-objects state file-id page-id) + (cfh/get-children-with-self frame-id))) + (persist-to-server [data-uri] - (let [blob (wapi/data-uri->blob data-uri)] - (->> (rp/cmd! :create-file-object-thumbnail - {:file-id file-id - :object-id object-id - :media blob - :tag "component"}) - (rx/catch rx/empty) - (rx/ignore)))) + ;; A change during the render (a rollback, an edit) makes + ;; it stale; that change brings its own render + (if (= (get-subtree state) (get-subtree @st/state)) + (let [blob (wapi/data-uri->blob data-uri)] + (->> (rp/cmd! :create-file-object-thumbnail + {:file-id file-id + :object-id object-id + :media blob + :tag "component"}) + (rx/catch rx/empty) + (rx/ignore))) + (rx/empty))) (do-render-thumbnail [] diff --git a/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs b/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs index 04c7d0c83d..74e702c681 100644 --- a/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs +++ b/frontend/src/app/main/ui/workspace/shapes/text/v2_editor.cljs @@ -192,6 +192,8 @@ ;; content in the editor, which would otherwise keep showing and ;; saving the rejected one. Saves failing do the same with the last ;; committed content, since the typing not yet committed is dropped. + ;; The WASM canvas draws the editor's layout, so it is laid out + ;; again from that content. rolled-back-sub (->> st/stream (rx/filter #(or (ptk/type? ::dps/rolled-back %) @@ -203,7 +205,9 @@ current (content/dom->cljs (dwt/get-editor-root instance))] (when (and (some? content) (not (dwt/same-styled-text? content current))) - (dwt/set-editor-root! instance (content/cljs->dom content)))))))] + (dwt/set-editor-root! instance (content/cljs->dom content)) + (when (features/active-feature? @st/state "render-wasm/v1") + (st/emit! (dwt/v2-sync-wasm-text-layout shape-id content))))))))] (.addEventListener ^js global/document "keyup" on-key-up) (.addEventListener ^js instance "focus" on-focus) diff --git a/frontend/test/frontend_tests/data/persistence_commits_test.cljs b/frontend/test/frontend_tests/data/persistence_commits_test.cljs index 2b6cde7258..2a85f3e4f3 100644 --- a/frontend/test/frontend_tests/data/persistence_commits_test.cljs +++ b/frontend/test/frontend_tests/data/persistence_commits_test.cljs @@ -160,13 +160,16 @@ "the rest of the undo group goes too"))) (t/deftest rollback-targets-name-the-files-pages-and-page-shapes-of-the-reverted-changes - (let [[f1 f2 p1 p2 p3 s1 s2 s3 s4 s5] (repeatedly 10 uuid/next) + (let [[f1 f2 p1 p2 p3 s1 s2 s3 s4 s5 c1] (repeatedly 11 uuid/next) plan {:files {f1 {:reverted [{:redo-changes [{:type :add-obj :page-id p1 :id s1} {:type :mov-objects :page-id p1 :shapes [s2 s3]} {:type :mod-obj :component-id (uuid/next) :id s4} - {:type :mod-page :id p2}]}]}} + {:type :mod-page :id p2} + {:type :set-comment-thread-position :page-id p1 + :comment-thread-id c1}]}]}} :compensate [{:file-id f2 :redo-changes [{:type :del-obj :page-id p3 :id s5}]}]}] - (t/is (= {:file-ids #{f1 f2} - :page-ids #{p1 p2 p3} - :shape-ids #{s1 s2 s3 s5}} + (t/is (= {:file-ids #{f1 f2} + :page-ids #{p1 p2 p3} + :shape-ids #{s1 s2 s3 s5} + :comment-thread-ids #{c1}} (pcm/rollback-targets plan))))) diff --git a/frontend/test/frontend_tests/data/persistence_rollback_test.cljs b/frontend/test/frontend_tests/data/persistence_rollback_test.cljs index 730bcc5b33..3efff3cb85 100644 --- a/frontend/test/frontend_tests/data/persistence_rollback_test.cljs +++ b/frontend/test/frontend_tests/data/persistence_rollback_test.cljs @@ -10,12 +10,15 @@ changes survive, and how the UI state follows. The pure plan behind it is tested in `frontend-tests.data.persistence-commits-test`." (:require + [app.common.geom.point :as gpt] + [app.common.time :as ct] [app.common.types.shape :as cts] [app.common.uuid :as uuid] [app.main.data.changes :as dch] [app.main.data.modal :as-alias modal] [app.main.data.persistence :as dps] [app.main.data.workspace :as dw] + [app.main.data.workspace.comments :as dwcm] [app.main.data.workspace.undo :as dwu] [app.main.errors :as errors] [app.util.i18n :as i18n] @@ -692,6 +695,85 @@ (t/is (nil? (get-in @store [:workspace-global :persistence-lock]))) (t/is (= "Restored" (thp/shape-name store file-id page-id shape-id)))))))) +(defn- comment-thread + "A comment thread of `file-id` on `page-id`, pinned on `frame-id`." + [file-id page-id frame-id position] + {:id (uuid/next) :page-id page-id :file-id file-id :project-id (uuid/next) + :owner-id (uuid/next) :file-name "File" :seqn 1 :content "Note" + :participants #{} :created-at (ct/now) :modified-at (ct/now) + :position position :frame-id frame-id}) + +;; Scenario: comment #1 sits on board Y. During a save Y is moved, which +;; moves pin #1 in the same action, and board R is drawn and gets comment +;; #2. The server rejects the move, so the move, the pin move and R are +;; rolled back. Comment positions are saved by their own requests, so the +;; server already holds the moved pin and #2 on R. Proves: pin #1 goes back +;; to its spot on screen and on the server, and #2, whose board is gone, +;; moves to the page root, as deleting the board does. +(t/deftest ^:async a-rollback-puts-comment-pins-back-and-detaches-them-from-removed-boards + (let [answer (rx/subject) + respond (fn [cmd _] + (if (= :update-file cmd) + (->> answer (rx/take 1) (rx/observe-on :async)) + (rx/of nil)))] + (await + (thp/with-persistence + (^:async fn [{:keys [file-id requests store]}] + (let [page-id (uuid/next) + shape-id (uuid/next) + board-y (cts/setup-shape {:id (uuid/next) :type :frame :name "Y" + :parent-id uuid/zero :frame-id uuid/zero}) + board-r (cts/setup-shape {:id (uuid/next) :type :frame :name "R" + :parent-id uuid/zero :frame-id uuid/zero}) + [p0 p1 p2] [(gpt/point 610 10) (gpt/point 760 10) (gpt/point 20 20)] + thread-1 (comment-thread file-id page-id (:id board-y) p1) + thread-2 (comment-thread file-id page-id (:id board-r) p2) + group (uuid/next) + move (grouped (thp/rename-commit file-id page-id (:id board-y) "Y" "Y moved") group) + pin (grouped (ptk/data-event + ::dch/commit + {:id (uuid/next) :file-id file-id :file-revn 0 :file-vern 0 + :source :local :features #{} + :redo-changes [{:type :set-comment-thread-position :page-id page-id + :comment-thread-id (:id thread-1) + :frame-id (:id board-y) :position p1}] + :undo-changes [{:type :set-comment-thread-position :page-id page-id + :comment-thread-id (:id thread-1) + :frame-id (:id board-y) :position p0}]}) + group) + draw (add-shape-commit file-id page-id board-r) + data (-> (thp/file-data page-id shape-id "A") + (thp/add-shape page-id (assoc board-y :name "Y moved")) + (thp/add-shape page-id board-r) + (assoc-in [:pages-index page-id :comment-thread-positions] + {(:id thread-1) {:frame-id (:id board-y) :position p1} + (:id thread-2) {:frame-id (:id board-r) :position p2}})) + sent (fn [cmd] (into [] (comp (filter #(= cmd (first %))) (map second)) @requests))] + ;; The local data and the rendered pins already hold the edits + (ptk/emit! store + (thp/open-file file-id page-id data) + #(assoc % :comment-threads {(:id thread-1) thread-1 (:id thread-2) thread-2}) + (dwcm/watch-comment-thread-position-changes (rx/subject))) + (ptk/emit! store move pin draw ::dps/force-persist) + (await (async/wait-for #(seq (sent :update-file)) "the edits are sent")) + (rx/push! answer (thp/rejection (:id @move))) + (await (async/wait-for #(seq (sent :update-comment-thread-frame)) + "the rollback updates the comments")) + (t/is (nil? (thp/shape-attr store file-id page-id (:id board-r) :id)) "R is gone") + (t/is (= p0 (get-in @store [:comment-threads (:id thread-1) :position])) + "pin #1 is back on screen") + (t/is (= [{:id (:id thread-1) :position p0 :frame-id (:id board-y)}] + (sent :update-comment-thread-position)) + "and on the server") + (t/is (= uuid/zero (get-in @store [:comment-threads (:id thread-2) :frame-id])) + "comment #2 leaves the removed board") + (t/is (= p2 (get-in @store [:comment-threads (:id thread-2) :position])) + "and keeps its spot") + (t/is (= [{:id (:id thread-2) :frame-id uuid/zero}] + (sent :update-comment-thread-frame))) + (rx/end! answer))) + respond)))) + (defn- rejection-notice "Saves a rename of the open file and answers it with `answer` applied to diff --git a/frontend/test/frontend_tests/data/workspace_focus_test.cljs b/frontend/test/frontend_tests/data/workspace_focus_test.cljs new file mode 100644 index 0000000000..ee1d3435c3 --- /dev/null +++ b/frontend/test/frontend_tests/data/workspace_focus_test.cljs @@ -0,0 +1,87 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns frontend-tests.data.workspace-focus-test + "Tests for focus mode when the focused shapes go away." + (:require + [app.common.data :as d] + [app.common.types.shape :as cts] + [app.common.uuid :as uuid] + [app.main.data.workspace.selection :as dws] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [potok.v2.core :as ptk])) + +(defn- focus-store + "A store on a page holding `ids`, in focus mode on `focused`, entered + with `selected` selected." + [ids focused selected] + (let [file-id (uuid/next) + page-id (uuid/next) + objects (into {uuid/zero (cts/setup-shape {:id uuid/zero :type :frame :shapes (vec ids)})} + (map (fn [id] + [id (cts/setup-shape {:id id :type :rect :name "Rect" + :parent-id uuid/zero :frame-id uuid/zero})])) + ids) + local {:selected (into (d/ordered-set) selected) :zoom 1}] + (ptk/store {:state {:current-file-id file-id + :current-page-id page-id + :files {file-id {:id file-id + :data {:pages [page-id] + :pages-index {page-id {:id page-id :objects objects}}}}} + :workspace-local (assoc local :selected (d/ordered-set)) + :workspace-pre-focus local + :workspace-focus-selected (set focused)} + :on-error #(t/is false (str %))}))) + +(defn- remove-shape + "Takes `id` out of the page of `store`, as a delete or a rollback does." + [store id] + (let [{:keys [current-file-id current-page-id]} @store] + (ptk/emit! store #(update-in % [:files current-file-id :data :pages-index current-page-id :objects] + dissoc id)))) + +;; Scenario: focus mode is on R, entered with R selected, and R goes away +;; (a rollback or a delete); later a new shape is drawn. Proves: focus mode +;; ends when its last shape goes, the selection from before it comes back +;; without R, and the new shape does not bring focus mode back. +(t/deftest ^:async focus-mode-ends-when-its-last-shape-is-removed + (let [[r s n] (repeatedly 3 uuid/next) + store (focus-store [r s] [r] [r]) + left (atom 0) + sub (->> (ptk/input-stream store) + (rx/filter (ptk/type? ::dws/focus-mode-left)) + (rx/subs! (fn [_] (swap! left inc))))] + (remove-shape store r) + (ptk/emit! store (dws/update-focus-shapes #{} #{r})) + (await (async/settle)) + (t/is (nil? (:workspace-focus-selected @store)) "focus mode is off") + (t/is (nil? (:workspace-pre-focus @store))) + (t/is (empty? (get-in @store [:workspace-local :selected])) + "the removed shape is not selected again") + (t/is (= 1 (:zoom (:workspace-local @store))) "the viewport from before comes back") + (ptk/emit! store (dws/update-focus-shapes #{n} #{})) + (await (async/settle)) + (t/is (nil? (:workspace-focus-selected @store)) "a new shape does not turn it on") + (t/is (= 1 @left) "the focus watcher is told to stop once") + (rx/dispose! sub))) + +;; Scenario: focus mode is on R and S, entered with both selected; R goes +;; away and the user leaves focus mode with F. Proves: focus mode stays on +;; while a focused shape is left, and leaving it selects only the shapes +;; that still exist. +(t/deftest ^:async leaving-focus-mode-selects-only-shapes-that-exist + (let [[r s] (repeatedly 2 uuid/next) + store (focus-store [r s] [r s] [r s])] + (remove-shape store r) + (ptk/emit! store (dws/update-focus-shapes #{} #{r})) + (await (async/settle)) + (t/is (= #{s} (:workspace-focus-selected @store))) + (ptk/emit! store (dws/toggle-focus-mode)) + (await (async/settle)) + (t/is (nil? (:workspace-focus-selected @store))) + (t/is (= [s] (vec (get-in @store [:workspace-local :selected])))))) diff --git a/frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs b/frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs index daa5e38955..1990099695 100644 --- a/frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs @@ -95,7 +95,7 @@ ;; Scenario: the save of the sync's commits fails for good, so waiting for ;; it ends with no value; later an edit of the same file is rejected. ;; Proves: the library is not marked as synced, its sync date goes back, -;; and the sync stops listening, so the later rollback leaves the date alone. +;; and the later rollback leaves the date alone. (t/deftest ^:async a-failed-sync-save-ends-the-sync (let [saved (rx/subject)] (await @@ -112,3 +112,35 @@ (await (async/settle)) (t/is (= later (get-in @store [:files library-id :synced-at])) "a later rollback does not wake the ended sync"))))))) + +;; Scenario: the user becomes a viewer right after "Update", so the save of +;; the sync's commits fails for good; when edit rights come back, the +;; release of the failed save undoes the sync. Proves: the update notice is +;; checked again, as after a rejected sync, instead of waiting for a reload. +(t/deftest ^:async a-released-sync-checks-the-update-notice-again + (let [file-id (uuid/next) + library-id (uuid/next) + store (library-store file-id library-id) + waits (atom 0) + notified (atom 0)] + (await + (mock/with-mocks* + {rx/timer (mock/stub (fn [& _] (rx/of 0))) + ;; The first wait ends with the failed save, the next ones saved + dps/wait-persisted (mock/stub (fn [& _] + (if (= 1 (swap! waits inc)) + (rx/empty) + (rx/of :saved)))) + dwl/notify-sync-file (mock/stub (fn [] + (swap! notified inc) + (ptk/data-event ::notified {}))) + rp/cmd! (mock/stub (fn [_ _] (rx/of nil)))} + (ptk/emit! store (dwl/sync-file file-id library-id)) + (await (async/wait-for #(= 1 @waits) "the save of the sync fails")) + (await (async/settle)) + (t/is (= synced-at (get-in @store [:files library-id :synced-at]))) + (t/is (zero? @notified) "a failed save alone shows no notice") + + ;; The role change that brings edit rights back releases the save + (ptk/emit! store (ptk/data-event ::dps/rolled-back {:file-ids #{file-id}})) + (await (async/wait-for #(= 1 @notified) "the release checks the update notice")))))) diff --git a/frontend/test/frontend_tests/logic/comp_watcher_test.cljs b/frontend/test/frontend_tests/logic/comp_watcher_test.cljs index f8d3bc8783..b9767480ba 100644 --- a/frontend/test/frontend_tests/logic/comp_watcher_test.cljs +++ b/frontend/test/frontend_tests/logic/comp_watcher_test.cljs @@ -11,6 +11,7 @@ [app.common.test-helpers.files :as cthf] [app.common.test-helpers.ids-map :as cthi] [app.common.test-helpers.shapes :as cths] + [app.main.data.changes :as dch] [app.main.data.helpers :as dsh] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.pages :as dwpg] @@ -98,3 +99,34 @@ (ptk/emit! store (dwpg/finalize-page (:id file) (cthf/current-page-id file))) (await (async/settle)))))) + +;; Scenario: a rollback reverts the opacity of the main's child. A rollback +;; is not a commit, so the change watch sees no component change. +;; Proves: the component's thumbnail is rendered again and uploaded, so +;; neither the tab nor the server keeps a thumbnail of the rejected data. +(t/deftest ^:async a-rollback-of-a-main-renders-and-uploads-its-thumbnail + (let [file (setup-file) + store (ths/setup-store file) + page-id (cthf/current-page-id file) + renders (atom [])] + (await + (mock/with-mocks* + {refs/workspace-data (l/derived dsh/lookup-file-data store) + st/state store + dwt.wasm/render-thumbnail (fn [_ _ frame-id & {:as opts}] + (swap! renders conj [frame-id opts]) + (noop-render-thumbnail))} + + (ptk/emit! store (dwl/watch-component-changes)) + (ptk/emit! store (dch/rebase-commits + (:id file) + {:undo [] + :remote [{:type :mod-obj :id (cthi/id :main-child) :page-id page-id + :operations [{:type :set :attr :opacity :val 0.5}]}] + :redo []})) + + (await (async/wait-for #(seq @renders) "the thumbnail to render")) + (t/is (= [[(cthi/id :main-root) {:persist? true}]] @renders)) + + (ptk/emit! store (dwpg/finalize-page (:id file) page-id)) + (await (async/settle)))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index e7f4ae6778..63e2a6cfa1 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -34,6 +34,7 @@ [frontend-tests.data.workspace-colors-test] [frontend-tests.data.workspace-comments-test] [frontend-tests.data.workspace-context-menu-test] + [frontend-tests.data.workspace-focus-test] [frontend-tests.data.workspace-interactions-test] [frontend-tests.data.workspace-libraries-sync-test] [frontend-tests.data.workspace-mcp-test] @@ -226,6 +227,7 @@ 'frontend-tests.data.workspace-colors-test 'frontend-tests.data.workspace-comments-test 'frontend-tests.data.workspace-context-menu-test + 'frontend-tests.data.workspace-focus-test 'frontend-tests.data.workspace-interactions-test 'frontend-tests.data.workspace-libraries-sync-test 'frontend-tests.data.workspace-mcp-test