🐛 Fix Heroicons arrow paths broken after SVG import (#5283) (#9156)

Signed-off-by: statxc <181730535+statxc@users.noreply.github.com>
Co-authored-by: Andrey Antukh <niwi@niwi.nz>

CHANGES.md

@@ -54,6 +54,7 @@
 ### :bug: Bugs fixed
 
 - Fix plugin API `fileVersion.restore()` promise hanging indefinitely on restore failure [Github #9092](https://github.com/penpot/penpot/issues/9092)
+- Fix imported stroke-only SVG paths losing their rounded join when authoring tools (e.g. Figma → Heroicons) split a continuous polyline into adjacent `M…L M…L` subpaths sharing an endpoint; on import these are now folded back into one chain so `stroke-linejoin` renders the elbow correctly in both editor and exports [Github #5283](https://github.com/penpot/penpot/issues/5283)
 - Fix plugin API `library.connectLibrary()` returning a non-Promise (or throwing synchronously) when the plugin lacks `library:write` permission — the method now always returns a `Promise` and rejects with a structured error message, matching the contract used by every other Promise-returning plugin method (`restore`, `remove`, `pin`, `saveVersion`, `findVersions`, …)
 - Fix LDAP provider params schema typo (`bind-passwor` → `bind-password`) introduced during the `clojure.spec` → `malli` migration; the schema slot now matches the runtime key actually read by `prepare-params` (`:password (:bind-password cfg)`) and `try-connectivity` (`(:bind-password cfg)`), so a wrong type for the password no longer slips through unvalidated
 - Fix `login-with-ldap` silently dropping its error message on the `ldap-not-initialized` restriction (typo `:hide` → `:hint`); the message `"ldap auth provider is not initialized"` now actually surfaces in logs and error responses instead of being discarded into an unread key

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

@@ -340,12 +340,26 @@
       :svg-viewbox vbox
       :svg-defs defs})))
 
