Enhance/PDF viewer (#7369)

* fix(pdf): WIP potential memory leaks
* enhance(pdf): sync page number in highlights metafile
* enhance(pdf): support preview highlight area image in a lightbox
* fix: clojurescript unit tests
* fix(pdf): page number overflow when more digits

src/main/frontend/components/block.cljs

@@ -61,6 +61,7 @@
             [frontend.util.drawer :as drawer]
             [frontend.util.property :as property]
             [frontend.util.text :as text-util]
+            [frontend.handler.notification :as notification]
             [goog.dom :as gdom]
             [goog.object :as gobj]
             [lambdaisland.glogi :as log]
@@ -264,14 +265,6 @@
     (when (seq images)
       (lightbox/preview-images! images))))
 
-(defn copy-image-to-clipboard
-  [src]
-  (-> (js/fetch src)
-      (.then (fn [data]
-               (-> (.blob data)
-                   (.then (fn [blob]
-                            (js/navigator.clipboard.write (clj->js [(js/ClipboardItem. (clj->js {(.-type blob) blob}))])))))))))
-
 (defonce *resizing-image? (atom false))
 (rum/defcs resizable-image <
   (rum/local nil ::size)
@@ -358,7 +351,8 @@
              :on-mouse-down util/stop
              :on-click      (fn [e]
                               (util/stop e)
-                         (copy-image-to-clipboard image-src))}
+                              (-> (util/copy-image-to-clipboard image-src)
+                                  (p/then #(notification/show! "Copied!" :success))))}
             (ui/icon "copy")]
 
            [:button.asset-action-btn
@@ -1045,7 +1039,7 @@
         [:a.asset-ref.is-pdf
          {:on-mouse-down (fn [_event]
                            (when-let [current (pdf-assets/inflate-asset s)]
-                             (state/set-state! :pdf/current current)))}
+                             (state/set-current-pdf! current)))}
          (or label-text
              (->elem :span (map-inline config label)))]
 

src/main/frontend/components/page.cljs

@@ -16,6 +16,7 @@
             [frontend.db.model :as model]
             [frontend.extensions.graph :as graph]
             [frontend.extensions.pdf.assets :as pdf-assets]
+            [frontend.extensions.pdf.utils :as pdf-utils]
             [frontend.format.block :as block]
             [frontend.handler.common :as common-handler]
             [frontend.handler.config :as config-handler]
@@ -282,7 +283,7 @@
           whiteboard-page? (model/whiteboard-page? page-name)
           untitled? (and whiteboard-page? (parse-uuid page-name)) ;; normal page cannot be untitled right?
           title (if hls-page?
-                  [:a.asset-ref (pdf-assets/fix-local-asset-pagename title)]
+                  [:a.asset-ref (pdf-utils/fix-local-asset-pagename title)]
                   (if fmt-journal? (date/journal-title->custom-format title) title))
           old-name (or title page-name)]
       [:h1.page-title.flex.cursor-pointer.gap-1.w-full

src/main/frontend/components/search.cljs

@@ -13,7 +13,7 @@
             [frontend.db.model :as model]
             [frontend.handler.search :as search-handler]
             [frontend.handler.whiteboard :as whiteboard-handler]
-            [frontend.extensions.pdf.assets :as pdf-assets]
+            [frontend.extensions.pdf.utils :as pdf-utils]
             [frontend.ui :as ui]
             [frontend.state :as state]
             [frontend.mixins :as mixins]
@@ -203,7 +203,7 @@
 (defn- search-item-render
   [search-q {:keys [type data alias]}]
   (let [search-mode (state/get-search-mode)
-        data (if (string? data) (pdf-assets/fix-local-asset-pagename data) data)]
+        data (if (string? data) (pdf-utils/fix-local-asset-pagename data) data)]
     [:div {:class "py-2"}
      (case type
        :graph-add-filter

src/main/frontend/components/sidebar.cljs

@@ -18,7 +18,7 @@
             [frontend.db :as db]
             [frontend.db-mixins :as db-mixins]
             [frontend.db.model :as db-model]
-            [frontend.extensions.pdf.assets :as pdf-assets]
+            [frontend.extensions.pdf.utils :as pdf-utils]
             [frontend.extensions.srs :as srs]
             [frontend.handler.common :as common-handler]
             [frontend.handler.editor :as editor-handler]
@@ -89,7 +89,7 @@
               (route-handler/redirect-to-whiteboard! name)
               (route-handler/redirect-to-page! name {:click-from-recent? recent?})))))}
      [:span.page-icon (if whiteboard-page? (ui/icon "whiteboard" {:extension? true}) icon)]
