🐛 Fix stroke-cap-start/end stored at wrong level in SVG imports (#9982)

common/src/app/common/files/migrations.cljc

@@ -1837,6 +1837,29 @@
   [data _]
   (d/update-when data :tokens-lib ctob/fix-missing-sets-in-themes))
 
+;; This will fix incorrectly created strokes from SVG imports
+;; that have the stroke-cap at the shape level instead of at the stroke level
+(defmethod migrate-data "0024-fix-stroke-cap-placement"
+  [data _]
+  (letfn [(fix-shape [shape]
+            (let [cap-start (get shape :stroke-cap-start)
+                  cap-end   (get shape :stroke-cap-end)]
+              (if (or (some? cap-start) (some? cap-end))
+                (cond-> (dissoc shape :stroke-cap-start :stroke-cap-end)
+                  (and (some? cap-start) (seq (:strokes shape)))
+                  (assoc-in [:strokes 0 :stroke-cap-start] cap-start)
+
+                  (and (some? cap-end) (seq (:strokes shape)))
+                  (assoc-in [:strokes 0 :stroke-cap-end] cap-end))
+                shape)))
+
+          (update-container [container]
+            (d/update-when container :objects d/update-vals fix-shape))]
+
+    (-> data
+        (update :pages-index d/update-vals update-container)
+        (d/update-when :components d/update-vals update-container))))
+
 (def available-migrations
   (into (d/ordered-set)
         ["legacy-2"
@@ -1917,4 +1940,5 @@
          "0020-sync-component-id-with-near-main"
          "0021-fix-shape-svg-attrs"
          "0022-normalize-component-root-and-resync"
-         "0023-repair-token-themes-with-inexistent-sets"]))
+         "0023-repair-token-themes-with-inexistent-sets"
+         "0024-fix-stroke-cap-placement"]))

common/src/app/common/files/shapes_builder.cljc

@@ -609,17 +609,13 @@
       (and (some? color) (some? width))
       (assoc-in [:strokes 0 :stroke-width] width)
 
