diff --git a/CHANGES.md b/CHANGES.md index 57885fabe9..edfacd3ad2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -133,6 +133,7 @@ - Fix broken update library notification link in the UI [Github #9070](https://github.com/penpot/penpot/issues/9070) - Fix plugin API `ShapeBase.component()` returning the outermost component instead of the immediate component in case of nested component instances [Github #9183](https://github.com/penpot/penpot/issues/9183) - Fix missing `labels.open` translation that surfaced the raw key as the typography font open-button `aria-label`, breaking screen-reader output (by @MilosM348) +- Fix plugin API `shape.fills` and `shape.strokes` arrays being read-only [Github #8357](https://github.com/penpot/penpot/issues/8357) ## 2.15.0 (Unreleased) diff --git a/frontend/src/app/plugins/format.cljs b/frontend/src/app/plugins/format.cljs index f0ff928fc8..9a488a5e71 100644 --- a/frontend/src/app/plugins/format.cljs +++ b/frontend/src/app/plugins/format.cljs @@ -26,6 +26,97 @@ (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] (if (= value :multiple) @@ -198,16 +289,17 @@ :fillImage (format-image fill-image)}))) (defn format-fills - [fills] - (cond - (= fills :multiple) - "mixed" + ([fills] (format-fills fills nil)) + ([fills commit-fn] + (cond + (= fills :multiple) "mixed" + (= fills "mixed") "mixed" - (= fills "mixed") - "mixed" + (and (some? fills) (fn? commit-fn)) + (mutable-proxy-array fills format-fill commit-fn) - (some? fills) - (format-array format-fill fills))) + :else + (format-array format-fill fills)))) ;; export interface Stroke { ;; strokeColor?: string; @@ -240,9 +332,11 @@ :strokeColorGradient (format-gradient stroke-color-gradient)}))) (defn format-strokes - [strokes] - (when (some? strokes) - (format-array format-stroke 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/shape.cljs b/frontend/src/app/plugins/shape.cljs index 63037e405c..b6b610920e 100644 --- a/frontend/src/app/plugins/shape.cljs +++ b/frontend/src/app/plugins/shape.cljs @@ -173,6 +173,38 @@ :hidden false} blur)) +(defn commit-fills! + [plugin-id ^js self value] + (let [shape (u/proxy->shape self) + id (:id shape) + value (parser/parse-fills value)] + (cond + (not (sm/validate [:vector types.fills/schema:fill] value)) + (u/not-valid plugin-id :fills value) + + (cfh/text-shape? shape) + (st/emit! (dwt/update-attrs id {:fills value})) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :fills "Plugin doesn't have 'content:write' permission") + + :else + (st/emit! (dwsh/update-shapes [id] #(assoc % :fills value)))))) + +(defn commit-strokes! + [plugin-id ^js self value] + (let [id (obj/get self "$id") + value (parser/parse-strokes value)] + (cond + (not (sm/validate [:vector cts/schema:stroke] value)) + (u/not-valid plugin-id :strokes value) + + (not (r/check-permission plugin-id "content:write")) + (u/not-valid plugin-id :strokes "Plugin doesn't have 'content:write' permission") + + :else + (st/emit! (dwsh/update-shapes [id] #(assoc % :strokes value)))))) + (defn shape-proxy? [p] (obj/type-of? p "ShapeProxy")) @@ -726,43 +758,19 @@ ;; Strokes and fills :fills {:this true - :get #(if (cfh/text-shape? data) - (-> % u/proxy->shape text-props :fills format/format-fills) - (-> % u/proxy->shape :fills format/format-fills)) - :set - (fn [self value] - (let [shape (u/proxy->shape self) - id (:id shape) - value (parser/parse-fills value)] - (cond - (not (sm/validate [:vector types.fills/schema:fill] value)) - (u/not-valid plugin-id :fills value) - - (cfh/text-shape? shape) - (st/emit! (dwt/update-attrs id {:fills value})) - - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :fills "Plugin doesn't have 'content:write' permission") - - :else - (st/emit! (dwsh/update-shapes [id] #(assoc % :fills value))))))} + :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 %)))) + :set (fn [self value] (commit-fills! plugin-id self value))} :strokes {:this true - :get #(-> % u/proxy->shape :strokes format/format-strokes) - :set - (fn [self value] - (let [id (obj/get self "$id") - value (parser/parse-strokes value)] - (cond - (not (sm/validate [:vector cts/schema:stroke] value)) - (u/not-valid plugin-id :strokes value) - - (not (r/check-permission plugin-id "content:write")) - (u/not-valid plugin-id :strokes "Plugin doesn't have 'content:write' permission") - - :else - (st/emit! (dwsh/update-shapes [id] #(assoc % :strokes value))))))} + :get (fn [^js self] + (format/format-strokes (-> self u/proxy->shape :strokes) + #(commit-strokes! plugin-id self %))) + :set (fn [self value] (commit-strokes! plugin-id self value))} :layoutChild {:this true diff --git a/frontend/test/frontend_tests/plugins/context_shapes_test.cljs b/frontend/test/frontend_tests/plugins/context_shapes_test.cljs index 39b44e861f..80a3051ba0 100644 --- a/frontend/test/frontend_tests/plugins/context_shapes_test.cljs +++ b/frontend/test/frontend_tests/plugins/context_shapes_test.cljs @@ -11,6 +11,7 @@ [app.common.uuid :as uuid] [app.main.store :as st] [app.plugins.api :as api] + [app.util.object :as obj] [cljs.test :as t :include-macros true] [frontend-tests.helpers.state :as ths] [frontend-tests.helpers.wasm :as thw])) @@ -30,7 +31,28 @@ ^js shape (.createRectangle context) get-shape-path - #(vector :files (aget file "$id") :data :pages-index (aget page "$id") :objects (aget shape "$id") %)] + #(vector :files (aget file "$id") :data :pages-index (aget page "$id") :objects (aget shape "$id") %) + + gradient + (fn [] + #js {:type "linear" + :startX 0.5 + :startY 0 + :endX 0.5 + :endY 1 + :width 1 + :stops #js [#js {:color "#b400ff" :opacity 1 :offset 0} + #js {:color "#0c3fd5" :opacity 1 :offset 1}]}) + + parsed-gradient + {:type :linear + :start-x 0.5 + :start-y 0 + :end-x 0.5 + :end-y 1 + :width 1 + :stops [{:color "#b400ff" :opacity 1 :offset 0} + {:color "#0c3fd5" :opacity 1 :offset 1}]}] (t/testing "Basic shape properites" (t/testing " - name" @@ -218,7 +240,75 @@ (t/is (= (get-in @store (get-shape-path :strokes)) [{:stroke-color "#fabada" :stroke-opacity 1 :stroke-width 5}])) (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeColor")) "#fabada")) (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeOpacity")) 1)) - (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeWidth")) 5)))) + (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeWidth")) 5))) + + (t/testing " - fills per-element property mutation (bug #8357)" + (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}]) + (obj/set! (aget (.-fills shape) 0) "fillColor" "#ff0000") + (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)) + (t/is (= (get-in @store (get-shape-path :fills)) + [{:fill-opacity 1 :fill-color-gradient parsed-gradient}])) + (t/is (nil? (-> (. shape -fills) (aget 0) (aget "fillColor"))))) + + (t/testing " - fills nested gradient mutation (bug #8357)" + (set! (.-fills shape) #js [#js {:fillColorGradient (gradient) :fillOpacity 1}]) + (let [fill-gradient (-> (. shape -fills) (aget 0) (aget "fillColorGradient")) + stop (-> fill-gradient (aget "stops") (aget 0))] + (obj/set! fill-gradient "startX" 0.25) + (obj/set! stop "color" "#ffffff") + (t/is (= (get-in @store (get-shape-path :fills)) + [{:fill-opacity 1 + :fill-color-gradient (-> parsed-gradient + (assoc :start-x 0.25) + (assoc-in [:stops 0 :color] "#ffffff"))}])))) + + (t/testing " - strokes per-element property mutation (bug #8357)" + (set! (.-strokes shape) #js [#js {:strokeColor "#fabada" :strokeOpacity 1 :strokeWidth 5}]) + (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)) + (t/is (= (get-in @store (get-shape-path :strokes)) + [{:stroke-opacity 1 :stroke-width 5 :stroke-color-gradient parsed-gradient}]))) + + (t/testing " - strokes nested gradient mutation (bug #8357)" + (set! (.-strokes shape) #js [#js {:strokeColorGradient (gradient) :strokeOpacity 1 :strokeWidth 5}]) + (let [stroke-gradient (-> (. shape -strokes) (aget 0) (aget "strokeColorGradient")) + stop (-> stroke-gradient (aget "stops") (aget 1))] + (obj/set! stroke-gradient "endY" 0.75) + (obj/set! stop "opacity" 0.25) + (t/is (= (get-in @store (get-shape-path :strokes)) + [{:stroke-opacity 1 + :stroke-width 5 + :stroke-color-gradient (-> parsed-gradient + (assoc :end-y 0.75) + (assoc-in [:stops 1 :opacity] 0.25))}]))))) (t/testing "Relative properties" (let [board (.createBoard context)]