+(defn- stroke-only-svg-path?
+  "Returns true when the SVG element renders only a stroke (fill=none).
+  Stroke-only paths can have their consecutive touching subpaths safely
+  merged into a continuous polyline so that `stroke-linejoin` applies at
+  shared endpoints, without affecting any fill-rule semantics."
+  [attrs]
+  (let [attr-fill  (some-> (:fill attrs) str/trim)
+        style-fill (some-> (get-in attrs [:style :fill]) str/trim)]
+    (= "none" (or attr-fill style-fill))))
+
 (defn create-path-shape [name frame-id svg-data {:keys [attrs] :as data}]
   (when (and (contains? attrs :d) (seq (:d attrs)))
-    (let [transform (csvg/parse-transform (:transform attrs))
-          content   (cond-> (path/from-string (:d attrs))
-                      (some? transform)
-                      (path.segm/transform-content transform))
+    (let [transform    (csvg/parse-transform (:transform attrs))
+          stroke-only? (stroke-only-svg-path? attrs)
+          content      (cond-> (path/from-string (:d attrs))
+                         stroke-only?
+                         (path/merge-touching-subpaths)
+
+                         (some? transform)
+                         (path.segm/transform-content transform))
 
           selrect    (path.segm/content->selrect content)
           points     (grc/rect->points selrect)

common/src/app/common/types/path.cljc

@@ -84,6 +84,19 @@
   (-> (subpath/close-subpaths content)
       (impl/from-plain)))
 
+(defn merge-touching-subpaths
+  "Given a content, fold consecutive subpaths whose endpoints coincide
+  into a single continuous subpath, returning a PathData instance.
+
+  Conservative counterpart of `close-subpaths`: only adjacent subpaths
+  are merged and none are reversed, so fill rules and stroke-dasharray
+  semantics are preserved. Used at SVG-import time on stroke-only paths
+  to recover the `stroke-linejoin` rendering when authoring tools split
+  a continuous polyline into adjacent `M..L M..L` subpaths."
+  [content]
+  (-> (subpath/merge-touching-subpaths content)
+      (impl/from-plain)))
+
 (defn apply-content-modifiers
   "Apply delta modifiers over the path content"
   [content modifiers]

common/src/app/common/types/path/subpath.cljc

@@ -128,6 +128,36 @@
 (def ^:private xf-mapcat-data
   (mapcat :data))
 
+(defn- join-adjacent
+  "Fold neighbouring subpaths into the accumulator only when the
+  current accumulator's end-point matches the next subpath's start-point.
+  Unlike `merge-paths` this does not reverse subpaths nor reorder them;
+  the original draw order is preserved so stroke-dasharray and animation
+  semantics stay intact."
+  [acc subpath]
+  (if-let [prev (peek acc)]
+    (if (and (not (is-closed? prev))
+             (not (is-closed? subpath))
+             (pt= (:to prev) (:from subpath)))
+      (conj (pop acc) (subpaths-join prev subpath))
+      (conj acc subpath))
+    (conj acc subpath)))
+
+(defn merge-touching-subpaths
+  "Merge consecutive subpaths whose endpoints coincide into a single
+  continuous subpath, preserving the original drawing order.
+
+  This is a conservative variant of `close-subpaths`: it never reverses
+  a subpath and only merges immediate neighbours, so closed regions and
+  fill semantics are left untouched. The intent is to recover the
+  `stroke-linejoin` rendering for SVG paths whose authoring tools split
+  a continuous polyline into adjacent `M..L M..L` subpaths (e.g. the
+  `m0 0` markers Figma emits when exporting Heroicons-like icons)."
+  [content]
+  (let [subpaths (get-subpaths content)
+        merged   (reduce join-adjacent [] subpaths)]
+    (into [] xf-mapcat-data merged)))
+
 (defn close-subpaths
   "Searches a path for possible subpaths that can create closed loops and merge them"
   [content]

common/test/common_tests/types/path_data_test.cljc

@@ -667,6 +667,41 @@
           result (path.subpath/close-subpaths content)]
       (t/is (seq result)))))
 
+(t/deftest subpath-merge-touching-subpaths
+  (t/testing "adjacent subpaths sharing an endpoint collapse into one chain"
+    ;; Heroicons-style fragment: continuous polyline split as M-L M-L M-L
+    ;; with the second/third subpath starting at the first's endpoint.
+    (let [content [{:command :move-to :params {:x 0.0  :y 10.0}}
+                   {:command :line-to :params {:x 10.0 :y 10.0}}
+                   {:command :move-to :params {:x 10.0 :y 10.0}}
+                   {:command :line-to :params {:x 5.0  :y 0.0}}
+                   {:command :move-to :params {:x 10.0 :y 10.0}}
+                   {:command :line-to :params {:x 5.0  :y 20.0}}]
+          result  (path.subpath/merge-touching-subpaths content)
+          moves   (filter #(= :move-to (:command %)) result)]
+      ;; Subpaths 1+2 share (10,10) → merged. Subpath 3 also starts at (10,10),
+      ;; but the merged chain now ends at (5,0), so it does NOT match and
+      ;; is preserved as its own subpath. Two move-tos in the final result.
+      (t/is (= 2 (count moves)))
+      (t/is (= 5 (count result)))))
+  (t/testing "non-touching subpaths are left untouched"
+    (let [content [{:command :move-to :params {:x 0.0  :y 0.0}}
+                   {:command :line-to :params {:x 5.0  :y 0.0}}
+                   {:command :move-to :params {:x 50.0 :y 50.0}}
+                   {:command :line-to :params {:x 60.0 :y 60.0}}]
+          result  (path.subpath/merge-touching-subpaths content)]
+      (t/is (= content (vec result)))))
+  (t/testing "closed subpath is not absorbed into a neighbour"
+    (let [content [{:command :move-to   :params {:x 0.0 :y 0.0}}
+                   {:command :line-to   :params {:x 5.0 :y 0.0}}
+                   {:command :line-to   :params {:x 5.0 :y 5.0}}
+                   {:command :line-to   :params {:x 0.0 :y 0.0}}
+                   {:command :move-to   :params {:x 0.0 :y 0.0}}
+                   {:command :line-to   :params {:x 1.0 :y 1.0}}]
+          result  (path.subpath/merge-touching-subpaths content)
+          moves   (filter #(= :move-to (:command %)) result)]
+      (t/is (= 2 (count moves))))))
+
 (t/deftest subpath-reverse-content
   (let [result (path.subpath/reverse-content simple-open-content)]
     (t/is (= (count simple-open-content) (count result)))
@@ -1100,6 +1135,24 @@
     (t/is (path/content? result))
     (t/is (seq (vec result)))))
 
+(t/deftest path-merge-touching-subpaths
+  (t/testing "regression for #5283 — heroicons arrow path serialises as a single chain"
+    ;; SVG `d` originally split a continuous polyline by inserting a
+    ;; redundant moveto at the elbow. Importing it must collapse the
+    ;; first two subpaths so that stroke-linejoin renders the rounded tip.
+    (let [content (path/from-string
+                   (str "M350.5,1846 L365.5,1846"
+                        " M365.5,1846 L358.75,1839.25"
+                        " M365.5,1846 L358.75,1852.75"))
+          merged   (path/merge-touching-subpaths content)
+          rendered (str merged)]
+      (t/is (path/content? merged))
+      ;; First two subpaths fold into M ... L ... L ... ; third stays
+      ;; separate (its start point matches the original M, not the merged
+      ;; chain's tail), so exactly two M commands remain.
+      (t/is (= 2 (count (re-seq #"M" rendered))))
+      (t/is (= 3 (count (re-seq #"L" rendered)))))))
+
 (t/deftest path-move-content
   (let [content  (path/content sample-content-square)
         move-vec (gpt/point 3.0 4.0)