-     [:span.page-title (pdf-assets/fix-local-asset-pagename original-name)]]))
+     [:span.page-title (pdf-utils/fix-local-asset-pagename original-name)]]))
 
 (defn get-page-icon [page-entity]
   (let [default-icon (ui/icon "page" {:extension? true})

src/main/frontend/extensions/pdf/assets.cljs

@@ -8,6 +8,10 @@
             [frontend.handler.editor :as editor-handler]
             [frontend.handler.page :as page-handler]
             [frontend.handler.assets :as assets-handler]
+            [frontend.handler.notification :as notification]
+            [frontend.ui :as ui]
+            [frontend.context.i18n :refer [t]]
+            [frontend.extensions.lightbox :as lightbox]
             [frontend.util.page-property :as page-property]
             [frontend.state :as state]
             [frontend.util :as util]
@@ -59,11 +63,11 @@
         data))))
 
 (defn persist-hls-data$
-  [{:keys [hls-file]} highlights]
+  [{:keys [hls-file]} highlights extra]
   (when hls-file
     (let [repo-cur (state/get-current-repo)
           repo-dir (config/get-repo-dir repo-cur)
-          data (pr-str {:highlights highlights})]
+          data (pr-str {:highlights highlights :extra extra})]
       (fs/write-file! repo-cur repo-dir hls-file data {:skip-compare? true}))))
 
 (defn resolve-hls-data-by-key$
@@ -226,7 +230,7 @@
             (do
               (state/set-state! :pdf/ref-highlight matched)
               ;; open pdf viewer
-              (state/set-state! :pdf/current (inflate-asset file-path)))
+              (state/set-current-pdf! (inflate-asset file-path)))
             (js/console.debug "[Unmatched highlight ref]" block)))))))
 
 (defn goto-block-ref!
@@ -242,32 +246,57 @@
    (when-let [name (:key current)]
      (rfe/push-state :page {:name (str "hls__" name)} (if id {:anchor (str "block-content-" + id)} nil)))))
 
