Browse Source

enhance(ux): flashcards (#12299)

Enhances the flashcard user experience by adding automatic query property management, fixing critical bugs, and improving the UI for managing card sets.

Key changes:

1. Automatic creation of query property blocks when tagging with #Query or subclasses via pipeline
2. Fixed critical bug in api-insert-new-block! where the end? parameter had inverted conditional logic
3. Added ability to create new #Cards blocks directly from the flashcard modal with a plus button
Tienson Qin 1 month ago
parent
commit
41cbba3688

+ 8 - 1
clj-e2e/dev/user.clj

@@ -6,6 +6,7 @@
             [logseq.e2e.config :as config]
             [logseq.e2e.editor-basic-test]
             [logseq.e2e.fixtures :as fixtures]
+            [logseq.e2e.flashcards-basic-test]
             [logseq.e2e.graph :as graph]
             [logseq.e2e.keyboard :as k]
             [logseq.e2e.locator :as loc]
@@ -46,6 +47,11 @@
   (->> (future (run-tests 'logseq.e2e.property-basic-test))
        (swap! *futures assoc :property-test)))
 
+(defn run-flashcards-basic-test
+  []
+  (->> (future (run-tests 'logseq.e2e.flashcards-basic-test))
+       (swap! *futures assoc :flashcards-test)))
+
 (defn run-property-scoped-choices-test
   []
   (->> (future (run-tests 'logseq.e2e.property-scoped-choices-test))
@@ -111,7 +117,8 @@
              'logseq.e2e.plugins-basic-test
              'logseq.e2e.reference-basic-test
              'logseq.e2e.property-basic-test
-             'logseq.e2e.tag-basic-test)
+             'logseq.e2e.tag-basic-test
+             'logseq.e2e.flashcards-basic-test)
   (System/exit 0))
 
 (defn start

+ 39 - 0
clj-e2e/src/logseq/e2e/api.clj

@@ -0,0 +1,39 @@
+(ns logseq.e2e.api
+  (:require
+   [clojure.string :as string]
+   [jsonista.core :as json]
+   [wally.main :as w]))
+
+(defn- to-snake-case
+  "Converts a string to snake_case. Handles camelCase, PascalCase, spaces, hyphens, and existing underscores.
+   Examples:
+     'HelloWorld' -> 'hello_world'
+     'Hello World' -> 'hello_world'
+     'hello-world' -> 'hello_world'
+     'Hello__World' -> 'hello_world'"
+  [s]
+  (when (string? s)
+    (-> s
+      ;; Normalize input: replace hyphens/spaces with underscores, collapse multiple underscores
+        (string/replace #"[-\s]+" "_")
+      ;; Split on uppercase letters (except at start) and join with underscore
+        (string/replace #"(?<!^)([A-Z])" "_$1")
+      ;; Remove redundant underscores and trim
+        (string/replace #"_+" "_")
+        (string/trim)
+        ;; Convert to lowercase
+        (string/lower-case))))
+
+(defn ls-api-call!
+  [api-keyword & args]
+  (let [tag (name api-keyword)
+        ns' (string/split tag #"\.")
+        ns? (and (seq ns') (= (count ns') 2))
+        inbuilt? (contains? #{"app" "editor"} (first ns'))
+        ns1 (string/lower-case (if (and ns? (not inbuilt?))
+                                 (str "sdk." (first ns')) "api"))
+        name1 (if ns? (to-snake-case (last ns')) tag)
+        estr (format "s => { const args = JSON.parse(s);const o=logseq.%1$s; return o['%2$s']?.apply(null, args || []); }" ns1 name1)
+        args (json/write-value-as-string (vec args))]
+    ;; (prn "Debug: eval-js #" estr args)
+    (w/eval-js estr args)))

+ 2 - 0
clj-e2e/src/logseq/e2e/keyboard.clj

@@ -17,6 +17,8 @@
 
 (def arrow-up #(press "ArrowUp"))
 (def arrow-down #(press "ArrowDown"))
+(def arrow-left #(press "ArrowLeft"))
+(def arrow-right #(press "ArrowRight"))
 
 (def meta+shift+arrow-up #(press (str (if mac? "Meta" "Alt") "+Shift+ArrowUp")))
 (def meta+shift+arrow-down #(press (str (if mac? "Meta" "Alt") "+Shift+ArrowDown")))

+ 83 - 0
clj-e2e/test/logseq/e2e/flashcards_basic_test.clj

@@ -0,0 +1,83 @@
+(ns logseq.e2e.flashcards-basic-test
+  (:require [clojure.test :refer [deftest testing use-fixtures]]
+            [logseq.e2e.api :refer [ls-api-call!]]
+            [logseq.e2e.assert :as assert]
+            [logseq.e2e.fixtures :as fixtures]
+            [logseq.e2e.keyboard :as k]
+            [logseq.e2e.locator :as loc]
+            [logseq.e2e.util :as util]
+            [wally.main :as w]
+            [wally.repl :as repl]))
+
+(use-fixtures :once fixtures/open-page)
+
+(use-fixtures :each
+  fixtures/validate-graph)
+
+(defn- open-flashcards
+  []
+  (util/double-esc)
+  (k/press ["t" "c"])
+  (assert/assert-is-visible "#cards-modal"))
+
+(defn- select-cards-option
+  [label]
+  (w/click "#cards-modal [role='combobox']")
+  (w/click (loc/filter "[role='option']" :has-text label)))
+
+(defn- click-flashcards-plus
+  []
+  (w/click "#ls-cards-add"))
+
+(defn- setup-flashcards-data!
+  [{:keys [page-name tag-a tag-b card-a card-b query-a query-b]}]
+  (ls-api-call! :editor.appendBlockInPage page-name (str card-a " #Card #" tag-a))
+  (ls-api-call! :editor.appendBlockInPage page-name (str card-b " #Card #" tag-b))
+  (let [cards (ls-api-call! :editor.getTag "logseq.class/Cards")
+        cards-id (get cards "id")
+        cards-a (ls-api-call! :editor.appendBlockInPage page-name "Cards A"
+                              {:properties {:block/tags #{cards-id}}})
+        cards-b (ls-api-call! :editor.appendBlockInPage page-name "Cards B"
+                              {:properties {:block/tags #{cards-id}}})
+        query-a-id (get cards-a ":logseq.property/query")
+        query-b-id (get cards-b ":logseq.property/query")]
+    (ls-api-call! :editor.updateBlock query-a-id query-a)
+    (ls-api-call! :editor.updateBlock query-b-id query-b)))
+
+(deftest flashcards-plus-and-switching-test
+  (testing "create #Cards blocks from flashcards dialog and switch card sets"
+    (let [tag-a "fc-tag-a"
+          tag-b "fc-tag-b"
+          card-a "Card A"
+          card-b "Card B"
+          query-a (str "[[" tag-a "]]")
+          query-b (str "[[" tag-b "]]")]
+      (util/goto-journals)
+      (let [page (ls-api-call! :editor.getCurrentPage)
+            page-name (get page "name")]
+        (setup-flashcards-data!
+         {:page-name page-name
+          :tag-a tag-a
+          :tag-b tag-b
+          :card-a card-a
+          :card-b card-b
+          :query-a query-a
+          :query-b query-b}))
+
+      (open-flashcards)
+      (click-flashcards-plus)
+      (w/wait-for ".ls-block .tag:has-text('Cards')")
+
+      (open-flashcards)
+      (select-cards-option "Cards A")
+      (assert/assert-is-visible (format "#cards-modal .ls-card :text('%s')" card-a))
+      (assert/assert-have-count (format "#cards-modal .ls-card :text('%s')" card-b) 0)
+      (assert/assert-is-visible (loc/filter "#cards-modal .text-sm.opacity-50" :has-text "1/1"))
+
+      (select-cards-option "Cards B")
+      (assert/assert-is-visible (format "#cards-modal .ls-card :text('%s')" card-b))
+      (assert/assert-have-count (format "#cards-modal .ls-card :text('%s')" card-a) 0)
+      (assert/assert-is-visible (loc/filter "#cards-modal .text-sm.opacity-50" :has-text "1/1"))
+
+      (select-cards-option "All cards")
+      (assert/assert-is-visible (loc/filter "#cards-modal .text-sm.opacity-50" :has-text "1/2")))))

+ 1 - 36
clj-e2e/test/logseq/e2e/plugins_basic_test.clj

@@ -1,9 +1,8 @@
 (ns logseq.e2e.plugins-basic-test
   (:require
    [clojure.set :as set]
-   [clojure.string :as string]
    [clojure.test :refer [deftest testing is use-fixtures]]
-   [jsonista.core :as json]
+   [logseq.e2e.api :refer [ls-api-call!]]
    [logseq.e2e.assert :as assert]
    [logseq.e2e.fixtures :as fixtures]
    [logseq.e2e.keyboard :as k]
@@ -19,45 +18,11 @@
   [property-name]
   (str ":plugin.property._test_plugin/" property-name))
 
-(defn- to-snake-case
-  "Converts a string to snake_case. Handles camelCase, PascalCase, spaces, hyphens, and existing underscores.
-   Examples:
-     'HelloWorld' -> 'hello_world'
-     'Hello World' -> 'hello_world'
-     'hello-world' -> 'hello_world'
-     'Hello__World' -> 'hello_world'"
-  [s]
-  (when (string? s)
-    (-> s
-      ;; Normalize input: replace hyphens/spaces with underscores, collapse multiple underscores
-        (clojure.string/replace #"[-\s]+" "_")
-      ;; Split on uppercase letters (except at start) and join with underscore
-        (clojure.string/replace #"(?<!^)([A-Z])" "_$1")
-      ;; Remove redundant underscores and trim
-        (clojure.string/replace #"_+" "_")
-        (clojure.string/trim)
-        ;; Convert to lowercase
-        (clojure.string/lower-case))))
-
 (defonce ^:private *property-idx (atom 0))
 (defn- new-property
   []
   (str "p" (swap! *property-idx inc)))
 
-(defn- ls-api-call!
-  [tag & args]
-  (let [tag (name tag)
-        ns' (string/split tag #"\.")
-        ns? (and (seq ns') (= (count ns') 2))
-        inbuilt? (contains? #{"app" "editor"} (first ns'))
-        ns1 (string/lower-case (if (and ns? (not inbuilt?))
-                                 (str "sdk." (first ns')) "api"))
-        name1 (if ns? (to-snake-case (last ns')) tag)
-        estr (format "s => { const args = JSON.parse(s);const o=logseq.%1$s; return o['%2$s']?.apply(null, args || []); }" ns1 name1)
-        args (json/write-value-as-string (vec args))]
-    ;; (prn "Debug: eval-js #" estr args)
-    (w/eval-js estr args)))
-
 (defn- assert-api-ls-block!
   ([ret] (assert-api-ls-block! ret 1))
   ([ret-or-uuid count]

+ 15 - 16
src/main/frontend/components/block.cljs

@@ -1947,21 +1947,7 @@
                               (= 1 (count block-ast-title))
                               (= "Link" (ffirst block-ast-title)))
                          (assoc :node-ref-link-only? true))]
-           (map-inline config' block-ast-title))
-
-         (when (and (seq block-ast-title) (ldb/class-instance?
-                                           (entity-plus/entity-memoized (db/get-db) :logseq.class/Cards)
-                                           block))
-           [(ui/tooltip
-             (shui/button
-              {:variant :ghost
-               :size :sm
-               :class "ml-2 !px-1 !h-5 text-xs text-muted-foreground"
-               :on-click (fn [e]
-                           (util/stop e)
-                           (state/pub-event! [:modal/show-cards (:db/id block)]))}
-              "Practice")
-             [:div "Practice cards"])])))))))
+           (map-inline config' block-ast-title))))))))
 
 (rum/defc block-title-aux
   [config block {:keys [query? *show-query?]}]
@@ -1995,13 +1981,26 @@
         (when (fn? on-title-click)
           {:on-click on-title-click})))
      (cond
-       (and query? (and blank? (or advanced-query? show-query?)))
+       (and query? blank? (or advanced-query? show-query?))
        [:span.opacity-75.hover:opacity-100 "Untitled query"]
        (and query? blank?)
        (query-builder-component/builder query {})
        :else
        (text-block-title config block))
      query-setting
+     (when (ldb/class-instance?
+            (entity-plus/entity-memoized (db/get-db) :logseq.class/Cards)
+            block)
+       [(ui/tooltip
+         (shui/button
+          {:variant :ghost
+           :size :sm
+           :class "!px-1 text-xs text-muted-foreground"
+           :on-click (fn [e]
+                       (util/stop e)
+                       (state/pub-event! [:modal/show-cards (:db/id block)]))}
+          "Practice")
+         [:div "Practice cards"])])
      (when-let [property (:logseq.property/created-from-property block)]
        (when-let [message (when (= :url (:logseq.property/type property))
                             (first (outliner-property/validate-property-value (db/get-db) property (:db/id block))))]

+ 68 - 21
src/main/frontend/extensions/fsrs.cljs

@@ -6,13 +6,16 @@
             [frontend.components.block :as component-block]
             [frontend.components.macro :as component-macro]
             [frontend.context.i18n :refer [t]]
+            [frontend.date :as date]
             [frontend.db :as db]
             [frontend.db-mixins :as db-mixins]
             [frontend.db.async :as db-async]
             [frontend.db.model :as db-model]
             [frontend.db.query-dsl :as query-dsl]
             [frontend.handler.block :as block-handler]
+            [frontend.handler.editor :as editor-handler]
             [frontend.handler.property :as property-handler]
+            [frontend.handler.route :as route-handler]
             [frontend.modules.shortcut.core :as shortcut]
             [frontend.state :as state]
             [frontend.ui :as ui]
@@ -81,8 +84,12 @@
 (defn- <get-due-card-block-ids
   [repo cards-id]
   (let [now-inst-ms (inst-ms (js/Date.))
-        cards (when (and cards-id (not= (keyword cards-id) :global)) (db/entity cards-id))
-        query (:block/title cards)
+        cards (when (and cards-id (not (contains? #{:global "global"} cards-id)))
+                (db/entity cards-id))
+        query (when cards
+                (when-let [query (:logseq.property/query cards)]
+                  (when-not (string/blank? (:block/title query))
+                    (:block/title query))))
         result (query-dsl/parse query {:db-graph? true})
         card-tag-id (:db/id (db/entity :logseq.class/Card))
         card-tag-children-ids (db-model/get-structured-children repo card-tag-id)
@@ -105,6 +112,15 @@
              q)]
     (db-async/<q repo {:transact-db? false} q' card-ids now-inst-ms (:rules result))))
 
+(defn- <create-cards-block!
+  []
+  (let [cards-tag-id (:db/id (db/entity :logseq.class/Cards))]
+    (editor-handler/api-insert-new-block! ""
+                                          {:page (date/today)
+                                           :properties {:block/tags #{cards-tag-id}}
+                                           :sibling? false
+                                           :end? true})))
+
 (defn- btn-with-shortcut [{:keys [shortcut id btn-text due on-click class]}]
   (let [bg-class (case id
                    "card-again" "primary-red"
@@ -230,21 +246,41 @@
             [:div.flex.justify-center (rating-btns repo block-entity *card-index *phase)])]]))))
 
 (declare update-due-cards-count)
-(rum/defcs cards-view < rum/reactive
+(rum/defcs ^:large-vars/cleanup-todo cards-view < rum/reactive
   (rum/local 0 ::card-index)
   (shortcut/mixin :shortcut.handler/cards false)
   {:init (fn [state]
            (let [*block-ids (atom nil)
                  *loading? (atom nil)
-                 cards-id (last (:rum/args state))]
+                 cards-id (last (:rum/args state))
+                 *cards-list (atom [{:db/id :global
+                                     :block/title "All cards"}])
+                 repo (state/get-current-repo)
+                 cards-class-id (:db/id (entity-plus/entity-memoized (db/get-db) :logseq.class/Cards))]
              (reset! *loading? true)
              (p/let [result (<get-due-card-block-ids (state/get-current-repo) cards-id)]
                (reset! *block-ids result)
                (reset! *loading? false))
+             (when cards-class-id
+               (p/let [cards (db-async/<get-tag-objects repo cards-class-id)
+                       cards (p/all (map (fn [block]
+                                           (if-not (string/blank? (:block/title block))
+                                             block
+                                             (when-let [query-block-id (:db/id (:logseq.property/query block))]
+                                               (p/let [query-block (db-async/<get-block (state/get-current-repo) query-block-id)]
+                                                 (assoc block :block/title (:block/title query-block))))))
+                                         cards))]
+                 (reset! *cards-list (concat [{:db/id :global
+                                               :block/title "All cards"}]
+                                             (remove
+                                              (fn [card]
+                                                (string/blank? (:block/title card)))
+                                              cards)))))
              (assoc state
                     ::block-ids *block-ids
                     ::cards-id (atom (or cards-id :global))
-                    ::loading? *loading?)))
+                    ::loading? *loading?
+                    ::cards-list *cards-list)))
    :will-unmount (fn [state]
                    (update-due-cards-count)
                    state)}
@@ -252,12 +288,10 @@
   (let [repo (state/get-current-repo)
         *cards-id (::cards-id state)
         cards-id (rum/react *cards-id)
-        all-cards (concat
-                   [{:db/id :global
-                     :block/title "All cards"}]
-                   (db-model/get-class-objects repo (:db/id (entity-plus/entity-memoized (db/get-db) :logseq.class/Cards)))
-                   ;; TODO: list all children tags of #Card
-                   )
+        *cards-list (::cards-list state)
+        all-cards (or (rum/react *cards-list)
+                      [{:db/id :global
+                        :block/title "All cards"}])
         *block-ids (::block-ids state)
         block-ids (rum/react *block-ids)
         loading? (rum/react (::loading? state))
@@ -269,20 +303,33 @@
         (shui/select
          {:on-value-change (fn [v]
                              (reset! *cards-id v)
-                             (p/let [result (<get-due-card-block-ids repo (if (= :global v) nil v))]
-                               (reset! *card-index 0)
-                               (reset! *block-ids result)))
+                             (let [cards-id' (when-not (contains? #{:global "global"} v) v)]
+                               (p/let [result (<get-due-card-block-ids repo cards-id')]
+                                 (reset! *card-index 0)
+                                 (reset! *block-ids result))))
           :default-value cards-id}
          (shui/select-trigger
           {:class "!px-2 !py-0 !h-8 w-64"}
           (shui/select-value
-           {:placeholder "Select cards"})
-          (shui/select-content
-           (shui/select-group
-            (for [card-entity all-cards]
-              (shui/select-item {:value (:db/id card-entity)}
-                                (:block/title card-entity)))))))
-
+           {:placeholder "Select cards"}))
+         (shui/select-content
+          (shui/select-group
+           (for [card-entity all-cards]
+             (shui/select-item {:value (:db/id card-entity)}
+                               (:block/title card-entity))))))
+        (shui/button
+         {:variant :ghost
+          :id "ls-cards-add"
+          :size :sm
+          :title "Add new query"
+          :class "!px-1 text-muted-foreground"
+          :on-click (fn []
+                      (p/let [saved-block (<create-cards-block!)]
+                        (shui/dialog-close!)
+                        (when saved-block
+                          (route-handler/redirect-to-page! (:block/uuid saved-block)
+                                                           {}))))}
+         (ui/icon "plus"))
         [:span.text-sm.opacity-50 (str (min (inc @*card-index) (count @*block-ids)) "/" (count @*block-ids))]]
        (let [block-id (nth block-ids @*card-index nil)]
          (cond

+ 2 - 2
src/main/frontend/handler/editor.cljs

@@ -572,8 +572,8 @@
 
                                         end?
                                         (if last-block
-                                          [block' false]
-                                          [last-block true])
+                                          [last-block true]
+                                          [block' false])
 
                                         last-block
                                         [last-block true]

+ 36 - 0
src/main/frontend/worker/pipeline.cljs

@@ -14,6 +14,7 @@
             [logseq.db.common.entity-plus :as entity-plus]
             [logseq.db.common.order :as db-order]
             [logseq.db.frontend.class :as db-class]
+            [logseq.db.frontend.property.build :as db-property-build]
             [logseq.db.sqlite.create-graph :as sqlite-create-graph]
             [logseq.db.sqlite.export :as sqlite-export]
             [logseq.graph-parser.exporter :as gp-exporter]
@@ -275,6 +276,39 @@
               (assoc :logseq.property.code/lang (:kv/value (d/entity db :logseq.kv/latest-code-lang))))])))
      datoms)))
 
+(defn- ensure-query-property-on-tag-additions
+  [tx-report]
+  (let [{:keys [db-after tx-data tx-meta]} tx-report
+        query-class (entity-plus/entity-memoized db-after :logseq.class/Query)
+        query-property (d/entity db-after :logseq.property/query)]
+    (when (and query-class
+               query-property
+               (not (rtc-tx-or-download-graph? tx-meta))
+               (not (:undo? tx-meta))
+               (not (:redo? tx-meta)))
+      (let [tagged-block-ids (->> tx-data
+                                  (filter (fn [d] (and (= :block/tags (:a d)) (:added d))))
+                                  (map :e)
+                                  (distinct))]
+        (mapcat
+         (fn [eid]
+           (when-let [block (d/entity db-after eid)]
+             (when (ldb/class-instance? query-class block)
+               (let [query-entity (:logseq.property/query block)]
+                 (when-not (and query-entity (:block/uuid query-entity))
+                   (let [query-text (if (string? query-entity) query-entity "")
+                         value-block (db-property-build/build-property-value-block
+                                      block
+                                      query-property
+                                      query-text
+                                      {:block-uuid (common-uuid/gen-uuid)})
+                         value-uuid (:block/uuid value-block)]
+                     [value-block
+                      (outliner-core/block-with-updated-at
+                       {:db/id (:db/id block)
+                        :logseq.property/query [:block/uuid value-uuid]})]))))))
+         tagged-block-ids)))))
+
 (defn- invoke-hooks-for-imported-graph [conn {:keys [tx-meta] :as tx-report}]
   (let [refs-tx-report (outliner-pipeline/transact-new-db-graph-refs conn tx-report)
         full-tx-data (concat (:tx-data tx-report) (:tx-data refs-tx-report))
@@ -402,6 +436,7 @@
         toggle-page-and-block-tx-data (when (empty? fix-inline-page-tx-data)
                                         (toggle-page-and-block db tx-report))
         display-blocks-tx-data (add-missing-properties-to-typed-display-blocks db-after tx-data tx-meta)
+        ensure-query-tx-data (ensure-query-property-on-tag-additions tx-report)
         commands-tx (when-not (or (:undo? tx-meta) (:redo? tx-meta) (rtc-tx-or-download-graph? tx-meta))
                       (commands/run-commands tx-report))
         insert-templates-tx (when-not (rtc-tx-or-download-graph? tx-meta)
@@ -410,6 +445,7 @@
     (concat revert-tx-data
             toggle-page-and-block-tx-data
             display-blocks-tx-data
+            ensure-query-tx-data
             commands-tx
             insert-templates-tx
             created-by-tx

+ 35 - 0
src/test/frontend/worker/pipeline_test.cljs

@@ -93,3 +93,38 @@
 
     ;; return global fn back to previous behavior
     (ldb/register-transact-pipeline-fn! identity)))
+
+(deftest ensure-query-property-on-tag-additions-test
+  (let [graph test-helper/test-db-name-db-version
+        conn (db-test/create-conn-with-blocks
+              {:pages-and-blocks [{:page {:block/title "page1"}
+                                   :blocks [{:block/title "b1"}
+                                            {:block/title "b2"}]}]
+               :classes {:QueryChild {:build/class-extends [:logseq.class/Query]}}})
+        page (ldb/get-page @conn "page1")
+        blocks (:block/_page page)
+        b1 (some #(when (= "b1" (:block/title %)) %) blocks)
+        b2 (some #(when (= "b2" (:block/title %)) %) blocks)
+        query-child (ldb/get-page @conn "QueryChild")]
+    (ldb/register-transact-pipeline-fn!
+     (fn [tx-report]
+       (worker-pipeline/transact-pipeline graph tx-report)))
+
+    (testing "tagging with #Query adds query property"
+      (ldb/transact! conn [[:db/add (:db/id b1) :block/tags :logseq.class/Query]])
+      (let [block (d/entity @conn (:db/id b1))
+            query (:logseq.property/query block)]
+        (is query)
+        (is (uuid? (:block/uuid query)))
+        (is (= "" (:block/title query)))))
+
+    (testing "tagging with class extending #Query adds query property"
+      (ldb/transact! conn [[:db/add (:db/id b2) :block/tags (:db/id query-child)]])
+      (let [block (d/entity @conn (:db/id b2))
+            query (:logseq.property/query block)]
+        (is query)
+        (is (uuid? (:block/uuid query)))
+        (is (= "" (:block/title query)))))
+
+    ;; return global fn back to previous behavior
+    (ldb/register-transact-pipeline-fn! identity)))