Sorter improvements (#6542)

* Fix bugs with autoscroll and drop indicator

* Improve sorter performance

* fix hover index

packages/core/src/canvas/index.ts

@@ -617,6 +617,7 @@ export default class CanvasModule extends Module<CanvasConfig> {
       const el = this.getCanvasView().el;
       this.autoScroller.start(el, el, {
         zoom: this.em.getZoomDecimal(),
+        ignoredElement: this.getSpotsEl(),
       });
     }
   }

packages/core/src/canvas/view/FrameView.ts

@@ -229,6 +229,7 @@ export default class FrameView extends ModuleView<Frame, HTMLIFrameElement> {
     this.autoScroller.start(this.el, this.getWindow(), {
       lastMaxHeight: this.getWrapper().offsetHeight - this.el.offsetHeight,
       zoom: this.em.getZoomDecimal(),
+      ignoredElement: this.em.Canvas.getSpotsEl(),
     });
   }
 

packages/core/src/utils/AutoScroller.ts

@@ -15,6 +15,7 @@ export default class AutoScroller {
    * are relative to the iframe's document, not the main window's.
    */
   private rectIsInScrollIframe: boolean = false;
+  private ignoredElement?: HTMLElement; // If the mouse is over this element, don't autoscroll
 
   constructor(
     autoscrollLimit: number = 50,
@@ -31,11 +32,20 @@ export default class AutoScroller {
     bindAll(this, 'start', 'autoscroll', 'updateClientY', 'stop');
   }
 
-  start(eventEl: HTMLElement, scrollEl: HTMLElement | Window, opts?: { lastMaxHeight?: number; zoom?: number }) {
+  start(
+    eventEl: HTMLElement,
+    scrollEl: HTMLElement | Window,
+    opts?: {
+      lastMaxHeight?: number;
+      zoom?: number;
+      ignoredElement?: HTMLElement;
+    },
+  ) {
     this.eventEl = eventEl;
     this.scrollEl = scrollEl;
     this.lastMaxHeight = opts?.lastMaxHeight || Number.POSITIVE_INFINITY;
     this.zoom = opts?.zoom || 1;
+    this.ignoredElement = opts?.ignoredElement;
 
     // By detaching those from the stack avoid browsers lags
     // Noticeable with "fast" drag of blocks
@@ -47,24 +57,32 @@ export default class AutoScroller {
 
   private autoscroll() {
     const scrollEl = this.scrollEl;
-    if (this.dragging && scrollEl) {
-      const clientY = this.lastClientY ?? 0;
-      const limitTop = this.autoscrollLimit;
-      const eventElHeight = this.getEventElHeight();
-      const limitBottom = eventElHeight - limitTop;
-      let nextTop = 0;
-
-      if (clientY < limitTop) nextTop += clientY - limitTop;
-      if (clientY > limitBottom) nextTop += clientY - limitBottom;
-
-      const scrollTop = this.getElScrollTop(scrollEl);
-      if (this.lastClientY !== undefined && nextTop !== 0 && this.lastMaxHeight - nextTop > scrollTop) {
-        scrollEl.scrollBy({ top: nextTop, left: 0, behavior: 'auto' });
-        this.onScroll?.();
-      }
+    if (!this.dragging || !scrollEl) return;
+    if (this.lastClientY === undefined) {
+      setTimeout(() => {
+        requestAnimationFrame(this.autoscroll);
+      }, 50);
+      return;
+    }
 
-      requestAnimationFrame(this.autoscroll);
+    const clientY = this.lastClientY ?? 0;
+    const limitTop = this.autoscrollLimit;
+    const eventElHeight = this.getEventElHeight();
+    const limitBottom = eventElHeight - limitTop;
+    let scrollAmount = 0;
+
+    if (clientY < limitTop) scrollAmount += clientY - limitTop;
+    if (clientY > limitBottom) scrollAmount += clientY - limitBottom;
+
+    const scrollTop = this.getElScrollTop(scrollEl);
+    scrollAmount = Math.min(scrollAmount, this.lastMaxHeight - scrollTop);
+    scrollAmount = Math.max(scrollAmount, -scrollTop);
+    if (scrollAmount !== 0) {
+      scrollEl.scrollBy({ top: scrollAmount, behavior: 'auto' });
+      this.onScroll?.();
     }
+
+    requestAnimationFrame(this.autoscroll);
   }
 
   private getEventElHeight() {
@@ -76,6 +94,12 @@ export default class AutoScroller {
   }
 
   private updateClientY(ev: Event) {
+    const target = ev.target as HTMLElement;
+
+    if (this.ignoredElement && this.ignoredElement.contains(target)) {
+      return;
+    }
+
     const scrollEl = this.scrollEl;
     ev.preventDefault();
 
@@ -99,5 +123,7 @@ export default class AutoScroller {
 
   stop() {
     this.toggleAutoscrollFx(false);
+    this.lastClientY = undefined;
+    this.ignoredElement = undefined;
   }
 }

packages/core/src/utils/sorter/BaseComponentNode.ts

@@ -152,6 +152,10 @@ export abstract class BaseComponentNode extends SortableTreeNode<Component> {
     return this.model.em.Components.canMove(this.model, source.model, this.getRealIndex(index)).result;
   }
 
+  equals(node?: BaseComponentNode): node is BaseComponentNode {
+    return !!node?._model && this._model.getId() === node._model.getId();
+  }
+
   /**
    * Abstract method to get the view associated with this component.
    * Subclasses must implement this method.

packages/core/src/utils/sorter/CanvasNewComponentNode.ts

@@ -3,7 +3,6 @@ import CanvasComponentNode from './CanvasComponentNode';
 import { getSymbolMain, getSymbolTop, isSymbol, isSymbolMain } from '../../dom_components/model/SymbolUtils';
 import Component from '../../dom_components/model/Component';
 import { ContentElement, ContentType } from './types';
-import { isComponent } from '../mixins';
 
 type CanMoveSource = Component | ContentType;
 

packages/core/src/utils/sorter/DropLocationDeterminer.ts

@@ -36,6 +36,8 @@ type lastMoveData<NodeType> = {
   hoveredNode?: NodeType;
   /** The index where the placeholder or dragged element should be inserted. */
   index?: number;
+  /** The index under the mouse pointer during this move. */
+  hoveredIndex?: number;
   /** Placement relative to the target ('before' or 'after'). */
   placement?: Placement;
   /** The mouse event, used if we want to move placeholder with scrolling. */
@@ -113,19 +115,28 @@ export class DropLocationDeterminer<T, NodeType extends SortableTreeNode<T>> ext
 
   private handleMove(mouseEvent: MouseEvent): void {
     this.adjustForScroll();
-
     const { targetNode: lastTargetNode } = this.lastMoveData;
     this.eventHandlers.onMouseMove?.(mouseEvent);
     const { mouseXRelative: mouseX, mouseYRelative: mouseY } = this.getMousePositionRelativeToContainer(
       mouseEvent.clientX,
       mouseEvent.clientY,
     );
-    const targetNode = this.getTargetNode(mouseEvent);
+
+    const mouseTargetEl = this.getMouseTargetElement(mouseEvent);
+    const targetEl = this.getFirstElementWithAModel(mouseTargetEl);
+    const hoveredModel = targetEl ? $(targetEl)?.data('model') : undefined;
+    const hoveredNode = hoveredModel ? this.getOrCreateHoveredNode(hoveredModel) : undefined;
+    const hoveredIndex = hoveredNode
+      ? this.getIndexInParent(hoveredNode!, hoveredNode!.nodeDimensions!, mouseX, mouseY)
+      : 0;
+    const targetNode = hoveredNode ? this.getValidParent(hoveredNode, 0, mouseX, mouseY) : undefined;
+
     const targetChanged = !targetNode?.equals(lastTargetNode);
     if (targetChanged) {
       this.eventHandlers.onTargetChange?.(lastTargetNode, targetNode);
     }
-    if (!targetNode) {
+
+    if (!targetNode || !hoveredNode) {
       this.triggerLegacyOnMoveCallback(mouseEvent, 0);
       this.triggerMoveEvent(mouseX, mouseY);
       this.restLastMoveData();
@@ -144,10 +155,11 @@ export class DropLocationDeterminer<T, NodeType extends SortableTreeNode<T>> ext
     }
 
     this.lastMoveData = {
-      ...this.lastMoveData,
       targetNode,
+      hoveredNode,
       mouseEvent,
       index,
+      hoveredIndex,
       placement,
       placeholderDimensions,
     };
@@ -249,39 +261,6 @@ export class DropLocationDeterminer<T, NodeType extends SortableTreeNode<T>> ext
     };
   }
 
-  /**
-   * Retrieves the target node based on the mouse event.
-   * Determines the element being hovered, its corresponding model, and
-   * calculates the valid parent node to use as the target node.
-   *
-   * @param mouseEvent - The mouse event containing the cursor position and target element.
-   * @returns The target node if a valid one is found, otherwise undefined.
-   */
-  private getTargetNode(mouseEvent: MouseEvent): NodeType | undefined {
-    this.cacheContainerPosition(this.containerContext.container);
-    const { mouseXRelative, mouseYRelative } = this.getMousePositionRelativeToContainer(
-      mouseEvent.clientX,
-      mouseEvent.clientY,
-    );
-
-    // Get the element under the mouse
-    const mouseTargetEl = this.getMouseTargetElement(mouseEvent);
-    const targetEl = this.getFirstElementWithAModel(mouseTargetEl);
-    if (!targetEl) return;
-    const hoveredModel = $(targetEl)?.data('model');
-    if (!hoveredModel) return;
-
-    let hoveredNode = this.getOrCreateHoveredNode(hoveredModel);
-
-    // Get the drop position index based on the mouse position
-    const { index } = this.getDropPosition(hoveredNode, mouseXRelative, mouseYRelative);
-
-    // Determine the valid target node (or its valid parent)
-    let targetNode = this.getValidParent(hoveredNode, index, mouseXRelative, mouseYRelative);
-
-    return this.getOrReuseTargetNode(targetNode);
-  }
-
   /**
    * Creates a new hovered node or reuses the last hovered node if it is the same.
    *
@@ -291,8 +270,11 @@ export class DropLocationDeterminer<T, NodeType extends SortableTreeNode<T>> ext
   private getOrCreateHoveredNode(hoveredModel: T): NodeType {
     const lastHoveredNode = this.lastMoveData.hoveredNode;
     const hoveredNode = new this.treeClass(hoveredModel);
-    const newHoveredNode = hoveredNode.equals(lastHoveredNode) ? lastHoveredNode : hoveredNode;
-    this.lastMoveData.hoveredNode = newHoveredNode;
+    const sameHoveredNode = hoveredNode.equals(lastHoveredNode);
+    const newHoveredNode = sameHoveredNode ? lastHoveredNode : hoveredNode;
+    newHoveredNode.nodeDimensions = sameHoveredNode
+      ? lastHoveredNode!.nodeDimensions!
+      : this.getDim(hoveredNode.element!);
 
     return newHoveredNode;
   }
@@ -396,16 +378,23 @@ export class DropLocationDeterminer<T, NodeType extends SortableTreeNode<T>> ext
   private getValidParent(targetNode: NodeType, index: number, mouseX: number, mouseY: number): NodeType | undefined {
     if (!targetNode) return;
 
-    const lastTargetNode = this.lastMoveData.targetNode;
-    const targetNotChanged = targetNode.equals(lastTargetNode);
-    targetNode.nodeDimensions = targetNotChanged ? lastTargetNode.nodeDimensions! : this.getDim(targetNode.element!);
+    const {
+      targetNode: lastTargetNode,
+      hoveredNode: lastHoveredNode,
+      hoveredIndex: lastHoveredIndex,
+    } = this.lastMoveData;
+
+    const sameHoveredNode = targetNode.equals(lastHoveredNode);
+    targetNode.nodeDimensions = sameHoveredNode ? lastHoveredNode!.nodeDimensions! : this.getDim(targetNode.element!);
+    const hoverIndex = this.getIndexInParent(targetNode, targetNode.nodeDimensions!, mouseX, mouseY);
+    const sameHoveredIndex = hoverIndex === lastHoveredIndex;
+    const sameHoverPosition = sameHoveredNode && sameHoveredIndex;
+    if (sameHoverPosition && lastTargetNode) return lastTargetNode;
+
     if (!targetNode.isWithinDropBounds(mouseX, mouseY)) {
       return this.handleParentTraversal(targetNode, mouseX, mouseY);
     }
 
-    const positionNotChanged = targetNotChanged && index === this.lastMoveData.index;
-    if (positionNotChanged) return lastTargetNode;
-
     const canMove = this.sourceNodes.some((node) => targetNode.canMove(node, index));
     this.triggerDragValidation(canMove, targetNode);
     if (canMove) return targetNode;
@@ -417,17 +406,16 @@ export class DropLocationDeterminer<T, NodeType extends SortableTreeNode<T>> ext
     const parent = targetNode.getParent() as NodeType;
     if (!parent) return;
 
-    const indexInParent = this.getIndexInParent(parent, targetNode, targetNode.nodeDimensions!, mouseX, mouseY);
+    const indexInParent = this.getIndexInParent(targetNode, targetNode.nodeDimensions!, mouseX, mouseY);
+    if (indexInParent === undefined) return;
+
     return this.getValidParent(parent, indexInParent, mouseX, mouseY);
   }
 
-  private getIndexInParent(
-    parent: NodeType,
-    targetNode: NodeType,
-    nodeDimensions: Dimension,
-    mouseX: number,
-    mouseY: number,
-  ) {
+  private getIndexInParent(targetNode: NodeType, nodeDimensions: Dimension, mouseX: number, mouseY: number) {
+    const parent = targetNode.getParent() as NodeType;
+    if (!parent) return;
+
     let indexInParent = parent?.indexOfChild(targetNode);
     nodeDimensions.dir = this.getDirection(targetNode.element!, parent.element!);