From 56e6dbe8d89e537e4a8109ff1dcac72d4769bc4a Mon Sep 17 00:00:00 2001 From: Juho Teperi Date: Fri, 24 Mar 2023 14:29:36 +0200 Subject: [PATCH] Fix #377, navigate to routes with fragment string in frontend --- .../src/frontend/core.cljs | 2 +- .../reitit-frontend/src/reitit/frontend.cljs | 11 +++++++ .../src/reitit/frontend/easy.cljs | 26 +++++++++------- .../src/reitit/frontend/history.cljs | 30 ++++++++++++------- test/cljs/reitit/frontend/core_test.cljs | 13 ++++++++ test/cljs/reitit/frontend/easy_test.cljs | 6 ++-- test/cljs/reitit/frontend/history_test.cljs | 2 ++ 7 files changed, 65 insertions(+), 25 deletions(-) diff --git a/examples/frontend-controllers/src/frontend/core.cljs b/examples/frontend-controllers/src/frontend/core.cljs index 6a2388f2..851e11ce 100644 --- a/examples/frontend-controllers/src/frontend/core.cljs +++ b/examples/frontend-controllers/src/frontend/core.cljs @@ -18,7 +18,7 @@ [:div [:ul [:li [:a {:href (rfe/href ::item {:id 1})} "Item 1"]] - [:li [:a {:href (rfe/href ::item {:id 2} {:foo "bar"})} "Item 2"]]] + [:li [:a {:href (rfe/href ::item {:id 2} {:foo "bar"} "zzz")} "Item 2"]]] (when id [:h2 "Selected item " id]) [:p "Query params: " [:pre (pr-str query)]] diff --git a/modules/reitit-frontend/src/reitit/frontend.cljs b/modules/reitit-frontend/src/reitit/frontend.cljs index 907a07a7..f7183b40 100644 --- a/modules/reitit-frontend/src/reitit/frontend.cljs +++ b/modules/reitit-frontend/src/reitit/frontend.cljs @@ -2,6 +2,7 @@ (:require [clojure.set :as set] [reitit.coercion :as coercion] [reitit.core :as r] + [reitit.impl :as impl] goog.Uri goog.Uri.QueryData)) @@ -36,6 +37,16 @@ (.setQueryData uri (goog.Uri.QueryData/createFromMap (clj->js new-query))) (.toString uri))) +(defn + ^{:see-also ["reitit.core/match->path"]} + match->path + "Create routing path from given match and optional query-string map and + optional fragment string." + [match query-params fragment] + (when-let [path (r/match->path match query-params)] + (cond-> path + (and fragment (seq fragment)) (str "#" (impl/form-encode fragment))))) + (defn match-by-path "Given routing tree and current path, return match with possibly coerced parameters. Return nil if no match found. diff --git a/modules/reitit-frontend/src/reitit/frontend/easy.cljs b/modules/reitit-frontend/src/reitit/frontend/easy.cljs index bcd47f88..b50bbb48 100644 --- a/modules/reitit-frontend/src/reitit/frontend/easy.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/easy.cljs @@ -52,11 +52,13 @@ pairs separated by &, i.e. \"?a=1&a=2\", if you want to encode them differently, convert the collections to strings first." ([name] - (rfh/href @history name nil nil)) + (rfh/href @history name nil nil nil)) ([name path-params] - (rfh/href @history name path-params nil)) + (rfh/href @history name path-params nil nil)) ([name path-params query-params] - (rfh/href @history name path-params query-params))) + (rfh/href @history name path-params query-params nil)) + ([name path-params query-params fragment] + (rfh/href @history name path-params query-params fragment))) (defn ^{:see-also ["reitit.frontend.history/push-state"]} @@ -74,11 +76,13 @@ See also: https://developer.mozilla.org/en-US/docs/Web/API/History/pushState" ([name] - (rfh/push-state @history name nil nil)) + (rfh/push-state @history name nil nil nil)) ([name path-params] - (rfh/push-state @history name path-params nil)) + (rfh/push-state @history name path-params nil nil)) ([name path-params query-params] - (rfh/push-state @history name path-params query-params))) + (rfh/push-state @history name path-params query-params nil)) + ([name path-params query-params fragment] + (rfh/push-state @history name path-params query-params fragment))) (defn ^{:see-also ["reitit.frontend.history/replace-state"]} @@ -96,11 +100,13 @@ See also: https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState" ([name] - (rfh/replace-state @history name nil nil)) + (rfh/replace-state @history name nil nil nil)) ([name path-params] - (rfh/replace-state @history name path-params nil)) + (rfh/replace-state @history name path-params nil nil)) ([name path-params query-params] - (rfh/replace-state @history name path-params query-params))) + (rfh/replace-state @history name path-params query-params nil)) + ([name path-params query-params fragment] + (rfh/replace-state @history name path-params query-params fragment))) ;; This duplicates previous two, but the map parameter will be easier way to ;; extend the functions, e.g. to work with fragment string. Toggling push vs @@ -125,7 +131,7 @@ https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState" ([name] (rfh/navigate @history name)) - ([name {:keys [path-params query-params replace] :as opts}] + ([name {:keys [path-params query-params replace fragment] :as opts}] (rfh/navigate @history name opts))) (defn diff --git a/modules/reitit-frontend/src/reitit/frontend/history.cljs b/modules/reitit-frontend/src/reitit/frontend/history.cljs index 5764e10f..0f93a958 100644 --- a/modules/reitit-frontend/src/reitit/frontend/history.cljs +++ b/modules/reitit-frontend/src/reitit/frontend/history.cljs @@ -10,7 +10,7 @@ (-init [this] "Create event listeners") (-stop [this] "Remove event listeners") (-on-navigate [this path] "Find a match for current routing path and call on-navigate callback") - (-get-path [this] "Get the current routing path") + (-get-path [this] "Get the current routing path, including query string and fragment") (-href [this path] "Converts given routing path to browser location")) ;; This version listens for both pop-state and hash-change for @@ -92,6 +92,7 @@ ;; isContentEditable property is inherited from parents, ;; so if the anchor is inside contenteditable div, the property will be true. (not (.-isContentEditable el)) + ;; NOTE: Why doesn't this use frontend variant instead of core? (reitit/match-by-path router (.getPath uri))))) (defrecord Html5History [on-navigate router listen-key click-listen-key] @@ -132,7 +133,8 @@ nil) (-get-path [this] (str (.. js/window -location -pathname) - (.. js/window -location -search))) + (.. js/window -location -search) + (.. js/window -location -hash))) (-href [this path] path)) @@ -193,8 +195,10 @@ ([history name path-params] (href history name path-params nil)) ([history name path-params query-params] + (href history name path-params query-params nil)) + ([history name path-params query-params fragment] (let [match (rf/match-by-name! (:router history) name path-params)] - (-href history (reitit/match->path match query-params))))) + (-href history (rf/match->path match query-params fragment))))) (defn ^{:see-also ["reitit.core/match->path"]} @@ -211,12 +215,14 @@ See also: https://developer.mozilla.org/en-US/docs/Web/API/History/pushState" ([history name] - (push-state history name nil nil)) + (push-state history name nil nil nil)) ([history name path-params] - (push-state history name path-params nil)) + (push-state history name path-params nil nil)) ([history name path-params query-params] + (push-state history name path-params query-params nil)) + ([history name path-params query-params fragment] (let [match (rf/match-by-name! (:router history) name path-params) - path (reitit/match->path match query-params)] + path (rf/match->path match query-params fragment)] ;; pushState and replaceState don't trigger popstate event so call on-navigate manually (.pushState js/window.history nil "" (-href history path)) (-on-navigate history path)))) @@ -237,12 +243,14 @@ See also: https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState" ([history name] - (replace-state history name nil nil)) + (replace-state history name nil nil nil)) ([history name path-params] - (replace-state history name path-params nil)) + (replace-state history name path-params nil nil)) ([history name path-params query-params] + (replace-state history name path-params query-params nil)) + ([history name path-params query-params fragment] (let [match (rf/match-by-name! (:router history) name path-params) - path (reitit/match->path match query-params)] + path (rf/match->path match query-params fragment)] (.replaceState js/window.history nil "" (-href history path)) (-on-navigate history path)))) @@ -265,9 +273,9 @@ https://developer.mozilla.org/en-US/docs/Web/API/History/replaceState" ([history name] (navigate history name nil)) - ([history name {:keys [path-params query-params replace] :as opts}] + ([history name {:keys [path-params query-params fragment replace] :as opts}] (let [match (rf/match-by-name! (:router history) name path-params) - path (reitit/match->path match query-params)] + path (rf/match->path match query-params fragment)] (if replace (.replaceState js/window.history nil "" (-href history path)) (.pushState js/window.history nil "" (-href history path))) diff --git a/test/cljs/reitit/frontend/core_test.cljs b/test/cljs/reitit/frontend/core_test.cljs index a0e0abe0..12d461fa 100644 --- a/test/cljs/reitit/frontend/core_test.cljs +++ b/test/cljs/reitit/frontend/core_test.cljs @@ -282,3 +282,16 @@ (testing "Need to coerce current values manually" (is (= "foo?foo=2" (rf/set-query-params "foo?foo=1" (fn [q] (update q :foo #(inc (js/parseInt %))))))))) + +(deftest match->path-test + (is (= "foo" + (rf/match->path {:path "foo"} nil nil) + (rf/match->path {:path "foo"} {} ""))) + (is (= "foo?a=1&b=&c=foo+bar" + ;; NOTE: This encoding differs from set-query + (rf/match->path {:path "foo"} {:a "1" :b "" :c "foo bar"} nil))) + (is (= "foo#aaa" + (rf/match->path {:path "foo"} nil "aaa"))) + (testing "Fragment encoding" + (is (= "foo#foo+bar+%25" + (rf/match->path {:path "foo"} nil "foo bar %"))))) diff --git a/test/cljs/reitit/frontend/easy_test.cljs b/test/cljs/reitit/frontend/easy_test.cljs index abe11545..e36d9eee 100644 --- a/test/cljs/reitit/frontend/easy_test.cljs +++ b/test/cljs/reitit/frontend/easy_test.cljs @@ -29,10 +29,10 @@ 1 (do (is (some? (:popstate-listener history))) (is (= "/" url) "start at root") - (rfe/push-state ::foo)) + (rfe/push-state ::foo nil {:a 1} "foo bar")) ;; 0. / - ;; 1. /foo - 2 (do (is (= "/foo" url) + ;; 1. /foo?a=1#foo+bar + 2 (do (is (= "/foo?a=1#foo+bar" url) "push-state") (.back js/window.history)) ;; 0. / diff --git a/test/cljs/reitit/frontend/history_test.cljs b/test/cljs/reitit/frontend/history_test.cljs index ffab3970..ff3df965 100644 --- a/test/cljs/reitit/frontend/history_test.cljs +++ b/test/cljs/reitit/frontend/history_test.cljs @@ -26,6 +26,8 @@ (rfh/href history ::bar {:id 5}))) (is (= "#/bar/5?q=x" (rfh/href history ::bar {:id 5} {:q "x"}))) + (is (= "#/bar/5?q=x#foo" + (rfh/href history ::bar {:id 5} {:q "x"} "foo"))) (let [{:keys [value messages]} (capture-console (fn [] (rfh/href history ::asd)))]