diff --git a/src/main/frontend/components/block.cljs b/src/main/frontend/components/block.cljs index 522dd0df3f..4813c68ab4 100644 --- a/src/main/frontend/components/block.cljs +++ b/src/main/frontend/components/block.cljs @@ -3255,8 +3255,9 @@ (asset-cp config block)]}) (if show-editor? [:div.mt-1 editor-cp] - [:div.text-xs.opacity-60.mt-1.cursor-text - {:on-click #(edit-block-content config block edit-input-id)} + [:div + (assoc block-asset/read-mode-title-attrs + :on-click #(edit-block-content config block edit-input-id)) (text-block-title (dissoc config :raw-title?) block)])] show-editor? diff --git a/src/main/frontend/components/block/asset.cljs b/src/main/frontend/components/block/asset.cljs index ec7eeba142..97655f7d15 100644 --- a/src/main/frontend/components/block/asset.cljs +++ b/src/main/frontend/components/block/asset.cljs @@ -32,3 +32,7 @@ [asset-block ext] (cond-> (str (:block/title asset-block)) ext (str "." (name ext)))) + +(def read-mode-title-attrs + {:class "asset-title-slot text-xs opacity-60 mt-1 cursor-text" + :style {:min-height 24}}) diff --git a/src/main/frontend/handler/editor.cljs b/src/main/frontend/handler/editor.cljs index 9995bc6596..133b79bfd0 100644 --- a/src/main/frontend/handler/editor.cljs +++ b/src/main/frontend/handler/editor.cljs @@ -748,6 +748,11 @@ (or (ldb/page? block1) (ldb/page? block2)))) +(defn- editor-block-preserved-on-empty-title-merge? + [block] + (or (ldb/asset? block) + (comments-model/comments-area? block))) + (defn delete-block-inner! [repo {:keys [block-id value config block-container current-block next-block delete-concat?]}] (when (and block-id (not (one-page-another-block current-block next-block))) @@ -785,7 +790,10 @@ (db-model/hidden-page? (:block/page block))) ; embed page nil - (and concat-prev-block? input-empty? delete-concat?) + (and concat-prev-block? + input-empty? + (not (editor-block-preserved-on-empty-title-merge? prev-block)) + delete-concat?) (let [children (:block/_parent (db/entity (:db/id current-block)))] ; del (p/do! (ui-outliner-tx/transact! @@ -806,7 +814,10 @@ (delete-block-aux! current-block)) (edit-block! (db/entity (:db/id next-block)) 0))) - (and concat-prev-block? (string/blank? (:block/title prev-block)) (not delete-concat?)) ; backspace + (and concat-prev-block? + (string/blank? (:block/title prev-block)) + (not (editor-block-preserved-on-empty-title-merge? prev-block)) + (not delete-concat?)) ; backspace (p/do! (ui-outliner-tx/transact! transact-opts @@ -2677,6 +2688,7 @@ single-block? (if e (inside-of-single-block (.-target e)) false) root-block? (= (:block.temp/container block) (str (:block/uuid block)))] (when (and (not (and top-block? (not (string/blank? value)))) + (not (editor-block-preserved-on-empty-title-merge? block)) (not root-block?) (not single-block?) (not custom-query?)) diff --git a/src/test/frontend/components/block/asset_test.cljs b/src/test/frontend/components/block/asset_test.cljs index 975333236c..47182a9d5c 100644 --- a/src/test/frontend/components/block/asset_test.cljs +++ b/src/test/frontend/components/block/asset_test.cljs @@ -16,3 +16,9 @@ (block-asset/link-file-name {:block/title "test"} :pdf))))) + +(deftest read-mode-title-attrs-test + (testing "reserves the asset title row at the same height as a one-line editor" + (let [attrs block-asset/read-mode-title-attrs] + (is (= {:min-height 24} (:style attrs))) + (is (= "asset-title-slot text-xs opacity-60 mt-1 cursor-text" (:class attrs)))))) diff --git a/src/test/frontend/handler/editor_async_test.cljs b/src/test/frontend/handler/editor_async_test.cljs index 543ce7e096..7eeb67d833 100644 --- a/src/test/frontend/handler/editor_async_test.cljs +++ b/src/test/frontend/handler/editor_async_test.cljs @@ -41,21 +41,31 @@ :stopped? stopped?})) (defn- delete-block - [db block {:keys [embed? on-delete]}] + [db block {:keys [embed? on-delete edit-content on-edit schedule-immediately?]}] (let [sibling-block (ldb/get-left-sibling (d/entity db (:db/id block))) first-block (ldb/get-left-sibling sibling-block) block-dom-id "ls-block-block-to-delete" + sibling-dom-id "ls-block-sibling-block" + sibling-dom #js {:id sibling-dom-id + :getAttribute #({"blockid" (str (:block/uuid sibling-block)) + "data-embed" (if embed? "true" "false")} %)} + edit-content (or edit-content (:block/title block)) previous-repo (:git/current-repo @state/state)] (swap! state/state assoc :git/current-repo test-helper/test-db) (-> (p/with-redefs [editor/get-state (constantly {:block-id (:block/uuid block) :block-parent-id block-dom-id - :config {:embed? embed?}}) + :config {:embed? embed?} + :value edit-content}) ;; stub for delete-block gdom/getElement (constantly #js {:id block-dom-id}) ;; stub since not testing moving - editor/edit-block! (constantly nil) - state/get-edit-content (constantly "") + editor/edit-block! (fn [block pos opts] + (when (fn? on-edit) + (on-edit block pos opts)) + nil) + state/get-edit-content (constantly edit-content) + util/get-prev-block-non-collapsed-non-embed (constantly sibling-dom) ;; stub b/c of js/document state/get-selection-blocks (constantly []) util/get-blocks-noncollapse (constantly (mapv @@ -66,10 +76,14 @@ "data-embed" (if embed? "true" "false")} %)}) [{:id "ls-block-first-block" :block-uuid (:block/uuid first-block)} - {:id "ls-block-sibling-block" + {:id sibling-dom-id :block-uuid (:block/uuid sibling-block)} {:id block-dom-id - :block-uuid (:block/uuid block)}]))] + :block-uuid (:block/uuid block)}])) + util/schedule (fn [f] + (if schedule-immediately? + (f) + (js/setTimeout f 0)))] (p/do! (editor/delete-block! test-helper/test-db) (when (fn? on-delete) @@ -137,8 +151,189 @@ [?b :block/title ""]] @conn) (map first))] - (is (= ["b1" "b2"] updated-blocks) "Visible page blocks stay on the page") - (is (empty? deleted-blocks) "Deleted block is removed from page db")))})))) + (is (= ["b1" "b2"] updated-blocks) "Visible page blocks stay on the page") + (is (empty? deleted-blocks) "Deleted block is removed from page db")))})))) + +(deftest-async backspace-before-block-merges-into-previous-blank-asset-block + (load-test-files + [{:page {:block/title "page1"} + :blocks + [{:block/title "b1"} + {:block/title "" + :logseq.property.asset/type "png" + :logseq.property.asset/checksum "blank-asset-checksum" + :logseq.property.asset/size 1} + {:block/title "after"}]}]) + (p/let [conn (db/get-db test-helper/test-db false) + block (->> (d/q '[:find (pull ?b [*]) + :where [?b :block/title "after"] + [?p :block/name "page1"] + [?b :block/page ?p]] + @conn) + ffirst)] + (delete-block @conn block + {:on-delete (fn [] + (let [visible-blocks (->> (d/q '[:find (pull ?b [*]) + :where + [?p :block/name "page1"] + [?b :block/page ?p] + [?b :block/title] + [(missing? $ ?b :logseq.property/deleted-at)]] + @conn) + (map first)) + visible-titles (map :block/title visible-blocks) + asset-blocks (filter :logseq.property.asset/type visible-blocks)] + (is (= ["b1" "after"] visible-titles)) + (is (= "after" (:block/title (first asset-blocks))) + "Backspace before the following block should merge its title into the asset block") + (is (= "png" (:logseq.property.asset/type (first asset-blocks))) + "Merging must keep the previous block renderable as an asset")))}))) + +(deftest-async backspace-before-block-merges-into-previous-blank-comments-block + (load-test-files + [{:page {:block/title "page1"} + :blocks + [{:block/title "b1"} + {:block/title "" + :build/tags [:logseq.class/Comments]} + {:block/title "after"}]}]) + (p/let [conn (db/get-db test-helper/test-db false) + block (->> (d/q '[:find (pull ?b [*]) + :where [?b :block/title "after"] + [?p :block/name "page1"] + [?b :block/page ?p]] + @conn) + ffirst)] + (delete-block @conn block + {:on-delete (fn [] + (let [visible-blocks (->> (d/q '[:find (pull ?b [* {:block/tags [:db/ident]}]) + :where + [?p :block/name "page1"] + [?b :block/page ?p] + [?b :block/title] + [(missing? $ ?b :logseq.property/deleted-at)]] + @conn) + (map first)) + visible-titles (map :block/title visible-blocks) + comments-blocks (filter comments-model/comments-area? visible-blocks)] + (is (= ["b1" "after"] visible-titles)) + (is (= "after" (:block/title (first comments-blocks))) + "Backspace before the following block should merge its title into the Comments block") + (is (= #{:logseq.class/Comments} + (set (map :db/ident (:block/tags (first comments-blocks))))) + "Merging must keep the previous block tagged as a Comments block")))}))) + +(deftest-async delete-at-empty-asset-end-merges-next-block-into-asset-block + (load-test-files + [{:page {:block/title "page1"} + :blocks + [{:block/title "b1"} + {:block/title "" + :logseq.property.asset/type "png" + :logseq.property.asset/checksum "blank-asset-checksum" + :logseq.property.asset/size 1} + {:block/title "after"}]}]) + (p/let [conn (db/get-db test-helper/test-db false) + asset-block (->> (d/q '[:find (pull ?b [*]) + :where [?b :logseq.property.asset/type "png"] + [?p :block/name "page1"] + [?b :block/page ?p]] + @conn) + ffirst) + next-block (->> (d/q '[:find (pull ?b [*]) + :where [?b :block/title "after"] + [?p :block/name "page1"] + [?b :block/page ?p]] + @conn) + ffirst) + asset-dom #js {:getAttribute #({"blockid" (str (:block/uuid asset-block)) + "containerid" nil} %)}] + (-> (p/with-redefs [state/get-edit-content (constantly "") + util/get-prev-block-non-collapsed-non-embed (constantly asset-dom) + editor/edit-block! (constantly nil)] + (p/do! + (editor/delete-block-inner! + test-helper/test-db + {:block-id (:block/uuid next-block) + :value (:block/title next-block) + :config {} + :block-container #js {} + :current-block asset-block + :next-block next-block + :delete-concat? true}) + (let [visible-blocks (->> (d/q '[:find (pull ?b [*]) + :where + [?p :block/name "page1"] + [?b :block/page ?p] + [?b :block/title] + [(missing? $ ?b :logseq.property/deleted-at)]] + @conn) + (map first)) + visible-titles (map :block/title visible-blocks) + asset-blocks (filter :logseq.property.asset/type visible-blocks)] + (is (= ["b1" "after"] visible-titles)) + (is (= "after" (:block/title (first asset-blocks))) + "Delete at the end of an empty asset title should merge the next title into the asset block") + (is (= "png" (:logseq.property.asset/type (first asset-blocks))) + "Delete merge must keep the current block renderable as an asset")))) + (p/finally (fn [] + (state/set-state! :editor/edit-block-fn nil)))))) + +(deftest-async delete-at-empty-comments-end-merges-next-block-into-comments-block + (load-test-files + [{:page {:block/title "page1"} + :blocks + [{:block/title "b1"} + {:block/title "" + :build/tags [:logseq.class/Comments]} + {:block/title "after"}]}]) + (p/let [conn (db/get-db test-helper/test-db false) + comments-block (->> (d/q '[:find (pull ?b [* {:block/tags [:db/ident]}]) + :where + [?p :block/name "page1"] + [?b :block/page ?p] + [?b :block/tags :logseq.class/Comments]] + @conn) + ffirst) + next-block (->> (d/q '[:find (pull ?b [*]) + :where [?b :block/title "after"] + [?p :block/name "page1"] + [?b :block/page ?p]] + @conn) + ffirst) + comments-dom #js {:getAttribute #({"blockid" (str (:block/uuid comments-block)) + "containerid" nil} %)}] + (-> (p/with-redefs [state/get-edit-content (constantly "") + util/get-prev-block-non-collapsed-non-embed (constantly comments-dom) + editor/edit-block! (constantly nil)] + (p/do! + (editor/delete-block-inner! + test-helper/test-db + {:block-id (:block/uuid next-block) + :value (:block/title next-block) + :config {} + :block-container #js {} + :current-block comments-block + :next-block next-block + :delete-concat? true}) + (let [visible-blocks (->> (d/q '[:find (pull ?b [* {:block/tags [:db/ident]}]) + :where + [?p :block/name "page1"] + [?b :block/page ?p] + [?b :block/title] + [(missing? $ ?b :logseq.property/deleted-at)]] + @conn) + (map first)) + visible-titles (map :block/title visible-blocks) + comments-blocks (filter comments-model/comments-area? visible-blocks)] + (is (= ["b1" "after"] visible-titles)) + (is (= "after" (:block/title (first comments-blocks))) + "Delete at the end of an empty Comments title should merge the next title into the Comments block") + (is (= #{:logseq.class/Comments} + (set (map :db/ident (:block/tags (first comments-blocks))))) + "Delete merge must keep the current block tagged as a Comments block")))) + (p/finally (fn [] + (state/set-state! :editor/edit-block-fn nil)))))) (deftest-async rapid-tab-after-new-block-indents-pending-block (let [current-block {:db/id 1 diff --git a/src/test/frontend/handler/editor_test.cljs b/src/test/frontend/handler/editor_test.cljs index 4d8881a0d0..8f3247d3de 100644 --- a/src/test/frontend/handler/editor_test.cljs +++ b/src/test/frontend/handler/editor_test.cljs @@ -12,6 +12,7 @@ [frontend.util :as util] [frontend.util.cursor :as cursor] [goog.dom :as gdom] + [logseq.db :as ldb] [logseq.outliner.core :as outliner-core] [promesa.core :as p])) @@ -360,6 +361,87 @@ (keydown-dollar-without-selection-result {:value "inline $$" :cursor-pos 8})))) +(defn- delete-block-at-zero-pos-result + [block] + (let [deleted? (atom false) + stopped? (atom false) + input #js {:value ""}] + (with-redefs [state/get-input (constantly input) + cursor/pos (constantly 0) + util/stop (fn [_] (reset! stopped? true)) + state/get-current-repo (constantly test-helper/test-db) + state/get-edit-block (constantly block) + db/entity (fn [_] block) + ldb/get-left-sibling (constantly nil) + editor/get-state (constantly {:config {}}) + editor/delete-block! (fn [_] (reset! deleted? true))] + (#'editor/delete-block-when-zero-pos! nil) + {:deleted? @deleted? + :stopped? @stopped?}))) + +(deftest delete-block-when-zero-pos-keeps-asset-block-test + (testing "Backspace at the start of an asset block does not delete the block" + (is (= {:deleted? false + :stopped? true} + (delete-block-at-zero-pos-result + {:db/id 1 + :block/uuid #uuid "11111111-1111-1111-1111-111111111111" + :block/title "" + :block/page {:db/id 10} + :logseq.property.asset/type "png"}))))) + +(deftest delete-block-when-zero-pos-keeps-comments-block-test + (testing "Backspace at the start of a Comments block does not delete the block" + (is (= {:deleted? false + :stopped? true} + (delete-block-at-zero-pos-result + {:db/id 1 + :block/uuid #uuid "11111111-1111-1111-1111-111111111111" + :block/title "" + :block/page {:db/id 10} + :block/tags [{:db/ident :logseq.class/Comments}]}))))) + +(deftest delete-block-when-zero-pos-keeps-regular-empty-block-behavior-test + (testing "Backspace at the start of a regular empty block still deletes it" + (is (= {:deleted? true + :stopped? true} + (delete-block-at-zero-pos-result + {:db/id 1 + :block/uuid #uuid "11111111-1111-1111-1111-111111111111" + :block/title "" + :block/page {:db/id 10}}))))) + +(deftest move-to-prev-block-edit-fn-focuses-merged-asset-title-test + (let [asset-block {:db/id 1 + :block/uuid #uuid "11111111-1111-1111-1111-111111111111" + :block/title "" + :logseq.property.asset/type "png"} + sibling-dom #js {:getAttribute #({"blockid" (str (:block/uuid asset-block)) + "containerid" nil} %)} + edit-calls (atom [])] + (with-redefs [db/entity (fn [lookup-ref] + (when (contains? #{[:block/uuid (:block/uuid asset-block)] + (:db/id asset-block)} + lookup-ref) + asset-block)) + editor/edit-block! (fn [block pos opts] + (swap! edit-calls conj {:block block + :pos pos + :opts opts}))] + (let [{:keys [new-content pos edit-block-f]} (#'editor/move-to-prev-block + test-helper/test-db + sibling-dom + "after")] + (is (= "after" new-content)) + (is (= 0 pos)) + (edit-block-f) + (is (= [{:block asset-block + :pos 0 + :opts {:custom-content "after" + :tail-len 5 + :container-id nil}}] + @edit-calls)))))) + (defn- handle-last-input-handler "Spied version of editor/handle-last-input" [{:keys [value cursor-pos editor-config]}]