🐛 Fix issue on shortcuts restore operation (#6462)

* 🐛 Fix issue on shortcuts restore operation

Happens when the order of shortcuts pop events is inconsistent with
push events. Using less strictly order policy for pop operations
allows relax this and make it eventually consistent.

* 💄 Add cosmetic changes on shortcuts hooks on colorpicker and    wport

* 📎 Update changelog

* 📎 Add PR feedback changes

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

@@ -8,7 +8,7 @@
   (:refer-clojure :exclude [meta reset!])
   (:require
    ["@penpot/mousetrap$default" :as mousetrap]
-   [app.common.data.macros :as dm]
+   [app.common.data :as d]
    [app.common.logging :as log]
    [app.common.schema :as sm]
    [app.config :as cf]
@@ -135,7 +135,7 @@
     [:fn {:optional true} fn?]
     [:tooltip {:optional true} :string]]])
 
-(def check-shortcuts!
+(def ^:private check-shortcuts
   (sm/check-fn schema:shortcuts))
 
 (defn- wrap-cb
@@ -167,23 +167,20 @@
    (mousetrap/reset)
    (bind! shortcuts)))
 
+(def ^:private conj*
+  (fnil conj (d/ordered-map)))
+
 (defn push-shortcuts
   [key shortcuts]
+  (assert (keyword? key) "expected a keyword for `key`")
+  (let [shortcuts (check-shortcuts shortcuts)]
+    (ptk/reify ::push-shortcuts
+      ptk/UpdateEvent
+      (update [_ state]
+        (update state :shortcuts conj* [key shortcuts]))
 
-  (dm/assert!
-   "expected valid parameters"
-   (and (keyword? key)
-        (check-shortcuts! shortcuts)))
-
-  (ptk/reify ::push-shortcuts
-    ptk/UpdateEvent
-    (update [_ state]
-      (-> state
-          (update :shortcuts (fnil conj '()) [key shortcuts])))
-
-    ptk/EffectEvent
-    (effect [_ state _]
-      (let [[_key shortcuts] (peek (:shortcuts state))]
+      ptk/EffectEvent
+      (effect [_ _ _]
         (reset! shortcuts)))))
 
 (defn pop-shortcuts
@@ -192,12 +189,9 @@
     ptk/UpdateEvent
     (update [_ state]
       (update state :shortcuts (fn [shortcuts]
-                                 (let [current-key (first (peek shortcuts))]
-                                   (if (= key current-key)
-                                     (pop shortcuts)
-                                     shortcuts)))))
+                                 (dissoc shortcuts key))))
+
     ptk/EffectEvent
     (effect [_ state _]
-      (let [[key* shortcuts] (peek (:shortcuts state))]
-        (when (not= key key*)
-          (reset! shortcuts))))))
+      (let [[_key shortcuts] (last (:shortcuts state))]
+        (reset! shortcuts)))))

frontend/src/app/main/ui/workspace/colorpicker.cljs

@@ -586,10 +586,10 @@
 
     (mf/with-effect []
       (st/emit! (st/emit! (dsc/push-shortcuts ::colorpicker sc/shortcuts)))
-      #(do
-         (st/emit! (dsc/pop-shortcuts ::colorpicker))
-         (when (and @dirty? @last-change on-close)
-           (on-close @last-change))))
+      (fn []
+        (st/emit! (dsc/pop-shortcuts ::colorpicker))
+        (when (and @dirty? @last-change on-close)
+          (on-close @last-change))))
 
     [:div {:class (stl/css :colorpicker-tooltip)
            :data-testid "colorpicker"

frontend/src/app/main/ui/workspace/viewport/hooks.cljs

@@ -472,16 +472,23 @@
 (defn setup-shortcuts
   [path-editing? drawing-path? text-editing? grid-editing?]
   (hooks/use-shortcuts ::workspace wsc/shortcuts)
-  (mf/use-effect
-   (mf/deps path-editing? drawing-path? grid-editing?)
-   (fn []
-     (cond
-       grid-editing?
-       (do (st/emit! (dsc/push-shortcuts ::grid gsc/shortcuts))
-           #(st/emit! (dsc/pop-shortcuts ::grid)))
-       (or drawing-path? path-editing?)
-       (do (st/emit! (dsc/push-shortcuts ::path psc/shortcuts))
-           #(st/emit! (dsc/pop-shortcuts ::path)))
-       text-editing?
-       (do (st/emit! (dsc/push-shortcuts ::text tsc/shortcuts))
-           #(st/emit! (dsc/pop-shortcuts ::text)))))))
+
+  (mf/with-effect [path-editing? drawing-path? grid-editing?]
+    (cond
+      grid-editing?
+      (do
+        (st/emit! (dsc/push-shortcuts ::grid gsc/shortcuts))
+        (fn []
+          (st/emit! (dsc/pop-shortcuts ::grid))))
+
+      (or drawing-path? path-editing?)
+      (do
+        (st/emit! (dsc/push-shortcuts ::path psc/shortcuts))
+        (fn []
+          (st/emit! (dsc/pop-shortcuts ::path))))
+
+      text-editing?
+      (do
+        (st/emit! (dsc/push-shortcuts ::text tsc/shortcuts))
+        (fn []
+          (st/emit! (dsc/pop-shortcuts ::text)))))))