Merge remote-tracking branch 'origin/staging'

✨ Reduce lock contention on uploading file object thumbnail

CHANGES.md

@@ -1,5 +1,19 @@
 # CHANGELOG
 
+## 2.0.2
+
+### :sparkles: Enhancements
+
+- Fix locking contention on cron subsystem (causes backend start blocking)
+- Fix locking contention on file object thumbails backend RPC calls
+
+
+## 2.0.1
+
+### :bug: Bugs fixed
+
+- Fix different issues related to components v2 migrations including [Github #4443](https://github.com/penpot/penpot/issues/4443)
+
 ## 2.0.0 - I Just Can't Get Enough
 
 ### :rocket: Epics and highlights

backend/src/app/migrations/v2.clj

@@ -18,7 +18,7 @@
           row_number() OVER (ORDER BY created_at DESC) AS rown
     FROM team
    WHERE deleted_at IS NULL
-     AND (features <@ '{components/v2}' OR features IS NULL)
+     AND (not (features @> '{components/v2}') OR features IS NULL)
    ORDER BY created_at DESC")
 
 (defn- get-teams
@@ -37,7 +37,7 @@
   ;; Run teams migration
   (run! (fn [{:keys [id rown]}]
           (try
-            (-> (assoc system ::db/rollback true)
+            (-> (assoc system ::db/rollback false)
                 (feat/migrate-team! id
                                     :rown rown
                                     :label "v2-migration"

backend/src/app/rpc/climit.clj

@@ -20,6 +20,7 @@
    [app.util.time :as dt]
    [app.worker :as-alias wrk]
    [clojure.edn :as edn]
+   [clojure.set :as set]
    [clojure.spec.alpha :as s]
    [datoteka.fs :as fs]
    [integrant.core :as ig]
@@ -91,67 +92,77 @@
               :timeout (:timeout config)
               :type :semaphore))
 
-(defmacro ^:private measure-and-log!
-  [metrics mlabels stats id action limit-id limit-label profile-id elapsed]
-  `(let [mpermits# (:max-permits ~stats)
-         mqueue#   (:max-queue ~stats)
-         permits#  (:permits ~stats)
-         queue#    (:queue ~stats)
-         queue#    (- queue# mpermits#)
-         queue#    (if (neg? queue#) 0 queue#)
-         level#    (if (pos? queue#) :warn :trace)]
 
-     (mtx/run! ~metrics
-               :id :rpc-climit-queue
-               :val queue#
-               :labels ~mlabels)
+(defn measure!
+  [metrics mlabels stats elapsed]
+  (let [mpermits (:max-permits stats)
+        permits  (:permits stats)
+        queue    (:queue stats)
+        queue    (- queue mpermits)
+        queue    (if (neg? queue) 0 queue)]
 
-     (mtx/run! ~metrics
-               :id :rpc-climit-permits
-               :val permits#
-               :labels ~mlabels)
+    (mtx/run! metrics
+              :id :rpc-climit-queue
+              :val queue
+              :labels mlabels)
 
-     (l/log level#
-            :hint ~action
-            :req ~id
-            :id ~limit-id
-            :label ~limit-label
-            :profile-id (str ~profile-id)
-            :permits permits#
-            :queue queue#
-            :max-permits mpermits#
-            :max-queue mqueue#
-            ~@(if (some? elapsed)
-                [:elapsed `(dt/format-duration ~elapsed)]
-                []))))
+    (mtx/run! metrics
+              :id :rpc-climit-permits
+              :val permits
+              :labels mlabels)
+
+    (when elapsed
+      (mtx/run! metrics
+                :id :rpc-climit-timing
+                :val (inst-ms elapsed)
+                :labels mlabels))))
+
+(defn log!
+  [action req-id stats limit-id limit-label params elapsed]
+  (let [mpermits (:max-permits stats)
+        queue    (:queue stats)
+        queue    (- queue mpermits)
+        queue    (if (neg? queue) 0 queue)
+        level    (if (pos? queue) :warn :trace)]
+
+    (l/log level
+           :hint action
+           :req req-id
+           :id limit-id
+           :label limit-label
+           :queue queue
+           :elapsed (some-> elapsed dt/format-duration)
+           :params (-> (select-keys params [::rpc/profile-id :file-id :profile-id])
+                       (set/rename-keys {::rpc/profile-id :profile-id})
+                       (update-vals str)))))
 
 (def ^:private idseq (AtomicLong. 0))
 
 (defn- invoke
-  [limiter metrics limit-id limit-key limit-label profile-id f params]
+  [limiter metrics limit-id limit-key limit-label handler params]
   (let [tpoint    (dt/tpoint)
         mlabels   (into-array String [(id->str limit-id)])
         limit-id  (id->str limit-id limit-key)
         stats     (pbh/get-stats limiter)
-        id        (.incrementAndGet ^AtomicLong idseq)]
+        req-id    (.incrementAndGet ^AtomicLong idseq)]
 
     (try
-      (measure-and-log! metrics mlabels stats id "enqueued" limit-id limit-label profile-id nil)
+      (measure! metrics mlabels stats nil)
+      (log! "enqueued" req-id stats limit-id limit-label params nil)
       (px/invoke! limiter (fn []
                             (let [elapsed (tpoint)
                                   stats   (pbh/get-stats limiter)]
-                              (measure-and-log! metrics mlabels stats id "acquired" limit-id limit-label profile-id elapsed)
-                              (mtx/run! metrics
-                                        :id :rpc-climit-timing
-                                        :val (inst-ms elapsed)
-                                        :labels mlabels)
-                              (apply f params))))
+
+                              (measure! metrics mlabels stats elapsed)
+                              (log! "acquired" req-id stats limit-id limit-label params elapsed)
+
+                              (handler params))))
 
       (catch ExceptionInfo cause
         (let [{:keys [type code]} (ex-data cause)]
           (if (= :bulkhead-error type)
             (let [elapsed (tpoint)]
-              (measure-and-log! metrics mlabels stats id "reject" limit-id limit-label profile-id elapsed)
+              (log! "rejected" req-id stats limit-id limit-label params elapsed)
               (ex/raise :type :concurrency-limit
                         :code code
                         :hint "concurrency limit reached"
@@ -161,7 +172,9 @@
       (finally
         (let [elapsed (tpoint)
               stats (pbh/get-stats limiter)]
-          (measure-and-log! metrics mlabels stats id "finished" limit-id limit-label profile-id elapsed))))))
+
+          (measure! metrics mlabels stats nil)
+          (log! "finished" req-id stats limit-id limit-label params elapsed))))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; MIDDLEWARE
@@ -219,10 +232,8 @@
                         (let [limit-key  (key-fn params)
                               cache-key  [limit-id limit-key]
                               limiter    (cache/get cache cache-key (partial create-limiter config))
-                              profile-id (if (= key-fn ::rpc/profile-id)
-                                           limit-key
-                                           (get params ::rpc/profile-id))]
-                          (invoke limiter metrics limit-id limit-key label profile-id handler [cfg params])))))
+                              handler    (partial handler cfg)]
+                          (invoke limiter metrics limit-id limit-key label handler params)))))
 
                   (do
                     (l/wrn :hint "no config found for specified queue" :id (id->str limit-id))
@@ -237,15 +248,15 @@
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (defn- build-exec-chain
-  [{:keys [::label ::profile-id ::rpc/climit ::mtx/metrics] :as cfg} f]
+  [{:keys [::label ::rpc/climit ::mtx/metrics] :as cfg} f]
   (let [config (get climit ::config)
         cache  (get climit ::cache)]
     (reduce (fn [handler [limit-id limit-key :as ckey]]
               (if-let [config (get config limit-id)]
-                (fn [& params]
-                  (let [limiter (cache/get cache ckey (partial create-limiter config))]
-                    (invoke limiter metrics limit-id limit-key label profile-id handler params)))
-
+                (fn [cfg params]
+                  (let [limiter (cache/get cache ckey (partial create-limiter config))
+                        handler (partial handler cfg)]
+                    (invoke limiter metrics limit-id limit-key label handler params)))
                 (do
                   (l/wrn :hint "config not found" :label label :id limit-id)
                   f)))
@@ -255,9 +266,9 @@
 (defn invoke!
   "Run a function in context of climit.
   Intended to be used in virtual threads."
-  [{:keys [::executor] :as cfg} f & params]
+  [{:keys [::executor] :as cfg} f params]
   (let [f (if (some? executor)
-            (fn [& params] (px/await! (px/submit! executor (fn [] (apply f params)))))
+            (fn [cfg params] (px/await! (px/submit! executor (fn [] (f cfg params)))))
             f)
         f (build-exec-chain cfg f)]
-    (apply f params)))
+    (f cfg params)))

backend/src/app/rpc/commands/files_thumbnails.clj

@@ -228,51 +228,52 @@
 ;; MUTATION COMMANDS
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-;; MUTATION COMMAND: create-file-object-thumbnail
+(def sql:get-file-object-thumbnail
+  "SELECT * FROM file_tagged_object_thumbnail
+    WHERE file_id = ? AND object_id = ? AND tag = ?
+      FOR UPDATE")
 
-(defn- create-file-object-thumbnail!
-  [{:keys [::db/conn ::sto/storage]} file-id object-id media tag]
+(def sql:create-file-object-thumbnail
+  "INSERT INTO file_tagged_object_thumbnail (file_id, object_id, tag, media_id)
+   VALUES (?, ?, ?, ?)
+       ON CONFLICT (file_id, object_id, tag)
+       DO UPDATE SET updated_at=?, media_id=?, deleted_at=null
+   RETURNING *")
 
-  (let [thumb (db/get* conn :file-tagged-object-thumbnail
-                       {:file-id file-id
-                        :object-id object-id
-                        :tag tag}
-                       {::db/remove-deleted false
-                        ::sql/for-update true})
-
-        path  (:path media)
+(defn- persist-thumbnail!
+  [storage media created-at]
+  (let [path  (:path media)
         mtype (:mtype media)
         hash  (sto/calculate-hash path)
         data  (-> (sto/content path)
-                  (sto/wrap-with-hash hash))
-        tnow  (dt/now)
+                  (sto/wrap-with-hash hash))]
 
-        media (sto/put-object! storage
-                               {::sto/content data
-                                ::sto/deduplicate? true
-                                ::sto/touched-at tnow
-                                :content-type mtype
-                                :bucket "file-object-thumbnail"})]
+    (sto/put-object! storage
+                     {::sto/content data
+                      ::sto/deduplicate? true
+                      ::sto/touched-at created-at
+                      :content-type mtype
+                      :bucket "file-object-thumbnail"})))
 
-    (if (some? thumb)
-      (do
-        ;; We mark the old media id as touched if it does not matches
-        (when (not= (:id media) (:media-id thumb))
-          (sto/touch-object! storage (:media-id thumb)))
-        (db/update! conn :file-tagged-object-thumbnail
-                    {:media-id (:id media)
-                     :deleted-at nil
-                     :updated-at tnow}
-                    {:file-id file-id
-                     :object-id object-id
-                     :tag tag}))
-      (db/insert! conn :file-tagged-object-thumbnail
-                  {:file-id file-id
-                   :object-id object-id
-                   :created-at tnow
-                   :updated-at tnow
-                   :tag tag
-                   :media-id (:id media)}))))
+
+
+(defn- create-file-object-thumbnail!
+  [{:keys [::sto/storage] :as cfg} file-id object-id media tag]
+  (let [tsnow     (dt/now)
+        media     (persist-thumbnail! storage media tsnow)
+        [th1 th2] (db/tx-run! cfg (fn [{:keys [::db/conn]}]
+                                    (let [th1 (db/exec-one! conn [sql:get-file-object-thumbnail file-id object-id tag])
+                                          th2 (db/exec-one! conn [sql:create-file-object-thumbnail
+                                                                  file-id object-id tag (:id media)
+                                                                  tsnow (:id media)])]
+                                      [th1 th2])))]
+
+    (when (and (some? th1)
+               (not= (:media-id th1)
+                     (:media-id th2)))
+      (sto/touch-object! storage (:media-id th1)))
+
+    th2))
 
 (def ^:private
   schema:create-file-object-thumbnail
@@ -296,16 +297,10 @@
   (media/validate-media-type! media)
   (media/validate-media-size! media)
 
-  (db/tx-run! cfg
-              (fn [{:keys [::db/conn] :as cfg}]
-                (files/check-edition-permissions! conn profile-id file-id)
-                (when-not (db/read-only? conn)
-                  (let [cfg (-> cfg
-                                (update ::sto/storage media/configure-assets-storage)
-                                (assoc ::rtry/when rtry/conflict-exception?)
-                                (assoc ::rtry/max-retries 5)
-                                (assoc ::rtry/label "create-file-object-thumbnail"))]
-                    (create-file-object-thumbnail! cfg file-id object-id media (or tag "frame")))))))
+  (db/run! cfg files/check-edition-permissions! profile-id file-id)
+
+  (let [cfg (update cfg ::sto/storage media/configure-assets-storage)]
+    (create-file-object-thumbnail! cfg file-id object-id media (or tag "frame"))))
 
 ;; --- MUTATION COMMAND: delete-file-object-thumbnail
 

backend/src/app/rpc/commands/media.clj

@@ -243,12 +243,13 @@
     ;; NOTE: we use the climit here in a dynamic invocation because we
     ;; don't want saturate the process-image limit with IO (download
     ;; of external image)
+
     (-> cfg
         (assoc ::climit/id [[:process-image/by-profile (:profile-id params)]
                             [:process-image/global]])
-        (assoc ::climit/profile-id (:profile-id params))
         (assoc ::climit/label "create-file-media-object-from-url")
-        (climit/invoke! db/run! cfg create-file-media-object params))))
+        (climit/invoke! #(db/run! %1 create-file-media-object %2) params))))
+
 
 ;; --- Clone File Media object (Upload and create from url)
 

backend/src/app/rpc/commands/profile.clj

@@ -233,7 +233,7 @@
                          :file-mtype (:mtype file)}}))))
 
 (defn- generate-thumbnail!
-  [file]
+  [_ file]
   (let [input   (media/run {:cmd :info :input file})
         thumb   (media/run {:cmd :profile-thumbnail
                             :format :jpeg
@@ -250,15 +250,15 @@
      :content-type (:mtype thumb)}))
 
 (defn upload-photo
-  [{:keys [::sto/storage ::wrk/executor] :as cfg} {:keys [file]}]
+  [{:keys [::sto/storage ::wrk/executor] :as cfg} {:keys [file] :as params}]
   (let [params (-> cfg
-                   (assoc ::climit/id :process-image/global)
+                   (assoc ::climit/id [[:process-image/by-profile (:profile-id params)]
+                                       [:process-image/global]])
                    (assoc ::climit/label "upload-photo")
                    (assoc ::climit/executor executor)
                    (climit/invoke! generate-thumbnail! file))]
     (sto/put-object! storage params)))
 
-
 ;; --- MUTATION: Request Email Change
 
 (declare ^:private request-email-change!)

backend/src/app/worker/cron.clj

@@ -27,14 +27,15 @@
   "insert into scheduled_task (id, cron_expr)
    values (?, ?)
        on conflict (id)
-       do update set cron_expr=?")
+       do nothing")
 
 (defn- synchronize-cron-entries!
-  [{:keys [::db/pool ::entries]}]
-  (db/with-atomic [conn pool]
-    (doseq [{:keys [id cron]} entries]
-      (l/trc :hint "register cron task" :id id :cron (str cron))
-      (db/exec-one! conn [sql:upsert-cron-task id (str cron) (str cron)]))))
+  [{:keys [::db/conn ::entries]}]
+  (doseq [{:keys [id cron]} entries]
+    (let [result   (db/exec-one! conn [sql:upsert-cron-task id (str cron)])
+          updated? (pos? (db/get-update-count result))]
+      (l/dbg :hint "register task" :id id :cron (str cron)
+             :status (if updated? "created" "exists")))))
 
 (defn- lock-scheduled-task!
   [conn id]
@@ -45,7 +46,7 @@
 (declare ^:private schedule-cron-task)
 
 (defn- execute-cron-task
-  [cfg {:keys [id] :as task}]
+  [cfg {:keys [id cron] :as task}]
   (px/thread
     {:name (str "penpot/cron-task/" id)}
     (let [tpoint (dt/tpoint)]
@@ -54,20 +55,25 @@
                           (db/exec-one! conn ["SET LOCAL statement_timeout=0;"])
                           (db/exec-one! conn ["SET LOCAL idle_in_transaction_session_timeout=0;"])
                           (when (lock-scheduled-task! conn id)
-                            (l/dbg :hint "start task" :task-id id)
+                            (db/update! conn :scheduled-task
+                                        {:cron-expr (str cron)
+                                         :modified-at (dt/now)}
+                                        {:id id}
+                                        {::db/return-keys false})
+                            (l/dbg :hint "start" :id id)
                             ((:fn task) task)
                             (let [elapsed (dt/format-duration (tpoint))]
-                              (l/dbg :hint "end task" :task-id id :elapsed elapsed)))))
+                              (l/dbg :hint "end" :id id :elapsed elapsed)))))
 
         (catch InterruptedException _
           (let [elapsed (dt/format-duration (tpoint))]
-            (l/debug :hint "task interrupted" :task-id id :elapsed elapsed)))
+            (l/debug :hint "task interrupted" :id id :elapsed elapsed)))
 
         (catch Throwable cause
           (let [elapsed (dt/format-duration (tpoint))]
             (binding [l/*context* (get-error-context cause task)]
               (l/err :hint "unhandled exception on running task"
-                     :task-id id
+                     :id id
                      :elapsed elapsed
                      :cause cause))))
         (finally
@@ -86,7 +92,7 @@
   (let [ts (ms-until-valid cron)
         ft (px/schedule! ts (partial execute-cron-task cfg task))]
 
-    (l/dbg :hint "schedule task" :task-id id
+    (l/dbg :hint "schedule task" :id id
            :ts (dt/format-duration ts)
            :at (dt/format-instant (dt/in-future ts)))
 
@@ -135,7 +141,8 @@
           cfg     (assoc cfg ::entries entries ::running running)]
 
       (l/inf :hint "started" :tasks (count entries))
-      (synchronize-cron-entries! cfg)
+
+      (db/tx-run! cfg synchronize-cron-entries!)
 
       (->> (filter some? entries)
            (run! (partial schedule-cron-task cfg)))

backend/test/backend_tests/rpc_file_test.clj

@@ -1158,7 +1158,7 @@
       ;; check that the unknown frame thumbnail is deleted
       (let [rows (th/db-query :file-tagged-object-thumbnail {:file-id (:id file)})]
         (t/is (= 2 (count rows)))
-        (t/is (= 1 (count (remove (comp some? :deleted-at) rows)))))
+        (t/is (= 1 (count (remove :deleted-at rows)))))
 
       (let [res (th/run-task! :objects-gc {:min-age 0})]
         (t/is (= 3 (:processed res))))

frontend/resources/styles/main/partials/texts.scss

@@ -21,6 +21,10 @@
     flex-direction: column;
   }
 
+  .public-DraftStyleDefault-block {
+    white-space: pre;
+  }
+
   &.align-top {
     .DraftEditor-root {
       justify-content: flex-start;

frontend/src/app/libs/file_builder.cljs

@@ -253,7 +253,7 @@
 
   (export [_]
     (->> (export-file file)
-         (rx/subs
+         (rx/subs!
           (fn [value]
             (when  (not (contains? value :type))
               (let [[file export-blob] value]

frontend/src/app/main/data/users.cljs

@@ -163,7 +163,7 @@
     (ptk/reify ::logged-in
       ev/Event
       (-data [_]
-        {::ev/name "signing"
+        {::ev/name "signin"
          ::ev/type "identify"
          :email (:email profile)
          :auth-backend (:auth-backend profile)

frontend/src/app/main/data/workspace.cljs

@@ -1711,8 +1711,14 @@
           (process-entry [[type data]]
             (case type
               :text
-              (if (str/empty? data)
+              (cond
+                (str/empty? data)
                 (rx/empty)
+
+                (re-find #"<svg\s" data)
+                (rx/of (paste-svg-text data))
+
+                :else
                 (rx/of (paste-text data)))
 
               :transit
@@ -1757,8 +1763,7 @@
                 text-data    (some-> pdata wapi/extract-text)
                 transit-data (ex/ignoring (some-> text-data t/decode-str))]
             (cond
-              (and (string? text-data)
-                   (str/includes? text-data "<svg "))
+              (and (string? text-data) (re-find #"<svg\s" text-data))
               (rx/of (paste-svg-text text-data))
 
               (seq image-data)

frontend/src/app/main/data/workspace/libraries_helpers.cljs

@@ -1491,9 +1491,22 @@
                                                  container
                                                  {:type :reg-objects
                                                   :shapes all-parents})]))))
-          (let [roperation {:type :set
+          (let [;; position-data is a special case because can be affected by :geometry-group and :content-group
+                ;; so, if the position-data changes but the geometry is touched we need to reset the position-data
+                ;; so it's calculated again
+                reset-pos-data?
+                (and (cfh/text-shape? origin-shape)
+                     (= attr :position-data)
+                     (not= (get origin-shape attr) (get dest-shape attr))
+                     (touched :geometry-group))
+
+                roperation {:type :set
                             :attr attr
-                            :val (get origin-shape attr)
+                            :val (cond
+                                   ;; If position data changes and the geometry group is touched
+                                   ;; we need to put to nil so we can regenerate it
+                                   reset-pos-data? nil
+                                   :else (get origin-shape attr))
                             :ignore-touched true}
                 uoperation {:type :set
                             :attr attr

frontend/src/app/main/ui/components/tab_container.scss

@@ -46,6 +46,7 @@
     @extend .button-icon;
     stroke: var(--tab-foreground-color);
   }
+
   .content {
     @include headlineSmallTypography;
     text-align: center;
@@ -53,17 +54,21 @@
     overflow: hidden;
     text-overflow: ellipsis;
   }
+
   &.current,
   &.current:hover {
     background: var(--tab-background-color-selected);
     border-color: var(--tab-border-color-selected);
     color: var(--tab-foreground-color-selected);
+
     svg {
       stroke: var(--tab-foreground-color-selected);
     }
   }
+
   &:hover {
     color: var(--tab-foreground-color-hover);
+
     svg {
       stroke: var(--tab-foreground-color-hover);
     }
@@ -78,6 +83,7 @@
   min-width: $s-24;
   padding: 0 $s-6;
   border-radius: $br-5;
+
   svg {
     @include flexCenter;
     height: $s-16;
@@ -87,6 +93,7 @@
     fill: none;
     color: transparent;
   }
+
   &:hover {
     svg {
       stroke: var(--icon-foreground-hover);
@@ -107,3 +114,10 @@
   display: flex;
   flex-direction: column;
 }
+
+//Firefox doesn't respect scrollbar-gutter
+@supports (-moz-appearance: none) {
+  .tab-container-content {
+    padding-right: $s-8;
+  }
+}

frontend/src/app/main/ui/workspace/sidebar/options/menus/constraints.cljs

@@ -181,14 +181,14 @@
              [:span {:class (stl/css :resalted-area)}]]]
            [:div {:class (stl/css :constraints-center)}
             [:button {:class (stl/css-case :constraint-btn true
-                                           :active (= constraints-h :center))
-                      :data-value "centerh"
+                                           :active (= constraints-v :center))
+                      :data-value "centerv"
                       :on-click on-constraint-button-clicked}
              [:span {:class (stl/css :resalted-area)}]]
             [:button {:class (stl/css-case :constraint-btn-special true
                                            :constraint-btn-rotated true
-                                           :active (= constraints-v :center))
-                      :data-value "centerv"
+                                           :active (= constraints-h :center))
+                      :data-value "centerh"
                       :on-click on-constraint-button-clicked}
              [:span {:class (stl/css :resalted-area)}]]]
            [:div {:class (stl/css :constraints-right)}

frontend/src/app/main/ui/workspace/sidebar/options/menus/fill.cljs

@@ -69,33 +69,34 @@
 
         on-add
         (mf/use-fn
-         (mf/deps ids)
+         (mf/deps ids fills)
          (fn [_]
            (st/emit! (dc/add-fill ids {:color default-color
                                        :opacity 1}))
 
-           (when (not (some? (seq fills))) (open-content))))
+           (when (or (= :multiple fills)
+                     (not (some? (seq fills))))
+             (open-content))))
 
         on-change
-        (mf/use-fn
-         (mf/deps ids)
-         (fn [index]
-           (fn [color]
-             (st/emit! (dc/change-fill ids color index)))))
+        (fn [index]
+          (fn [color]
+            (st/emit! (dc/change-fill ids color index))))
 
         on-reorder
-        (mf/use-fn
-         (mf/deps ids)
-         (fn [new-index]
-           (fn [index]
-             (st/emit! (dc/reorder-fills ids index new-index)))))
+        (fn [new-index]
+          (fn [index]
+            (st/emit! (dc/reorder-fills ids index new-index))))
 
         on-remove
         (fn [index]
           (fn []
             (st/emit! (dc/remove-fill ids {:color default-color
                                            :opacity 1} index))
-            (when (= 1 (count (seq fills))) (close-content))))
+            (when (or (= :multiple fills)
+                      (= 1 (count (seq fills))))
+              (close-content))))
+
         on-remove-all
         (fn [_]
           (st/emit! (dc/remove-all-fills ids {:color clr/black