From ec4504e47582e8cb72286ce92ca926b5c405b8d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20Schl=C3=BCter?= Date: Wed, 17 Oct 2018 12:58:37 +0200 Subject: [PATCH] Add loading indicator to breadcrumbs (#29) Also moves the current route data into a level 3 subscription (finally). --- src/cljs/airsonic_ui/api/subs.cljs | 45 +++++++++++++++---- src/cljs/airsonic_ui/views.cljs | 4 +- src/cljs/airsonic_ui/views/breadcrumbs.cljs | 17 +++++--- src/sass/app.sass | 4 +- test/cljs/airsonic_ui/api/subs_test.cljs | 48 ++++++++++++++------- 5 files changed, 83 insertions(+), 35 deletions(-) diff --git a/src/cljs/airsonic_ui/api/subs.cljs b/src/cljs/airsonic_ui/api/subs.cljs index b9260fa..fbb0881 100644 --- a/src/cljs/airsonic_ui/api/subs.cljs +++ b/src/cljs/airsonic_ui/api/subs.cljs @@ -3,12 +3,22 @@ [re-frame.core :refer [reg-sub]] [airsonic-ui.helpers :refer [kebabify]])) +(defn responses + "Returns the response cache" + [db _] + (:api/responses db)) + +(reg-sub :api/responses responses) + (defn response-for "Returns the cached response for a single endpoint" - [db [_ endpoint params]] - (get-in db [:api/responses [endpoint params]])) + [responses [_ endpoint params]] + (get responses [endpoint params])) -(reg-sub :api/response-for response-for) +(reg-sub + :api/response-for + :<- [:api/responses] + response-for) (defn endpoint->kw "Given an endpoint like `getAlbumList2`, returns a cleaned keyword like @@ -21,12 +31,29 @@ (str/replace #"\d+$" "") (kebabify))) -(defn route-data - "Given a list of event vectors, returns that responses for all API requests." - [db [_ route-events]] - (->> (filter #(= :api/request (first %)) route-events) +(defn current-route-data + "Returns all responses for the current route" + [[responses current-route-events] _] + (->> (filter #(= :api/request (first %)) current-route-events) (mapcat (fn [[_ endpoint params]] - [(endpoint->kw endpoint) (get-in db [:api/responses [endpoint params]])])) + [(endpoint->kw endpoint) (get responses [endpoint params])])) (apply hash-map))) -(reg-sub :api/route-data route-data) +(reg-sub + :api/current-route-data + :<- [:api/responses] + :<- [:routes/events-for-current-route] + current-route-data) + +(defn content-pending? + "Tells us if any of the requests fired for the current route are + awaiting responses." + [current-route-data _] + (->> (vals current-route-data) + (map :api/is-loading?) + (some true?))) + +(reg-sub + :api/content-pending? + :<- [:api/current-route-data] + content-pending?) diff --git a/src/cljs/airsonic_ui/views.cljs b/src/cljs/airsonic_ui/views.cljs index 4693854..4654d32 100644 --- a/src/cljs/airsonic_ui/views.cljs +++ b/src/cljs/airsonic_ui/views.cljs @@ -104,9 +104,7 @@ "Provides the complete UI to browse the media library, interact with search results etc" [[route-id :as route]] - (let [;; TODO: Move this to a layer 3 subscription ↓ - route-events @(subscribe [:routes/events-for-current-route]) - content @(subscribe [:api/route-data route-events])] + (let [content @(subscribe [:api/current-route-data])] [:div [:section.section [breadcrumbs route content] diff --git a/src/cljs/airsonic_ui/views/breadcrumbs.cljs b/src/cljs/airsonic_ui/views/breadcrumbs.cljs index 164a960..bbaf715 100644 --- a/src/cljs/airsonic_ui/views/breadcrumbs.cljs +++ b/src/cljs/airsonic_ui/views/breadcrumbs.cljs @@ -1,16 +1,21 @@ (ns airsonic-ui.views.breadcrumbs - (:require [airsonic-ui.routes :as routes :refer [url-for]])) + (:require [re-frame.core :refer [subscribe]] + [airsonic-ui.routes :as routes :refer [url-for]] + [airsonic-ui.views.loading-spinner :refer [loading-spinner]])) ;; Breadcrumbs are implemented in such a way that they provide a stringent ;; hierarchy no matter how you came to the url. They should allow easy ;; navigation upwards that hierarchy (e.g. album -> artist) (defn- bulma-breadcrumbs [& items] - [:div.container>nav.breadcrumb {:aria-label "breadcrumbs"} - [:ul - (for [[idx [href label]] (map-indexed vector (butlast items))] - [:li {:key idx} [:a {:href href} label]]) - [:li.is-active>a (last items)]]]) + (let [content-pending? @(subscribe [:api/content-pending?])] + [:div.container + [:nav.breadcrumb {:aria-label "breadcrumbs"} + [:ul + (for [[idx [href label]] (map-indexed vector (butlast items))] + [:li {:key idx} [:a {:href href} label]]) + [:li.is-active>a (last items) + (when content-pending? [loading-spinner])]]]])) (defmulti breadcrumbs (fn dispatch-on [[route-id] content] route-id)) diff --git a/src/sass/app.sass b/src/sass/app.sass index 5c54d57..9449a26 100644 --- a/src/sass/app.sass +++ b/src/sass/app.sass @@ -169,11 +169,11 @@ @keyframes you-spin-my-head-right-round from transform: rotate(0deg) - transform-origin: 49% 50% + transform-origin: 50% 46% to transform: rotate(359deg) - transform-origin: 49% 50% + transform-origin: 50% 46% .loading-spinner .icon diff --git a/test/cljs/airsonic_ui/api/subs_test.cljs b/test/cljs/airsonic_ui/api/subs_test.cljs index 463567e..abe5692 100644 --- a/test/cljs/airsonic_ui/api/subs_test.cljs +++ b/test/cljs/airsonic_ui/api/subs_test.cljs @@ -6,9 +6,9 @@ (deftest single-response (testing "Should return the response for a specified endpoint" - (let [db {:api/responses {["search2" {:query "query term"}] :result}}] - (is (= :result (sub/response-for db [:api/response-for "search2" {:query "query term"}]))) - (is (nil? (sub/response-for db [:api/response-for "search2" {:query "another query term"}])))))) + (let [responses (sub/responses {:api/responses {["search2" {:query "query term"}] :result}} [:api/responses])] + (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 endpoint-keywordification (testing "Should strip prefixes" @@ -18,18 +18,36 @@ (is (= :album-list (sub/endpoint->kw "getAlbumList2"))) (is (= :search (sub/endpoint->kw "search3"))))) +(def responses {["getAlbumList2" {:type "recent" :size 18}] + {:album [{:genre "foo", :artistId "12345"} + {:genre "electronic", :artistId "9999"}]} + + ["getArtistInfo" {:id "128"}] + {:biography "Interesting bio" + :largeImageUrl "https://lastfm-img2.akamaized.net/i/u/300x300/fb416b59cd694587aca0b2dec8f41198.png"}}) + (deftest responses-for-route (testing "Should return all cached responses for a route" - (let [route-events [[:api/request "getAlbumList2" {:type "recent", :size 18}] - [:event/should-be-ignored] - [:api/request "getArtistInfo" {:id "128"}]] - db {:api/responses {["getAlbumList2" {:type "recent" :size 18}] - {:album [{:genre "foo", :artistId "12345"} - {:genre "electronic", :artistId "9999"}]} + (let [current-route-events [[:api/request "getAlbumList2" {:type "recent", :size 18}] + [:event/should-be-ignored] + [:api/request "getArtistInfo" {:id "128"}]]] + (is (= {:album-list (get responses ["getAlbumList2" {:type "recent" :size 18}]) + :artist-info (get responses ["getArtistInfo" {:id "128"}])} + (sub/current-route-data [responses current-route-events] + [:api/current-route-data])))))) - ["getArtistInfo" {:id "128"}] - {:biography "Interesting bio" - :largeImageUrl "https://lastfm-img2.akamaized.net/i/u/300x300/fb416b59cd694587aca0b2dec8f41198.png"}}}] - (is (= {:album-list (get-in db [:api/responses ["getAlbumList2" {:type "recent" :size 18}]]) - :artist-info (get-in db [:api/responses ["getArtistInfo" {:id "128"}]])} - (sub/route-data db [:api/route-data route-events])))))) +(deftest content-pending + (testing "Should indicate if there are outstanding requests for the current route" + (let [current-route-events [[:api/request "getAlbumList2" {:type "recent", :size 18}] + [:event/should-be-ignored] + [:api/request "getArtistInfo" {:id "128"}]] + done responses + in-progress (assoc-in responses + [["getAlbumList2" {:type "recent" :size 18}] :api/is-loading?] + true)] + (is (true? (-> (sub/current-route-data [in-progress current-route-events] + [:api/current-route-data]) + (sub/content-pending? [:api/content-pending?])))) + (is (not (true? (-> (sub/current-route-data [done current-route-events] + [:api/current-route-data]) + (sub/content-pending? [:api/content-pending?]))))))))