Browse Source

fix: slow query ux (#11548)

* enhance: intuitive query input

* fix: Manually editing Query property doesn't update its results

fix https://linear.app/logseq/issue/LOG-3249/manually-editing-query-property-doesnt-update-its-results

* fix: don't close popup when editing hidden property

* enhance: display query setting icon when hovering title

* Use block's title as query if its query is still empty

* enhance: display both simple and advanced query as below when not collapsed
Tienson Qin 1 year ago
parent
commit
879a30593a

+ 5 - 4
deps/db/src/logseq/db/frontend/property.cljs

@@ -61,10 +61,11 @@
                                           :schema {:type :checkbox
                                                    :public? true
                                                    :view-context :class}}
-   :logseq.property/query {:title "Query"
-                           :schema {:type :default
-                                    :public? true
-                                    :view-context :block}}
+   :logseq.property/query       {:title "Query"
+                                 :schema {:type :default
+                                          :public? true
+                                          :hide? true
+                                          :view-context :block}}
    :logseq.property/page-tags {:title "Page Tags"
                                :schema {:type :page
                                         :public? true

+ 3 - 1
deps/outliner/src/logseq/outliner/property.cljs

@@ -230,7 +230,9 @@
 (defn- find-or-create-property-value
   "Find or create a property value. Only to be used with properties that have ref types"
   [conn property-id v]
-  (or (get-property-value-eid @conn property-id v)
+  ;; FIXME: some properties should always create new values
+  (or (when-not (contains? #{:logseq.property/query} property-id)
+        (get-property-value-eid @conn property-id v))
       (let [v-uuid (create-property-text-block! conn nil property-id v {})]
         (:db/id (d/entity @conn [:block/uuid v-uuid])))))
 

+ 4 - 7
src/main/frontend/commands.cljs

@@ -141,7 +141,8 @@
 (defn db-based-query
   []
   [[:editor/input "" {:last-pattern command-trigger}]
-   [:editor/set-property :block/tags :logseq.class/Query]])
+   [:editor/set-property :block/tags :logseq.class/Query]
+   [:editor/set-property :logseq.property/query ""]])
 
 (defn file-based-query
   []
@@ -402,12 +403,8 @@
           (conj ["Properties" (->properties)])))
 
       ;; advanced
-      [["Query"
-        (query-steps)
-        query-doc
-        :icon/query
-        "ADVANCED"]
-       ["Advanced Query" (advanced-query-steps) "Create an advanced query block" :icon/advanced-query]
+      [["Query" (query-steps) query-doc :icon/query "ADVANCED"]
+       ["Advanced Query" (advanced-query-steps) "Create an advanced query block" :icon/query]
        (when-not db?
          ["Zotero" (zotero-steps) "Import Zotero journal article" :icon/circle-letter-z])
        ["Query function" [[:editor/input "{{function }}" {:backward-pos 2}]] "Create a query function" :icon/queryCode]

+ 80 - 24
src/main/frontend/components/block.cljs

@@ -2037,10 +2037,11 @@
 (declare block-content)
 
 (declare src-cp)
+(declare block-title)
 
-(defn- text-block-title
+(rum/defc ^:large-vars/cleanup-todo text-block-title
   [config {:block/keys [marker pre-block? properties] :as block}]
-  (let [block-title (:block.temp/ast-title block)
+  (let [block-ast-title (:block.temp/ast-title block)
         config (assoc config :block block)
         level (:level config)
         slide? (boolean (:slide? config))
@@ -2121,33 +2122,66 @@
          (when-not area? [(hl-ref)])
 
          (conj
-          (map-inline config block-title)
+          (map-inline config block-ast-title)
           (when (= block-type :whiteboard-shape) [:span.mr-1 (ui/icon "whiteboard-element" {:extension? true})]))
 
          ;; highlight ref block (area)
          (when area? [(hl-ref)])
 
-         (when (and (seq block-title) (ldb/class-instance? (db/entity :logseq.class/Cards) block))
-           [(shui/button
-             {:variant :ghost
-              :size :sm
-              :class "ml-2 !px-1 !h-5 text-xs text-muted-foreground"
-              :on-click (fn [_]
-                          (state/pub-event! [:modal/show-cards (:db/id block)]))}
-             "Practice")])))))))
+         (when (and (seq block-ast-title) (ldb/class-instance? (db/entity :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"])])))))))
 
-(defn build-block-title
-  [config block]
-  (let [node-type (:logseq.property.node/display-type block)]
-    (case node-type
-      :code
+(rum/defcs block-title < rum/reactive db-mixins/query
+  (rum/local false ::hover?)
+  [state config block]
+  (let [*hover? (::hover? state)
+        node-type (:logseq.property.node/display-type block)
+        query? (ldb/class-instance? (db/entity :logseq.class/Query) block)
+        query (:logseq.property/query block)
+        empty-query-title? (and query? (string/blank? (:block/title query)))
+        query-block? (:logseq.property/_query block)
+        advanced-query? (= :code (:logseq.property.node/display-type query))]
+    (cond
+      (= :code node-type)
       [:div.flex.flex-1.w-full
        (src-cp (assoc config :block block) {:language (:logseq.property.code/mode block)})]
 
-      ;; TODO: switched to https://cortexjs.io/mathlive/ for editing
-      :math
+      query-block?
+      (query-builder-component/builder (:block/title block)
+                                       {:block block
+                                        :query-object? true})
+
+        ;; TODO: switched to https://cortexjs.io/mathlive/ for editing
+      (= :math node-type)
       (latex/latex (str (:container-id config) "-" (:db/id block)) (:block/title block) true false)
 
+      (and empty-query-title? (not advanced-query?))
+      [:div.flex.flex-row.w-full.gap-1
+       {:on-mouse-over #(reset! *hover? true)
+        :on-mouse-out #(reset! *hover? false)}
+       (query-builder-component/builder (:block/title block)
+                                        {:block block
+                                         :query-object? true})
+       (when @*hover?
+         (shui/button
+          {:variant "outline"
+           :size :sm
+           :class "!h-6"
+           :on-pointer-down (fn [e]
+                              (util/stop e)
+                              (editor-handler/query-edit-title! block))}
+          [:span "Edit query title"]))]
+
+      :else
       (text-block-title config block))))
 
 (rum/defc span-comma
@@ -2604,6 +2638,7 @@
                                             (block-content-on-pointer-down e block block-id content edit-input-id config))))))]
     [:div.block-content.inline
      (cond-> {:id (str "block-content-" uuid)
+              :key (str "block-content-" uuid)
               :on-pointer-up (fn [e]
                                (when (and
                                       (state/in-selection-mode?)
@@ -2623,7 +2658,7 @@
       [:div.flex.flex-row.justify-between.block-content-inner
        (when-not plugin-slotted?
          [:div.block-head-wrap
-          (build-block-title config block)])
+          (block-title config block)])
 
        (file-block/clock-summary-cp block ast-body)]
 
@@ -3294,14 +3329,36 @@
         (when (and @*show-right-menu? (not in-whiteboard?) (not (or table? property?)))
           (block-right-menu config block editing?))])
 
+     (when-not (:table? config)
+       (let [query* (:logseq.property/query (db/entity (:db/id block)))
+             query (when query* (db/sub-block (:db/id query*)))
+             advanced-query? (= :code (:logseq.property.node/display-type query))]
+         (when query
+           (cond
+             (and advanced-query? (not collapsed?))
+             [:div.flex.flex-1.my-1 {:style {:margin-left 42}}
+              (src-cp (assoc config :block query)
+                      {:language (:logseq.property.code/mode query)})]
+
+             (and (not advanced-query?) (not collapsed?) (not (string/blank? (:block/title query))))
+             [:div.flex.flex-1.my-1 {:style {:margin-left 42}}
+              (query-builder-component/builder (:block/title query)
+                                               {:block query
+                                                :query-object? true})]))))
+
      (when (and db-based? (not collapsed?) (not (or table? property?)))
        [:div (when-not (:page-title? config) {:style {:padding-left 45}})
         (db-properties-cp config block {:in-block-container? true})])
 
-     (when (and db-based? (not collapsed?) (not (or table? property?)) (not (string/blank? (:block/title (:logseq.property/query block)))))
-       (let [query (:block/title (:logseq.property/query block))
+     (when (and db-based? (not collapsed?) (not (or table? property?))
+                (ldb/class-instance? (db/entity :logseq.class/Query) block))
+       (let [query-block (:logseq.property/query block)
+             query-block-title (:block/title query-block)
+             query (if (string/blank? query-block-title)
+                     (:block/title (db/entity (:db/id block)))
+                     query-block-title)
              result (common-util/safe-read-string query)
-             advanced-query? (and (map? result) (:query result))]
+             advanced-query? (map? result)]
          [:div {:style {:padding-left 42}}
           (query/custom-query (wrap-query-components (assoc config
                                                             :dsl-query? (not advanced-query?)
@@ -3654,7 +3711,7 @@
       (if html-export?
         (latex/html-export content true false)
         (if (config/db-based-graph? (state/get-current-repo))
-         [:div.warning "'#+BEGIN_EXPORT latex' is deprecated. Use '/Math block' command instead."]
+          [:div.warning "'#+BEGIN_EXPORT latex' is deprecated. Use '/Math block' command instead."]
           (latex/latex (str (d/squuid)) content true false)))
 
       ["Custom" "query" _options _result content]
@@ -3667,7 +3724,6 @@
             (log/error :read-string-error e)
             (ui/block-error "Invalid query:" {:content content}))))
 
-
       ["Custom" "note" _options result _content]
       (ui/admonition "note" (markup-elements-cp config result))
 

+ 1 - 1
src/main/frontend/components/block.css

@@ -212,7 +212,7 @@
 }
 
 .block-title-wrap {
-  @apply w-full inline;
+  @apply w-full inline-flex;
 
   > .ui__checkbox {
     @apply relative top-[2px];

+ 4 - 6
src/main/frontend/components/property.cljs

@@ -218,10 +218,7 @@
 
             (and (= :default type)
                  (not (seq (:property/closed-values property))))
-            (p/do!
-             (pv/<create-new-block! block property "")
-             (shui/popup-hide!)
-             (shui/dialog-close!))
+            (pv/<create-new-block! block property "")
 
             (or (not= :default type)
                 (and (= :default type) (seq (:property/closed-values property))))
@@ -339,7 +336,8 @@
                                    block-types (if (and page? (not= block-type :page))
                                                  #{:page block-type}
                                                  #{block-type})]
-                               (or (and (not page?) (contains? #{:block/alias} (:block/title m)))
+                               (or (contains? #{:logseq.property/query} (:db/ident m))
+                                   (and (not page?) (contains? #{:block/alias} (:db/ident m)))
                                    ;; Filters out properties from being in wrong :view-context and :never view-contexts
                                    (and (not= view-context :all) (not (contains? block-types view-context))))))
         property (rum/react *property)
@@ -654,7 +652,7 @@
                           (state/set-selection-blocks! [block])
                           (some-> js/document.activeElement (.blur)))
                         (d/remove-class! target "ls-popup-closed")))}
-       (let [properties' (remove (fn [[k _v]] (contains? #{:logseq.property/icon} k)) full-properties)]
+       (let [properties' (remove (fn [[k _v]] (contains? #{:logseq.property/icon :logseq.property/query} k)) full-properties)]
          (properties-section block (if class-schema? properties properties') opts))
 
        (rum/with-key (new-property block opts) (str id "-add-property"))])))

+ 4 - 53
src/main/frontend/components/property/value.cljs

@@ -4,7 +4,6 @@
             [datascript.impl.entity :as de]
             [dommy.core :as d]
             [frontend.components.icon :as icon-component]
-            [frontend.components.query.builder :as query-builder-component]
             [frontend.components.select :as select]
             [frontend.components.title :as title]
             [frontend.config :as config]
@@ -29,15 +28,13 @@
             [goog.dom :as gdom]
             [goog.functions :refer [debounce]]
             [lambdaisland.glogi :as log]
-            [logseq.common.util :as common-util]
             [logseq.common.util.macro :as macro-util]
             [logseq.db :as ldb]
             [logseq.db.frontend.property :as db-property]
             [logseq.db.frontend.property.type :as db-property-type]
             [logseq.shui.ui :as shui]
             [promesa.core :as p]
-            [rum.core :as rum]
-            [frontend.modules.outliner.op :as outliner-op]))
+            [rum.core :as rum]))
 
 (rum/defc property-empty-btn-value
   [property & opts]
@@ -109,8 +106,9 @@
 (defn <create-new-block!
   [block property value & {:keys [edit-block?]
                            :or {edit-block? true}}]
-  (shui/popup-hide!)
-  (shui/dialog-close!)
+  (when-not (get-in property [:block/schema :hide?])
+    (shui/popup-hide!)
+    (shui/dialog-close!))
   (p/let [block
           (if (and (= :default (get-in property [:block/schema :type]))
                    (not (db-property/many? property)))
@@ -715,50 +713,6 @@
         :on-click (fn [] (<create-new-block! block property ""))}
        (property-empty-btn-value property)])))
 
-(rum/defcs query-cp <
-  (rum/local false ::show-setting?)
-  [state block property v-block]
-  (let [result (common-util/safe-read-string (:block/title v-block))
-        advanced-query? (or (and (map? result) (:query result))
-                            (string/starts-with? (string/triml (:block/title v-block)) "{"))]
-    [:div.flex.flex-1.flex-row.gap-1.justify-between
-     [:div.flex.flex-1 (property-normal-block-value block property v-block)]
-     (when-not advanced-query?
-       (shui/button
-        {:variant :ghost
-         :size :sm
-         :class "jtrigger px-1 text-muted-foreground"
-         :title "Update query"
-         :on-click (fn [e]
-                     (shui/popup-show!
-                      (.-target e)
-                      (fn []
-                        (let [block (db/entity (:db/id v-block))
-                              query (:block/title block)]
-                          [:div.p-4.h-64.flex.flex-col.gap-4 {:style {:width "42rem"}}
-                           [:div.flex.flex-1
-                            (query-builder-component/builder query {:property property
-                                                                    :block block})]
-
-                           [:div
-                            (shui/button
-                             {:variant :ghost
-                              :size :sm
-                              :class "text-muted-foreground"
-                              :on-click (fn []
-                                          (p/do!
-                                           (ui-outliner-tx/transact!
-                                            {:outliner-op :save-block}
-                                            (db-property-handler/set-block-properties! (:db/id block)
-                                                                                       {:logseq.property.node/display-type :code
-                                                                                        :logseq.property.code/mode "clojure"})
-                                            (outliner-op/save-block! {:db/id (:db/id block) :block/title ""}))
-
-                                           (shui/popup-hide!)))}
-                             "Switch to advanced query")]]))
-                      {:align :end}))}
-        (ui/icon "settings" {:size 18})))]))
-
 (rum/defcs property-block-value < rum/reactive db-mixins/query
   {:init (fn [state]
            (let [block (first (:rum/args state))]
@@ -775,9 +729,6 @@
                                "Invalid block value, please delete the current property."]]
           (when v-block
             (cond
-              (= (:db/ident property) :logseq.property/query)
-              (query-cp block property v-block)
-
               (:block/page v-block)
               (property-normal-block-value block property v-block)
 

+ 6 - 2
src/main/frontend/components/query.cljs

@@ -126,6 +126,7 @@
 
 (rum/defcs custom-query* < rum/reactive rum/static db-mixins/query
   (rum/local nil ::query-result-atom)
+  (rum/local nil ::prev-q)
   {:init (fn [state]
            (let [[{:keys [dsl-query? db-graph? built-in-query?] :as config}
                   {:keys [collapsed?]}] (:rum/args state)]
@@ -138,7 +139,8 @@
            (assoc state :query-error (atom nil)
                   :fulltext-query-result (atom nil)))}
   [state {:keys [db-graph? dsl-query? built-in-query?] :as config} {:keys [builder query view collapsed?] :as q}]
-  (let [*query-result-atom (::query-result-atom state)
+  (let [*prev-q (::prev-q state)
+        *query-result-atom (::query-result-atom state)
         *query-error (:query-error state)
         *fulltext-query-result (:fulltext-query-result state)
         current-block-uuid (or (:block/uuid (:block config))
@@ -150,11 +152,13 @@
         table? (when-not db-graph?
                  (or (get-in current-block [:block/properties :query-table])
                      (and (string? query) (string/ends-with? (string/trim query) "table"))))
+        q-changed? (not= @*prev-q q)
         result (when (or built-in-collapsed? (not collapsed?'))
-                 (or @*query-result-atom
+                 (or (if q-changed? false @*query-result-atom)
                      (let [result (query-result/get-query-result config q *query-error *fulltext-query-result current-block-uuid {:table? table?})]
                        (reset! *query-result-atom result)
                        result)))
+        _ (when q-changed? (reset! *prev-q q))
         ;; Args for displaying query header and results
         view-fn (if (keyword? view) (get-in (state/sub-config) [:query/views view]) view)
         view-f (and view-fn (sci/eval-string (pr-str view-fn)))

+ 2 - 2
src/main/frontend/components/query/builder.cljs

@@ -361,7 +361,7 @@
   (let [f (first clause)]
     (cond
       (string? clause)
-      (str "search: " clause)
+      (str "Search: " clause)
 
       (= (keyword f) :page-ref)
       (page-ref/->page-ref (second clause))
@@ -557,7 +557,7 @@
                                                  repo (state/get-current-repo)
                                                  block (db/pull [:block/uuid (:block/uuid block)])]
                                              (when block
-                                               (if (:property config)
+                                               (if (:query-object? config)
                                                  (editor-handler/save-block! repo (:block/uuid block) q)
                                                  (let [content (string/replace (:block/title block)
                                                                                #"\{\{query[^}]+\}\}"

+ 2 - 6
src/main/frontend/components/query/result.cljs

@@ -39,10 +39,7 @@
                                  (let [result (->> blocks
                                                    (keep (fn [b]
                                                            (when-not (= (:block/uuid b) current-block-uuid)
-                                                             [:block/uuid (:block/uuid b)])))
-                                                  ;; Why pull-many here instead of `d/entity`?
-                                                   (db/pull-many (state/get-current-repo) '[*])
-                                                   (remove nil?))]
+                                                             (db/entity [:block/uuid (:block/uuid b)])))))]
                                    (reset! *fulltext-query-result result))))
                              *fulltext-query-result)
 
@@ -54,8 +51,7 @@
                      (catch :default e
                        (reset! *query-error e)
                        (atom nil)))]
-    (or query-atom
-        (atom nil))))
+    (or query-atom (atom nil))))
 
 (defn get-group-by-page [{:keys [result-transform query] :as query-m}
                          {:keys [table?]}]

+ 1 - 1
src/main/frontend/components/query/view.cljs

@@ -36,7 +36,7 @@
   (let [*result (::result state)
         result' (or @*result (init-result result view-entity))
         columns' (columns (assoc config :container-id (::container-id state)) result')]
-    [:div.query-result.w-full.mt-1
+    [:div.query-result.w-full
      {:on-pointer-down util/stop-propagation}
      (views/view view-entity
                  {:title-key :views.table/live-query-title

+ 1 - 1
src/main/frontend/extensions/fsrs.cljs

@@ -81,7 +81,7 @@
   [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 (:logseq.property/query cards))
+        query (:block/title cards)
         result (query-dsl/parse query {:db-graph? true})
         q '[:find [?b ...]
             :in $ ?now-inst-ms %

+ 15 - 1
src/main/frontend/handler/editor.cljs

@@ -3355,7 +3355,8 @@
   (let [property-keys (->> (keys (:block/properties block))
                            (remove db-property/db-attribute-properties)
                            (remove #(outliner-property/property-with-other-position? (db/entity %))))]
-    (or (and (seq property-keys)
+    (or (ldb/class-instance? (db/entity :logseq.class/Query) block)
+        (and (seq property-keys)
              (not (db-pu/all-hidden-properties? property-keys)))
         (and (seq (:block/tags block))
              (some (fn [t]
@@ -3828,3 +3829,16 @@
   (.setData (gobj/get event "dataTransfer")
             (if page? "page-name" "block-uuid")
             (str block-or-page-name)))
+
+(defn query-edit-title!
+  [block]
+  (let [query-block (:logseq.property/query block)
+        current-query (:block/title block)]
+    (p/do!
+     (state/clear-edit!)
+     (ui-outliner-tx/transact!
+      {:outliner-op :save-block}
+      (save-block-inner! block "" {})
+      (when query-block
+        (save-block-inner! query-block current-query {})))
+     (js/setTimeout #(edit-block! (db/entity (:db/id block)) :max) 100))))

+ 14 - 14
src/main/frontend/worker/db/migrate.cljs

@@ -101,23 +101,23 @@
   [props-to-rename]
   (fn [conn _search-db]
     (when (ldb/db-based-graph? @conn)
-     (let [props-tx (mapv (fn [[old new]]
-                            (merge {:db/id (:db/id (d/entity @conn old))
-                                    :db/ident new}
-                                   (when-let [new-title (get-in db-property/built-in-properties [new :title])]
-                                     {:block/title new-title
-                                      :block/name (common-util/page-name-sanity-lc new-title)})))
-                          props-to-rename)]
+      (let [props-tx (mapv (fn [[old new]]
+                             (merge {:db/id (:db/id (d/entity @conn old))
+                                     :db/ident new}
+                                    (when-let [new-title (get-in db-property/built-in-properties [new :title])]
+                                      {:block/title new-title
+                                       :block/name (common-util/page-name-sanity-lc new-title)})))
+                           props-to-rename)]
        ;; Property changes need to be in their own tx for subsequent uses of properties to take effect
-       (ldb/transact! conn props-tx {:db-migrate? true})
+        (ldb/transact! conn props-tx {:db-migrate? true})
 
-       (mapcat (fn [[old new]]
+        (mapcat (fn [[old new]]
                  ;; can't use datoms b/c user properties aren't indexed
-                 (->> (d/q '[:find ?b ?prop-v :in $ ?prop :where [?b ?prop ?prop-v]] @conn old)
-                      (mapcat (fn [[id prop-value]]
-                                [[:db/retract id old]
-                                 [:db/add id new prop-value]]))))
-               props-to-rename)))))
+                  (->> (d/q '[:find ?b ?prop-v :in $ ?prop :where [?b ?prop ?prop-v]] @conn old)
+                       (mapcat (fn [[id prop-value]]
+                                 [[:db/retract id old]
+                                  [:db/add id new prop-value]]))))
+                props-to-rename)))))
 
 (defn- update-block-type-many->one
   [conn _search-db]