+(defn open-lightbox
+  [e]
+  (let [images (js/document.querySelectorAll ".hl-area img")
+        images (to-array images)
+        images (if-not (= (count images) 1)
+                 (let [^js image (.closest (.-target e) ".hl-area")
+                       image (. image querySelector "img")]
+                   (->> images
+                        (sort-by (juxt #(.-y %) #(.-x %)))
+                        (split-with (complement #{image}))
+                        reverse
+                        (apply concat)))
+                 images)
+        images (for [^js it images] {:src (.-src it)
+                                     :w (.-naturalWidth it)
+                                     :h (.-naturalHeight it)})]
+
+    (when (seq images)
+      (lightbox/preview-images! images))))
+
 (rum/defc area-display
   [block]
   (when-let [asset-path' (and block (pdf-utils/get-area-block-asset-url
                                      block (db-utils/pull (:db/id (:block/page block)))))]
     (let [asset-path (editor-handler/make-asset-url asset-path')]
       [:span.hl-area
-       [:img {:src asset-path}]])))
+       [:span.actions
+        (when-not config/publishing?
+          [:button.asset-action-btn.px-1
+           {:title         (t :asset/copy)
+            :tabIndex      "-1"
+            :on-mouse-down util/stop
+            :on-click      (fn [e]
+                             (util/stop e)
+                             (-> (util/copy-image-to-clipboard (gp-config/remove-asset-protocol asset-path))
+                                 (p/then #(notification/show! "Copied!" :success))))}
+           (ui/icon "copy")])
 
-(defn fix-local-asset-pagename
-  [filename]
-  (when-not (string/blank? filename)
-    (let [local-asset? (re-find #"[0-9]{13}_\d$" filename)
-          hls?         (re-find #"^hls__" filename)
-          len          (count filename)]
-      (if (or local-asset? hls?)
-        (-> filename
-            (subs 0 (if local-asset? (- len 15) len))
-            (string/replace #"^hls__" "")
-            (string/replace "_" " ")
-            (string/trimr))
-        filename))))
+        [:button.asset-action-btn.px-1
+         {:title         (t :asset/maximize)
+          :tabIndex      "-1"
+          :on-mouse-down util/stop
+          :on-click      open-lightbox}
+
+         (ui/icon "maximize")]]
+       [:img {:src asset-path}]])))
 
 (defn human-page-name
   [page-name]
   (cond
     (string/starts-with? page-name "hls__")
-    (fix-local-asset-pagename page-name)
+    (pdf-utils/fix-local-asset-pagename page-name)
 
     :else (util/trim-safe page-name)))

src/main/frontend/extensions/pdf/highlights.cljs

@@ -12,7 +12,6 @@
             [frontend.commands :as commands]
             [frontend.rum :refer [use-atom]]
             [frontend.state :as state]
-            [frontend.storage :as storage]
             [frontend.util :as util]
             [medley.core :as medley]
             [promesa.core :as p]
@@ -44,14 +43,12 @@
   (rum/use-effect!
    (fn []
      (when viewer
-       (when-let [current (:pdf/current @state/state)]
-         (let [active-hl (:pdf/ref-highlight @state/state)
-               page-key  (:filename current)
-               last-page (and page-key
-                              (util/safe-parse-int (storage/get (str "ls-pdf-last-page-" page-key))))]
-
-           (when (and last-page (nil? active-hl))
-             (set! (.-currentPageNumber viewer) last-page))))))
+       (when-let [_ (:pdf/current @state/state)]
+         (let [active-hl (:pdf/ref-highlight @state/state)]
+           (when-not active-hl
+             (.on (.-eventBus viewer) (name :restore-last-page)
+                  (fn [last-page]
+                    (set! (.-currentPageNumber viewer) (util/safe-parse-int last-page)))))))))
    [viewer])
   nil)
 
@@ -665,7 +662,7 @@
        })]))
 
 (rum/defc pdf-viewer
-  [url initial-hls ^js pdf-document ops]
+  [_url initial-hls initial-page ^js pdf-document ops]
 
   (let [*el-ref (rum/create-ref)
         [state, set-state!] (rum/use-state {:viewer nil :bus nil :link nil :el nil})
@@ -675,7 +672,8 @@
 
     ;; instant pdfjs viewer
     (rum/use-effect!
-     (fn [] (let [^js event-bus    (js/pdfjsViewer.EventBus.)
+     (fn []
+       (let [^js event-bus    (js/pdfjsViewer.EventBus.)
              ^js link-service (js/pdfjsViewer.PDFLinkService. #js {:eventBus event-bus :externalLinkTarget 2})
              ^js el           (rum/deref *el-ref)
              ^js viewer       (js/pdfjsViewer.PDFViewer.
@@ -687,21 +685,37 @@
                                     :textLayerMode     2
                                     :annotationMode    2
                                     :removePageBorders true})]
+
          (. link-service setDocument pdf-document)
          (. link-service setViewer viewer)
 
-              ;; TODO: debug
-              (set! (. js/window -lsPdfViewer) viewer)
+         ;; events
+         (doto event-bus
+           ;; it must be initialized before set-up document
+           (.on "pagesinit"
+                (fn []
+                  (set! (. viewer -currentScaleValue) "auto")
+                  (set-page-ready! true)))
+
+           (.on (name :ls-update-extra-state)
+                #(when-let [extra (bean/->clj %)]
+                   (apply (:set-hls-extra! ops) [extra]))))
 
          (p/then (. viewer setDocument pdf-document)
                  #(set-state! {:viewer viewer :bus event-bus :link link-service :el el}))
 
-              ;;TODO: destroy
-              (fn []
-                (when-let [last-page (.-currentPageNumber viewer)]
-                  (storage/set (str "ls-pdf-last-page-" (util/node-path.basename url)) last-page))
+         ;; TODO: debug
+         (set! (. js/window -lsPdfViewer) viewer)
 
-                (when pdf-document (.destroy pdf-document)))))
+         ;; set initial page
+         (js/setTimeout
+          #(set! (.-currentPageNumber viewer) initial-page) 16)
+
+         ;; destroy
+         (fn []
+           (.destroy pdf-document)
+           (set! (. js/window -lsPdfViewer) nil)
+           (.cleanup viewer))))
      [])
 
     ;; interaction events
@@ -710,20 +724,13 @@
        (when-let [^js viewer (:viewer state)]
          (let [fn-textlayer-ready
                (fn [^js p]
-                 (set-ano-state! {:loaded-pages (conj (:loaded-pages ano-state) (int (.-pageNumber p)))}))
-
-               fn-page-ready
-               (fn []
-                 (set! (. viewer -currentScaleValue) "auto")
-                 (set-page-ready! true))]
+                 (set-ano-state! {:loaded-pages (conj (:loaded-pages ano-state) (int (.-pageNumber p)))}))]
 
            (doto (.-eventBus viewer)
-             (.on "pagesinit" fn-page-ready)
              (.on "textlayerrendered" fn-textlayer-ready))
 
            #(do
               (doto (.-eventBus viewer)
-                (.off "pagesinit" fn-page-ready)
                 (.off "textlayerrendered" fn-textlayer-ready))))))
 
      [(:viewer state)
@@ -750,23 +757,27 @@
 (rum/defc ^:large-vars/data-var pdf-loader
   [{:keys [url hls-file] :as pdf-current}]
   (let [*doc-ref       (rum/use-ref nil)
-        [state, set-state!] (rum/use-state {:error nil :pdf-document nil :status nil})
-        [hls-state, set-hls-state!] (rum/use-state {:initial-hls nil :latest-hls nil})
+        [loader-state, set-loader-state!] (rum/use-state {:error nil :pdf-document nil :status nil})
+        [hls-state, set-hls-state!] (rum/use-state {:initial-hls nil :latest-hls nil :extra nil :loaded false})
+        [initial-page, set-initial-page!] (rum/use-state 0)
         set-dirty-hls! (fn [latest-hls]  ;; TODO: incremental
-                         (set-hls-state! {:initial-hls [] :latest-hls latest-hls}))]
+                         (set-hls-state! #(merge % {:initial-hls [] :latest-hls latest-hls})))
+        set-hls-extra! (fn [extra]
+                         (set-hls-state! #(merge % {:extra extra})))]
 
     ;; load highlights
     (rum/use-effect!
      (fn []
        (p/catch
         (p/let [data (pdf-assets/load-hls-data$ pdf-current)
-                highlights (:highlights data)]
-          (set-hls-state! {:initial-hls highlights}))
+                {:keys [highlights extra]} data]
+          (set-initial-page! (util/safe-parse-int (:page extra)))
+          (set-hls-state! {:initial-hls highlights :latest-hls highlights :extra extra :loaded true}))
 
         ;; error
         (fn [e]
           (js/console.error "[load hls error]" e)
-          (set-hls-state! {:initial-hls []})))
+          (set-hls-state! {:initial-hls [] :loaded true})))
 
        ;; cancel
        #())
@@ -775,15 +786,16 @@
     ;; cache highlights
     (rum/use-effect!
      (fn []
-       (when-let [hls (:latest-hls hls-state)]
+       (when (= :completed (:status loader-state))
          (p/catch
-          (pdf-assets/persist-hls-data$ pdf-current hls)
+          (pdf-assets/persist-hls-data$
+           pdf-current (:latest-hls hls-state) (:extra hls-state))
 
           ;; write hls file error
           (fn [e]
             (js/console.error "[write hls error]" e)))))
 
-     [(:latest-hls hls-state)])
+     [(:latest-hls hls-state) (:extra hls-state)])
 
     ;; load document
     (rum/use-effect!
@@ -795,20 +807,18 @@
                        ;;:cMapUrl       "https://cdn.jsdelivr.net/npm/pdfjs-dist@2.8.335/cmaps/"
                        :cMapPacked    true}]
 
-         (set-state! {:status :loading})
+         (set-loader-state! {:status :loading})
 
          (-> (get-doc$ (clj->js opts))
-             (p/then #(set-state! {:pdf-document %}))
-             (p/catch #(set-state! {:error %}))
-             (p/finally #(set-state! {:status :completed})))
-
+             (p/then #(set-loader-state! {:pdf-document % :status :completed}))
+             (p/catch #(set-loader-state! {:error %})))
          #()))
      [url])
 
     (rum/use-effect!
      (fn []
-       (when-let [error (:error state)]
-         (dd "[ERROR loader]" (:error state))
+       (when-let [error (:error loader-state)]
+         (dd "[ERROR loader]" (:error loader-state))
          (case (.-name error)
            "MissingPDFException"
            (do
@@ -835,24 +845,24 @@
               :error
               false)
              (state/set-state! :pdf/current nil)))))
-     [(:error state)])
+     [(:error loader-state)])
 
     (rum/bind-context
      [*highlights-ctx* hls-state]
      [:div.extensions__pdf-loader {:ref *doc-ref}
-      (let [status-doc  (:status state)
+      (let [status-doc  (:status loader-state)
             initial-hls (:initial-hls hls-state)]
 
-        (if (or (= status-doc :loading)
-                (nil? initial-hls))
+        (if (= status-doc :loading)
 
           [:div.flex.justify-center.items-center.h-screen.text-gray-500.text-lg
            svg/loading]
 
+          (when-let [pdf-document (and (:loaded hls-state) (:pdf-document loader-state))]
             [(rum/with-key (pdf-viewer
-                          url initial-hls
-                          (:pdf-document state)
-                          {:set-dirty-hls! set-dirty-hls!}) "pdf-viewer")]))])))
+                            url initial-hls initial-page pdf-document
+                            {:set-dirty-hls! set-dirty-hls!
+                             :set-hls-extra! set-hls-extra!}) "pdf-viewer")])))])))
 
 (rum/defc pdf-container
   [{:keys [identity] :as pdf-current}]

src/main/frontend/extensions/pdf/pdf.css

@@ -107,9 +107,15 @@ input::-webkit-inner-spin-button {
             width: 35px;
             text-align: right;
             padding-right: 4px;
+            padding-left: 2px;
             height: 18px;
             border: none;
             background: transparent;
+            font-size: 15px;
+            
+            &.is-long {
+              font-size: 12px;
+            }
           }
         }
 
