From 162507264c56c0809caf80fddf8bb3dc9b0222a0 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 13 Sep 2024 11:22:20 +0200 Subject: [PATCH 1/3] :bug: Reexecute file migration 26 again for shapes that has transform prop as nil --- common/src/app/common/files/defaults.cljc | 2 +- common/src/app/common/files/migrations.cljc | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/common/src/app/common/files/defaults.cljc b/common/src/app/common/files/defaults.cljc index 4c2dd1d96..12351cdf5 100644 --- a/common/src/app/common/files/defaults.cljc +++ b/common/src/app/common/files/defaults.cljc @@ -6,4 +6,4 @@ (ns app.common.files.defaults) -(def version 52) +(def version 53) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 218103f32..797f2fe24 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -1072,4 +1072,5 @@ {:id 49 :migrate-up migrate-up-49} {:id 50 :migrate-up migrate-up-50} {:id 51 :migrate-up migrate-up-51} - {:id 52 :migrate-up migrate-up-52}]) + {:id 52 :migrate-up migrate-up-52} + {:id 53 :migrate-up migrate-up-26}]) From 179d534237be361060c24407c21d931a956f4a99 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Fri, 13 Sep 2024 15:17:02 +0200 Subject: [PATCH 2/3] :bug: Fix several issues related to invalid colors inserted on shadows --- common/src/app/common/files/defaults.cljc | 2 +- common/src/app/common/files/migrations.cljc | 30 +++- common/src/app/common/types/color.cljc | 39 ++--- common/src/app/common/types/shape.cljc | 3 + common/src/app/common/types/shape/shadow.cljc | 3 + .../src/app/main/data/workspace/colors.cljs | 153 ++++++++++++++---- .../app/main/data/workspace/libraries.cljs | 43 ++--- frontend/src/app/main/ui/hooks.cljs | 8 + .../ui/workspace/sidebar/assets/colors.cljs | 4 +- .../options/menus/color_selection.cljs | 77 +++++---- .../sidebar/options/rows/color_row.cljs | 14 +- frontend/src/app/util/color.cljs | 3 - 12 files changed, 266 insertions(+), 113 deletions(-) diff --git a/common/src/app/common/files/defaults.cljc b/common/src/app/common/files/defaults.cljc index 12351cdf5..b15aac919 100644 --- a/common/src/app/common/files/defaults.cljc +++ b/common/src/app/common/files/defaults.cljc @@ -6,4 +6,4 @@ (ns app.common.files.defaults) -(def version 53) +(def version 54) diff --git a/common/src/app/common/files/migrations.cljc b/common/src/app/common/files/migrations.cljc index 797f2fe24..4a649c265 100644 --- a/common/src/app/common/files/migrations.cljc +++ b/common/src/app/common/files/migrations.cljc @@ -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}]) diff --git a/common/src/app/common/types/color.cljc b/common/src/app/common/types/color.cljc index 78a7f8114..ed4def689 100644 --- a/common/src/app/common/types/color.cljc +++ b/common/src/app/common/types/color.cljc @@ -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 diff --git a/common/src/app/common/types/shape.cljc b/common/src/app/common/types/shape.cljc index b502c4c65..cf437d2dc 100644 --- a/common/src/app/common/types/shape.cljc +++ b/common/src/app/common/types/shape.cljc @@ -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] diff --git a/common/src/app/common/types/shape/shadow.cljc b/common/src/app/common/types/shape/shadow.cljc index 0c3389893..b1ec5342f 100644 --- a/common/src/app/common/types/shape/shadow.cljc +++ b/common/src/app/common/types/shape/shadow.cljc @@ -27,3 +27,6 @@ [:color ::ctc/color]]) (sm/register! ::shadow schema:shadow) + +(def check-shadow! + (sm/check-fn schema:shadow)) diff --git a/frontend/src/app/main/data/workspace/colors.cljs b/frontend/src/app/main/data/workspace/colors.cljs index dc0a44d4a..004edc696 100644 --- a/frontend/src/app/main/data/workspace/colors.cljs +++ b/frontend/src/app/main/data/workspace/colors.cljs @@ -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 _] diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index a6c6cb8b3..cfa7e5f12 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -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}] diff --git a/frontend/src/app/main/ui/hooks.cljs b/frontend/src/app/main/ui/hooks.cljs index 14bff1559..912fb8bc8 100644 --- a/frontend/src/app/main/ui/hooks.cljs +++ b/frontend/src/app/main/ui/hooks.cljs @@ -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)] diff --git a/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs b/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs index 965f02bb3..7f7e67d92 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/assets/colors.cljs @@ -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 diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs index b0c5b0cdc..e6c904beb 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/menus/color_selection.cljs @@ -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? diff --git a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs index dcbabace6..d6ad61822 100644 --- a/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs +++ b/frontend/src/app/main/ui/workspace/sidebar/options/rows/color_row.cljs @@ -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)))) diff --git a/frontend/src/app/util/color.cljs b/frontend/src/app/util/color.cljs index aeb95d007..078102d90 100644 --- a/frontend/src/app/util/color.cljs +++ b/frontend/src/app/util/color.cljs @@ -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) From 86c5ca42135f04a26d17e5b9549ab85c1bbfe54c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 16 Sep 2024 18:21:25 +0200 Subject: [PATCH 3/3] :bug: Fix incorrect redirect handling on request-access go-home button --- frontend/src/app/main.cljs | 1 - frontend/src/app/main/data/users.cljs | 1 + frontend/src/app/main/ui.cljs | 4 +- frontend/src/app/main/ui/dashboard.cljs | 25 ++- frontend/src/app/main/ui/routes.cljs | 6 +- frontend/src/app/main/ui/static.cljs | 236 +++++++++++++++--------- frontend/src/app/util/dom.cljs | 6 +- frontend/src/app/util/router.cljs | 12 +- 8 files changed, 182 insertions(+), 109 deletions(-) diff --git a/frontend/src/app/main.cljs b/frontend/src/app/main.cljs index 3cbbe0d70..517376ba2 100644 --- a/frontend/src/app/main.cljs +++ b/frontend/src/app/main.cljs @@ -143,4 +143,3 @@ (reinit)))) (set! (.-stackTraceLimit js/Error) 50) - diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index 9cf0f9966..f4ea04997 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -171,6 +171,7 @@ (let [team-id (get-current-team-id profile) welcome-file-id (dm/get-in profile [:props :welcome-file-id]) redirect-href (:login-redirect @s/session)] + (cond (some? redirect-href) (binding [s/*sync* true] diff --git a/frontend/src/app/main/ui.cljs b/frontend/src/app/main/ui.cljs index 8a53010de..9de71e8e2 100644 --- a/frontend/src/app/main/ui.cljs +++ b/frontend/src/app/main/ui.cljs @@ -42,7 +42,8 @@ (mf/lazy-component app.main.ui.workspace/workspace)) (mf/defc main-page - {::mf/props :obj} + {::mf/props :obj + ::mf/private true} [{:keys [route profile]}] (let [{:keys [data params]} route props (get profile :props) @@ -68,6 +69,7 @@ (:onboarding-viewed props) (not= (:release-notes-viewed props) (:main cf/version)) (not= "0.0" (:main cf/version)))] + [:& (mf/provider ctx/current-route) {:value route} (case (:name data) (:auth-login diff --git a/frontend/src/app/main/ui/dashboard.cljs b/frontend/src/app/main/ui/dashboard.cljs index 78c9902a9..0ec58ba16 100644 --- a/frontend/src/app/main/ui/dashboard.cljs +++ b/frontend/src/app/main/ui/dashboard.cljs @@ -42,9 +42,7 @@ (let [search-term (get-in route [:params :query :search-term]) team-id (get-in route [:params :path :team-id]) project-id (get-in route [:params :path :project-id])] - (cond-> - {:search-term search-term} - + (cond-> {:search-term search-term} (uuid-str? team-id) (assoc :team-id (uuid team-id)) @@ -84,10 +82,10 @@ (mf/use-effect on-resize) - [:div {:class (stl/css :dashboard-content) :style {:pointer-events (when file-menu-open? "none")} - :on-click clear-selected-fn :ref container} + :on-click clear-selected-fn + :ref container} (case section :dashboard-projects [:* @@ -146,7 +144,8 @@ (l/derived :current-team-id st/state)) (mf/defc dashboard - [{:keys [route profile] :as props}] + {::mf/props :obj} + [{:keys [route profile]}] (let [section (get-in route [:data :name]) params (parse-params route) @@ -181,13 +180,13 @@ [:& (mf/provider ctx/current-team-id) {:value team-id} [:& (mf/provider ctx/current-project-id) {:value project-id} - ;; NOTE: dashboard events and other related functions assumes - ;; that the team is a implicit context variable that is - ;; available using react context or accessing - ;; the :current-team-id on the state. We set the key to the - ;; team-id because we want to completely refresh all the - ;; components on team change. Many components assumes that the - ;; team is already set so don't put the team into mf/deps. + ;; NOTE: dashboard events and other related functions assumes + ;; that the team is a implicit context variable that is + ;; available using react context or accessing + ;; the :current-team-id on the state. We set the key to the + ;; team-id because we want to completely refresh all the + ;; components on team change. Many components assumes that the + ;; team is already set so don't put the team into mf/deps. (when (and team initialized?) [:main {:class (stl/css :dashboard) :key (:id team)} diff --git a/frontend/src/app/main/ui/routes.cljs b/frontend/src/app/main/ui/routes.cljs index ff943a0f7..7b26b8561 100644 --- a/frontend/src/app/main/ui/routes.cljs +++ b/frontend/src/app/main/ui/routes.cljs @@ -106,9 +106,9 @@ (st/emit! (rt/navigated match)) :else - ;; We just recheck with an additional profile request; this avoids - ;; some race conditions that causes unexpected redirects on - ;; invitations workflows (and probably other cases). + ;; We just recheck with an additional profile request; this + ;; avoids some race conditions that causes unexpected redirects + ;; on invitations workflows (and probably other cases). (->> (rp/cmd! :get-profile) (rx/subs! (fn [{:keys [id] :as profile}] (cond diff --git a/frontend/src/app/main/ui/static.cljs b/frontend/src/app/main/ui/static.cljs index e069a9486..8f6ea87b2 100644 --- a/frontend/src/app/main/ui/static.cljs +++ b/frontend/src/app/main/ui/static.cljs @@ -7,19 +7,21 @@ (ns app.main.ui.static (:require-macros [app.main.style :as stl]) (:require + ["rxjs" :as rxjs] [app.common.data :as d] [app.common.pprint :as pp] [app.common.uri :as u] [app.main.data.common :as dc] [app.main.data.events :as ev] + [app.main.refs :as refs] [app.main.repo :as rp] [app.main.store :as st] [app.main.ui.auth.login :refer [login-methods]] [app.main.ui.auth.recovery-request :refer [recovery-request-page recovery-sent-page]] - [app.main.ui.auth.register :refer [register-methods register-validate-form register-success-page terms-register]] + [app.main.ui.auth.register :as register] [app.main.ui.dashboard.sidebar :refer [sidebar]] [app.main.ui.icons :as i] - [app.main.ui.viewer.header :as header] + [app.main.ui.viewer.header :as viewer.header] [app.util.dom :as dom] [app.util.i18n :refer [tr]] [app.util.router :as rt] @@ -29,10 +31,14 @@ [potok.v2.core :as ptk] [rumext.v2 :as mf])) +;; FIXME: this is a workaround until we export this class on beicon library +(def TimeoutError rxjs/TimeoutError) + (mf/defc error-container* {::mf/props :obj} [{:keys [children]}] - (let [profile-id (:profile-id @st/state)] + (let [profile-id (:profile-id @st/state) + on-nav-root (mf/use-fn #(st/emit! (rt/nav-root)))] [:section {:class (stl/css :exception-layout)} [:button {:class (stl/css :exception-header) @@ -44,7 +50,7 @@ [:div {:class (stl/css :deco-before)} i/logo-error-screen] (when-not profile-id [:button {:class (stl/css :login-header) - :on-click rt/nav-root} + :on-click on-nav-root} (tr "labels.login")]) [:div {:class (stl/css :exception-content)} @@ -65,8 +71,8 @@ {::mf/props :obj} [{:keys [show-dialog]}] (let [current-section (mf/use-state :login) - user-email (mf/use-state "") - register-token (mf/use-state "") + user-email (mf/use-state "") + register-token (mf/use-state "") set-section (mf/use-fn @@ -85,29 +91,37 @@ #(reset! current-section :login)) success-login - (fn [] - (reset! show-dialog false) - (.reload js/window.location true)) + (mf/use-fn + (fn [] + (reset! show-dialog false) + (st/emit! (rt/reload true)))) success-register - (fn [data] - (reset! register-token (:token data)) - (reset! current-section :register-validate)) + (mf/use-fn + (fn [data] + (reset! register-token (:token data)) + (reset! current-section :register-validate))) register-email-sent - (fn [email] - (reset! user-email email) - (reset! current-section :register-email-sent)) + (mf/use-fn + (fn [email] + (reset! user-email email) + (reset! current-section :register-email-sent))) recovery-email-sent - (fn [email] - (reset! user-email email) - (reset! current-section :recovery-email-sent))] + (mf/use-fn + (fn [email] + (reset! user-email email) + (reset! current-section :recovery-email-sent))) + + on-nav-root + (mf/use-fn #(st/emit! (rt/nav-root)))] [:div {:class (stl/css :overlay)} [:div {:class (stl/css :dialog-login)} [:div {:class (stl/css :modal-close)} - [:button {:class (stl/css :modal-close-button) :on-click rt/nav-root} + [:button {:class (stl/css :modal-close-button) + :on-click on-nav-root} i/close]] [:div {:class (stl/css :login)} [:div {:class (stl/css :logo)} i/logo] @@ -125,13 +139,14 @@ (tr "auth.register") " " [:a {:data-section "register" - :on-click set-section} (tr "auth.register-submit")]]] + :on-click set-section} + (tr "auth.register-submit")]]] :register [:* [:div {:class (stl/css :logo-title)} (tr "not-found.login.signup-free")] [:div {:class (stl/css :logo-subtitle)} (tr "not-found.login.start-using")] - [:& register-methods {:on-success-callback success-register :hide-separator true}] + [:& register/register-methods {:on-success-callback success-register :hide-separator true}] #_[:hr {:class (stl/css :separator)}] [:div {:class (stl/css :separator)}] [:div {:class (stl/css :change-section)} @@ -141,12 +156,13 @@ :on-click set-section} (tr "auth.login-here")]] [:div {:class (stl/css :links)} [:hr {:class (stl/css :separator)}] - [:& terms-register]]] + [:& register/terms-register]]] :register-validate [:div {:class (stl/css :form-container)} - [:& register-validate-form {:params {:token @register-token} - :on-success-callback register-email-sent}] + [:& register/register-validate-form + {:params {:token @register-token} + :on-success-callback register-email-sent}] [:div {:class (stl/css :links)} [:div {:class (stl/css :register)} [:a {:data-section "register" @@ -155,7 +171,7 @@ :register-email-sent [:div {:class (stl/css :form-container)} - [:& register-success-page {:params {:email @user-email :hide-logo true}}]] + [:& register/register-success-page {:params {:email @user-email :hide-logo true}}]] :recovery-request [:& recovery-request-page {:go-back-callback set-section-login @@ -175,40 +191,49 @@ [:button {:class (stl/css :modal-close-button) :on-click on-close} i/close]] [:div {:class (stl/css :dialog-title)} title] - (for [txt content] - [:div txt]) + (for [[index content] (d/enumerate content)] + [:div {:key index} content]) [:div {:class (stl/css :sign-info)} (when cancel-text - [:button {:class (stl/css :cancel-button) :on-click on-close} cancel-text]) + [:button {:class (stl/css :cancel-button) + :on-click on-close} + cancel-text]) [:button {:on-click on-click} button-text]]]])) - (mf/defc request-access {::mf/props :obj} [{:keys [file-id team-id is-default workspace?]}] - (let [profile (:profile @st/state) + (let [profile (mf/deref refs/profile) requested* (mf/use-state {:sent false :already-requested false}) requested (deref requested*) show-dialog (mf/use-state true) + on-close (mf/use-fn (mf/deps profile) (fn [] (st/emit! (rt/nav :dashboard-projects {:team-id (:default-team-id profile)})))) + on-success (mf/use-fn #(reset! requested* {:sent true :already-requested false})) + on-error (mf/use-fn #(reset! requested* {:sent true :already-requested true})) + on-request-access (mf/use-fn (mf/deps file-id team-id workspace?) (fn [] - (let [params (if (some? file-id) {:file-id file-id :is-viewer (not workspace?)} {:team-id team-id}) - mdata {:on-success on-success :on-error on-error}] - (st/emit! (dc/create-team-access-request (with-meta params mdata))))))] - + (let [params (if (some? file-id) + {:file-id file-id + :is-viewer (not workspace?)} + {:team-id team-id}) + mdata {:on-success on-success + :on-error on-error}] + (st/emit! (dc/create-team-access-request + (with-meta params mdata))))))] [:* (if (some? file-id) @@ -220,17 +245,24 @@ [:div {:class (stl/css :project-name)} (tr "not-found.no-permission.project-name")] [:div {:class (stl/css :file-name)} (tr "not-found.no-permission.penpot-file")]]] [:div {:class (stl/css :workspace-right)}]] + [:div {:class (stl/css :viewer)} - [:& header/header {:project {:name (tr "not-found.no-permission.project-name")} - :index 0 - :file {:name (tr "not-found.no-permission.penpot-file")} - :page nil - :frame nil - :permissions {:is-logged true} - :zoom 1 - :section :interactions - :shown-thumbnails false - :interactions-mode nil}]]) + ;; FIXME: the viewer header was never designed to be reused + ;; from other parts of the application, and this code looks + ;; like a fast workaround reusing it as-is without a proper + ;; component adaptation for be able to use it easily it on + ;; viewer context or static error page context + [:& viewer.header/header {:project + {:name (tr "not-found.no-permission.project-name")} + :index 0 + :file {:name (tr "not-found.no-permission.penpot-file")} + :page nil + :frame nil + :permissions {:is-logged true} + :zoom 1 + :section :interactions + :shown-thumbnails false + :interactions-mode nil}]]) [:div {:class (stl/css :dashboard)} [:div {:class (stl/css :dashboard-sidebar)} @@ -392,64 +424,92 @@ [:div {:class (stl/css :sign-info)} [:button {:on-click on-reset} (tr "labels.retry")]]])) +(defn- load-info + "Load exception page info" + [path-params] + (let [default {:loaded true} + stream (cond + (:file-id path-params) + (->> (rp/cmd! :get-file-info {:id (:file-id path-params)}) + (rx/map (fn [info] + {:loaded true + :file-id (:id info)}))) + + (:team-id path-params) + (->> (rp/cmd! :get-team-info {:id (:team-id path-params)}) + (rx/map (fn [info] + {:loaded true + :team-id (:id info) + :team-default (:is-default info)}))) + + :else + (rx/of default))] + + (->> stream + (rx/timeout 3000) + (rx/catch (fn [cause] + (if (instance? TimeoutError cause) + (rx/of default) + (rx/throw cause))))))) + + (mf/defc exception-page* {::mf/props :obj} [{:keys [data route] :as props}] - (let [file-info (mf/use-state {:pending true}) - team-info (mf/use-state {:pending true}) - type (:type data) - path (:path route) + (let [type (:type data) + path (:path route) - workspace? (str/includes? path "workspace") - dashboard? (str/includes? path "dashboard") - view? (str/includes? path "view") + query-params (:query-params route) + path-params (:path-params route) - request-access? (and - (or workspace? dashboard? view?) - (or (not (str/empty? (:file-id @file-info))) (not (str/empty? (:team-id @team-info))))) + workspace? (str/includes? path "workspace") + dashboard? (str/includes? path "dashboard") + view? (str/includes? path "view") - query-params (u/map->query-string (:query-params route)) - pparams (:path-params route) - on-file-info (mf/use-fn - (fn [info] - (reset! file-info {:file-id (:id info)}))) - on-team-info (mf/use-fn - (fn [info] - (reset! team-info {:team-id (:id info) :is-default (:is-default info)})))] + ;; We stora the request access info int this state + info* (mf/use-state nil) + info (deref info*) - (mf/with-effect [type path query-params pparams @file-info @team-info] - (st/emit! (ptk/event ::ev/event {::ev/name "exception-page" :type type :path path :query-params query-params})) + loaded? (get info :loaded false) - (when (and (:file-id pparams) (:pending @file-info)) - (->> (rp/cmd! :get-file-info {:id (:file-id pparams)}) - (rx/subs! on-file-info))) + request-access? + (and + (or workspace? dashboard? view?) + (or (:file-id info) + (:team-id info)))] - (when (and (:team-id pparams) (:pending @team-info)) - (->> (rp/cmd! :get-team-info {:id (:team-id pparams)}) - (rx/subs! on-team-info)))) + (mf/with-effect [type path query-params path-params] + (let [query-params (u/map->query-string query-params) + event-params {::ev/name "exception-page" + :type type + :path path + :query-params query-params}] + (st/emit! (ptk/event ::ev/event event-params)))) - (case (:type data) - :not-found + (mf/with-effect [path-params info] + (when-not (:loaded info) + (->> (load-info path-params) + (rx/subs! (partial reset! info*))))) + + (when loaded? (if request-access? - [:& request-access {:file-id (:file-id @file-info) - :team-id (:team-id @team-info) - :is-default (:is-default @team-info) + [:& request-access {:file-id (:file-id info) + :team-id (:team-id info) + :is-default (:team-default info) :workspace? workspace?}] - [:> not-found* {}]) - :authentication - (if request-access? - [:& request-access {:file-id (:file-id @file-info) - :team-id (:team-id @team-info) - :is-default (:is-default @team-info) - :workspace? workspace?}] - [:> not-found* {}]) + (case (:type data) + :not-found + [:> not-found* {}] - :bad-gateway - [:> bad-gateway* props] + :authentication + [:> not-found* {}] - :service-unavailable - [:& service-unavailable] + :bad-gateway + [:> bad-gateway* props] - [:> internal-error* props]))) + :service-unavailable + [:& service-unavailable] + + [:> internal-error* props]))))) diff --git a/frontend/src/app/util/dom.cljs b/frontend/src/app/util/dom.cljs index 08b89d364..488a0d728 100644 --- a/frontend/src/app/util/dom.cljs +++ b/frontend/src/app/util/dom.cljs @@ -760,8 +760,10 @@ (.back (.-history js/window))) (defn reload-current-window - [] - (.reload (.-location js/window))) + ([] + (.reload globals/location)) + ([force?] + (.reload globals/location force?))) (defn scroll-by! ([element x y] diff --git a/frontend/src/app/util/router.cljs b/frontend/src/app/util/router.cljs index 9180d5d77..1a92d4ec7 100644 --- a/frontend/src/app/util/router.cljs +++ b/frontend/src/app/util/router.cljs @@ -148,7 +148,17 @@ (defn nav-root "Navigate to the root page." [] - (set! (.-href globals/location) "/")) + (ptk/reify ::nav-root + ptk/EffectEvent + (effect [_ _ _] + (set! (.-href globals/location) "/")))) + +(defn reload + [force?] + (ptk/reify ::reload + ptk/EffectEvent + (effect [_ _ _] + (ts/asap (partial dom/reload-current-window force?))))) (defn nav-raw [& {:keys [href uri]}]