From 930bf553900f9e9143cdce38225b8e7d99122e37 Mon Sep 17 00:00:00 2001 From: heyarne Date: Sun, 8 Dec 2019 00:56:45 +0100 Subject: [PATCH] Move artists into library (#68) * Use more sensible naming for api responses * Move artist overview into library; closes #50 and #52 * Fix sass live-reload * Move editor config out of shadow-cljs.edn --- package.json | 6 ++-- shadow-cljs.edn | 4 +-- src/assets/index.html | 2 +- src/cljs/airsonic_ui/api/events.cljs | 12 +++---- .../airsonic_ui/components/artist/views.cljs | 8 +++-- .../airsonic_ui/components/library/views.cljs | 13 +++++--- src/cljs/airsonic_ui/events.cljs | 2 +- src/cljs/airsonic_ui/views.cljs | 10 +++--- src/cljs/airsonic_ui/views/breadcrumbs.cljs | 32 +++++++++++++------ src/cljs/bulma/tabs.cljs | 2 +- src/sass/app.sass | 20 ++++++++++-- test/cljs/airsonic_ui/api/events_test.cljs | 16 +++++----- test/cljs/airsonic_ui/events_test.cljs | 2 +- 13 files changed, 82 insertions(+), 47 deletions(-) diff --git a/package.json b/package.json index 055215f..4ae5782 100644 --- a/package.json +++ b/package.json @@ -5,15 +5,15 @@ "main": "index.js", "scripts": { "build:cljs": "shadow-cljs release app", - "build:sass": "node-sass --output-style compressed src/sass/app.sass | postcss -o public/app/style.css", + "build:sass": "node-sass --output-style compressed src/sass/app.sass | postcss -o public/app/app.css", "build": "rm -r public/*; run-p copy:* build:*", "copy:assets": "cp -R src/assets/* public/", "copy:icons": "cp -R node_modules/open-iconic/font/fonts public", "deploy": "npm run build && gh-pages -d public -m \"Deploying $(git rev-parse --short HEAD)\"", "dev:cljs": "shadow-cljs watch app test", - "dev:sass": "npm run build:sass; node-sass -w src/sass/app.sass | postcss -o public/app/style.css", + "dev:sass": "node-sass -w src/sass/app.sass -o public/app", "dev:test": "karma start --reporters notify,progress --auto-watch", - "dev": "rm -r public/*; npm-run-all copy:* test:compile -p dev:*", + "dev": "rm -r public/*; npm-run-all build:sass copy:* test:compile -p dev:*", "test": "run-s test:compile test:run", "test:compile": "shadow-cljs compile test", "test:run": "karma start --single-run" diff --git a/shadow-cljs.edn b/shadow-cljs.edn index 27a7179..bae1304 100644 --- a/shadow-cljs.edn +++ b/shadow-cljs.edn @@ -13,9 +13,7 @@ ;; debugging [day8.re-frame/re-frame-10x "0.4.5"] #_[day8.re-frame/tracing "0.5.1"] - [philoskim/debux "0.5.6"] - ;; for CIDER - [cider/cider-nrepl "0.21.1"]] + [philoskim/debux "0.5.6"]] :nrepl {:port 9000} diff --git a/src/assets/index.html b/src/assets/index.html index b238243..940932a 100644 --- a/src/assets/index.html +++ b/src/assets/index.html @@ -4,7 +4,7 @@ Airsonic - + diff --git a/src/cljs/airsonic_ui/api/events.cljs b/src/cljs/airsonic_ui/api/events.cljs index 764dee0..b99d4f0 100644 --- a/src/cljs/airsonic_ui/api/events.cljs +++ b/src/cljs/airsonic_ui/api/events.cljs @@ -15,13 +15,13 @@ {:http-xhrio {:method :get :uri (api/url (:credentials db) endpoint params) :response-format (ajax/json-response-format {:keywords? true}) - :on-success [:api/good-response endpoint params] - :on-failure [:api/failed-response endpoint params]} + :on-success [:api.response/ok endpoint params] + :on-failure [:api.response/failed endpoint params]} :db (assoc-in db (conj (cache-path endpoint params) :api/is-loading?) true)}) (reg-event-fx :api/request api-request) -(defn good-api-response +(defn api-success "Handles when the server responded. There could still be an error while processing the request on the server side which we have to account for." [{:keys [db]} [_ endpoint params response]] @@ -32,9 +32,9 @@ {:dispatch [:notification/show :error (api/error-msg e)] :db (update-in db response-cache dissoc :api/is-loading?)})))) -(reg-event-fx :api/good-response good-api-response) +(reg-event-fx :api.response/ok api-success) -(defn failed-api-response +(defn api-failure "Handler for catastrophic failures (network errors and such things)" [fx [ev endpoint params]] (let [response-cache (cons :db (cache-path endpoint params))] @@ -42,4 +42,4 @@ :dispatch [:notification/show :error "Communication with server failed. Check browser logs for details."] :db (update-in fx response-cache dissoc :api/is-loading?)})) -(reg-event-fx :api/failed-response failed-api-response) +(reg-event-fx :api.response/failed api-failure) diff --git a/src/cljs/airsonic_ui/components/artist/views.cljs b/src/cljs/airsonic_ui/components/artist/views.cljs index 32fbb0b..271b282 100644 --- a/src/cljs/airsonic_ui/components/artist/views.cljs +++ b/src/cljs/airsonic_ui/components/artist/views.cljs @@ -1,5 +1,6 @@ (ns airsonic-ui.components.artist.views (:require [airsonic-ui.components.collection.views :as collection] + [airsonic-ui.components.library.views :as library] [airsonic-ui.routes :as routes] [clojure.string :as str])) @@ -68,7 +69,7 @@ (defn overview "Displays the alphabetical listing of all artists along with some additional information about the collection" - [{:keys [artists]}] + [current-route {:keys [artists]}] (let [artists (:index artists) ;; TODO: Calculations in views should be avoided artists-count (count (mapcat :artist artists)) @@ -76,8 +77,9 @@ (map :albumCount) (reduce +))] [:div - [:section.hero.is-small>div.hero-body + [library/tab-section current-route] + [:section.hero.single-line.is-small>div.hero-body [:div.container [:h1.title "Artists"] - [:p.subtitle.is-5.has-text-grey [:strong artists-count] " artists in your collection with " [:strong album-count] " albums"]]] + [:p.subtitle.is-5.has-text-grey [:strong artists-count] " artists with " [:strong album-count] " albums"]]] [:section.section>div.container [alphabetical-listing artists]]])) diff --git a/src/cljs/airsonic_ui/components/library/views.cljs b/src/cljs/airsonic_ui/components/library/views.cljs index 98247df..cf4a833 100644 --- a/src/cljs/airsonic_ui/components/library/views.cljs +++ b/src/cljs/airsonic_ui/components/library/views.cljs @@ -55,7 +55,8 @@ (->> [[[::routes/library {:kind "recent"}] "Recently played"] [[::routes/library {:kind "newest"}] "Newest additions"] - [[::routes/library {:kind "starred"}] "Starred"]] + [[::routes/library {:kind "starred"}] "Starred"] + [[::routes/artist.overview] "Artists"]] (map (fn [[[id params :as route] label]] (cond-> {:href (apply routes/url-for route) :label label} @@ -63,12 +64,17 @@ (= (:kind params) (:kind current-params))) (assoc :active? true)))))) +(defn tab-section [current-route] + [:section.section.ui-tab-bar.is-small>div.container + [tabs {:items (tab-items current-route)}]]) + (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}} :as current-route] {:keys [scan-status]}] + (println "scan-status" scan-status) (let [library @(subscribe [:library/paginated kind]) page (int page) current-items (get library page) @@ -77,14 +83,13 @@ :items library :url-fn url-fn}]] [:div - [:section.hero.is-small>div.hero-body>div.container + [tab-section current-route] + [:section.hero.single-line.is-small>div.hero-body>div.container [:h2.title "Your library"] (if (:count scan-status) [:p.subtitle.is-5.has-text-grey [:strong (:count scan-status)] " items"] (when (:scanning scan-status) [:p.subtitle.is-5.has-text-grey "Scanning…"]))] - [:section.section.is-small>div.container - [tabs {:items (tab-items current-route)}]] [:section.section.is-tiny>div.container pagination-links] [:section.section.is-tiny>div.container [collection/listing current-items]] [:section.section.is-tiny>div.container pagination-links]])) diff --git a/src/cljs/airsonic_ui/events.cljs b/src/cljs/airsonic_ui/events.cljs index 85308eb..905b6e9 100644 --- a/src/cljs/airsonic_ui/events.cljs +++ b/src/cljs/airsonic_ui/events.cljs @@ -67,7 +67,7 @@ :uri (api/url credentials "getUser" {:username (:u credentials)}) :response-format (ajax/json-response-format {:keywords? true}) :on-success [:credentials/authentication-response credentials] - :on-failure [:api/failed-response]}}) ; <- we don't need endpoint and params here because the response is not cached + :on-failure [:api.response/failed]}}) ; <- we don't need endpoint and params here because the response is not cached (rf/reg-event-fx :credentials/send-authentication-request authentication-request) diff --git a/src/cljs/airsonic_ui/views.cljs b/src/cljs/airsonic_ui/views.cljs index 947298e..c1d771f 100644 --- a/src/cljs/airsonic_ui/views.cljs +++ b/src/cljs/airsonic_ui/views.cljs @@ -60,17 +60,19 @@ [:nav.navbar.is-fixed-top.is-dark {:role "navigation", :aria-label "search and navigation"} ;; user is `nil` when we're not logged in, we can hide the extended navigation [:div.navbar-brand - [:div.navbar-item + [:div.navbar-item [:a {:href (url-for ::routes/library)} [:img {:src logo-url}]] [:div.navbar-burger.burger {:on-click toggle-navbar-active!} - (for [idx (range 3)] ^{:key (str "burger-" idx)} [:span])]]] + [:span] + [:span] + [:span]]]] (when user [(if @navbar-active? :div.navbar-menu.is-active :div.navbar-menu) [:div.navbar-start [:div.navbar-item [search/form]]] [:div.navbar-end [:a.navbar-item {:href (url-for ::routes/current-queue) - :title "Current queue"} [icon :audio-spectrum]] + :title "Current queue"} [:span.heart-beat [icon :audio-spectrum]]] (when stream-role [navbar-dropdown "Library" [[{:href (url-for ::routes/library {:kind "recent"})} "Recently played"] @@ -112,7 +114,7 @@ [breadcrumbs route content] (case route-id ::routes/library [library/main route content] - ::routes/artist.overview [artist/overview content] + ::routes/artist.overview [artist/overview route content] ::routes/artist.detail [artist/detail content] ::routes/album.detail [collection/detail content] ::routes/search [search/results content] diff --git a/src/cljs/airsonic_ui/views/breadcrumbs.cljs b/src/cljs/airsonic_ui/views/breadcrumbs.cljs index c958315..bab6a05 100644 --- a/src/cljs/airsonic_ui/views/breadcrumbs.cljs +++ b/src/cljs/airsonic_ui/views/breadcrumbs.cljs @@ -17,36 +17,48 @@ (when content-pending? [:span.loader])]]]])) (defmulti breadcrumbs - (fn dispatch-on [[route-id] content] route-id)) + ;; the first parameter is always the current route, the second parameter is + ;; whatever the subscriptions return as the current content (e.g. album title) + (fn dispatch-on [[route-id] _] route-id)) (defmethod breadcrumbs :default [_ _] - [bulma-breadcrumbs "Start"]) + [bulma-breadcrumbs "Airsonic"]) -(def start [(url-for ::routes/library) "Start"]) +(defmethod breadcrumbs ::routes/library [[_ params] _] + [bulma-breadcrumbs + [(url-for ::routes/library {:kind "recent"}) "Library"] + (case (:kind params) + "recent" "Recently played" + "newest" "Newest additions" + "starred" "Starred")]) (defmethod breadcrumbs ::routes/artist.overview [_ _] - [bulma-breadcrumbs start "Artists"]) + [bulma-breadcrumbs + [(url-for ::routes/library {:kind "recent"}) "Library"] + "Artists"]) (defmethod breadcrumbs ::routes/artist.detail [_ {:keys [artist]}] - [bulma-breadcrumbs start + [bulma-breadcrumbs + [(url-for ::routes/library {:kind "recent"}) "Library"] [(url-for ::routes/artist.overview) "Artists"] (:name artist)]) (defmethod breadcrumbs ::routes/album.detail [_ {:keys [album]}] - [bulma-breadcrumbs start + [bulma-breadcrumbs + [(url-for ::routes/library {:kind "recent"}) "Library"] [(url-for ::routes/artist.overview) "Artists"] [(url-for ::routes/artist.detail {:id (:artistId album)}) (:artist album)] (:name album)]) (defmethod breadcrumbs ::routes/search [_ _] - [bulma-breadcrumbs start "Search"]) + [bulma-breadcrumbs "Search"]) (defmethod breadcrumbs ::routes/podcast.overview [_ _] ;; TODO: Detail view - [bulma-breadcrumbs start "Podcasts"]) + [bulma-breadcrumbs "Podcasts"]) (defmethod breadcrumbs ::routes/current-queue [_ _] - [bulma-breadcrumbs start "Current Queue"]) + [bulma-breadcrumbs "Current Queue"]) (defmethod breadcrumbs ::routes/about [_ _] - [bulma-breadcrumbs start "About"]) + [bulma-breadcrumbs "About"]) diff --git a/src/cljs/bulma/tabs.cljs b/src/cljs/bulma/tabs.cljs index 67146e6..95f36f8 100644 --- a/src/cljs/bulma/tabs.cljs +++ b/src/cljs/bulma/tabs.cljs @@ -1,7 +1,7 @@ (ns bulma.tabs) (defn tabs [{:keys [items]}] - [:div.tabs + [:div.tabs.is-boxed [:ul (for [[idx {:keys [href label active?]}] (map-indexed vector items)] ^{:key idx} [:li (when active? {:class "is-active"}) diff --git a/src/sass/app.sass b/src/sass/app.sass index 6113ddb..ef7660c 100644 --- a/src/sass/app.sass +++ b/src/sass/app.sass @@ -228,14 +228,30 @@ &.is-tiny padding: 0.75rem 1.5rem + // tab bar on top + &.ui-tab-bar + padding-bottom: 0.75rem + // occurs on many pages at the top to show details .hero &.is-small + .section - padding-top: 0 + padding: 1.5rem 1.5rem + + &.is-tiny + .section + padding: 0.75rem 1.5rem .media-content align-self: center + // modifies our headlines to be next to each other + +tablet + &.single-line .container + display: flex + align-items: baseline + .title + flex-grow: 1 + margin-bottom: 0 + // floating notifications .notifications:not(:empty) @extend .container @@ -358,4 +374,4 @@ tr.sortable-is-moving.is-playing // Navigation fixes .navbar-brand > .navbar-item > a display: flex - align-items: center \ No newline at end of file + align-items: center diff --git a/test/cljs/airsonic_ui/api/events_test.cljs b/test/cljs/airsonic_ui/api/events_test.cljs index 06b6b1c..7b053fc 100644 --- a/test/cljs/airsonic_ui/api/events_test.cljs +++ b/test/cljs/airsonic_ui/api/events_test.cljs @@ -7,7 +7,7 @@ (deftest api-failure-notifcations (testing "Should show an error notification when airsonic responds with an error" - (let [fx (events/good-api-response {} [:api/good-response "ping" nil (:error fixtures/responses)]) + (let [fx (events/api-success {} [:api.response/ok "ping" nil (:error fixtures/responses)]) ev (:dispatch fx)] (is (= :notification/show (first ev))) (is (= :error (second ev)))))) @@ -18,13 +18,13 @@ (testing "Should be cached" (testing "when the response was successful" (let [endpoint "getScanStatus" - successful (events/good-api-response {} [:api/good-response endpoint nil (:ok fixtures/responses)]) - unsuccessful (events/good-api-response {} [:api/good-response endpoint nil (:error fixtures/responses)])] + successful (events/api-success {} [:api.response/ok endpoint nil (:ok fixtures/responses)]) + unsuccessful (events/api-success {} [:api.response/ok endpoint nil (:error fixtures/responses)])] (is (map? (cache successful [endpoint]))) (is (nil? (cache unsuccessful [endpoint]))))) (testing "in an unwrapped format" (let [endpoint "getScanStatus" - fx (events/good-api-response {} [:api/good-response endpoint nil (:ok fixtures/responses)])] + fx (events/api-success {} [:api.response/ok endpoint nil (:ok fixtures/responses)])] (is (= #{:count :scanning} (set (keys (cache fx [endpoint])))))))) (testing "When being issued" (let [endpoint "getScanStatus" @@ -34,16 +34,16 @@ (is (contains? fx :http-xhrio))) (testing "should indicate that a request is ongoing" (is (true? (:api/is-loading? (cache fx [endpoint]))) "for non-cached responses") - (is (true? (-> (events/good-api-response fx [:api/good-response endpoint nil (:ok fixtures/responses)]) + (is (true? (-> (events/api-success fx [:api.response/ok endpoint nil (:ok fixtures/responses)]) (events/api-request [:api/request endpoint]) (cache [endpoint]) :api/is-loading?)) "for cached responses")) (testing "should remove the indication that a request is ongoing when there is a response" - (is (not (:api/is-loading? (-> (events/good-api-response fx [:api/good-response endpoint nil (:ok fixtures/responses)]) + (is (not (:api/is-loading? (-> (events/api-success fx [:api.response/ok endpoint nil (:ok fixtures/responses)]) (cache [endpoint])))) "for a good response") - (is (not (:api/is-loading? (-> (merge fx (events/good-api-response fx [:api/good-response endpoint nil (:error fixtures/responses)])) + (is (not (:api/is-loading? (-> (merge fx (events/api-success fx [:api.response/ok endpoint nil (:error fixtures/responses)])) (cache [endpoint])))) "when an error is returned") - (is (not (:api/is-loading? (-> (merge fx (events/failed-api-response fx [:api/failed-response endpoint])) + (is (not (:api/is-loading? (-> (merge fx (events/api-failure fx [:api.response/failed endpoint])) (cache [endpoint])))) "when communication with the server failed")))) (testing "Should be able to avoid the cache" ;; FIXME: Implement this diff --git a/test/cljs/airsonic_ui/events_test.cljs b/test/cljs/airsonic_ui/events_test.cljs index 983d318..56db73e 100644 --- a/test/cljs/airsonic_ui/events_test.cljs +++ b/test/cljs/airsonic_ui/events_test.cljs @@ -62,7 +62,7 @@ (testing "invokes correct callback on server response" (is (= [:credentials/authentication-response fixtures/credentials] (:on-success request)))) (testing "invokes correct callback when server is not reachable" - (is (= [:api/failed-response] (:on-failure request)))))) + (is (= [:api.response/failed] (:on-failure request)))))) (deftest authentication-response (testing "On success"