github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-28 21:40:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Open the first page in view mode when the URL has no page id

Browse Source 
A view URL without a page-id crashed builds with assertions enabled:
the viewer passes a nil page-id to `initialize`, whose schema accepted
a missing key but not a nil value. Release builds skip that check and
draw the first page, but every viewer event reads the page from the
route, so zoom to fit and fill, the next-board arrow, Navigate To
interactions, and comment pins did not work.

`initialize` now accepts a nil page-id, `fetch-bundle` makes it
optional, and `bundle-fetched` replaces a route without page-id by one
naming the file's first page. The viewer then initializes again on the
completed route, as it does when the user changes page. A file with no
pages keeps its route.

AI-assisted-by: claude-opus-5-5
Signed-off-by: Álvaro Tejero Cantero <alvorithm@teje.ro>
This commit is contained in:
Álvaro Tejero Cantero 2026-10-28 16:37:24 +01:00
parent 8951c57ee6
commit ad7fc402c0
No known key found for this signature in database
2 changed files with 75 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
frontend/src/app/main/data/viewer.cljs
View File

@ -57,7 +57,7 @@
[:map {:title "initialize"}
[:file-id ::sm/uuid]
[:share-id {:optional true} [:maybe ::sm/uuid]]
[:page-id {:optional true} ::sm/uuid]])
[:page-id {:optional true} [:maybe ::sm/uuid]]])
(defn initialize
[{:keys [file-id share-id] :as params}]
@ -116,7 +116,7 @@
(def ^:private
schema:fetch-bundle
[:map {:title "fetch-bundle"}
[:page-id ::sm/uuid]
[:page-id {:optional true} ::sm/uuid]
[:file-id ::sm/uuid]
[:share-id {:optional true} ::sm/uuid]])
@ -305,19 +305,27 @@
index (some-> (rt/get-query-param qparams :index) parse-long)
frame-id (some-> (:frame-id qparams) uuid/parse)
page-id (some-> (rt/get-query-param qparams :page-id) uuid/parse)
file-id (some-> (rt/get-query-param qparams :file-id) uuid/parse)]
file-id (some-> (rt/get-query-param qparams :file-id) uuid/parse)
first-page-id (-> file :data :pages first)]
(rx/merge
(rx/of (case (:zoom qparams)
"fit" zoom-to-fit
"fill" zoom-to-fill
nil))
(rx/of
(update-page-position-data file-id page-id)
(cond
(some? frame-id) (go-to-frame frame-id)
(some? index) (go-to-frame-by-index index)
:else (go-to-frame-auto)))))))))
(if (and (nil? page-id) (some? first-page-id))
;; Every viewer event reads the page from the route, so a URL
;; without page-id names the first page; the viewer then
;; initializes again on the completed route.
(rx/of (rt/nav :viewer
(assoc (rt/get-params state) :page-id first-page-id)
{::rt/replace true}))
(rx/merge
(rx/of (case (:zoom qparams)
"fit" zoom-to-fit
"fill" zoom-to-fill
nil))
(rx/of
(update-page-position-data file-id page-id)
(cond
(some? frame-id) (go-to-frame frame-id)
(some? index) (go-to-frame-by-index index)
:else (go-to-frame-auto))))))))))
(defn fetch-comment-threads
[{:keys [file-id page-id share-id] :as params}]

56
frontend/test/frontend_tests/data/viewer_test.cljs
View File

@ -131,9 +131,13 @@
;; because update-zoom-querystring navigated unconditionally.
;; At HEAD the guard breaks the cycle, so a bundle fetch must not
;; emit any navigation.
(let [state (-> (base-state {:frames [{:selrect {:width 100 :height 100}}]
:index 0})
(assoc-in [:route :query-params :zoom] "fill"))]
(let [state (as-> (base-state {:frames [{:selrect {:width 100 :height 100}}]
:index 0}) state
;; The router stores the same query map under both keys.
(let [query (assoc (rt/get-params state) :zoom "fill")]
(-> state
(assoc-in [:route :params :query] query)
(assoc-in [:route :query-params] query))))]
(let [events (watch-events (dv/bundle-fetched
{:file {:id page-id
:data {:pages [page-id]
@ -151,3 +155,49 @@
;; ones that can restart the zoom cycle.
navigations (filter #(= :app.main.router/navigate (ptk/type %)) events)]
(t/is (empty? navigations)))))
(defn- minimal-bundle
[page-ids]
{:file {:id (uuid/custom 2 2)
:data {:pages (vec page-ids)
:pages-index (into {} (map (fn [id] [id {:objects {}}])) page-ids)}}
:project {}
:team {:features []}
:share-links []
:libraries []
:users []
:permissions {}
:thumbnails {}})
(defn- route-without-page-id
[]
(let [query {:file-id (str (uuid/custom 2 2)) :section "interactions" :index "0"}]
{:route {:params {:query query} :query-params query}}))
(t/deftest initialize-without-page-id-fetches-the-bundle
;; The viewer route passes `:page-id nil` when the URL has none; the
;; params check used to reject it before any fetch.
(let [events (watch-events (dv/initialize {:file-id (uuid/custom 2 2)
:page-id nil
:share-id nil})
{})]
(t/is (= [::dv/fetch-bundle] (map ptk/type (remove nil? events))))))
(t/deftest bundle-fetched-without-page-id-names-the-first-page
(t/testing "the route is replaced by one naming the first page"
(let [first-page (uuid/custom 3 1)
events (watch-events (dv/bundle-fetched
(minimal-bundle [first-page (uuid/custom 3 2)]))
(route-without-page-id))
{:keys [id params options]} (some-> (first events) deref)]
(t/is (= 1 (count events)))
(t/is (= :viewer id))
(t/is (= first-page (:page-id params)))
(t/is (= "interactions" (:section params)))
(t/is (true? (::rt/replace options)))))
(t/testing "a file without pages keeps the route"
(let [events (watch-events (dv/bundle-fetched (minimal-bundle []))
(route-without-page-id))
navigations (filter #(= :app.main.router/navigate (ptk/type %)) events)]
(t/is (empty? navigations)))))