@@ -841,6 +847,16 @@ input::-webkit-inner-spin-button {
       overflow: hidden;
       margin-top: 4px;
 
+      .actions {
+        @apply absolute right-1 top-1 flex opacity-0 transition-opacity;
+      }
+
+      &:hover {
+        .actions {
+          @apply opacity-100;
+        }
+      }
+
       img {
         margin: 0;
         box-shadow: none;

src/main/frontend/extensions/pdf/toolbar.cljs

@@ -433,6 +433,14 @@
          #(js-delete (. el -dataset) "theme")))
      [viewer-theme])
 
+    ;; export page state
+    (rum/use-effect!
+     (fn []
+       (when viewer
+         (.dispatch (.-eventBus viewer) (name :ls-update-extra-state)
+                    #js {:page current-page-num})))
+     [viewer current-page-num])
+
     ;; pager hooks
     (rum/use-effect!
      (fn []
@@ -511,14 +519,16 @@
         [:span.nu.flex.items-center.opacity-70
          [:input {:ref            *page-ref
                   :type           "number"
+                  :class          (util/classnames [{:is-long (> (util/safe-parse-int current-page-num) 999)}])
                   :default-value  current-page-num
                   :on-mouse-enter #(.select ^js (.-target %))
                   :on-key-up      (fn [^js e]
                                     (let [^js input (.-target e)
                                           value     (util/safe-parse-int (.-value input))]
+                                      (set-current-page-num! value)
                                       (when (and (= (.-keyCode e) 13) value (> value 0))
-                                        (set! (. viewer -currentPageNumber)
-                                              (if (> value total-page-num) total-page-num value)))))}]
+                                        (->> (if (> value total-page-num) total-page-num value)
+                                             (set! (. viewer -currentPageNumber))))))}]
          [:small "/ " total-page-num]]
 
         [:span.ct.flex.items-center

src/main/frontend/extensions/pdf/utils.cljs

@@ -173,6 +173,20 @@
           (string/replace #"\|#\|([a-zA-Z_])" " $1")
           (string/replace sp "")))))
 