-      (and (some? linecap) (cfh/path-shape? shape)
+      (and (some? color) (some? linecap) (cfh/path-shape? shape)
            (or (= linecap :round) (= linecap :square)))
+      (assoc-in [:strokes 0 :stroke-cap-start] linecap)
 
-      (assoc :stroke-cap-start linecap
-             :stroke-cap-end linecap
-             :stroke-linecap linecap)
-
-      (d/any-key? (dm/get-in shape [:strokes 0])
-                  :strokeColor :strokeOpacity :strokeWidth
-                  :strokeLinecap :strokeCapStart :strokeCapEnd)
-      (assoc-in [:strokes 0 :stroke-style] :svg))))
+      (and (some? color) (some? linecap) (cfh/path-shape? shape)
+           (or (= linecap :round) (= linecap :square)))
+      (assoc-in [:strokes 0 :stroke-cap-end] linecap))))
 
 (defn setup-opacity [shape]
   (cond-> shape

common/test/common_tests/files/shapes_builder_test.cljc

@@ -50,3 +50,44 @@
 
 (t/deftest resolve-element-name-empty-attrs-uses-tag-fallback
   (t/is (some? (sb/resolve-element-name :path {}))))
+
+;; Regression for https://tree.taiga.io/project/penpot/issue/8277
+;; stroke-linecap and stroke-linejoin on the SVG root must be inherited
+;; by child path shapes, and stroke-cap-start/end must be stored inside
+;; the stroke entry (not at the shape top level).
+(t/deftest svg-root-stroke-linecap-inherited-to-path-shapes
+  (let [svg-data {:name "icon"
+                  :tag :svg
+                  :attrs {:xmlns "http://www.w3.org/2000/svg"
+                          :width "24" :height "24"
+                          :viewBox "0 0 24 24"
+                          :fill "none"
+                          :stroke "currentColor"
+                          :stroke-width "2"
+                          :stroke-linecap "round"
+                          :stroke-linejoin "round"}
+                  :content [{:tag :line
+                             :attrs {:x1 "12" :y1 "8" :x2 "12" :y2 "12"}
+                             :content []}]}
+        [_root children] (sb/create-svg-shapes svg-data {:x 0 :y 0} {} nil nil #{} false)
+        path-shapes      (filter #(= :path (:type %)) children)]
+
+    ;; At least one path shape was created from the <line> element
+    (t/is (seq path-shapes))
+
+    (doseq [shape path-shapes]
+      (let [stroke (first (:strokes shape))]
+        ;; svg-attrs must carry stroke-linecap and stroke-linejoin for the renderers
+        (t/is (= "round" (get-in shape [:svg-attrs :strokeLinecap])))
+        (t/is (= "round" (get-in shape [:svg-attrs :strokeLinejoin])))
+
+        ;; stroke-cap-start/end must be inside the stroke entry, not at shape level
+        (t/is (= :round (:stroke-cap-start stroke))
+              "stroke-cap-start should be in the stroke entry")
+        (t/is (= :round (:stroke-cap-end stroke))
+              "stroke-cap-end should be in the stroke entry")
+        (t/is (nil? (:stroke-cap-start shape))
+              "stroke-cap-start must NOT be at the shape top level")
+
+        ;; stroke-style is not set at import time (nil means default solid rendering)
+        (t/is (nil? (:stroke-style stroke)))))))

common/test/common_tests/files_migrations_test.cljc

@@ -10,6 +10,7 @@
    [app.common.files.migrations :as cfm]
    [app.common.pprint :as pp]
    [app.common.types.file :as ctf]
+   [app.common.uuid :as uuid]
    [clojure.test :as t]))
 
 (defmethod cfm/migrate-data "test/1" [data _] (update data :sum inc))
@@ -26,3 +27,42 @@
             file' (cfm/migrate file nil)]
         (t/is (= cfm/available-migrations (:migrations file')))
         (t/is (= 3 (:sum (:data file'))))))))
+
+(t/deftest migration-0024-fix-stroke-cap-placement
+  (let [shape-id (uuid/next)
+        page-id  (uuid/next)
+        data     {:pages-index
+                  {page-id
+                   {:objects
+                    {shape-id {:id         shape-id
+                               :type       :path
+                               :stroke-cap-start :round
+                               :stroke-cap-end   :round
+                               :strokes    [{:stroke-color "#000000"
+                                             :stroke-opacity 1
+                                             :stroke-style :svg
+                                             :stroke-width 2}]}}}}}
+        data'    (cfm/migrate-data data "0024-fix-stroke-cap-placement")]
+
+    (let [shape (get-in data' [:pages-index page-id :objects shape-id])]
+      (t/is (nil? (:stroke-cap-start shape)) "top-level cap removed")
+      (t/is (nil? (:stroke-cap-end shape)) "top-level cap removed")
+      (t/is (= :round (get-in shape [:strokes 0 :stroke-cap-start])) "cap moved into stroke")
+      (t/is (= :round (get-in shape [:strokes 0 :stroke-cap-end])) "cap moved into stroke"))))
+
+(t/deftest migration-0024-fix-stroke-cap-no-strokes
+  (let [shape-id (uuid/next)
+        page-id  (uuid/next)
+        data     {:pages-index
+                  {page-id
+                   {:objects
+                    {shape-id {:id               shape-id
+                               :type             :path
+                               :stroke-cap-start :round
+                               :stroke-cap-end   :round
+                               :strokes          []}}}}}
+        data'    (cfm/migrate-data data "0024-fix-stroke-cap-placement")]
+
+    (let [shape (get-in data' [:pages-index page-id :objects shape-id])]
+      (t/is (nil? (:stroke-cap-start shape)) "top-level cap removed even with no strokes")
+      (t/is (nil? (:stroke-cap-end shape)) "top-level cap removed even with no strokes"))))