From 0fe59cac940c7890fea254518d2079f628ab90df Mon Sep 17 00:00:00 2001 From: Alonso Torres Date: Wed, 27 May 2026 14:05:28 +0200 Subject: [PATCH] :bug: Fix problem with fill/stroke proxy properties (#9647) --- frontend/src/app/plugins/fills.cljs | 69 +++++++++++ frontend/src/app/plugins/format.cljs | 108 ------------------ frontend/src/app/plugins/gradients.cljs | 71 ++++++++++++ frontend/src/app/plugins/shape.cljs | 14 ++- frontend/src/app/plugins/strokes.cljs | 77 +++++++++++++ frontend/src/app/plugins/text.cljs | 3 +- .../plugins/context_shapes_test.cljs | 59 +++++++--- 7 files changed, 267 insertions(+), 134 deletions(-) create mode 100644 frontend/src/app/plugins/fills.cljs create mode 100644 frontend/src/app/plugins/gradients.cljs create mode 100644 frontend/src/app/plugins/strokes.cljs diff --git a/frontend/src/app/plugins/fills.cljs b/frontend/src/app/plugins/fills.cljs new file mode 100644 index 0000000000..15895ed12b --- /dev/null +++ b/frontend/src/app/plugins/fills.cljs @@ -0,0 +1,69 @@ +;; 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 app.plugins.fills + (:require + [app.plugins.format :as format] + [app.plugins.gradients :as gradients] + [app.plugins.parser :as parser] + [app.util.object :as obj])) + +(defn fill-proxy + [fill-data on-change!] + (let [state (atom fill-data)] + (obj/reify {:name "FillProxy"} + :fillColor + {:get (fn [] (:fill-color @state)) + :set (fn [v] + (swap! state #(-> % (assoc :fill-color v) (dissoc :fill-color-gradient :fill-image))) + (on-change!))} + + :fillOpacity + {:get (fn [] (:fill-opacity @state)) + :set (fn [v] (swap! state assoc :fill-opacity v) (on-change!))} + + :fillColorGradient + {:get (fn [] + (when-let [gradient (:fill-color-gradient @state)] + (let [gradient-state (atom gradient) + gradient-change! (fn [] + (swap! state assoc :fill-color-gradient @gradient-state) + (on-change!))] + (gradients/gradient-proxy gradient-state gradient-change!)))) + :set (fn [v] + (swap! state #(-> % (assoc :fill-color-gradient (parser/parse-gradient v)) (dissoc :fill-color :fill-image))) + (on-change!))} + + :fillColorRefFile + {:get (fn [] (format/format-id (:fill-color-ref-file @state))) + :set (fn [v] (swap! state assoc :fill-color-ref-file (parser/parse-id v)) (on-change!))} + + :fillColorRefId + {:get (fn [] (format/format-id (:fill-color-ref-id @state))) + :set (fn [v] (swap! state assoc :fill-color-ref-id (parser/parse-id v)) (on-change!))} + + :fillImage + {:get (fn [] (format/format-image (:fill-image @state))) + :set (fn [v] + (swap! state #(-> % (assoc :fill-image (parser/parse-image-data v)) (dissoc :fill-color :fill-color-gradient))) + (on-change!))}))) + +(defn format-fills + ([fills] (format-fills fills nil)) + ([fills commit-fn] + (cond + (= fills :multiple) "mixed" + (= fills "mixed") "mixed" + + (and (some? fills) (fn? commit-fn)) + (let [arr-ref (atom nil) + on-change! (fn [] (commit-fn @arr-ref)) + arr (apply array (mapv #(fill-proxy % on-change!) fills))] + (reset! arr-ref arr) + arr) + + :else + (format/format-array format/format-fill fills)))) diff --git a/frontend/src/app/plugins/format.cljs b/frontend/src/app/plugins/format.cljs index 9a488a5e71..c5947d75c0 100644 --- a/frontend/src/app/plugins/format.cljs +++ b/frontend/src/app/plugins/format.cljs @@ -26,96 +26,6 @@ (when (some? coll) (apply array (keep format-fn coll)))) -(defn- numeric-index? - [prop] - (and (string? prop) (boolean (re-matches #"\d+" prop)))) - -(defn- normalize-exclusive-color-props! - [target prop] - (case prop - "fillColor" - (do - (js-delete target "fillColorGradient") - (js-delete target "fillImage")) - - "fillColorGradient" - (do - (js-delete target "fillColor") - (js-delete target "fillImage")) - - "fillImage" - (do - (js-delete target "fillColor") - (js-delete target "fillColorGradient")) - - "strokeColor" - (js-delete target "strokeColorGradient") - - "strokeColorGradient" - (js-delete target "strokeColor") - - nil)) - -(declare wrap-mutable-value) - -(defn- wrap-mutable-object - [^js js-obj commit!] - (doseq [prop (js/Object.keys js-obj)] - (obj/set! js-obj prop (wrap-mutable-value (obj/get js-obj prop) commit!))) - (js/Proxy. js-obj - #js {:set (fn [target prop value] - (obj/set! target prop (wrap-mutable-value value commit!)) - (normalize-exclusive-color-props! target prop) - (commit!) - true) - :deleteProperty (fn [target prop] - (js-delete target prop) - (commit!) - true)})) - -(defn- wrap-mutable-array - [^js js-arr commit!] - (doseq [index (range (.-length js-arr))] - (obj/set! js-arr index (wrap-mutable-value (obj/get js-arr index) commit!))) - (js/Proxy. js-arr - #js {:set (fn [target prop value] - (if (or (numeric-index? prop) (= prop "length")) - (do - (if (numeric-index? prop) - (obj/set! target prop (wrap-mutable-value value commit!)) - (obj/set! target prop value)) - (commit!) - true) - false)) - :deleteProperty (fn [target prop] - (if (numeric-index? prop) - (do - (js-delete target prop) - true) - false))})) - -(defn- wrap-mutable-value - [value commit!] - (cond - (obj/array? value) - (wrap-mutable-array value commit!) - - (obj/plain-object? value) - (wrap-mutable-object value commit!) - - :else - value)) - -(defn wrap-mutable-element - [^js js-obj commit!] - (when (some? js-obj) - (wrap-mutable-value js-obj commit!))) - -(defn mutable-proxy-array - [coll format-fn commit-fn] - (let [raw-arr (format-array format-fn coll) - commit! (fn [] (commit-fn raw-arr))] - (wrap-mutable-array raw-arr commit!))) (defn format-mixed [value] @@ -288,18 +198,6 @@ :fillColorRefId (format-id fill-color-ref-id) :fillImage (format-image fill-image)}))) -(defn format-fills - ([fills] (format-fills fills nil)) - ([fills commit-fn] - (cond - (= fills :multiple) "mixed" - (= fills "mixed") "mixed" - - (and (some? fills) (fn? commit-fn)) - (mutable-proxy-array fills format-fill commit-fn) - - :else - (format-array format-fill fills)))) ;; export interface Stroke { ;; strokeColor?: string; @@ -331,12 +229,6 @@ :strokeCapEnd (format-key stroke-cap-end) :strokeColorGradient (format-gradient stroke-color-gradient)}))) -(defn format-strokes - ([strokes] (format-strokes strokes nil)) - ([strokes commit-fn] - (if (and (some? strokes) (fn? commit-fn)) - (mutable-proxy-array strokes format-stroke commit-fn) - (format-array format-stroke strokes)))) ;; export interface Blur { ;; id?: string; diff --git a/frontend/src/app/plugins/gradients.cljs b/frontend/src/app/plugins/gradients.cljs new file mode 100644 index 0000000000..809c8c768e --- /dev/null +++ b/frontend/src/app/plugins/gradients.cljs @@ -0,0 +1,71 @@ +;; 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 app.plugins.gradients + (:require + [app.plugins.format :as format] + [app.plugins.parser :as parser] + [app.util.object :as obj])) + +(defn stop-proxy + [stop-data on-change!] + (let [state (atom stop-data)] + (obj/reify {:name "GradientStopProxy"} + :color + {:get (fn [] (:color @state)) + :set (fn [v] (swap! state assoc :color v) (on-change!))} + + :opacity + {:get (fn [] (:opacity @state)) + :set (fn [v] (swap! state assoc :opacity v) (on-change!))} + + :offset + {:get (fn [] (:offset @state)) + :set (fn [v] (swap! state assoc :offset v) (on-change!))}))) + +;; gradient-proxy takes an external atom `state` so the caller +;; (fill-proxy / stroke-proxy) can read @state after any change. +(defn gradient-proxy + [state on-change!] + (obj/reify {:name "GradientProxy"} + :type + {:get (fn [] (format/format-key (:type @state))) + :set (fn [v] (swap! state assoc :type (parser/parse-keyword v)) (on-change!))} + + :startX + {:get (fn [] (:start-x @state)) + :set (fn [v] (swap! state assoc :start-x v) (on-change!))} + + :startY + {:get (fn [] (:start-y @state)) + :set (fn [v] (swap! state assoc :start-y v) (on-change!))} + + :endX + {:get (fn [] (:end-x @state)) + :set (fn [v] (swap! state assoc :end-x v) (on-change!))} + + :endY + {:get (fn [] (:end-y @state)) + :set (fn [v] (swap! state assoc :end-y v) (on-change!))} + + :width + {:get (fn [] (:width @state)) + :set (fn [v] (swap! state assoc :width v) (on-change!))} + + :stops + {:get (fn [] + (let [stops-ref (atom nil) + stop-change! + (fn [] + (let [new-stops (into [] (map parser/parse-gradient-stop) @stops-ref)] + (swap! state assoc :stops new-stops) + (on-change!))) + arr (apply array (mapv #(stop-proxy % stop-change!) (:stops @state)))] + (reset! stops-ref arr) + arr)) + :set (fn [v] + (swap! state assoc :stops (into [] (map parser/parse-gradient-stop) v)) + (on-change!))})) diff --git a/frontend/src/app/plugins/shape.cljs b/frontend/src/app/plugins/shape.cljs index 133b282237..dda0f704ca 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -48,12 +48,14 @@ [app.main.data.workspace.variants :as dwv] [app.main.repo :as rp] [app.main.store :as st] + [app.plugins.fills :as fills] [app.plugins.flex :as flex] [app.plugins.format :as format] [app.plugins.grid :as grid] [app.plugins.parser :as parser] [app.plugins.register :as r] [app.plugins.ruler-guides :as rg] + [app.plugins.strokes :as strokes] [app.plugins.system-events :as se] [app.plugins.text :as text] [app.plugins.tokens :refer [applied-tokens-plugin->applied-tokens token-attr-plugin->token-attr token-attr?]] @@ -760,17 +762,17 @@ :fills {:this true :get (fn [^js self] - (let [fills (if (cfh/text-shape? data) - (-> self u/proxy->shape text-props :fills) - (-> self u/proxy->shape :fills))] - (format/format-fills fills #(commit-fills! plugin-id self %)))) + (let [fill-data (if (cfh/text-shape? data) + (-> self u/proxy->shape text-props :fills) + (-> self u/proxy->shape :fills))] + (fills/format-fills fill-data #(commit-fills! plugin-id self %)))) :set (fn [self value] (commit-fills! plugin-id self value))} :strokes {:this true :get (fn [^js self] - (format/format-strokes (-> self u/proxy->shape :strokes) - #(commit-strokes! plugin-id self %))) + (strokes/format-strokes (-> self u/proxy->shape :strokes) + #(commit-strokes! plugin-id self %))) :set (fn [self value] (commit-strokes! plugin-id self value))} :layoutChild diff --git a/frontend/src/app/plugins/strokes.cljs b/frontend/src/app/plugins/strokes.cljs new file mode 100644 index 0000000000..59e305d4aa --- /dev/null +++ b/frontend/src/app/plugins/strokes.cljs @@ -0,0 +1,77 @@ +;; 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 app.plugins.strokes + (:require + [app.plugins.format :as format] + [app.plugins.gradients :as gradients] + [app.plugins.parser :as parser] + [app.util.object :as obj])) + +(defn stroke-proxy + [stroke-data on-change!] + (let [state (atom stroke-data)] + (obj/reify {:name "StrokeProxy"} + :strokeColor + {:get (fn [] (:stroke-color @state)) + :set (fn [v] + (swap! state #(-> % (assoc :stroke-color v) (dissoc :stroke-color-gradient))) + (on-change!))} + + :strokeColorRefFile + {:get (fn [] (format/format-id (:stroke-color-ref-file @state))) + :set (fn [v] (swap! state assoc :stroke-color-ref-file (parser/parse-id v)) (on-change!))} + + :strokeColorRefId + {:get (fn [] (format/format-id (:stroke-color-ref-id @state))) + :set (fn [v] (swap! state assoc :stroke-color-ref-id (parser/parse-id v)) (on-change!))} + + :strokeOpacity + {:get (fn [] (:stroke-opacity @state)) + :set (fn [v] (swap! state assoc :stroke-opacity v) (on-change!))} + + :strokeStyle + {:get (fn [] (format/format-key (:stroke-style @state))) + :set (fn [v] (swap! state assoc :stroke-style (parser/parse-keyword v)) (on-change!))} + + :strokeWidth + {:get (fn [] (:stroke-width @state)) + :set (fn [v] (swap! state assoc :stroke-width v) (on-change!))} + + :strokeAlignment + {:get (fn [] (format/format-key (:stroke-alignment @state))) + :set (fn [v] (swap! state assoc :stroke-alignment (parser/parse-keyword v)) (on-change!))} + + :strokeCapStart + {:get (fn [] (format/format-key (:stroke-cap-start @state))) + :set (fn [v] (swap! state assoc :stroke-cap-start (parser/parse-keyword v)) (on-change!))} + + :strokeCapEnd + {:get (fn [] (format/format-key (:stroke-cap-end @state))) + :set (fn [v] (swap! state assoc :stroke-cap-end (parser/parse-keyword v)) (on-change!))} + + :strokeColorGradient + {:get (fn [] + (when-let [gradient (:stroke-color-gradient @state)] + (let [gradient-state (atom gradient) + gradient-change! (fn [] + (swap! state assoc :stroke-color-gradient @gradient-state) + (on-change!))] + (gradients/gradient-proxy gradient-state gradient-change!)))) + :set (fn [v] + (swap! state #(-> % (assoc :stroke-color-gradient (parser/parse-gradient v)) (dissoc :stroke-color))) + (on-change!))}))) + +(defn format-strokes + ([strokes] (format-strokes strokes nil)) + ([strokes commit-fn] + (if (and (some? strokes) (fn? commit-fn)) + (let [arr-ref (atom nil) + on-change! (fn [] (commit-fn @arr-ref)) + arr (apply array (mapv #(stroke-proxy % on-change!) strokes))] + (reset! arr-ref arr) + arr) + (format/format-array format/format-stroke strokes)))) diff --git a/frontend/src/app/plugins/text.cljs b/frontend/src/app/plugins/text.cljs index aa4539c980..d36f34603c 100644 --- a/frontend/src/app/plugins/text.cljs +++ b/frontend/src/app/plugins/text.cljs @@ -19,6 +19,7 @@ [app.main.features :as features] [app.main.fonts :as fonts] [app.main.store :as st] + [app.plugins.fills :as fills] [app.plugins.format :as format] [app.plugins.parser :as parser] [app.plugins.register :as r] @@ -367,7 +368,7 @@ (fn [self] (let [range-data (-> self u/proxy->shape :content (content-range->text+styles start end))] - (->> range-data (map :fills) u/mixed-value format/format-fills))) + (->> range-data (map :fills) u/mixed-value fills/format-fills))) :set (fn [_ value] (let [value (parser/parse-fills value)] diff --git a/frontend/test/frontend_tests/plugins/context_shapes_test.cljs b/frontend/test/frontend_tests/plugins/context_shapes_test.cljs index 80a3051ba0..5ae0b9b6dd 100644 --- a/frontend/test/frontend_tests/plugins/context_shapes_test.cljs +++ b/frontend/test/frontend_tests/plugins/context_shapes_test.cljs @@ -248,20 +248,6 @@ (t/is (= (get-in @store (get-shape-path :fills)) [{:fill-color "#ff0000" :fill-opacity 1}])) (t/is (= (-> (. shape -fills) (aget 0) (aget "fillColor")) "#ff0000"))) - (t/testing " - fills element replacement (bug #8357)" - (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}]) - (aset (.-fills shape) 0 #js {:fillColor "#00ff00" :fillOpacity 0.5}) - (t/is (= (get-in @store (get-shape-path :fills)) [{:fill-color "#00ff00" :fill-opacity 0.5}]))) - - (t/testing " - fills push/pop (bug #8357)" - (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}]) - (.push (.-fills shape) #js {:fillColor "#00ff00" :fillOpacity 1}) - (t/is (= (get-in @store (get-shape-path :fills)) - [{:fill-color "#fabada" :fill-opacity 1} - {:fill-color "#00ff00" :fill-opacity 1}])) - (.pop (.-fills shape)) - (t/is (= (get-in @store (get-shape-path :fills)) [{:fill-color "#fabada" :fill-opacity 1}]))) - (t/testing " - fills gradient assignment replaces solid color (bug #8357)" (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}]) (obj/set! (aget (.-fills shape) 0) "fillColorGradient" (gradient)) @@ -286,11 +272,6 @@ (obj/set! (aget (.-strokes shape) 0) "strokeColor" "#0000ff") (t/is (= (get-in @store (get-shape-path :strokes)) [{:stroke-color "#0000ff" :stroke-opacity 1 :stroke-width 5}]))) - (t/testing " - strokes element replacement (bug #8357)" - (set! (.-strokes shape) #js [#js {:strokeColor "#fabada" :strokeOpacity 1 :strokeWidth 5}]) - (aset (.-strokes shape) 0 #js {:strokeColor "#00ff00" :strokeOpacity 0.5 :strokeWidth 2}) - (t/is (= (get-in @store (get-shape-path :strokes)) [{:stroke-color "#00ff00" :stroke-opacity 0.5 :stroke-width 2}]))) - (t/testing " - strokes gradient assignment replaces solid color (bug #8357)" (set! (.-strokes shape) #js [#js {:strokeColor "#fabada" :strokeOpacity 1 :strokeWidth 5}]) (obj/set! (aget (.-strokes shape) 0) "strokeColorGradient" (gradient)) @@ -310,6 +291,46 @@ (assoc :end-y 0.75) (assoc-in [:stops 1 :opacity] 0.25))}]))))) + (t/testing "Text shape fills" + (let [^js text (.createText context "Hello")] + + (t/testing " - flat fill set and read-back" + (set! (.-fills text) #js [#js {:fillColor "#aa00aa" :fillOpacity 0.9}]) + (t/is (= (-> (. text -fills) (aget 0) (aget "fillColor")) "#aa00aa")) + (t/is (= (-> (. text -fills) (aget 0) (aget "fillOpacity")) 0.9))) + + (t/testing " - in-place fill color mutation" + (set! (.-fills text) #js [#js {:fillColor "#fabada" :fillOpacity 1}]) + (obj/set! (aget (.-fills text) 0) "fillColor" "#00ccdd") + (obj/set! (aget (.-fills text) 0) "fillOpacity" 0.5) + (t/is (= (-> (. text -fills) (aget 0) (aget "fillColor")) "#00ccdd")) + (t/is (= (-> (. text -fills) (aget 0) (aget "fillOpacity")) 0.5))) + + (t/testing " - gradient fill set" + (set! (.-fills text) #js [#js {:fillColorGradient (gradient) :fillOpacity 1}]) + (let [g (-> (. text -fills) (aget 0) (aget "fillColorGradient"))] + (t/is (= (aget g "type") "linear")) + (t/is (= (-> g (aget "stops") (aget 0) (aget "color")) "#b400ff")) + (t/is (= (-> g (aget "stops") (aget 1) (aget "color")) "#0c3fd5")))) + + (t/testing " - gradient stop mutation" + (set! (.-fills text) #js [#js {:fillColorGradient (gradient) :fillOpacity 1}]) + (let [fill-gradient (-> (. text -fills) (aget 0) (aget "fillColorGradient")) + stop (-> fill-gradient (aget "stops") (aget 0))] + (obj/set! fill-gradient "startX" 0.1) + (obj/set! stop "color" "#ffff00") + (obj/set! stop "opacity" 0.5) + (let [g2 (-> (. text -fills) (aget 0) (aget "fillColorGradient"))] + (t/is (= (aget g2 "startX") 0.1)) + (t/is (= (-> g2 (aget "stops") (aget 0) (aget "color")) "#ffff00")) + (t/is (= (-> g2 (aget "stops") (aget 0) (aget "opacity")) 0.5))))) + + (t/testing " - fillColor clears fillColorGradient" + (set! (.-fills text) #js [#js {:fillColorGradient (gradient) :fillOpacity 1}]) + (obj/set! (aget (.-fills text) 0) "fillColor" "#123456") + (t/is (= (-> (. text -fills) (aget 0) (aget "fillColor")) "#123456")) + (t/is (nil? (-> (. text -fills) (aget 0) (aget "fillColorGradient"))))))) + (t/testing "Relative properties" (let [board (.createBoard context)] (set! (.-x board) 100)