From cefdcd542e4257e60be1cc2e28d59bdc017aec98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 17 Oct 2018 16:01:29 +0200 Subject: [PATCH] Add real pagination; fixes #28 (#30) --- src/cljs/airsonic_ui/api/subs.cljs | 16 +++- .../airsonic_ui/components/library/subs.cljs | 17 ++++ .../airsonic_ui/components/library/views.cljs | 96 ++++++++++++------- src/cljs/airsonic_ui/config.cljs | 3 + src/cljs/airsonic_ui/core.cljs | 1 + src/cljs/airsonic_ui/routes.cljs | 17 ++-- src/cljs/airsonic_ui/views.cljs | 6 +- test/cljs/airsonic_ui/api/subs_test.cljs | 8 ++ .../components/library/subs_test.cljs | 12 +++ 9 files changed, 133 insertions(+), 43 deletions(-) create mode 100644 src/cljs/airsonic_ui/components/library/subs.cljs create mode 100644 test/cljs/airsonic_ui/components/library/subs_test.cljs diff --git a/src/cljs/airsonic_ui/api/subs.cljs b/src/cljs/airsonic_ui/api/subs.cljs index fbb0881..1eea97c 100644 --- a/src/cljs/airsonic_ui/api/subs.cljs +++ b/src/cljs/airsonic_ui/api/subs.cljs @@ -11,7 +11,8 @@ (reg-sub :api/responses responses) (defn response-for - "Returns the cached response for a single endpoint" + "Returns the cached response for a single endpoint, respecting passed + url pramters." [responses [_ endpoint params]] (get responses [endpoint params])) @@ -20,9 +21,20 @@ :<- [:api/responses] response-for) +(defn responses-for-endpoint + "Returns a seq of all responses for an endpoint, ignoring url parameters and + looking only at the path" + [responses [_ endpoint]] + (into {} (filter (fn [[[k _] _]] (= endpoint k)) responses))) + +(reg-sub + :api/responses-for-endpoint + :<- [:api/responses] + responses-for-endpoint) + (defn endpoint->kw "Given an endpoint like `getAlbumList2`, returns a cleaned keyword like - `:album-list``. + `:album-list`. Rules: Kebab-case everything, remove prefixes like `get`, `create`, `delete`, `update` and strip trailing numbers." diff --git a/src/cljs/airsonic_ui/components/library/subs.cljs b/src/cljs/airsonic_ui/components/library/subs.cljs new file mode 100644 index 0000000..0003278 --- /dev/null +++ b/src/cljs/airsonic_ui/components/library/subs.cljs @@ -0,0 +1,17 @@ +(ns airsonic-ui.components.library.subs + (:require [re-frame.core :as re-frame] + [airsonic-ui.config :as conf])) + +(defn complete-library + "Concatenates all responses of one type of library to make paging through + it a bit easier." + [responses [_ kind]] + (->> (filter (fn [[[_ params] _]] + (= kind (:type params))) responses) + (sort-by (fn [[[_ params] _]] (:offset params))) + (mapcat (fn [[_ vals]] (:album vals))))) + +(re-frame/reg-sub + :library/complete + :<- [:api/responses-for-endpoint "getAlbumList2"] + complete-library) diff --git a/src/cljs/airsonic_ui/components/library/views.cljs b/src/cljs/airsonic_ui/components/library/views.cljs index f61cc07..e4bd43a 100644 --- a/src/cljs/airsonic_ui/components/library/views.cljs +++ b/src/cljs/airsonic_ui/components/library/views.cljs @@ -1,5 +1,7 @@ (ns airsonic-ui.components.library.views - (:require [airsonic-ui.routes :as routes :refer [url-for]] + (:require [re-frame.core :refer [subscribe]] + [airsonic-ui.config :as conf] + [airsonic-ui.routes :as routes :refer [url-for]] [airsonic-ui.components.collection.views :as collection] [airsonic-ui.helpers :refer [add-classes]])) @@ -11,41 +13,71 @@ {:class-name "is-active"}) [:a {:href (apply url-for route)} label]]))]]) +;; the pagination should be used like this +;; [pagination {:per-page 12 +;; :max-pages nil +;; :url-fn generate-url +;; :current-page 0 +;; :items [,,,] +;; :on-change (fn [current-page items] +;; (reset! current-items items))}] + +(defn num-pages [items per-page max-pages] + (min (Math/ceil (/ (count items) per-page)) max-pages)) + (defn pagination "Builds a pagination, calling `url-fn` for every rendered page link with the page as its argument. When `max-pages` is `nil` an infinite pagination will be rendered." - [{:keys [url-fn max-pages current-page]}] - [:nav.pagination {:role "pagination", :aria-label "pagination"} - [:a.pagination-previous (if (> current-page 1) - {:href (url-fn (dec current-page))} - {:disabled true}) "Previous page"] - [:a.pagination-next (if (= max-pages current-page) - {:disabled true} - {:href (url-fn (inc current-page))}) "Next page"] - [:ul.pagination-list - (when (> current-page 3) - ^{:key "ellipsis-before"} [:li>span.pagination-ellipsis "…"]) - (for [page (range (max 1 (- current-page 2)) - (if max-pages - (min (+ current-page 3) (inc max-pages)) - (+ current-page 3)))] - (let [current-page? (= page current-page)] - ^{:key page} [(cond-> :li>a.pagination-link - current-page? (add-classes :is-current)) - (cond-> {:href (url-fn page), :aria-label (str "Page " page)} - (= page current-page) (assoc :aria-current "page")) page])) - (when (or (not max-pages) (< current-page (- max-pages 2))) - ^{:key "ellipsis-after"} [:li>span.pagination-ellipsis "…"])]]) + [{:keys [items per-page max-pages current-page url-fn on-change] + :or {max-pages (.-MAX_VALUE js/Number)}}] + (let [num-pages (num-pages items per-page max-pages) + first-page? (= current-page 1) + last-page? (= current-page num-pages)] + (println "range" + (count items) + "num-pages" + num-pages) + [:nav.pagination {:role "pagination", :aria-label "pagination"} + [:a.pagination-previous (if first-page? + {:disabled true} + {:href (url-fn (dec current-page))}) "Previous page"] + [:a.pagination-next (if last-page? + {:disabled true} + {:href (url-fn (inc current-page))}) "Next page"] + [:ul.pagination-list + (when (> current-page 3) + ^{:key "ellipsis-before"} [:li>span.pagination-ellipsis "…"]) + (for [page (range (max 1 (- current-page 2)) + (min (+ current-page 3) (inc num-pages)))] + (let [current-page? (= page current-page)] + ^{:key page} [(cond-> :li>a.pagination-link + current-page? (add-classes :is-current)) + (cond-> {:href (url-fn page), :aria-label (str "Page " page)} + (= page current-page) (assoc :aria-current "page")) page])) + (when (< current-page (- num-pages 2)) + ^{:key "ellipsis-after"} [:li>span.pagination-ellipsis "…"])]])) -(defn main [route {:keys [scan-status album-list]}] - (let [[_ {:keys [criteria]} {:keys [page] :or {page 1}}] route - tab-items [[[::routes/library {:criteria "recent"} nil] "Recently played"] - [[::routes/library {:criteria "newest"} nil] "Newest additions"] - [[::routes/library {:criteria "starred"} nil] "Starred"]] +(def tab-items [[[::routes/library {:kind "recent"} nil] "Recently played"] + [[::routes/library {:kind "newest"} nil] "Newest additions"] + [[::routes/library {:kind "starred"} nil] "Starred"]]) + +(defn main + "Renders the pagination and shows a list of all albums with their cover art. + The first parameter is the route that's passed in, the second one is the + content that has been fetched for that route." + [[_ {:keys [kind]} {:keys [page] + :or {page 1}}] + {:keys [scan-status]}] + (let [library @(subscribe [:library/complete kind]) + ;; FIXME: vv Views shouldn't do calculations vv + visible (->> (drop (* (dec page) conf/albums-per-page) library) + (take conf/albums-per-page)) + url-fn #(url-for ::routes/library {:kind kind} {:page %}) pagination [pagination {:current-page (int page) - :max-pages 5 - :url-fn #(url-for ::routes/library {:criteria criteria} {:page %})}]] + :per-page conf/albums-per-page + :items library + :url-fn url-fn}]] [:div [:section.hero.is-small>div.hero-body>div.container [:h2.title "Your library"] @@ -54,7 +86,7 @@ (when (:scanning scan-status) [:p.subtitle.is-5.has-text-grey "Scanning…"]))] [:section.section>div.container - [tabs {:items tab-items :active-item {:criteria criteria}}] + [tabs {:items tab-items :active-item {:kind kind}}] pagination - [:section.section [collection/listing (:album album-list)]] + [:section.section [collection/listing visible]] pagination]])) diff --git a/src/cljs/airsonic_ui/config.cljs b/src/cljs/airsonic_ui/config.cljs index f3eeae0..8e61c5c 100644 --- a/src/cljs/airsonic_ui/config.cljs +++ b/src/cljs/airsonic_ui/config.cljs @@ -2,3 +2,6 @@ (def debug? ^boolean goog.DEBUG) + +;; how many covers are shown per page when browsing the library +(def albums-per-page 20) diff --git a/src/cljs/airsonic_ui/core.cljs b/src/cljs/airsonic_ui/core.cljs index d668679..93e6579 100644 --- a/src/cljs/airsonic_ui/core.cljs +++ b/src/cljs/airsonic_ui/core.cljs @@ -11,6 +11,7 @@ [airsonic-ui.api.events] [airsonic-ui.api.subs] [airsonic-ui.components.audio-player.events] + [airsonic-ui.components.library.subs] [airsonic-ui.components.search.events] [airsonic-ui.components.search.subs] [airsonic-ui.events :as events] diff --git a/src/cljs/airsonic_ui/routes.cljs b/src/cljs/airsonic_ui/routes.cljs index fcb2ed1..3a178e8 100644 --- a/src/cljs/airsonic_ui/routes.cljs +++ b/src/cljs/airsonic_ui/routes.cljs @@ -1,14 +1,15 @@ (ns airsonic-ui.routes (:require [bide.core :as r] [cljs.reader :refer [read-string]] - [re-frame.core :as re-frame])) + [re-frame.core :as re-frame] + [airsonic-ui.config :as conf])) (def default-route ::login) (defonce router (r/router [["/" ::login] ["/library" ::library] - ["/library/:criteria" ::library] + ["/library/:kind" ::library] ["/artists" ::artist.overview] ["/artists/:id" ::artist.detail] ["/album/:id" ::album.detail] @@ -42,11 +43,15 @@ (defmethod -route-events :default [route-id params query]) (defmethod -route-events ::library - [route-id {:keys [criteria]} {:keys [page]}] - (if criteria + [route-id {:keys [kind]} {:keys [page] :or {page 1}}] + (if kind [[:api/request "getScanStatus"] - [:api/request "getAlbumList2" {:type criteria, :size 20, :offset (* 20 (dec page))}]] - [:routes/do-navigation [route-id {:criteria "recent"} {:page 1}]])) + ;; we fetch more than just the albums needed for the current page so we can + ;; page through it faster + [:api/request "getAlbumList2" {:type kind + :size (* 3 conf/albums-per-page) + :offset (* page conf/albums-per-page)}]] + [:routes/do-navigation [route-id {:kind "recent"} {:page 1}]])) (defmethod -route-events ::artist.overview [route-id params query] diff --git a/src/cljs/airsonic_ui/views.cljs b/src/cljs/airsonic_ui/views.cljs index 4654d32..04a7234 100644 --- a/src/cljs/airsonic_ui/views.cljs +++ b/src/cljs/airsonic_ui/views.cljs @@ -71,9 +71,9 @@ :title "Current queue"} [icon :audio-spectrum]] (when stream-role [navbar-dropdown "Library" - [[{:href (url-for ::routes/library {:criteria "recent"})} "Recently played"] - [{:href (url-for ::routes/library {:criteria "newest"})} "Newest additions"] - [{:href (url-for ::routes/library {:criteria "starred"})} "Starred"] + [[{:href (url-for ::routes/library {:kind "recent"})} "Recently played"] + [{:href (url-for ::routes/library {:kind "newest"})} "Newest additions"] + [{:href (url-for ::routes/library {:kind "starred"})} "Starred"] [{:href (url-for ::routes/artist.overview)} "By artist"]]]) (when podcast-role #_(let [podcast-url (url-for ::routes/podcast.overview)] diff --git a/test/cljs/airsonic_ui/api/subs_test.cljs b/test/cljs/airsonic_ui/api/subs_test.cljs index abe5692..43399c8 100644 --- a/test/cljs/airsonic_ui/api/subs_test.cljs +++ b/test/cljs/airsonic_ui/api/subs_test.cljs @@ -10,6 +10,14 @@ (is (= :result (sub/response-for responses [:api/response-for "search2" {:query "query term"}]))) (is (nil? (sub/response-for responses [:api/response-for "search2" {:query "another query term"}])))))) +(deftest responses-for-endpoint + (testing "Should concatenate all responses for an endpoint" + (let [responses {["search2" {:query "query term"}] :result1 + ["something-else" nil] :ignored-result + ["search2" {:query "another query term"}] :result2}] + (is (= (dissoc responses ["something-else" nil]) + (sub/responses-for-endpoint responses [:api/responses-for-endpoint "search2"])))))) + (deftest endpoint-keywordification (testing "Should strip prefixes" (is (= :artist-info (sub/endpoint->kw "getArtistInfo"))) diff --git a/test/cljs/airsonic_ui/components/library/subs_test.cljs b/test/cljs/airsonic_ui/components/library/subs_test.cljs new file mode 100644 index 0000000..df818f1 --- /dev/null +++ b/test/cljs/airsonic_ui/components/library/subs_test.cljs @@ -0,0 +1,12 @@ +(ns airsonic-ui.components.library.subs-test + (:require [cljs.test :refer-macros [deftest testing is]] + [airsonic-ui.components.library.subs :as sub])) + +(def responses {["getAlbumList2" {:type "recent" :offset 1}] {:album '(5 6 7 8)} + ["getAlbumList2" {:type "recent" :offset 0}] {:album '(1 2 3 4)} + ["getAlbumList2" {:type "newest" :offset 1}] {:album '(9 8 7 6)}}) + +(deftest complete-library + (testing "Should concatenate all album list responses for a given type of list" + (is (= '(1 2 3 4 5 6 7 8) + (sub/complete-library responses [:library/complete "recent"])))))