mirror of
https://github.com/penpot/penpot.git
synced 2026-05-07 00:58:48 +00:00
♻️ Migrate fontfaces and viewer thumbnails components to modern syntax (#9293)
Step toward issue #9260 (incremental migration of legacy UI components to the modern `*`-suffixed syntax, removing the per-render JS-to-Clojure props conversion overhead). Two unrelated namespaces, both clean Case-A migrations grouped in a single PR for review efficiency. frontend/src/app/main/ui/shapes/text/fontfaces.cljs --------------------------------------------------- Three components, all previously using `::mf/wrap-props false` with a custom memoizer (`#(mf/memo' % (mf/check-props ["fonts"]))`) and reading `fonts` via `(obj/get props "fonts")`. The custom memoizer existed because the legacy components received raw JS-object props, where the default `mf/memo` Clojure-equality comparison would always fail. - `fontfaces-style-html` → `fontfaces-style-html*` - `fontfaces-style-render` → `fontfaces-style-render*` - `fontfaces-style` → `fontfaces-style*` Migration: - Standard destructuring `[{:keys [fonts]}]` replaces the `[props]` + `(obj/get props "fonts")` pattern. - `::mf/wrap-props false` removed. - Custom memoizer collapses to `::mf/wrap [mf/memo]`. With modern destructuring the props are Clojure data, so default `=`-based memo is structurally correct (and a stronger guarantee than the previous shallow JS-prop check). - `app.util.object` require dropped from the namespace — it was only used for the `obj/get props "fonts"` reads that are now gone. - Internal call site of `fontfaces-style-render*` (within `fontfaces-style*`) keeps its `[:>` form, just with the new name. External call sites updated: - `frontend/src/app/main/render.cljs` — three sites (`[:& ff/fontfaces-style {:fonts fonts}]` × 3) → `[:> ff/fontfaces-style* {:fonts fonts}]`. - `frontend/src/app/main/ui/workspace/shapes.cljs` — one site, call signature unchanged. (Note: this caller passes `:shapes` rather than `:fonts`; the legacy component already ignored `:shapes` because it only read `(obj/get props "fonts")`, so the modern destructuring `{:keys [fonts]}` preserves the same behavior. Pre-existing bug, intentionally left out of scope.) frontend/src/app/main/ui/viewer/thumbnails.cljs ----------------------------------------------- Four components, all using standard destructuring already (no `::mf/wrap-props false`, no `unchecked-get`). Migration is the straight `*` rename plus `?`-prop renames per the prompt's mapping: - `thumbnails-content` → `thumbnails-content*` (`expanded?` → `is-expanded`) - `thumbnails-summary` → `thumbnails-summary*` (no `?`-props) - `thumbnail-item` → `thumbnail-item*` (`selected?` → `is-selected`; `::mf/wrap [mf/memo #(mf/deferred …)]` preserved) - `thumbnails-panel` → `thumbnails-panel*` (`show?` → `show` — no `is-` prefix needed, reads naturally as a verb) Internal callsites of all three sub-components in `thumbnails-panel*` updated to `[:> …*` with renamed kwargs (`:expanded?` → `:is-expanded`, `:selected?` → `:is-selected`). The `expanded?` symbol still appears in `thumbnails-panel*`'s body — it's a local `let`-binding deref'd from the `expanded-state` atom, not a component param, so the `?` suffix is preserved per the prompt's "local bindings stay" rule. External call sites updated: - `frontend/src/app/main/ui/viewer.cljs` — one site, plus the `:refer [thumbnails-panel]` → `:refer [thumbnails-panel*]` require update. Github #9260 Signed-off-by: FairyPigDev <luislee3108@gmail.com> Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
parent
e8ac5f26db
commit
4892799cf6
@ -244,7 +244,7 @@
|
||||
(remove cfh/frame-shape?)
|
||||
(mapcat #(cfh/get-children-with-self objects (:id %))))
|
||||
fonts (ff/shapes->fonts shapes)]
|
||||
[:& ff/fontfaces-style {:fonts fonts}])
|
||||
[:> ff/fontfaces-style* {:fonts fonts}])
|
||||
|
||||
(for [item shapes]
|
||||
[:& shape-wrapper {:shape item
|
||||
@ -481,7 +481,7 @@
|
||||
:style {:-webkit-print-color-adjust :exact}
|
||||
:fill "none"}
|
||||
|
||||
[:& ff/fontfaces-style {:fonts fonts}]
|
||||
[:> ff/fontfaces-style* {:fonts fonts}]
|
||||
[:& shape-wrapper {:shape object}]]]]))
|
||||
|
||||
(mf/defc objects-svg
|
||||
@ -520,7 +520,7 @@
|
||||
:xmlnsXlink "http://www.w3.org/1999/xlink"
|
||||
:style {:-webkit-print-color-adjust :exact}
|
||||
:fill "none"}
|
||||
[:& ff/fontfaces-style {:fonts fonts}]
|
||||
[:> ff/fontfaces-style* {:fonts fonts}]
|
||||
(for [shape shapes]
|
||||
[:& shape-wrapper {:key (dm/str (:id shape)) :shape shape}])]]]))
|
||||
|
||||
|
||||
@ -9,7 +9,6 @@
|
||||
[app.common.data :as d]
|
||||
[app.common.files.helpers :as cfh]
|
||||
[app.main.fonts :as fonts]
|
||||
[app.util.object :as obj]
|
||||
[beicon.v2.core :as rx]
|
||||
[clojure.set :as set]
|
||||
[cuerdas.core :as str]
|
||||
@ -38,24 +37,17 @@
|
||||
|
||||
(mf/ref-val fonts-css-ref)))
|
||||
|
||||
(mf/defc fontfaces-style-html
|
||||
{::mf/wrap-props false
|
||||
::mf/wrap [#(mf/memo' % (mf/check-props ["fonts"]))]}
|
||||
[props]
|
||||
|
||||
(let [fonts (obj/get props "fonts")
|
||||
|
||||
;; Fetch its CSS fontfaces
|
||||
(mf/defc fontfaces-style-html*
|
||||
{::mf/wrap [mf/memo]}
|
||||
[{:keys [fonts]}]
|
||||
(let [;; Fetch its CSS fontfaces
|
||||
fonts-css (use-fonts-css fonts)]
|
||||
|
||||
[:style fonts-css]))
|
||||
|
||||
(mf/defc fontfaces-style-render
|
||||
{::mf/wrap-props false
|
||||
::mf/wrap [#(mf/memo' % (mf/check-props ["fonts"]))]}
|
||||
[props]
|
||||
(let [fonts (obj/get props "fonts")
|
||||
;; Fetch its CSS fontfaces
|
||||
(mf/defc fontfaces-style-render*
|
||||
{::mf/wrap [mf/memo]}
|
||||
[{:keys [fonts]}]
|
||||
(let [;; Fetch its CSS fontfaces
|
||||
fonts-css (use-fonts-css fonts)]
|
||||
[:style fonts-css]))
|
||||
|
||||
@ -76,11 +68,8 @@
|
||||
(map (comp fonts/get-content-fonts :content))
|
||||
(reduce set/union #{})))
|
||||
|
||||
(mf/defc fontfaces-style
|
||||
{::mf/wrap-props false
|
||||
::mf/wrap [#(mf/memo' % (mf/check-props ["fonts"]))]}
|
||||
[props]
|
||||
(let [;; Retrieve the fonts ids used by the text shapes
|
||||
fonts (obj/get props "fonts")]
|
||||
(when (d/not-empty? fonts)
|
||||
[:> fontfaces-style-render {:fonts fonts}])))
|
||||
(mf/defc fontfaces-style*
|
||||
{::mf/wrap [mf/memo]}
|
||||
[{:keys [fonts]}]
|
||||
(when (d/not-empty? fonts)
|
||||
[:> fontfaces-style-render* {:fonts fonts}]))
|
||||
|
||||
@ -32,7 +32,7 @@
|
||||
[app.main.ui.viewer.interactions :as interactions]
|
||||
[app.main.ui.viewer.login]
|
||||
[app.main.ui.viewer.share-link]
|
||||
[app.main.ui.viewer.thumbnails :refer [thumbnails-panel]]
|
||||
[app.main.ui.viewer.thumbnails :refer [thumbnails-panel*]]
|
||||
[app.util.dom :as dom]
|
||||
[app.util.dom.normalize-wheel :as nw]
|
||||
[app.util.globals :as globals]
|
||||
@ -555,11 +555,11 @@
|
||||
:class (stl/css-case :thumbnails-close true
|
||||
:invisible (not (:show-thumbnails local false)))}]
|
||||
|
||||
[:& thumbnails-panel {:frames frames
|
||||
:show? (:show-thumbnails local false)
|
||||
:page page
|
||||
:index index
|
||||
:thumbnail-data (:thumbnails file)}]
|
||||
[:> thumbnails-panel* {:frames frames
|
||||
:show (:show-thumbnails local false)
|
||||
:page page
|
||||
:index index
|
||||
:thumbnail-data (:thumbnails file)}]
|
||||
|
||||
[:section#viewer-section {:ref viewer-section-ref
|
||||
:data-viewer-section true
|
||||
|
||||
@ -21,8 +21,8 @@
|
||||
[app.util.timers :as ts]
|
||||
[rumext.v2 :as mf]))
|
||||
|
||||
(mf/defc thumbnails-content
|
||||
[{:keys [children expanded? total] :as props}]
|
||||
(mf/defc thumbnails-content*
|
||||
[{:keys [children is-expanded total]}]
|
||||
(let [container (mf/use-ref)
|
||||
width (mf/use-var (.. js/document -documentElement -clientWidth))
|
||||
element-width (mf/use-var 152)
|
||||
@ -58,7 +58,7 @@
|
||||
(reset! width (obj/get dom "clientWidth"))))]
|
||||
|
||||
(mf/use-effect on-mount)
|
||||
(if expanded?
|
||||
(if is-expanded
|
||||
[:div {:class (stl/css :thumbnails-content)}
|
||||
[:div {:class (stl/css :thumbnails-list-expanded)} children]]
|
||||
|
||||
@ -75,8 +75,8 @@
|
||||
:style {:right (str (* @offset 152) "px")}}
|
||||
children]]])))
|
||||
|
||||
(mf/defc thumbnails-summary
|
||||
[{:keys [on-toggle-expand on-close total] :as props}]
|
||||
(mf/defc thumbnails-summary*
|
||||
[{:keys [on-toggle-expand on-close total]}]
|
||||
[:div {:class (stl/css :thumbnails-summary)}
|
||||
[:span {:class (stl/css :counter)}
|
||||
(tr "labels.num-of-frames" (i18n/c total))]
|
||||
@ -86,10 +86,10 @@
|
||||
[:button {:class (stl/css :close-btn)
|
||||
:on-click on-close} deprecated-icon/close]]])
|
||||
|
||||
(mf/defc thumbnail-item
|
||||
(mf/defc thumbnail-item*
|
||||
{::mf/wrap [mf/memo
|
||||
#(mf/deferred % ts/idle-then-raf)]}
|
||||
[{:keys [selected? frame on-click index objects page-id thumbnail-data]}]
|
||||
[{:keys [is-selected frame on-click index objects page-id thumbnail-data]}]
|
||||
|
||||
(let [children-ids (cfh/get-children-ids objects (:id frame))
|
||||
children-bounds (gsh/shapes->rect (concat [frame] (->> children-ids (keep (d/getf objects)))))]
|
||||
@ -97,7 +97,7 @@
|
||||
[:button {:class (stl/css :thumbnail-item)
|
||||
:on-click #(on-click % index)}
|
||||
[:div {:class (stl/css-case :thumbnail-preview true
|
||||
:selected selected?)}
|
||||
:selected is-selected)}
|
||||
[:& render/frame-svg {:frame (-> frame
|
||||
(assoc :thumbnail (get thumbnail-data (dm/str page-id (:id frame))))
|
||||
(assoc :children-bounds children-bounds))
|
||||
@ -107,11 +107,11 @@
|
||||
:title (:name frame)}
|
||||
(:name frame)]]))
|
||||
|
||||
(mf/defc thumbnails-panel
|
||||
[{:keys [frames page index show? thumbnail-data] :as props}]
|
||||
(mf/defc thumbnails-panel*
|
||||
[{:keys [frames page index show thumbnail-data]}]
|
||||
(let [expanded-state (mf/use-state false)
|
||||
expanded? (deref expanded-state)
|
||||
container (mf/use-ref)
|
||||
expanded? (deref expanded-state)
|
||||
container (mf/use-ref)
|
||||
|
||||
objects (:objects page)
|
||||
on-close #(st/emit! dv/toggle-thumbnails-panel)
|
||||
@ -132,20 +132,20 @@
|
||||
[:section {:class (stl/css-case :viewer-thumbnails true
|
||||
:expanded expanded?)
|
||||
;; This is better as an inline-style so it won't make a reflow of every frame inside
|
||||
:style {:display (when (not show?) "none")}
|
||||
:style {:display (when (not show) "none")}
|
||||
:ref container}
|
||||
|
||||
[:& thumbnails-summary {:on-toggle-expand toggle-expand
|
||||
:on-close on-close
|
||||
:total (count frames)}]
|
||||
[:& thumbnails-content {:expanded? expanded?
|
||||
:total (count frames)}
|
||||
[:> thumbnails-summary* {:on-toggle-expand toggle-expand
|
||||
:on-close on-close
|
||||
:total (count frames)}]
|
||||
[:> thumbnails-content* {:is-expanded expanded?
|
||||
:total (count frames)}
|
||||
(for [[i frame] (d/enumerate frames)]
|
||||
[:& thumbnail-item {:index i
|
||||
:key (dm/str (:id frame) "-" i)
|
||||
:frame frame
|
||||
:page-id (:id page)
|
||||
:objects objects
|
||||
:on-click on-item-click
|
||||
:selected? (= i index)
|
||||
:thumbnail-data thumbnail-data}])]]))
|
||||
[:> thumbnail-item* {:index i
|
||||
:key (dm/str (:id frame) "-" i)
|
||||
:frame frame
|
||||
:page-id (:id page)
|
||||
:objects objects
|
||||
:on-click on-item-click
|
||||
:is-selected (= i index)
|
||||
:thumbnail-data thumbnail-data}])]]))
|
||||
|
||||
@ -80,7 +80,7 @@
|
||||
(let [xform (comp
|
||||
(remove cfh/frame-shape?)
|
||||
(mapcat #(cfh/get-children-with-self objects (:id %))))]
|
||||
[:& ff/fontfaces-style {:shapes (into [] xform shapes)}])
|
||||
[:> ff/fontfaces-style* {:shapes (into [] xform shapes)}])
|
||||
|
||||
[:g.frame-children
|
||||
(for [shape shapes]
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user