+(defn fix-local-asset-pagename
+  [filename]
+  (when-not (string/blank? filename)
+    (let [local-asset? (re-find #"[0-9]{13}_\d$" filename)
+          hls?         (re-find #"^hls__" filename)
+          len          (count filename)]
+      (if (or local-asset? hls?)
+        (-> filename
+            (subs 0 (if local-asset? (- len 15) len))
+            (string/replace #"^hls__" "")
+            (string/replace "_" " ")
+            (string/trimr))
+        filename))))
+
 ;; TODO: which viewer instance?
 (defn next-page
   []

src/main/frontend/state.cljs

@@ -296,10 +296,6 @@
 ;;  (re-)fetches get-current-repo needlessly
 ;; TODO: Add consistent validation. Only a few config options validate at get time
 
-(defn get-current-pdf
-  []
-  (:pdf/current @state))
-
 (def default-config
   "Default config for a repo-specific, user config"
   {:feature/enable-search-remove-accents? true
@@ -1967,3 +1963,16 @@ Similar to re-frame subscriptions"
   []
   (when (mobile-util/native-ios?)
     (get-in @state [:mobile/container-urls :iCloudContainerUrl])))
+
+(defn get-current-pdf
+  []
+  (:pdf/current @state))
+
+(defn set-current-pdf!
+  [inflated-file]
+  (let [settle-file! #(set-state! :pdf/current inflated-file)]
+    (if-not (get-current-pdf)
+      (settle-file!)
+      (when (apply not= (map :identity [inflated-file (get-current-pdf)]))
+        (set-state! :pdf/current nil)
+        (js/setTimeout #(settle-file!) 16)))))

src/main/frontend/util.cljc

@@ -1419,3 +1419,13 @@
               (<= (+ (.-bottom r) 64)
                   (or (.-innerHeight js/window)
                       (js/document.documentElement.clientHeight))))))))
+
+#?(:cljs
+   (defn copy-image-to-clipboard
+     [src]
+     (-> (js/fetch src)
+         (.then (fn [data]
+                  (-> (.blob data)
+                      (.then (fn [blob]
+                               (js/navigator.clipboard.write (clj->js [(js/ClipboardItem. (clj->js {(.-type blob) blob}))]))))
+                      (.catch js/console.error)))))))

src/test/frontend/extensions/pdf/assets_test.cljs

@@ -1,15 +1,15 @@
 (ns frontend.extensions.pdf.assets-test
   (:require [clojure.test :as test :refer [are deftest testing]]
-            [frontend.extensions.pdf.assets :as assets]))
+            [frontend.extensions.pdf.utils :as pdf-utils]))
 
 (deftest fix-local-asset-pagename
   (testing "matched filenames"
-    (are [x y] (= y (assets/fix-local-asset-pagename x))
+    (are [x y] (= y (pdf-utils/fix-local-asset-pagename x))
       "2015_Book_Intertwingled_1659920114630_0" "2015 Book Intertwingled"
       "hls__2015_Book_Intertwingled_1659920114630_0" "2015 Book Intertwingled"
       "hls/2015_Book_Intertwingled_1659920114630_0" "hls/2015 Book Intertwingled"))
   (testing "non matched filenames"
-    (are [x y] (= y (assets/fix-local-asset-pagename x))
+    (are [x y] (= y (pdf-utils/fix-local-asset-pagename x))
       "foo" "foo"
       "foo_bar" "foo_bar"
       "foo__bar" "foo__bar"