🐛 Fixes more glitches of dnd sortable trees

frontend/src/uxbox/main/ui/hooks.cljs

@@ -87,11 +87,52 @@
         (dissoc state :timer))
       state)))
 
+(def sortable-ctx (mf/create-context nil))
+
+(mf/defc sortable-container
+  [{:keys [children] :as props}]
+  (let [global-drag-end (mf/use-memo #(rx/subject))]
+    [:& (mf/provider sortable-ctx) {:value global-drag-end}
+     children]))
+
+
+;; The dnd API is problematic for nested elements, such a sortable items tree.
+;; The approach used here to solve bad situations is:
+;; - Capture all events in the leaf draggable elements, and stop propagation.
+;; - Ignore events originated in non-draggable children.
+;; - At drag operation end, all elements that have received some enter/over
+;;   event and have not received the corresponding leave event, are notified
+;;   so they can clean up.
+;;
+;; Do not remove commented out lines, they are useful to debug events when
+;; things go weird.
+
 (defn use-sortable
   [& {:keys [data-type data on-drop on-drag on-hold detect-center?] :as opts}]
   (let [ref   (mf/use-ref)
         state (mf/use-state {:over nil
-                             :timer nil})
+                             :timer nil
+                             :subscr nil})
+
+        global-drag-end (mf/use-ctx sortable-ctx)
+
+        cleanup
+        (fn []
+          ;; (js/console.log "cleanup" (:name data))
+          (when-let [subscr (:subscr @state)]
+            ;; (js/console.log "unsubscribing" (:name data))
+            (rx/unsub! (:subscr @state)))
+          (swap! state (fn [state]
+                              (-> state
+                                  (cancel-timer)
+                                  (dissoc :over :subscr)))))
+
+        subscribe-to-drag-end
+        (fn []
+          (when (nil? (:subscr @state))
+            ;; (js/console.log "subscribing" (:name data))
+            (swap! state
+                   #(assoc % :subscr (rx/sub! global-drag-end cleanup)))))
 
         on-drag-start
         (fn [event]
@@ -108,6 +149,7 @@
           (dom/prevent-default event) ;; prevent default to allow drag enter
           (when-not (dnd/from-child? event)
             (dom/stop-propagation event)
+            (subscribe-to-drag-end)
             ;; (dnd/trace event data "drag-enter")
             (when (fn? on-hold)
               (swap! state (fn [state]
@@ -121,6 +163,7 @@
             (dom/prevent-default event) ;; prevent default to allow drag over
             (when-not (dnd/from-child? event)
               (dom/stop-propagation event)
+              (subscribe-to-drag-end)
               ;; (dnd/trace event data "drag-over")
               (let [side (dnd/drop-side event detect-center?)]
                 (swap! state assoc :over side)))))
@@ -130,10 +173,7 @@
           (when-not (dnd/from-child? event)
             (dom/stop-propagation event)
             ;; (dnd/trace event data "drag-leave")
-            (swap! state (fn [state]
+            (cleanup)))
-                           (-> state
-                             (cancel-timer)
-                             (dissoc :over))))))
 
         on-drop'
         (fn [event]
@@ -141,30 +181,30 @@
           ;; (dnd/trace event data "drop")
           (let [side (dnd/drop-side event detect-center?)
                 drop-data (dnd/get-data event data-type)]
-            (swap! state (fn [state]
+            (cleanup)
-                           (-> state
+            (rx/push! global-drag-end nil)
-                             (cancel-timer)
-                             (dissoc :over))))
             (when (fn? on-drop)
               (on-drop side drop-data))))
 
         on-drag-end
         (fn [event]
+          (dom/stop-propagation event)
           ;; (dnd/trace event data "drag-end")
-          (swap! state (fn [state]
+          (rx/push! global-drag-end nil)
-                         (-> state
+          (cleanup))
-                           (cancel-timer)
-                           (dissoc :over)))))
 
         on-mount
         (fn []
           (let [dom (mf/ref-val ref)]
             (.setAttribute dom "draggable" true)
 
+            ;; Register all events in the (default) bubble mode, so that they
+            ;; are captured by the most leaf item. The handler will stop
+            ;; propagation, so they will not go up in the containment tree.
             (.addEventListener dom "dragstart" on-drag-start false)
             (.addEventListener dom "dragenter" on-drag-enter false)
             (.addEventListener dom "dragover" on-drag-over false)
-            (.addEventListener dom "dragleave" on-drag-leave true)
+            (.addEventListener dom "dragleave" on-drag-leave false)
             (.addEventListener dom "drop" on-drop' false)
             (.addEventListener dom "dragend" on-drag-end false)
             #(do

frontend/src/uxbox/main/ui/workspace/sidebar/layers.cljs

@@ -12,6 +12,7 @@
   (:require
    [okulary.core :as l]
    [rumext.alpha :as mf]
+   [beicon.core :as rx]
    [uxbox.main.ui.icons :as i]
    [uxbox.common.data :as d]
    [uxbox.common.uuid :as uuid]
@@ -266,21 +267,22 @@
   (let [selected (mf/deref refs/selected-shapes)
         root (get objects uuid/zero)]
     [:ul.element-list
-     (for [[index id] (reverse (d/enumerate (:shapes root)))]
+     [:& hooks/sortable-container {}
-       (let [obj (get objects id)]
+       (for [[index id] (reverse (d/enumerate (:shapes root)))]
-         (if (= (:type obj) :frame)
+         (let [obj (get objects id)]
-           [:& frame-wrapper
+           (if (= (:type obj) :frame)
-            {:item obj
+             [:& frame-wrapper
-             :selected selected
+              {:item obj
-             :index index
+               :selected selected
-             :objects objects
+               :index index
-             :key id}]
+               :objects objects
-           [:& layer-item
+               :key id}]
-            {:item obj
+             [:& layer-item
-             :selected selected
+              {:item obj
-             :index index
+               :selected selected
-             :objects objects
+               :index index
-             :key id}])))]))
+               :objects objects
+               :key id}])))]]))
 
 (defn- strip-objects
   [objects]

frontend/src/uxbox/main/ui/workspace/sidebar/sitemap.cljs

@@ -129,13 +129,14 @@
   (let [pages (d/enumerate (:pages file))
         deletable? (> (count pages) 1)]
     [:ul.element-list
-     (for [[index page-id] pages]
+     [:& hooks/sortable-container {}
-       [:& page-item-wrapper
+       (for [[index page-id] pages]
-        {:page-id page-id
+         [:& page-item-wrapper
-         :index index
+          {:page-id page-id
-         :deletable? deletable?
+           :index index
-         :selected? (= page-id (:id current-page))
+           :deletable? deletable?
-         :key page-id}])]))
+           :selected? (= page-id (:id current-page))
+           :key page-id}])]]))
 
 ;; --- Sitemap Toolbox
 

frontend/src/uxbox/util/dom/dnd.cljs

@@ -21,15 +21,37 @@
 ;; https://www.w3schools.com/jsref/event_relatedtarget.asp
 ;; https://stackoverflow.com/questions/14194324/firefox-firing-dragleave-when-dragging-over-text?noredirect=1&lq=1
 ;; https://stackoverflow.com/questions/7110353/html5-dragleave-fired-when-hovering-a-child-element
+;;
+;; The main issue is that when we have a draggable element, for example
+;;   <li draggable="true">
+;;     <span>some text</span>
+;;     other text
+;;   </li>
+;;
+;; The api will generate enter and leave events when cursor moves within the internal
+;; elements (in this example the span and the other text). But the target of the event
+;; is the draggable element (the real initiator comes in the "relatedTarget" attribute).
+;; This causes that the draggable element receives events that tells that the cursor
+;; has moved from itself to itself, and this often causes strange behaviors.
+;;
+;; A common solution is to ignore events originated from child elements (look at
+;; from-child? function). This creates additional problems when there are nested draggable
+;; objects, for example a hierarchical tree with nested <li>s.
 
 (defn trace
-  ;; This function is useful to debug the erratic dnd interface behaviour when something weird occurs
+  ;; This function is useful to debug the dnd interface behaviour when something weird occurs.
   [event data label]
-  (js/console.log
+  (let [currentTarget (.-currentTarget event)
-    label
+        relatedTarget (.-relatedTarget event)]
-    "[" (:name data) "]"
+    (js/console.log
-    (if (.-currentTarget event) (.-textContent (.-currentTarget event)) "null")
+      label
-    (if (.-relatedTarget event) (.-textContent (.-relatedTarget event)) "null")))
+      "[" (:name data) "]"
+      ;; (if currentTarget
+      ;;   (str "<" (.-localName currentTarget) " " (.-textContent currentTarget) ">")
+      ;;   "null")
+      (if relatedTarget
+        (str "<" (.-localName relatedTarget) " " (.-textContent relatedTarget) ">")
+        "null"))))
 
 (defn set-data!
   ([e data]
@@ -93,9 +115,10 @@
   (let [ypos   (.-offsetY e)
         target (.-currentTarget e)
         height (.-clientHeight target)
+        innerHeight (.-clientHeight (.-firstChild target))
         thold  (/ height 2)
-        thold1 (* height 0.2)
+        thold1 (* innerHeight 0.2)
-        thold2 (* height 0.8)]
+        thold2 (* innerHeight 0.8)]
     (if detect-center?
       (cond
         (< ypos thold1) :top