From 763f1ee13d5eb711d4da6e004db93c4811dc0d7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9s=20Moya?= <andres.moya@kaleidos.net>
Date: Fri, 19 Jun 2020 12:06:22 +0200
Subject: [PATCH] :bug: Fixes more glitches of dnd sortable trees
---
frontend/src/uxbox/main/ui/hooks.cljs | 68 +++++++++++++++----
.../main/ui/workspace/sidebar/layers.cljs | 32 +++++----
.../main/ui/workspace/sidebar/sitemap.cljs | 15 ++--
frontend/src/uxbox/util/dom/dnd.cljs | 39 ++++++++---
4 files changed, 110 insertions(+), 44 deletions(-)
diff --git a/frontend/src/uxbox/main/ui/hooks.cljs b/frontend/src/uxbox/main/ui/hooks.cljs
index 412864c92..9e52ec3bb 100644
--- a/frontend/src/uxbox/main/ui/hooks.cljs
+++ b/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]
- (-> state
- (cancel-timer)
- (dissoc :over))))))
+ (cleanup)))
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]
- (-> state
- (cancel-timer)
- (dissoc :over))))
+ (cleanup)
+ (rx/push! global-drag-end nil)
(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]
- (-> state
- (cancel-timer)
- (dissoc :over)))))
+ (rx/push! global-drag-end nil)
+ (cleanup))
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
diff --git a/frontend/src/uxbox/main/ui/workspace/sidebar/layers.cljs b/frontend/src/uxbox/main/ui/workspace/sidebar/layers.cljs
index 0589ed1cd..614458c4e 100644
--- a/frontend/src/uxbox/main/ui/workspace/sidebar/layers.cljs
+++ b/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)))]
- (let [obj (get objects id)]
- (if (= (:type obj) :frame)
- [:& frame-wrapper
- {:item obj
- :selected selected
- :index index
- :objects objects
- :key id}]
- [:& layer-item
- {:item obj
- :selected selected
- :index index
- :objects objects
- :key id}])))]))
+ [:& hooks/sortable-container {}
+ (for [[index id] (reverse (d/enumerate (:shapes root)))]
+ (let [obj (get objects id)]
+ (if (= (:type obj) :frame)
+ [:& frame-wrapper
+ {:item obj
+ :selected selected
+ :index index
+ :objects objects
+ :key id}]
+ [:& layer-item
+ {:item obj
+ :selected selected
+ :index index
+ :objects objects
+ :key id}])))]]))
(defn- strip-objects
[objects]
diff --git a/frontend/src/uxbox/main/ui/workspace/sidebar/sitemap.cljs b/frontend/src/uxbox/main/ui/workspace/sidebar/sitemap.cljs
index 03eae976b..b82136d51 100644
--- a/frontend/src/uxbox/main/ui/workspace/sidebar/sitemap.cljs
+++ b/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]
- [:& page-item-wrapper
- {:page-id page-id
- :index index
- :deletable? deletable?
- :selected? (= page-id (:id current-page))
- :key page-id}])]))
+ [:& hooks/sortable-container {}
+ (for [[index page-id] pages]
+ [:& page-item-wrapper
+ {:page-id page-id
+ :index index
+ :deletable? deletable?
+ :selected? (= page-id (:id current-page))
+ :key page-id}])]]))
;; --- Sitemap Toolbox
diff --git a/frontend/src/uxbox/util/dom/dnd.cljs b/frontend/src/uxbox/util/dom/dnd.cljs
index b78ac9019..ed840b88c 100644
--- a/frontend/src/uxbox/util/dom/dnd.cljs
+++ b/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
- label
- "[" (:name data) "]"
- (if (.-currentTarget event) (.-textContent (.-currentTarget event)) "null")
- (if (.-relatedTarget event) (.-textContent (.-relatedTarget event)) "null")))
+ (let [currentTarget (.-currentTarget event)
+ relatedTarget (.-relatedTarget event)]
+ (js/console.log
+ label
+ "[" (: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)
- thold2 (* height 0.8)]
+ thold1 (* innerHeight 0.2)
+ thold2 (* innerHeight 0.8)]
(if detect-center?
(cond
(< ypos thold1) :top