Fixed drag-and-drop list placeholder issues (plone#2884)

- Fix placeholder position to work with subtree margins. The original calculation assumes that there are no margins. The update works for all cases included when the subtree contains margins that extend to the node's parent. - Remove placeholder when dropping in all cases. - Make placeholder disappear when the element is moved outside the parent (thus, causes the place to be removed).
nirban07 · Dec 1, 2021 · 86038b3 · 86038b3
1 parent 9bd72bf
commit 86038b3
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 
 - In search block, read SearchableText search param, to use it as search text input
   @tiberiuichim
+- Fixed drag-and-drop list placeholder issues @reebalazs
 
 ### Internal
 

diff --git a/src/components/manage/DragDropList/DragDropList.jsx b/src/components/manage/DragDropList/DragDropList.jsx
@@ -1,8 +1,52 @@
-import React from 'react';
+import React, { useRef } from 'react';
 import { isEmpty } from 'lodash';
 import { DragDropContext, Draggable, Droppable } from 'react-beautiful-dnd';
 import { v4 as uuid } from 'uuid';
 
+const getPlaceholder = (draggedDOM, sourceIndex, destinationIndex) => {
+  // Because of the margin rendering rules, there is no easy
+  // way to calculate the offset of the placeholder.
+  //
+  // (Note that this is the reason we cannot use the solutions
+  // published on the net, because they assume that we are in control
+  // of the content and there are no additional margins involved.)
+  //
+  // To get a placeholder that looks good in all cases, we
+  // fill up the space between the previous and the next element.
+  const childrenArray = [...draggedDOM.parentNode.children];
+  // Remove the source element
+  childrenArray.splice(sourceIndex, 1);
+  // Also remove the placeholder that the library always inserts at the end
+  childrenArray.splice(-1, 1);
+  const parentRect = draggedDOM.parentNode.getBoundingClientRect();
+  const prevNode = childrenArray[destinationIndex - 1];
+  const nextNode = childrenArray[destinationIndex];
+  let top, bottom;
+  if (prevNode) {
+    const prevRect = prevNode.getBoundingClientRect();
+    top = prevRect.top + prevRect.height - parentRect.top;
+  } else {
+    top = 0;
+  }
+  if (nextNode) {
+    const nextRect = nextNode.getBoundingClientRect();
+    bottom = nextRect.top - parentRect.top;
+  } else {
+    bottom =
+      parentRect.bottom +
+      draggedDOM.getBoundingClientRect().height -
+      parentRect.top;
+  }
+  return {
+    clientY: top,
+    clientHeight: bottom - top,
+    clientX: parseFloat(
+      window.getComputedStyle(draggedDOM.parentNode).paddingLeft,
+    ),
+    clientWidth: draggedDOM.clientWidth,
+  };
+};
+
 const DragDropList = (props) => {
   const {
     childList,
@@ -14,99 +58,68 @@ const DragDropList = (props) => {
   } = props; //renderChild
   const [placeholderProps, setPlaceholderProps] = React.useState({});
   const [uid] = React.useState(uuid());
+  // queueing timed action
+  const timer = useRef(null);
 
-  const handleDragStart = React.useCallback((event) => {
+  const onDragStart = React.useCallback((event) => {
+    clearTimeout(timer.current);
     const queryAttr = 'data-rbd-draggable-id';
     const domQuery = `[${queryAttr}='${event.draggableId}']`;
     const draggedDOM = document.querySelector(domQuery);
-
     if (!draggedDOM) {
       return;
     }
-
-    const { clientHeight, clientWidth } = draggedDOM;
     const sourceIndex = event.source.index;
-    var clientY =
-      parseFloat(window.getComputedStyle(draggedDOM.parentNode).paddingTop) +
-      [...draggedDOM.parentNode.children]
-        .slice(0, sourceIndex)
-        .reduce((total, curr) => {
-          const style = curr.currentStyle || window.getComputedStyle(curr);
-          const marginBottom = parseFloat(style.marginBottom);
-          return total + curr.clientHeight + marginBottom;
-        }, 0);
-
-    setPlaceholderProps({
-      clientHeight,
-      clientWidth,
-      clientY,
-      clientX: parseFloat(
-        window.getComputedStyle(draggedDOM.parentNode).paddingLeft,
-      ),
-    });
+    setPlaceholderProps(getPlaceholder(draggedDOM, sourceIndex, sourceIndex));
   }, []);
 
+  const onDragEnd = React.useCallback(
+    (result) => {
+      clearTimeout(timer.current);
+      onMoveItem(result);
+      setPlaceholderProps({});
+    },
+    [onMoveItem],
+  );
+
   const onDragUpdate = React.useCallback((update) => {
+    clearTimeout(timer.current);
+    setPlaceholderProps({});
     if (!update.destination) {
       return;
     }
     const draggableId = update.draggableId;
-    const destinationIndex = update.destination.index;
-
     const queryAttr = 'data-rbd-draggable-id';
     const domQuery = `[${queryAttr}='${draggableId}']`;
     const draggedDOM = document.querySelector(domQuery);
-
     if (!draggedDOM) {
       return;
     }
-    const { clientHeight, clientWidth } = draggedDOM;
     const sourceIndex = update.source.index;
-    const childrenArray = [...draggedDOM.parentNode.children];
-    const movedItem = childrenArray[sourceIndex];
-    childrenArray.splice(sourceIndex, 1);
-
-    const updatedArray = [
-      ...childrenArray.slice(0, destinationIndex),
-      movedItem,
-      ...childrenArray.slice(destinationIndex + 1),
-    ];
-
-    var clientY =
-      parseFloat(window.getComputedStyle(draggedDOM.parentNode).paddingTop) +
-      updatedArray.slice(0, destinationIndex).reduce((total, curr) => {
-        if (!curr) return total;
-        const style = curr.currentStyle || window.getComputedStyle(curr);
-        const marginBottom = parseFloat(style.marginBottom);
-        return total + curr.clientHeight + marginBottom;
-      }, 0);
-
-    setPlaceholderProps({
-      clientHeight,
-      clientWidth,
-      clientY,
-      clientX: parseFloat(
-        window.getComputedStyle(draggedDOM.parentNode).paddingLeft,
-      ),
-    });
+    const destinationIndex = update.destination.index;
+    // Wait until the animations have finished, to make it look good.
+    timer.current = setTimeout(
+      () =>
+        setPlaceholderProps(
+          getPlaceholder(draggedDOM, sourceIndex, destinationIndex),
+        ),
+      250,
+    );
   }, []);
 
   const AsDomComponent = as;
   return (
     <DragDropContext
-      onDragEnd={(result) => {
-        const isMoved = onMoveItem(result);
-        if (isMoved) setPlaceholderProps({});
-      }}
-      onDragStart={handleDragStart}
+      onDragStart={onDragStart}
       onDragUpdate={onDragUpdate}
+      onDragEnd={onDragEnd}
     >
       <Droppable droppableId={uid}>
         {(provided, snapshot) => (
           <AsDomComponent
             ref={provided.innerRef}
             {...provided.droppableProps}
-            style={{ position: 'relative', ...style }}
+            style={{ ...style, position: 'relative' }}
             aria-labelledby={forwardedAriaLabelledBy}
           >
             {childList
@@ -116,19 +129,23 @@ const DragDropList = (props) => {
                   draggableId={childId.toString()}
                   index={index}
                   key={childId}
+                  style={{
+                    userSelect: 'none',
+                  }}
                 >
                   {(draginfo) => children({ child, childId, index, draginfo })}
                 </Draggable>
               ))}
             {provided.placeholder}
-            {!isEmpty(placeholderProps) && (
+            {!isEmpty(placeholderProps) && snapshot.isDraggingOver && (
               <div
                 style={{
                   position: 'absolute',
-                  top: `${placeholderProps.clientY}px`,
-                  height: `${placeholderProps.clientHeight + 18}px`,
+                  top: placeholderProps.clientY,
+                  left: placeholderProps.clientX,
+                  height: placeholderProps.clientHeight,
                   background: '#eee',
-                  width: `${placeholderProps.clientWidth}px`,
+                  width: placeholderProps.clientWidth,
                   borderRadius: '3px',
                 }}
               />

diff --git a/src/components/manage/Widgets/__snapshots__/ObjectListWidget.test.js.snap b/src/components/manage/Widgets/__snapshots__/ObjectListWidget.test.js.snap
@@ -90,7 +90,7 @@ exports[`renders an object list widget component 1`] = `
       aria-labelledby="fieldset-my-field-set-field-label-my-widget"
       data-rbd-droppable-context-id="0"
       data-rbd-droppable-id="id-0"
-      style="position: relative; box-shadow: 0 1px 1px rgba(0,0,0,0.15), 0 10px 0 -5px #eee, 0 10px 1px -4px rgba(0,0,0,0.15);"
+      style="box-shadow: 0 1px 1px rgba(0,0,0,0.15), 0 10px 0 -5px #eee, 0 10px 1px -4px rgba(0,0,0,0.15); position: relative;"
     >
       <div
         data-rbd-draggable-context-id="0"