0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2025-03-18 10:41:29 -05:00

🐛 Fix several issues related to invalid colors inserted on shadows

This commit is contained in:
Andrey Antukh 2024-09-13 15:17:02 +02:00
parent 162507264c
commit 179d534237
12 changed files with 266 additions and 113 deletions

View file

@ -6,4 +6,4 @@
(ns app.common.files.defaults)
(def version 53)
(def version 54)

View file

@ -863,11 +863,9 @@
(assoc shadow :color color)))
(update-object [object]
(d/update-when object :shadow
#(into []
(comp (map fix-shadow)
(filter valid-shadow?))
%)))
(let [xform (comp (map fix-shadow)
(filter valid-shadow?))]
(d/update-when object :shadow #(into [] xform %))))
(update-container [container]
(d/update-when container :objects update-vals update-object))]
@ -1029,6 +1027,25 @@
(update data :pages-index update-vals update-page)))
(defn migrate-up-54
"Fixes shapes with invalid colors in shadow: it first tries a non
destructive fix, and if it is not possible, then, shadow is removed"
[data]
(letfn [(fix-shadow [shadow]
(update shadow :color d/without-nils))
(update-shape [shape]
(let [xform (comp (map fix-shadow)
(filter valid-shadow?))]
(d/update-when shape :shadow #(into [] xform %))))
(update-container [container]
(d/update-when container :objects update-vals update-shape))]
(-> data
(update :pages-index update-vals update-container)
(update :components update-vals update-container))))
(def migrations
"A vector of all applicable migrations"
[{:id 2 :migrate-up migrate-up-2}
@ -1073,4 +1090,5 @@
{:id 50 :migrate-up migrate-up-50}
{:id 51 :migrate-up migrate-up-51}
{:id 52 :migrate-up migrate-up-52}
{:id 53 :migrate-up migrate-up-26}])
{:id 53 :migrate-up migrate-up-26}
{:id 54 :migrate-up migrate-up-54}])

View file

@ -80,21 +80,23 @@
[:opacity {:optional true} [:maybe ::sm/safe-number]]
[:offset ::sm/safe-number]]]]])
(def schema:color-attrs
[:map {:title "ColorAttrs"}
[:id {:optional true} ::sm/uuid]
[:name {:optional true} :string]
[:path {:optional true} [:maybe :string]]
[:value {:optional true} [:maybe :string]]
[:color {:optional true} [:maybe ::rgb-color]]
[:opacity {:optional true} [:maybe ::sm/safe-number]]
[:modified-at {:optional true} ::sm/inst]
[:ref-id {:optional true} ::sm/uuid]
[:ref-file {:optional true} ::sm/uuid]
[:gradient {:optional true} [:maybe schema:gradient]]
[:image {:optional true} [:maybe schema:image-color]]
[:plugin-data {:optional true} ::ctpg/plugin-data]])
(def schema:color
[:and
[:map {:title "Color"}
[:id {:optional true} ::sm/uuid]
[:name {:optional true} :string]
[:path {:optional true} [:maybe :string]]
[:value {:optional true} [:maybe :string]]
[:color {:optional true} [:maybe ::rgb-color]]
[:opacity {:optional true} [:maybe ::sm/safe-number]]
[:modified-at {:optional true} ::sm/inst]
[:ref-id {:optional true} ::sm/uuid]
[:ref-file {:optional true} ::sm/uuid]
[:gradient {:optional true} [:maybe schema:gradient]]
[:image {:optional true} [:maybe schema:image-color]]
[:plugin-data {:optional true} ::ctpg/plugin-data]]
[:and schema:color-attrs
[::sm/contains-any {:strict true} [:color :gradient :image]]])
(def schema:recent-color
@ -111,12 +113,13 @@
(sm/register! ::gradient schema:gradient)
(sm/register! ::image-color schema:image-color)
(sm/register! ::recent-color schema:recent-color)
(sm/register! ::color-attrs schema:color-attrs)
(def valid-color?
(sm/lazy-validator schema:color))
(def check-color!
(sm/check-fn schema:color))
(def valid-recent-color?
(sm/lazy-validator schema:recent-color))
(def check-recent-color!
(sm/check-fn schema:recent-color))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; HELPERS

View file

@ -125,6 +125,9 @@
(sm/register! ::stroke schema:stroke)
(def check-stroke!
(sm/check-fn schema:stroke))
(def schema:shape-base-attrs
[:map {:title "ShapeMinimalRecord"}
[:id ::sm/uuid]

View file

@ -27,3 +27,6 @@
[:color ::ctc/color]])
(sm/register! ::shadow schema:shadow)
(def check-shadow!
(sm/check-fn schema:shadow))

View file

@ -12,6 +12,9 @@
[app.common.files.helpers :as cfh]
[app.common.schema :as sm]
[app.common.text :as txt]
[app.common.types.color :as ctc]
[app.common.types.shape :refer [check-stroke!]]
[app.common.types.shape.shadow :refer [check-shadow!]]
[app.main.broadcast :as mbc]
[app.main.data.events :as ev]
[app.main.data.modal :as md]
@ -21,7 +24,6 @@
[app.main.data.workspace.state-helpers :as wsh]
[app.main.data.workspace.texts :as dwt]
[app.main.data.workspace.undo :as dwu]
[app.util.color :as uc]
[app.util.storage :refer [storage]]
[beicon.v2.core :as rx]
[cuerdas.core :as str]
@ -165,6 +167,15 @@
(defn add-fill
[ids color]
(dm/assert!
"expected a valid color struct"
(ctc/check-color! color))
(dm/assert!
"expected a valid coll of uuid's"
(every? uuid? ids))
(ptk/reify ::add-fill
ptk/WatchEvent
(watch [_ state _]
@ -175,6 +186,15 @@
(defn remove-fill
[ids color position]
(dm/assert!
"expected a valid color struct"
(ctc/check-color! color))
(dm/assert!
"expected a valid coll of uuid's"
(every? uuid? ids))
(ptk/reify ::remove-fill
ptk/WatchEvent
(watch [_ state _]
@ -187,13 +207,21 @@
(defn remove-all-fills
[ids color]
(dm/assert!
"expected a valid color struct"
(ctc/check-color! color))
(dm/assert!
"expected a valid coll of uuid's"
(every? uuid? ids))
(ptk/reify ::remove-all-fills
ptk/WatchEvent
(watch [_ state _]
(let [remove-all (fn [shape _] (assoc shape :fills []))]
(transform-fill state ids color remove-all)))))
(defn change-hide-fill-on-export
[ids hide-fill-on-export]
(ptk/reify ::change-hide-fill-on-export
@ -272,17 +300,25 @@
;; example using the color selection from
;; multiple shapes) let's use the first stop
;; color
attrs (cond-> attrs
(:gradient attrs) (get-in [:gradient :stops 0]))
new-attrs (-> (merge (get-in shape [:shadow index :color]) attrs)
(d/without-nils))]
(assoc-in shape [:shadow index :color] new-attrs))))))))
attrs (cond-> attrs
(:gradient attrs)
(dm/get-in [:gradient :stops 0]))
attrs' (-> (dm/get-in shape [:shadow index :color])
(merge attrs)
(d/without-nils))]
(assoc-in shape [:shadow index :color] attrs'))))))))
(defn add-shadow
[ids shadow]
(dm/assert!
"expected a valid shadow struct"
(check-shadow! shadow))
(dm/assert!
"expected a valid coll of uuid's"
(sm/check-coll-of-uuid! ids))
(every? uuid? ids))
(ptk/reify ::add-shadow
ptk/WatchEvent
@ -293,6 +329,15 @@
(defn add-stroke
[ids stroke]
(dm/assert!
"expected a valid stroke struct"
(check-stroke! stroke))
(dm/assert!
"expected a valid coll of uuid's"
(every? uuid? ids))
(ptk/reify ::add-stroke
ptk/WatchEvent
(watch [_ _ _]
@ -301,6 +346,11 @@
(defn remove-stroke
[ids position]
(dm/assert!
"expected a valid coll of uuid's"
(every? uuid? ids))
(ptk/reify ::remove-stroke
ptk/WatchEvent
(watch [_ _ _]
@ -314,6 +364,11 @@
(defn remove-all-strokes
[ids]
(dm/assert!
"expected a valid coll of uuid's"
(every? uuid? ids))
(ptk/reify ::remove-all-strokes
ptk/WatchEvent
(watch [_ _ _]
@ -376,7 +431,7 @@
:on-change handle-change-color}
:allow-click-outside true})))))))
(defn color-att->text
(defn- color-att->text
[color]
{:fill-color (when (:color color) (str/lower (:color color)))
:fill-opacity (:opacity color)
@ -395,26 +450,57 @@
(some? has-color?)
(assoc-in [:fills index] parsed-new-color))))
(def ^:private schema:change-color-operation
[:map
[:prop [:enum :fill :stroke :shadow :content]]
[:shape-id ::sm/uuid]
[:index :int]])
(def ^:private schema:change-color-operations
[:vector schema:change-color-operation])
(def ^:private check-change-color-operations!
(sm/check-fn schema:change-color-operations))
(defn change-color-in-selected
[new-color shapes-by-color old-color]
[operations new-color old-color]
(dm/verify!
"expected valid change color operations"
(check-change-color-operations! operations))
(dm/verify!
"expected a valid color struct for new-color param"
(ctc/check-color! new-color))
(dm/verify!
"expected a valid color struct for old-color param"
(ctc/check-color! old-color))
(ptk/reify ::change-color-in-selected
ptk/WatchEvent
(watch [_ _ _]
(let [undo-id (js/Symbol)]
(rx/concat
(rx/of (dwu/start-undo-transaction undo-id))
(->> (rx/from shapes-by-color)
(rx/map (fn [shape] (case (:prop shape)
:fill (change-fill [(:shape-id shape)] new-color (:index shape))
:stroke (change-stroke [(:shape-id shape)] new-color (:index shape))
:shadow (change-shadow [(:shape-id shape)] new-color (:index shape))
:content (dwt/update-text-with-function
(:shape-id shape)
(partial change-text-color old-color new-color (:index shape)))))))
(->> (rx/from operations)
(rx/map (fn [{:keys [shape-id index] :as operation}]
(case (:prop operation)
:fill (change-fill [shape-id] new-color index)
:stroke (change-stroke [shape-id] new-color index)
:shadow (change-shadow [shape-id] new-color index)
:content (dwt/update-text-with-function
shape-id
(partial change-text-color old-color new-color index))))))
(rx/of (dwu/commit-undo-transaction undo-id)))))))
(defn apply-color-from-palette
[color stroke?]
(dm/assert!
"should be a valid color"
(ctc/check-color! color))
(ptk/reify ::apply-color-from-palette
ptk/WatchEvent
(watch [_ state _]
@ -437,9 +523,10 @@
result (cond-> result (not group?) (conj cur))]
(recur (rest pending) result))))]
(if stroke?
(rx/of (change-stroke ids (merge uc/empty-color color) 0))
(rx/of (change-fill ids (merge uc/empty-color color) 0)))))))
(rx/of (change-stroke ids color 0))
(rx/of (change-fill ids color 0)))))))
(declare activate-colorpicker-color)
(declare activate-colorpicker-gradient)
@ -448,15 +535,22 @@
(defn apply-color-from-colorpicker
[color]
(dm/assert!
"expected valid color structure"
(ctc/check-color! color))
(ptk/reify ::apply-color-from-colorpicker
ptk/WatchEvent
(watch [_ _ _]
(rx/of
(cond
(:image color) (activate-colorpicker-image)
(:color color) (activate-colorpicker-color)
(= :linear (get-in color [:gradient :type])) (activate-colorpicker-gradient :linear-gradient)
(= :radial (get-in color [:gradient :type])) (activate-colorpicker-gradient :radial-gradient))))))
;; FIXME: revisit this
(let [gradient-type (dm/get-in color [:gradient :type])]
(rx/of
(cond
(:image color) (activate-colorpicker-image)
(:color color) (activate-colorpicker-color)
(= :linear gradient-type) (activate-colorpicker-gradient :linear-gradient)
(= :radial gradient-type) (activate-colorpicker-gradient :radial-gradient)))))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@ -596,7 +690,8 @@
(update :current-color merge changes)
(update :current-color materialize-color-components)
(update :current-color #(if (not= type :image) (dissoc % :image) %))
;; current color can be a library one I'm changing via colorpicker
;; current color can be a library one
;; I'm changing via colorpicker
(d/dissoc-in [:current-color :id])
(d/dissoc-in [:current-color :file-id]))]
(if-let [stop (:editing-stop state)]
@ -614,7 +709,8 @@
:colorpicker
:type)
formated-color (get-color-from-colorpicker-state (:colorpicker state))
;; Type is set to color on closing the colorpicker, but we can can close it while still uploading an image fill
;; Type is set to color on closing the colorpicker, but we
;; can can close it while still uploading an image fill
ignore-color? (and (= selected-type :color) (nil? (:color formated-color)))]
(when (and add-recent? (not ignore-color?))
(rx/of (dwl/add-recent-color formated-color)))))))
@ -686,6 +782,7 @@
(defn select-color
[position add-color]
;; FIXME: revisit
(ptk/reify ::select-color
ptk/WatchEvent
(watch [_ state _]

View file

@ -116,8 +116,13 @@
(update :id #(or % (uuid/next)))
(assoc :name (or (get-in color [:image :name])
(:color color)
(uc/gradient-type->string (get-in color [:gradient :type])))))]
(dm/assert! ::ctc/color color)
(uc/gradient-type->string (get-in color [:gradient :type]))))
(d/without-nils))]
(dm/assert!
"expect valid color structure"
(ctc/check-color! color))
(ptk/reify ::add-color
ev/Event
(-data [_] color)
@ -135,8 +140,8 @@
[color]
(dm/assert!
"expected valid recent color map"
(ctc/valid-recent-color? color))
"expected valid recent color structure"
(ctc/check-recent-color! color))
(ptk/reify ::add-recent-color
ptk/UpdateEvent
@ -155,7 +160,7 @@
(update [_ state]
(assoc-in state [:workspace-local :color-for-rename] nil))))
(defn- do-update-color
(defn- update-color*
[it state color file-id]
(let [data (get state :workspace-data)
[path name] (cfh/parse-path-name (:name color))
@ -171,19 +176,20 @@
(defn update-color
[color file-id]
(let [color (d/without-nils color)]
(dm/assert!
"expected valid parameters"
(ctc/valid-color? color))
(dm/assert!
"expected valid color data structure"
(ctc/check-color! color))
(dm/assert!
"expected file-id"
(uuid? file-id))
(dm/assert!
"expected file-id"
(uuid? file-id))
(ptk/reify ::update-color
ptk/WatchEvent
(watch [it state _]
(do-update-color it state color file-id))))
(ptk/reify ::update-color
ptk/WatchEvent
(watch [it state _]
(update-color* it state color file-id)))))
(defn rename-color
[file-id id new-name]
@ -198,9 +204,10 @@
(if (str/empty? new-name)
(rx/empty)
(let [data (get state :workspace-data)
object (get-in data [:colors id])
object (assoc object :name new-name)]
(do-update-color it state object file-id)))))))
color (get-in data [:colors id])
color (assoc color :name new-name)
color (d/without-nils color)]
(update-color* it state color file-id)))))))
(defn delete-color
[{:keys [id] :as params}]

View file

@ -260,6 +260,14 @@
(when ^boolean obj
(apply (.-f obj) args)))))))
(defn use-ref-value
"Returns a ref that will be automatically updated when the value is changed"
[v]
(let [ref (mf/use-ref v)]
(mf/with-effect [v]
(mf/set-ref-val! ref v))
ref))
(defn use-equal-memo
[val]
(let [ref (mf/use-ref nil)]

View file

@ -42,7 +42,7 @@
(cond-> color
(:value color) (assoc :color (:value color) :opacity 1)
(:value color) (dissoc :value)
true (assoc :file-id file-id)))
:always (assoc :file-id file-id)))
color-id (:id color)
@ -70,7 +70,7 @@
(fn [event]
(st/emit!
(dwl/add-recent-color color)
(dc/apply-color-from-palette (merge uc/empty-color color) (kbd/alt? event)))))
(dc/apply-color-from-palette color (kbd/alt? event)))))
rename-color
(mf/use-fn

View file

@ -14,6 +14,7 @@
[app.main.data.workspace.selection :as dws]
[app.main.store :as st]
[app.main.ui.components.title-bar :refer [title-bar]]
[app.main.ui.hooks :as h]
[app.main.ui.workspace.sidebar.options.rows.color-row :refer [color-row]]
[app.util.i18n :as i18n :refer [tr]]
[rumext.v2 :as mf]))
@ -21,82 +22,96 @@
(defn- prepare-colors
[shapes file-id shared-libs]
(let [data (into [] (remove nil? (ctc/extract-all-colors shapes file-id shared-libs)))
grouped-colors (group-by :attrs data)
groups (d/group-by :attrs #(dissoc % :attrs) data)
all-colors (distinct (mapv :attrs data))
tmp (group-by #(some? (:id %)) all-colors)
library-colors (get tmp true)
colors (get tmp false)]
{:grouped-colors grouped-colors
{:groups groups
:all-colors all-colors
:colors colors
:library-colors library-colors}))
(def xf:map-shape-id
(map :shape-id))
(mf/defc color-selection-menu
{::mf/wrap [#(mf/memo' % (mf/check-props ["shapes"]))]
::mf/wrap-props false}
[{:keys [shapes file-id shared-libs]}]
(let [{:keys [grouped-colors library-colors colors]} (mf/with-memo [shapes file-id shared-libs]
(prepare-colors shapes file-id shared-libs))
(let [{:keys [groups library-colors colors]} (mf/with-memo [shapes file-id shared-libs]
(prepare-colors shapes file-id shared-libs))
state* (mf/use-state true)
open? (deref state*)
state* (mf/use-state true)
open? (deref state*)
has-colors? (or (some? (seq colors)) (some? (seq library-colors)))
has-colors? (or (some? (seq colors)) (some? (seq library-colors)))
toggle-content (mf/use-fn #(swap! state* not))
toggle-content (mf/use-fn #(swap! state* not))
expand-lib-color (mf/use-state false)
expand-color (mf/use-state false)
grouped-colors* (mf/use-var nil)
prev-colors* (mf/use-var [])
groups-ref (h/use-ref-value groups)
prev-colors-ref (mf/use-ref nil)
;; grouped-colors* (mf/use-var nil)
;; prev-colors* (mf/use-var [])
on-change
(mf/use-fn
(fn [new-color old-color from-picker?]
(let [old-color (-> old-color (dissoc :name :path) d/without-nils)
(prn "new-color" new-color)
(prn "old-color" old-color)
(let [old-color (-> old-color
(dissoc :name :path)
(d/without-nils))
;; When dragging on the color picker sometimes all
;; the shapes hasn't updated the color to the prev
;; value so we need this extra calculation
shapes-by-old-color (get @grouped-colors* old-color)
prev-color (d/seek #(get @grouped-colors* %) @prev-colors*)
shapes-by-prev-color (get @grouped-colors* prev-color)
shapes-by-color (or shapes-by-prev-color shapes-by-old-color)]
groups (mf/ref-val groups-ref)
prev-colors (mf/ref-val prev-colors-ref)
prev-color (d/seek (partial get groups) prev-colors)
cops-old (get groups old-color)
cops-prev (get groups prev-colors)
cops (or cops-prev cops-old)
old-color (or prev-color old-color)]
(when from-picker?
(swap! prev-colors* conj (-> new-color (dissoc :name :path) d/without-nils)))
(let [color (-> new-color
(dissoc :name :path)
(d/without-nils))]
(mf/set-ref-val! prev-colors-ref
(conj prev-colors color))))
(st/emit! (dc/change-color-in-selected new-color shapes-by-color (or prev-color old-color))))))
(st/emit! (dc/change-color-in-selected cops new-color old-color)))))
on-open
(mf/use-fn
(fn []
(reset! prev-colors* [])))
(mf/use-fn #(mf/set-ref-val! prev-colors-ref []))
on-close
(mf/use-fn
(fn []
(reset! prev-colors* [])))
(mf/use-fn #(mf/set-ref-val! prev-colors-ref []))
on-detach
(mf/use-fn
(fn [color]
(let [shapes-by-color (get @grouped-colors* color)
new-color (assoc color :id nil :file-id nil)]
(st/emit! (dc/change-color-in-selected new-color shapes-by-color color)))))
(let [groups (mf/ref-val groups-ref)
cops (get groups color)
color' (dissoc color :id :file-id)]
(st/emit! (dc/change-color-in-selected cops color' color)))))
select-only
(mf/use-fn
(fn [color]
(let [shapes-by-color (get @grouped-colors* color)
ids (into (d/ordered-set) (map :shape-id) shapes-by-color)]
(let [groups (mf/ref-val groups-ref)
cops (get groups color)
ids (into (d/ordered-set) xf:map-shape-id cops)]
(st/emit! (dws/select-shapes ids)))))]
(mf/with-effect [grouped-colors]
(reset! grouped-colors* grouped-colors))
[:div {:class (stl/css :element-set)}
[:div {:class (stl/css :element-title)}
[:& title-bar {:collapsable has-colors?

View file

@ -113,9 +113,9 @@
handle-value-change
(mf/use-fn
(mf/deps color on-change)
(fn [new-value]
(fn [value]
(let [color (-> color
(assoc :color new-value)
(assoc :color value)
(dissoc :gradient))]
(st/emit! (dwl/add-recent-color color)
(on-change color)))))
@ -146,7 +146,9 @@
:else
color)
{:keys [x y]} (dom/get-client-position event)
cpos (dom/get-client-position event)
x (dm/get-prop cpos :x)
y (dm/get-prop cpos :y)
props {:x x
:y y
@ -154,14 +156,14 @@
:disable-opacity disable-opacity
:disable-image disable-image
;; on-change second parameter means if the source is the color-picker
:on-change #(on-change (merge uc/empty-color %) true)
:on-change #(on-change % true)
:on-close (fn [value opacity id file-id]
(when on-close
(on-close value opacity id file-id)))
:data color}]
(when on-open
(on-open (merge uc/empty-color color)))
(when (fn? on-open)
(on-open color))
(modal/show! :colorpicker props))))

View file

@ -80,9 +80,6 @@
(= id :multiple)
(= file-id :multiple)))
(def empty-color
(into {} (map #(vector % nil)) [:color :id :file-id :gradient :opacity :image]))
(defn get-color-name
[color]
(or (:color-library-name color)