🐛 Fix errors with code generation (#10217)

* 🐛 Fix errors with code generation

* 🐛 Add cancelation of effects in inspect code
This commit is contained in:
Alonso Torres 2026-06-18 17:55:55 +02:00 committed by GitHub
parent b4532486e3
commit 6b50e2d822
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 281 additions and 37 deletions

View File

@ -61,7 +61,7 @@
only-flex?
"Flex element"
only-grid?
"Flex element"
"Grid element"
:else
"Layout element")]

View File

@ -87,12 +87,12 @@
value map))
(defn gen-all-code
[style-code markup-code images-data]
[style-code markup-code images-data fonts-data]
(let [markup-code (cond-> markup-code
embed-images? (replace-map images-data))
style-code (cond-> style-code
embed-images? (replace-map images-data))]
embed-images? (replace-map (merge images-data fonts-data)))]
(str/format page-template style-code markup-code)))
(mf/defc code*
@ -101,11 +101,13 @@
markup-type* (mf/use-state "html")
fontfaces-css* (mf/use-state nil)
images-data* (mf/use-state nil)
fonts-data* (mf/use-state nil)
style-type (deref style-type*)
markup-type (deref markup-type*)
fontfaces-css (deref fontfaces-css*)
images-data (deref images-data*)
fonts-data (deref fonts-data*)
collapsed* (mf/use-state #{})
collapsed-css? (contains? @collapsed* :css)
@ -199,9 +201,9 @@
handle-copy-all-code
(mf/use-fn
(mf/deps style-code markup-code images-data)
(mf/deps style-code markup-code images-data fonts-data)
(fn []
(clipboard/to-clipboard (gen-all-code style-code markup-code images-data))
(clipboard/to-clipboard (gen-all-code style-code markup-code images-data fonts-data))
(let [origin (if (= :workspace from)
"workspace"
"viewer")]
@ -228,8 +230,8 @@
(conj collapsed panel-type)))))))
copy-css-fn
(mf/use-fn
(mf/deps style-code images-data)
#(replace-map style-code images-data))
(mf/deps style-code images-data fonts-data)
#(replace-map style-code (merge images-data fonts-data)))
copy-html-fn
(mf/use-fn
@ -237,24 +239,41 @@
#(replace-map markup-code images-data))]
(mf/with-effect [fonts]
(->> (rx/from fonts)
(rx/merge-map fonts/fetch-font-css)
(rx/reduce conj [])
(rx/subs!
(fn [result]
(let [css (str/join "\n" result)]
(reset! fontfaces-css* css))))))
(let [sub (->> (rx/from fonts)
(rx/merge-map fonts/fetch-font-css)
(rx/reduce conj [])
(rx/subs!
(fn [result]
(let [css (str/join "\n" result)]
(reset! fontfaces-css* css)))))]
#(rx/dispose! sub)))
;; Resolve the font URLs to data URIs. The inspect view keeps the original
;; URLs (more readable), but copying embeds the fonts so the styles render
;; outside of Penpot, where the original URLs require auth/CORS.
(mf/with-effect [fontfaces-css]
(let [sub (->> (rx/from (fonts/extract-fontface-urls (or fontfaces-css "")))
(rx/merge-map
(fn [uri]
(->> (http/fetch-data-uri uri true)
(rx/catch (fn [_] (rx/of (hash-map uri uri)))))))
(rx/reduce conj {})
(rx/subs!
(fn [result]
(reset! fonts-data* result))))]
#(rx/dispose! sub)))
(mf/with-effect [images-urls]
(->> (rx/from images-urls)
(rx/merge-map
(fn [[_ uri]]
(->> (http/fetch-data-uri uri true)
(rx/catch (fn [_] (rx/of (hash-map uri uri)))))))
(rx/reduce conj {})
(rx/subs!
(fn [result]
(reset! images-data* result)))))
(let [sub (->> (rx/from images-urls)
(rx/merge-map
(fn [[_ uri]]
(->> (http/fetch-data-uri uri true)
(rx/catch (fn [_] (rx/of (hash-map uri uri)))))))
(rx/reduce conj {})
(rx/subs!
(fn [result]
(reset! images-data* result))))]
#(rx/dispose! sub)))
[:div {:class (stl/css-case :element-options true
:viewer-code-block (= :viewer from))}

View File

@ -38,7 +38,7 @@
indent))
(cfh/text-shape? shape)
(let [text-shape-html (rds/renderToStaticMarkup (mf/element text/text-shape* #js {:shape shape :is-code true}))
(let [text-shape-html (rds/renderToStaticMarkup (mf/element text/text-shape* #js {:shape shape :isCode true}))
text-shape-html (str/replace text-shape-html #"style\s*=\s*[\"'][^\"']*[\"']" "")]
(dm/fmt "%<div class=\"%\">\n%\n%</div>"
indent

View File

@ -50,7 +50,10 @@ body {
(def shape-wrapper-css-properties
#{:flex-shrink
:margin
:margin-block-start
:margin-block-end
:margin-inline-start
:margin-inline-end
:max-height
:min-height
:max-width
@ -113,7 +116,6 @@ body {
;; Flex/grid self properties
:flex-shrink
:margin
:margin-block-start
:margin-block-end
:margin-inline-start

View File

@ -35,13 +35,25 @@
parent-value (dm/get-in parent [:selrect coord])
;; In CSS an absolutely positioned element is placed relative to the
;; parent's padding box (i.e. inside its border), but Penpot
;; coordinates are relative to the border box. We discount the parent
;; border width so the element keeps its position when the parent has
;; a border.
parent-stroke (first (:strokes parent))
border-width (if (and (some? parent-stroke)
(not= :none (:stroke-style parent-stroke))
(not (cgc/svg-markup? parent)))
(d/nilv (:stroke-width parent-stroke) 0)
0)
[selrect _ _]
(-> (:points shape)
(gsh/transform-points (gsh/shape->center parent) (:transform-inverse parent (gmt/matrix)))
(gsh/calculate-geometry))
shape-value (get selrect coord)]
(- shape-value parent-value))))
(- shape-value parent-value border-width))))
(defn get-shape-size
[shape objects type]
@ -62,8 +74,13 @@
(and (ctl/flex-layout-immediate-child? objects shape) (= sizing :fill))
nil
(or (and (ctl/any-layout? shape) (= sizing :auto) (not (cgc/svg-markup? shape)))
(and (ctl/grid-layout-immediate-child? objects shape) (= sizing :fill)))
;; Grid fill children stretch to fill their cell (minus margins) via
;; justify-self/align-self: stretch, so we avoid emitting an explicit
;; 100% size that would overflow the track when a margin is present.
(and (ctl/grid-layout-immediate-child? objects shape) (= sizing :fill))
nil
(and (ctl/any-layout? shape) (= sizing :auto) (not (cgc/svg-markup? shape)))
sizing
(some? (:selrect shape))
@ -412,8 +429,10 @@
(defn- get-margin
[{:keys [layout-item-margin] :as shape} objects]
(when (ctl/any-layout-immediate-child? objects shape)
;; Absolutely positioned children are out of the layout flow, so their
;; margin must not be emitted.
(when (and (ctl/any-layout-immediate-child? objects shape)
(not (ctl/position-absolute? shape)))
(let [default-margin {:m1 0 :m2 0 :m3 0 :m4 0}
{:keys [m1 m2 m3 m4]} (merge default-margin layout-item-margin)]
(when (or (not= m1 0) (not= m2 0) (not= m3 0) (not= m4 0))
@ -421,22 +440,22 @@
(defn- get-margin-block-start
[{:keys [layout-item-margin] :as shape} objects]
(when (and (ctl/any-layout-immediate-child? objects shape) (:m1 layout-item-margin) (not= (:m1 layout-item-margin) 0))
(when (and (ctl/any-layout-immediate-child? objects shape) (not (ctl/position-absolute? shape)) (:m1 layout-item-margin) (not= (:m1 layout-item-margin) 0))
[(:m1 layout-item-margin)]))
(defn- get-margin-inline-end
[{:keys [layout-item-margin] :as shape} objects]
(when (and (ctl/any-layout-immediate-child? objects shape) (:m2 layout-item-margin) (not= (:m2 layout-item-margin) 0))
(when (and (ctl/any-layout-immediate-child? objects shape) (not (ctl/position-absolute? shape)) (:m2 layout-item-margin) (not= (:m2 layout-item-margin) 0))
[(:m2 layout-item-margin)]))
(defn- get-margin-block-end
[{:keys [layout-item-margin] :as shape} objects]
(when (and (ctl/any-layout-immediate-child? objects shape) (:m3 layout-item-margin) (not= (:m3 layout-item-margin) 0))
(when (and (ctl/any-layout-immediate-child? objects shape) (not (ctl/position-absolute? shape)) (:m3 layout-item-margin) (not= (:m3 layout-item-margin) 0))
[(:m3 layout-item-margin)]))
(defn- get-margin-inline-start
[{:keys [layout-item-margin] :as shape} objects]
(when (and (ctl/any-layout-immediate-child? objects shape) (:m4 layout-item-margin) (not= (:m4 layout-item-margin) 0))
(when (and (ctl/any-layout-immediate-child? objects shape) (not (ctl/position-absolute? shape)) (:m4 layout-item-margin) (not= (:m4 layout-item-margin) 0))
[(:m4 layout-item-margin)]))
@ -488,7 +507,10 @@
(let [parent (get objects (:parent-id shape))
cell (ctl/get-cell-by-shape-id parent (:id shape))
align-self (:align-self cell)]
(when (not= align-self :auto) align-self))))
(cond
(not= align-self :auto) align-self
;; Fill children rely on stretch to fill the cell minus their margins.
(= :fill (:layout-item-v-sizing shape)) :stretch))))
(defn- get-justify-self
[shape objects]
@ -497,7 +519,10 @@
(let [parent (get objects (:parent-id shape))
cell (ctl/get-cell-by-shape-id parent (:id shape))
justify-self (:justify-self cell)]
(when (not= justify-self :auto) justify-self))))
(cond
(not= justify-self :auto) justify-self
;; Fill children rely on stretch to fill the cell minus their margins.
(= :fill (:layout-item-h-sizing shape)) :stretch))))
(defn- get-grid-auto-flow
[shape]

View File

@ -0,0 +1,196 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS INC
(ns frontend-tests.code-gen-style-test
"Regression tests for the inspect code-generation (HTML/CSS export).
Each test guards against a concrete bug found in the CSS/HTML generation
of layout children and text shapes."
(:require
[app.common.geom.matrix :as gmt]
[app.common.geom.point :as gpt]
[app.common.geom.rect :as grc]
[app.common.uuid :as uuid]
[app.util.code-gen.markup-html :as html]
[app.util.code-gen.style-css :as css]
[cljs.test :refer [deftest is testing] :include-macros true]
[cuerdas.core :as str]))
;; --- Builders ------------------------------------------------------------
(defn- pts
"Rectangular point ring matching a x/y/w/h box."
[x y w h]
[(gpt/point x y)
(gpt/point (+ x w) y)
(gpt/point (+ x w) (+ y h))
(gpt/point x (+ y h))])
(defn- frame
[id & {:as extra}]
(merge {:id id :name "Board" :type :frame
:parent-id uuid/zero :frame-id uuid/zero
:selrect (grc/make-rect 0 0 200 200)
:points (pts 0 0 200 200)}
extra))
(defn- grid-frame
"A grid board with a single auto cell that holds `child-id`."
[id child-id & {:as extra}]
(let [cell-id (uuid/next)]
(merge (frame id)
{:name "Grid"
:layout :grid
:layout-grid-rows [{:type :flex :value 1}]
:layout-grid-columns [{:type :flex :value 1}]
:layout-grid-cells {cell-id {:id cell-id :row 1 :column 1
:row-span 1 :column-span 1
:position :auto
:align-self :auto :justify-self :auto
:shapes [child-id]}}}
extra)))
(defn- child
[id parent-id & {:as extra}]
(merge {:id id :name "Child" :type :rect :parent-id parent-id
:selrect (grc/make-rect 0 0 50 50)
:points (pts 0 0 50 50)
:transform (gmt/matrix)}
extra))
(defn- objects [& shapes]
(into {} (map (juxt :id identity)) shapes))
(def ^:private sample-margin
{:m1 10 :m2 20 :m3 30 :m4 40})
;; --- Margins on layout children -----------------------------------------
(deftest grid-child-margins-use-logical-longhand
(testing "margins of grid children map to logical longhand properties"
(let [pid (uuid/next)
cid (uuid/next)
c (child cid pid
:layout-item-margin sample-margin
:layout-item-margin-type :multiple)
objs (objects (grid-frame pid cid) c)]
(is (= "10px" (css/get-css-value objs c :margin-block-start)))
(is (= "20px" (css/get-css-value objs c :margin-inline-end)))
(is (= "30px" (css/get-css-value objs c :margin-block-end)))
(is (= "40px" (css/get-css-value objs c :margin-inline-start))))))
(deftest grid-child-css-has-no-redundant-margin-shorthand
(testing "the generated rule emits logical longhand margins, not a shorthand"
(let [pid (uuid/next)
cid (uuid/next)
c (child cid pid
:layout-item-margin sample-margin
:layout-item-margin-type :multiple)
out (css/get-shape-css-selector (objects (grid-frame pid cid) c) c)]
(is (str/includes? out "margin-block-start: 10px;"))
(is (not (re-find #"margin:" out))
"must not emit the `margin` shorthand alongside the longhand props"))))
(deftest wrapped-layout-child-margin-only-on-wrapper
(testing "a rotated (wrapped) child keeps margins on the wrapper, not the inner element"
(let [pid (uuid/next)
cid (uuid/next)
c (child cid pid
:layout-item-margin {:m1 10 :m2 0 :m3 0 :m4 0}
:layout-item-margin-type :multiple
:transform (gmt/rotate-matrix 30))
out (css/get-shape-css-selector (objects (grid-frame pid cid) c) c)
;; the inner element is the only rule carrying the transform matrix
inner (->> (str/split out "}")
(filter #(str/includes? % "transform:"))
(first))]
(is (str/includes? out "-wrapper {"))
(is (str/includes? out "margin-block-start: 10px;")
"margin is emitted (on the wrapper)")
(is (some? inner))
(is (not (str/includes? inner "margin"))
"the inner transformed element must not double the margin"))))
(deftest absolute-positioned-child-has-no-margin
(testing "absolutely positioned layout children drop their margin"
(let [pid (uuid/next)
cid (uuid/next)
c (child cid pid
:layout-item-margin sample-margin
:layout-item-margin-type :multiple
:layout-item-absolute true)
objs (objects (grid-frame pid cid) c)]
(is (nil? (css/get-css-value objs c :margin)))
(is (nil? (css/get-css-value objs c :margin-block-start))))))
;; --- Grid fill sizing ----------------------------------------------------
(deftest grid-fill-child-stretches-instead-of-fixed-size
(testing "fill-sized grid children rely on stretch instead of width/height: 100%"
(let [pid (uuid/next)
cid (uuid/next)
c (child cid pid
:layout-item-h-sizing :fill
:layout-item-v-sizing :fill)
objs (objects (grid-frame pid cid) c)]
(is (nil? (css/get-css-value objs c :width))
"no explicit width: 100% that would overflow the cell with a margin")
(is (nil? (css/get-css-value objs c :height)))
(is (= "stretch" (css/get-css-value objs c :justify-self)))
(is (= "stretch" (css/get-css-value objs c :align-self))))))
;; --- Absolute positioning vs parent border -------------------------------
(deftest absolute-position-discounts-parent-border
(testing "absolute coords are measured from the padding box, so the parent border is discounted"
(let [pid (uuid/next)
cid (uuid/next)
parent (frame pid :strokes [{:stroke-width 80
:stroke-style :solid
:stroke-color "#000000"
:stroke-alignment :inner}])
c (child cid pid
:selrect (grc/make-rect 100 100 50 50)
:points (pts 100 100 50 50))
objs (objects parent c)]
;; left/top = 100 (shape) - 0 (parent) - 80 (border) = 20
(is (= "20px" (css/get-css-value objs c :left)))
(is (= "20px" (css/get-css-value objs c :top))))))
(deftest absolute-position-without-border-is-unchanged
(testing "without a parent border the absolute coords are the raw offset"
(let [pid (uuid/next)
cid (uuid/next)
c (child cid pid
:selrect (grc/make-rect 100 100 50 50)
:points (pts 100 100 50 50))
objs (objects (frame pid) c)]
(is (= "100px" (css/get-css-value objs c :left)))
(is (= "100px" (css/get-css-value objs c :top))))))
;; --- Text node markup ----------------------------------------------------
(def ^:private text-content
{:type "root"
:children [{:type "paragraph-set"
:children [{:type "paragraph"
:children [{:text "Hello"
:fills [{:fill-color "#000000" :fill-opacity 1}]}]}]}]})
(deftest text-markup-emits-node-id-classes
(testing "generated text markup carries the $id classes the CSS rules target"
(let [tid (uuid/next)
text {:id tid :name "Text" :type :text
:parent-id uuid/zero :frame-id uuid/zero
:selrect (grc/make-rect 0 0 100 20)
:points (pts 0 0 100 20)
:grow-type :fixed
:content text-content}
markup (html/generate-markup (objects text) [text])]
(is (string? markup))
(is (str/includes? markup "root-0")
"the text nodes must expose their $id as a class for the CSS to apply")
(is (str/includes? markup "root-0-paragraph-set-0-paragraph-0")))))

View File

@ -5,6 +5,7 @@
[clojure.string :as str]
[clojure.tools.cli :refer [parse-opts]]
[frontend-tests.basic-shapes-test]
[frontend-tests.code-gen-style-test]
[frontend-tests.copy-as-svg-test]
[frontend-tests.data.nitrate-test]
[frontend-tests.data.repo-test]
@ -57,6 +58,7 @@
(def test-namespaces
'[frontend-tests.basic-shapes-test
frontend-tests.code-gen-style-test
frontend-tests.copy-as-svg-test
frontend-tests.data.nitrate-test
frontend-tests.data.repo-test