Fix correctly scrolling the screen when dragging events, instructions, objects (#9082)

Author: ClementPasteau

newIDE/app/src/EventsSheet/EventsTree/DropContainer.js

@@ -21,12 +21,6 @@ const sharedStyles = {
     border: '2px solid black',
     outline: '1px solid white',
   },
-  autoScroll: {
-    width: '100%',
-    position: 'absolute',
-    height: '10%',
-    zIndex: 2,
-  },
 };
 
 type DropTargetContainerStyle = {|
@@ -331,68 +325,3 @@ export function DropContainer({
     </div>
   );
 }
-
-export function AutoScroll({
-  direction,
-  DnDComponent,
-  activateTargets,
-  onHover,
-}: {|
-  direction: 'top' | 'bottom',
-  DnDComponent: DropTargetComponent<SortableTreeNode>,
-  activateTargets: boolean,
-  onHover: () => void,
-|}): React.MixedElement {
-  const delayActivationTimer = React.useRef<?TimeoutID>(null);
-  const [show, setShow] = React.useState(false);
-
-  // This drop target overlaps with sibling drag source and cancels drag immediately.
-  // See: https://github.com/react-dnd/react-dnd/issues/766#issuecomment-388943403
-  // Delaying the render of the drop target seems to solve the issue.
-  React.useEffect(
-    () => {
-      if (activateTargets) {
-        delayActivationTimer.current = setTimeout(() => {
-          setShow(true);
-        }, 100);
-      } else {
-        setShow(false);
-        clearTimeout(delayActivationTimer.current);
-        delayActivationTimer.current = null;
-      }
-      return () => {
-        delayActivationTimer.current &&
-          clearTimeout(delayActivationTimer.current);
-      };
-    },
-    [activateTargets]
-  );
-
-  return (
-    <DnDComponent
-      canDrop={() => true}
-      drop={() => {
-        return;
-      }}
-    >
-      {({ isOverLazy, connectDropTarget }) => {
-        if (isOverLazy) {
-          onHover();
-        }
-        const dropTarget = (
-          <div
-            style={{
-              ...sharedStyles.autoScroll,
-              ...(direction === 'top' ? { top: 0 } : { bottom: 0 }),
-
-              // Uncomment for debugging purposes.
-              // backgroundColor: 'black',
-              // opacity: isOverLazy ? 1 : 0,
-            }}
-          />
-        );
-        return show ? connectDropTarget(dropTarget) : null;
-      }}
-    </DnDComponent>
-  );
-}

newIDE/app/src/EventsSheet/EventsTree/SortableEventsTree.js

@@ -1,6 +1,7 @@
 // @flow
 import * as React from 'react';
-import { VariableSizeList } from 'react-window';
+import { VariableSizeList, areEqual } from 'react-window';
+import { useIsDragging } from '../../UI/DragAndDrop/UseIsDragging';
 import classNames from 'classnames';
 import { AutoSizer } from 'react-virtualized';
 import type { ProjectScopedContainersAccessor } from '../../InstructionOrExpression/EventsScope';
@@ -354,6 +355,13 @@ const TreeRow = ({
   );
 };
 
+// Rows only re-render when their data changes, not on every scroll.
+const MemoizedTreeRow = React.memo<{
+  index: number,
+  style: any,
+  data: RowItemData,
+}>(TreeRow, areEqual);
+
 // -- Main component --
 
 const SortableEventsTree = ({
@@ -459,6 +467,31 @@ const SortableEventsTree = ({
     ]
   );
 
+  const isDragging = useIsDragging();
+  const isDraggingRef = React.useRef(false);
+  isDraggingRef.current = isDragging;
+  // react-window disables pointer events while scrolling: the drop targets of
+  // the rows must stay active while the sheet auto scrolls during a drag.
+  // Stable component (rows would be unmounted otherwise) reading a ref.
+  const listInnerElementType = React.useMemo(
+    () =>
+      React.forwardRef<{ style: Object }, HTMLDivElement>(
+        ({ style, ...otherProps }, ref) => (
+          <div
+            ref={ref}
+            style={{
+              ...style,
+              pointerEvents: isDraggingRef.current
+                ? undefined
+                : style.pointerEvents,
+            }}
+            {...otherProps}
+          />
+        )
+      ),
+    []
+  );
+
   const itemKey = React.useCallback((index: number, data: RowItemData) => {
     const entry = data.flatData[index];
     return entry ? String(entry.node.key) : String(index);
@@ -480,8 +513,9 @@ const SortableEventsTree = ({
             itemKey={itemKey}
             onScroll={handleScroll}
             overscanCount={10}
+            innerElementType={listInnerElementType}
           >
-            {TreeRow}
+            {MemoizedTreeRow}
           </VariableSizeList>
         )}
       </AutoSizer>

newIDE/app/src/EventsSheet/EventsTree/index.js

@@ -45,7 +45,8 @@ import TutorialMessage from '../../Hints/TutorialMessage';
 import getTutorial from '../../Hints/getTutorial';
 import { makeDragSourceAndDropTarget } from '../../UI/DragAndDrop/DragSourceAndDropTarget';
 import { makeDropTarget } from '../../UI/DragAndDrop/DropTarget';
-import { AutoScroll, DropContainer } from './DropContainer';
+import { DropContainer } from './DropContainer';
+import { useAutoScrollDuringAnyDrag } from '../../UI/DragAndDrop/UseAutoScrollDuringDrag';
 import {
   isDescendant,
   isElseEventValid,
@@ -511,9 +512,13 @@ const EventsTree: React.ComponentType<{
   const _hoverTimerId = React.useRef<?TimeoutID>(null);
 
   const [draggedNode, setDraggedNode] = React.useState(null);
-  const [isScrolledTop, setIsScrolledTop] = React.useState(true);
-  const [isScrolledBottom, setIsScrolledBottom] = React.useState(false);
   const lastKnownScrollPosition = React.useRef(0);
+  // Scroll when events, but also instructions or variable declarations,
+  // are dragged close to the edges.
+  useAutoScrollDuringAnyDrag(
+    () => (_list.current ? _list.current.container : null),
+    { maxSpeed: 900 }
+  );
 
   // This is the data that will be displayed by the tree - reconstructed at each render
   // (because events could have changed, some could have been deleted, so we can't keep
@@ -1227,14 +1232,6 @@ const EventsTree: React.ComponentType<{
     []
   );
 
-  const _scrollUp = React.useCallback(() => {
-    _list.current && _list.current.container.scrollBy({ top: -5 });
-  }, []);
-
-  const _scrollDown = React.useCallback(() => {
-    _list.current && _list.current.container.scrollBy({ top: 5 });
-  }, []);
-
   const zoomLevel = props.fontSize || 14;
 
   // Update treeDataRoot with the events tree. Done at each render as events
@@ -1249,9 +1246,7 @@ const EventsTree: React.ComponentType<{
   React.useLayoutEffect(() => {
     // Recompute row heights on every render. This is needed for any change
     // that affects which event is at which row (deletion, fold/unfold, move)
-    // — not just changes to individual event heights. Scroll-triggered
-    // re-renders (isScrolledTop / isScrolledBottom state flips) are infrequent
-    // and recalculate the same heights, so the extra work is negligible.
+    // — not just changes to individual event heights.
     if (_list.current) {
       _list.current.recomputeRowHeights();
     }
@@ -1282,26 +1277,6 @@ const EventsTree: React.ComponentType<{
         )}px`,
       }}
     >
-      {/* Disable for touchscreen because the dragged DOM node gets deleted, the */}
-      {/* touch events are lost and the dnd does not drop anymore (hypothesis). */}
-      {props.screenType !== 'touch' && (
-        <>
-          <AutoScroll
-            DnDComponent={EventDropTarget}
-            direction="top"
-            // $FlowFixMe[constant-condition]
-            activateTargets={!!draggedNode && !isScrolledTop}
-            onHover={_scrollUp}
-          />
-          <AutoScroll
-            DnDComponent={EventDropTarget}
-            direction="bottom"
-            // $FlowFixMe[constant-condition]
-            activateTargets={!!draggedNode && !isScrolledBottom}
-            onHover={_scrollDown}
-          />
-        </>
-      )}
       <SortableTree
         treeData={
           // Pass a new array each time, otherwise the tree will not re-render.
@@ -1332,10 +1307,6 @@ const EventsTree: React.ComponentType<{
             }
             lastKnownScrollPosition.current = event.scrollTop;
             props.onScroll && props.onScroll();
-            setIsScrolledTop(event.scrollTop === 0);
-            setIsScrolledBottom(
-              event.clientHeight + event.scrollTop >= event.scrollHeight
-            );
           },
           // 'smart': no-op if the row is already visible; centers it only when
           // it is more than one viewport away. This prevents undo of an in-view

newIDE/app/src/EventsSheet/EventsTree/style.css

@@ -10,6 +10,9 @@
  * Draggable handle on the left of an event
  */
 .gd-events-sheet .move-handle {
+    /* Also prevents the browser from running its own selection auto-scroll
+       while an event is dragged. */
+    user-select: none;
     width: 10px;
     margin: 1px 0.5px;
     flex-grow: 0;

newIDE/app/src/UI/DragAndDrop/DragAndDropContextProvider.js

@@ -58,6 +58,82 @@ const EndDragOnTouchCancel = ({
   return null;
 };
 
+/**
+ * Touch events are always dispatched to the element the finger first touched.
+ * If this element is removed from the document during a drag (a virtualized
+ * list unmounts the dragged row once auto scrolling moved it out of view),
+ * the events don't reach the drag backend anymore: the drag can't be updated
+ * or ended, and auto scrolling never stops. Keep this element in the document
+ * until the drag ends (the backend only handles the removal of the drag
+ * source itself, not of one of its ancestors).
+ */
+const KeepTouchedElementConnectedDuringDrag = ({
+  documentToWatch,
+}: {|
+  documentToWatch: Document,
+|}) => {
+  const dragDropManager = useDragDropManager();
+  React.useEffect(
+    () => {
+      let touchedElement: ?Element = null;
+      let observer: ?MutationObserver = null;
+      let hiddenContainer: ?HTMLElement = null;
+
+      const onTouchStart = (event: TouchEvent) => {
+        touchedElement = event.target instanceof Element ? event.target : null;
+      };
+      const onTouchEnd = () => {
+        touchedElement = null;
+      };
+      const stopWatching = () => {
+        if (observer) observer.disconnect();
+        observer = null;
+        if (hiddenContainer) hiddenContainer.remove();
+        hiddenContainer = null;
+      };
+      const reconnectTouchedElement = () => {
+        const element = touchedElement;
+        if (!element || element.isConnected) return;
+        let detachedRoot: Node = element;
+        while (detachedRoot.parentNode) detachedRoot = detachedRoot.parentNode;
+        if (!hiddenContainer) {
+          const container = documentToWatch.createElement('div');
+          container.style.display = 'none';
+          documentToWatch.body && documentToWatch.body.appendChild(container);
+          hiddenContainer = container;
+        }
+        hiddenContainer.appendChild(detachedRoot);
+      };
+
+      const monitor = dragDropManager.getMonitor();
+      const onStateChange = () => {
+        if (!monitor.isDragging()) {
+          stopWatching();
+          return;
+        }
+        // Not a touch drag, or already watching.
+        if (observer || !touchedElement || !touchedElement.isConnected) return;
+        observer = new MutationObserver(reconnectTouchedElement);
+        observer.observe(documentToWatch, { childList: true, subtree: true });
+      };
+
+      documentToWatch.addEventListener('touchstart', onTouchStart, true);
+      documentToWatch.addEventListener('touchend', onTouchEnd, true);
+      documentToWatch.addEventListener('touchcancel', onTouchEnd, true);
+      const unsubscribe = monitor.subscribeToStateChange(onStateChange);
+      return () => {
+        documentToWatch.removeEventListener('touchstart', onTouchStart, true);
+        documentToWatch.removeEventListener('touchend', onTouchEnd, true);
+        documentToWatch.removeEventListener('touchcancel', onTouchEnd, true);
+        unsubscribe();
+        stopWatching();
+      };
+    },
+    [dragDropManager, documentToWatch]
+  );
+  return null;
+};
+
 type Props = {|
   children: React.Node,
 
@@ -108,6 +184,9 @@ const DragAndDropContextProvider = ({
       <EndDragOnTouchCancel
         documentToWatch={window ? window.document : document}
       />
+      <KeepTouchedElementConnectedDuringDrag
+        documentToWatch={window ? window.document : document}
+      />
       {children}
     </DndProvider>
   );