From e7b4010eba304d2655721877c6cab51a4dbd9998 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 24 Nov 2021 12:37:55 +0100 Subject: [PATCH] :bug: Fix undo on page relocate/sorting. --- .../src/app/common/pages/changes_builder.cljc | 11 +++- common/src/app/common/pages/helpers.cljc | 15 ++++-- .../test/app/common/pages_helpers_test.cljc | 50 +++++++++++++++++++ common/test/app/common/pages_test.cljc | 2 +- frontend/src/app/main/data/users.cljs | 2 - frontend/src/app/main/data/workspace.cljs | 25 ++++------ .../src/app/main/data/workspace/changes.cljs | 9 ++-- .../src/app/main/data/workspace/undo.cljs | 3 +- .../main/ui/workspace/sidebar/sitemap.cljs | 5 +- 9 files changed, 89 insertions(+), 33 deletions(-) create mode 100644 common/test/app/common/pages_helpers_test.cljc diff --git a/common/src/app/common/pages/changes_builder.cljc b/common/src/app/common/pages/changes_builder.cljc index 45fcebd23..741afb62e 100644 --- a/common/src/app/common/pages/changes_builder.cljc +++ b/common/src/app/common/pages/changes_builder.cljc @@ -12,7 +12,8 @@ ;; Auxiliary functions to help create a set of changes (undo + redo) -(defn empty-changes [origin page-id] +(defn empty-changes + [origin page-id] (let [changes {:redo-changes [] :undo-changes [] :origin origin}] @@ -165,3 +166,11 @@ (update :undo-changes #(as-> % $ (reduce add-undo-change-parent $ ids) (reduce add-undo-change-shape $ ids)))))) + + +(defn move-page + [chdata index prev-index] + (let [page-id (::page-id (meta chdata))] + (-> chdata + (update :redo-changes conj {:type :mov-page :id page-id :index index}) + (update :undo-changes conj {:type :mov-page :id page-id :index prev-index})))) diff --git a/common/src/app/common/pages/helpers.cljc b/common/src/app/common/pages/helpers.cljc index 909908820..b27dc1c49 100644 --- a/common/src/app/common/pages/helpers.cljc +++ b/common/src/app/common/pages/helpers.cljc @@ -213,11 +213,16 @@ (defn insert-at-index [objects index ids] (let [[before after] (split-at index objects) - p? (set ids)] - (d/concat [] - (remove p? before) - ids - (remove p? after)))) + p? (complement (set ids)) + before' (filterv p? before) + after' (filterv p? after)] + + (if (and (not= (count before) (count before')) + (pos? (count after'))) + (let [before' (conj before' (first after')) + after' (into [] (rest after'))] + (d/concat [] before' ids after')) + (d/concat [] before' ids after')))) (defn append-at-the-end [prev-ids ids] diff --git a/common/test/app/common/pages_helpers_test.cljc b/common/test/app/common/pages_helpers_test.cljc new file mode 100644 index 000000000..f7790af62 --- /dev/null +++ b/common/test/app/common/pages_helpers_test.cljc @@ -0,0 +1,50 @@ +;; 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) UXBOX Labs SL + +(ns app.common.pages-helpers-test + (:require + [clojure.test :as t] + [clojure.pprint :refer [pprint]] + [app.common.pages.helpers :as cph])) + +(t/deftest insert-at-index + ;; insert different object + (t/is (= (cph/insert-at-index [:a :b] 1 [:c :d]) + [:a :c :d :b])) + + ;; insert on the start + (t/is (= (cph/insert-at-index [:a :b] 0 [:c]) + [:c :a :b])) + + ;; insert on the end 1 + (t/is (= (cph/insert-at-index [:a :b] 2 [:c]) + [:a :b :c])) + + ;; insert on the end with not existing index + (t/is (= (cph/insert-at-index [:a :b] 10 [:c]) + [:a :b :c])) + + ;; insert existing in a contiguos index + (t/is (= (cph/insert-at-index [:a :b] 1 [:a]) + [:b :a])) + + ;; insert existing in the same index + (t/is (= (cph/insert-at-index [:a :b] 0 [:a]) + [:a :b])) + + ;; insert existing in other index case 1 + (t/is (= (cph/insert-at-index [:a :b :c] 2 [:a]) + [:b :c :a])) + + ;; insert existing in other index case 2 + (t/is (= (cph/insert-at-index [:a :b :c :d] 0 [:d]) + [:d :a :b :c])) + + ;; insert existing in other index case 3 + (t/is (= (cph/insert-at-index [:a :b :c :d] 1 [:a]) + [:b :a :c :d])) + + ) diff --git a/common/test/app/common/pages_test.cljc b/common/test/app/common/pages_test.cljc index abfa5f8f3..464e72bbd 100644 --- a/common/test/app/common/pages_test.cljc +++ b/common/test/app/common/pages_test.cljc @@ -574,7 +574,7 @@ ;; After - (t/is (= [shape-4-id shape-3-id group-1-id] + (t/is (= [shape-4-id group-1-id shape-3-id] (get-in res [:pages-index page-id :objects cp/root :shapes]))) ;; (pprint (get-in data [:pages-index page-id :objects cp/root])) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index 445d9aa19..9ce97da63 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -81,8 +81,6 @@ (when-not (contains? ids ctid) (swap! storage dissoc ::current-team-id))))))) - - (defn fetch-teams [] (ptk/reify ::fetch-teams diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 05d96e30d..8f606efc3 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -14,6 +14,7 @@ [app.common.geom.shapes :as gsh] [app.common.math :as mth] [app.common.pages :as cp] + [app.common.pages.changes-builder :as pcb] [app.common.pages.helpers :as cph] [app.common.pages.spec :as spec] [app.common.spec :as us] @@ -350,16 +351,16 @@ ;; TODO: properly handle positioning on undo. +;; TODO: for some reason, the page-id here in some circumstances is `nil` (defn delete-page [id] (ptk/reify ::delete-page ptk/WatchEvent (watch [it state _] (let [page (get-in state [:workspace-data :pages-index id]) - rchg {:type :del-page - :id id} - uchg {:type :add-page - :page page}] + rchg {:type :del-page :id id} + uchg {:type :add-page :page page}] + (rx/of (dch/commit-changes {:redo-changes [rchg] :undo-changes [uchg] :origin it}) @@ -1114,17 +1115,11 @@ (ptk/reify ::relocate-pages ptk/WatchEvent (watch [it state _] - (let [cidx (-> (get-in state [:workspace-data :pages]) - (d/index-of id)) - rchg {:type :mov-page - :id id - :index index} - uchg {:type :mov-page - :id id - :index cidx}] - (rx/of (dch/commit-changes {:redo-changes [rchg] - :undo-changes [uchg] - :origin it})))))) + (let [prev-index (-> (get-in state [:workspace-data :pages]) + (d/index-of id)) + changes (-> (pcb/empty-changes it id) + (pcb/move-page index prev-index))] + (rx/of (dch/commit-changes changes)))))) ;; --- Shape / Selection Alignment and Distribution diff --git a/frontend/src/app/main/data/workspace/changes.cljs b/frontend/src/app/main/data/workspace/changes.cljs index 99c050db6..66bd33d53 100644 --- a/frontend/src/app/main/data/workspace/changes.cljs +++ b/frontend/src/app/main/data/workspace/changes.cljs @@ -114,20 +114,20 @@ :changes changes})))) (defn commit-changes - [{:keys [redo-changes undo-changes origin save-undo? file-id] - :or {save-undo? true}}] - + [{:keys [redo-changes undo-changes origin save-undo? file-id] :or {save-undo? true}}] (log/debug :msg "commit-changes" :js/redo-changes redo-changes :js/undo-changes undo-changes) + (let [error (volatile! nil) + strace (.-stack (ex-info "" {}))] - (let [error (volatile! nil)] (ptk/reify ::commit-changes cljs.core/IDeref (-deref [_] {:file-id file-id :hint-events @st/last-events :hint-origin (ptk/type origin) + :hint-strace strace :changes redo-changes}) ptk/UpdateEvent @@ -135,7 +135,6 @@ (let [current-file-id (get state :current-file-id) file-id (or file-id current-file-id) path (if (= file-id current-file-id) - [:workspace-data] [:workspace-libraries file-id :data])] (try diff --git a/frontend/src/app/main/data/workspace/undo.cljs b/frontend/src/app/main/data/workspace/undo.cljs index dd2ded3ce..d25c22ea5 100644 --- a/frontend/src/app/main/data/workspace/undo.cljs +++ b/frontend/src/app/main/data/workspace/undo.cljs @@ -70,7 +70,8 @@ (accumulate-undo-entry state entry) (add-undo-entry state entry))))) -(defonce empty-tx {:undo-changes [] :redo-changes []}) +(def empty-tx + {:undo-changes [] :redo-changes []}) (defn start-undo-transaction [] (ptk/reify ::start-undo-transaction diff --git a/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs b/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs index 9acacb977..fc4e6c9ab 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/sitemap.cljs @@ -82,10 +82,9 @@ on-drop (mf/use-callback - (mf/deps id) + (mf/deps id index) (fn [side {:keys [id] :as data}] - (let [index (if (= :bot side) (inc index) index)] - (st/emit! (dw/relocate-page id index))))) + (st/emit! (dw/relocate-page id index)))) on-duplicate (fn [_]