From 55d560152e72c9eef281c3f5af9dff3f6cfc2258 Mon Sep 17 00:00:00 2001 From: Daniel Starns Date: Fri, 4 Oct 2024 10:38:20 -0700 Subject: [PATCH 1/2] fix: memory leak data sources (#6188) * fix: memory leak data sources * fix: not private method --- .../model/DataVariableListenerManager.ts | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/core/src/data_sources/model/DataVariableListenerManager.ts b/packages/core/src/data_sources/model/DataVariableListenerManager.ts index e3aa37256..5e0ee8c4a 100644 --- a/packages/core/src/data_sources/model/DataVariableListenerManager.ts +++ b/packages/core/src/data_sources/model/DataVariableListenerManager.ts @@ -29,14 +29,18 @@ export default class DataVariableListenerManager { this.listenToDataVariable(); } + private onChange = () => { + const value = this.dataVariable.getDataValue(); + this.updateValueFromDataVariable(value); + }; + listenToDataVariable() { - const { em, dataVariable, model, updateValueFromDataVariable } = this; + const { em, dataVariable, model } = this; const { path } = dataVariable.attributes; const normPath = stringToPath(path || '').join('.'); - const prevListeners = this.dataListeners || []; const [ds, dr] = this.em.DataSources.fromPath(path); - prevListeners.forEach((ls) => model.stopListening(ls.obj, ls.event, updateValueFromDataVariable)); + this.removeListeners(); const dataListeners: DataVariableListener[] = []; ds && dataListeners.push({ obj: ds.records, event: 'add remove reset' }); @@ -47,14 +51,14 @@ export default class DataVariableListenerManager { { obj: em, event: `${DataSourcesEvents.path}:${normPath}` }, ); - dataListeners.forEach((ls) => - model.listenTo(ls.obj, ls.event, () => { - const value = dataVariable.getDataValue(); - - updateValueFromDataVariable(value); - }), - ); + dataListeners.forEach((ls) => model.listenTo(ls.obj, ls.event, this.onChange)); this.dataListeners = dataListeners; } + + private removeListeners() { + const { model } = this; + this.dataListeners.forEach((ls) => model.stopListening(ls.obj, ls.event, this.onChange)); + this.dataListeners = []; + } } From 559a9b5556f6c2f9567dc811588e6259a531db6b Mon Sep 17 00:00:00 2001 From: mohamed yahia Date: Sat, 5 Oct 2024 08:02:06 +0300 Subject: [PATCH 2/2] Fix sorter performance (#6187) --- .../core/src/commands/view/SelectPosition.ts | 9 +- packages/core/src/utils/Droppable.ts | 7 - .../src/utils/sorter/BaseComponentNode.ts | 41 ++- .../utils/sorter/CanvasNewComponentNode.ts | 49 ++- .../core/src/utils/sorter/ComponentSorter.ts | 19 ++ packages/core/src/utils/sorter/Dimension.ts | 116 +++++++ .../utils/sorter/DropLocationDeterminer.ts | 318 +++++++++++------- .../core/src/utils/sorter/PlaceholderClass.ts | 31 +- packages/core/src/utils/sorter/RateLimiter.ts | 31 ++ .../core/src/utils/sorter/SortableTreeNode.ts | 14 +- packages/core/src/utils/sorter/Sorter.ts | 8 +- packages/core/src/utils/sorter/SorterUtils.ts | 17 +- packages/core/src/utils/sorter/types.ts | 13 +- 13 files changed, 500 insertions(+), 173 deletions(-) create mode 100644 packages/core/src/utils/sorter/Dimension.ts create mode 100644 packages/core/src/utils/sorter/RateLimiter.ts diff --git a/packages/core/src/commands/view/SelectPosition.ts b/packages/core/src/commands/view/SelectPosition.ts index 659df96b3..e308589e3 100644 --- a/packages/core/src/commands/view/SelectPosition.ts +++ b/packages/core/src/commands/view/SelectPosition.ts @@ -13,7 +13,7 @@ export default { const utils = this.em.Utils; const container = sourceElements[0].ownerDocument.body; - if (utils && !this.sorter) + if (utils) this.sorter = new utils.ComponentSorter({ em: this.em, treeClass: CanvasComponentNode, @@ -36,13 +36,6 @@ export default { }); if (opts.onStart) this.sorter.eventHandlers.legacyOnStartSort = opts.onStart; - this.em.on( - 'frame:scroll', - ((...agrs: any[]) => { - const canvasScroll = this.canvas.getCanvasView().frame === agrs[0].frame; - if (canvasScroll) this.sorter.recalculateTargetOnScroll(); - }).bind(this), - ); sourceElements && sourceElements.length > 0 && this.sorter.startSort(sourceElements.map((element) => ({ element }))); diff --git a/packages/core/src/utils/Droppable.ts b/packages/core/src/utils/Droppable.ts index 5a0fa10a0..94399aa02 100644 --- a/packages/core/src/utils/Droppable.ts +++ b/packages/core/src/utils/Droppable.ts @@ -187,13 +187,6 @@ export default class Droppable { sorter.eventHandlers.legacyOnEnd = sorterOptions.legacyOnEnd; sorter.containerContext.customTarget = sorterOptions.customTarget; } - this.em.on( - 'frame:scroll', - ((...agrs: any[]) => { - const canvasScroll = this.canvas.getCanvasView().frame === agrs[0].frame; - if (canvasScroll) sorter.recalculateTargetOnScroll(); - }).bind(this), - ); let dropModel = this.getTempDropModel(content); const el = dropModel.view?.el; const sources = el ? [{ element: el, dragSource: dragSourceOrigin }] : []; diff --git a/packages/core/src/utils/sorter/BaseComponentNode.ts b/packages/core/src/utils/sorter/BaseComponentNode.ts index 2247f09df..98bcb8779 100644 --- a/packages/core/src/utils/sorter/BaseComponentNode.ts +++ b/packages/core/src/utils/sorter/BaseComponentNode.ts @@ -8,6 +8,8 @@ import { SortableTreeNode } from './SortableTreeNode'; * Subclasses must implement the `view` and `element` methods. */ export abstract class BaseComponentNode extends SortableTreeNode { + private displayCache: Map = new Map(); + /** * Get the list of child components. * @returns {BaseComponentNode[] | null} - The list of children wrapped in @@ -19,20 +21,37 @@ export abstract class BaseComponentNode extends SortableTreeNode { /** * Get the list of displayed children, i.e., components that have a valid HTML element. + * Cached values are used to avoid recalculating the display status unnecessarily. * @returns {BaseComponentNode[] | null} - The list of displayed children wrapped in * BaseComponentNode, or null if there are no displayed children. */ private getDisplayedChildren(): BaseComponentNode[] | null { const children = this.model.components(); - const displayedChildren = children.filter((child) => { - const element = child.getEl(); - - return isDisplayed(element); - }); + const displayedChildren = children.filter((child) => this.isChildDisplayed(child)); return displayedChildren.map((comp: Component) => new (this.constructor as any)(comp)); } + /** + * Check if a child is displayed, using cached value if available. + * @param {Component} child - The child component to check. + * @returns {boolean} - Whether the child is displayed. + */ + private isChildDisplayed(child: Component): boolean { + // Check if display status is cached + if (this.displayCache.has(child)) { + return this.displayCache.get(child)!; + } + + const element = child.getEl(); + const displayed = isDisplayed(element); + + // Cache the display status + this.displayCache.set(child, displayed); + + return displayed; + } + /** * Get the parent component of this node. * @returns {BaseComponentNode | null} - The parent wrapped in BaseComponentNode, @@ -114,8 +133,7 @@ export abstract class BaseComponentNode extends SortableTreeNode { for (let i = 0; i < children.length; i++) { const child = children.at(i); - const element = child.getEl(); - const displayed = isDisplayed(element); + const displayed = this.isChildDisplayed(child); if (displayed) displayedCount++; if (displayedCount === index + 1) return i; @@ -208,8 +226,13 @@ export abstract class BaseComponentNode extends SortableTreeNode { } } -function isDisplayed(element: HTMLElement | undefined) { - if (!!!element) return false; +/** + * Function to check if an element is displayed in the DOM. + * @param {HTMLElement | undefined} element - The element to check. + * @returns {boolean} - Whether the element is displayed. + */ +function isDisplayed(element: HTMLElement | undefined): boolean { + if (!element) return false; return ( element instanceof HTMLElement && window.getComputedStyle(element).display !== 'none' && diff --git a/packages/core/src/utils/sorter/CanvasNewComponentNode.ts b/packages/core/src/utils/sorter/CanvasNewComponentNode.ts index 8447001a8..045538c97 100644 --- a/packages/core/src/utils/sorter/CanvasNewComponentNode.ts +++ b/packages/core/src/utils/sorter/CanvasNewComponentNode.ts @@ -3,23 +3,61 @@ 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; export default class CanvasNewComponentNode extends CanvasComponentNode { + /** + * A cache of shallow editor models stored in the source node. + * This is a map where each content item is associated with its corresponding shallow wrapper model. + */ + private _cachedShallowModels: Map = new Map(); + + /** + * Ensures the shallow editor model for the given contentItem is cached in the source node. + * If not cached, retrieves it from the shallow wrapper and stores it in the cache. + * @param contentItem - The content item to retrieve or cache. + * @returns {Component | null} - The shallow wrapper model, either cached or retrieved. + */ + cacheSrcModelForContent(contentItem: ContentElement | Component): Component | undefined { + if (isComponent(contentItem)) { + return contentItem; + } + + if (this._cachedShallowModels.has(contentItem)) { + return this._cachedShallowModels.get(contentItem)!; + } + + const wrapper = this.model.em.Components.getShallowWrapper(); + const srcModel = wrapper?.append(contentItem)[0]; + // Replace getEl as the element would be removed in the shallow wrapper after 100ms + const el = srcModel?.getEl(); + srcModel!.getEl = () => el; + + if (srcModel) { + this._cachedShallowModels.set(contentItem, srcModel); + } + + return srcModel; + } + canMove(source: CanvasNewComponentNode, index: number): boolean { const realIndex = this.getRealIndex(index); const { model: symbolModel, content, dragDef } = source._dragSource; const canMoveSymbol = !symbolModel || !this.isSourceSameSymbol(symbolModel); const sourceContent: CanMoveSource = (isFunction(content) ? dragDef : content) || source.model; + if (Array.isArray(sourceContent)) { return ( - sourceContent.every((contentItem, i) => this.canMoveSingleContent(contentItem, realIndex + i)) && canMoveSymbol + sourceContent.every((contentItem, i) => + this.canMoveSingleContent(source.cacheSrcModelForContent(contentItem)!, realIndex + i), + ) && canMoveSymbol ); } - return this.canMoveSingleContent(sourceContent, realIndex) && canMoveSymbol; + return this.canMoveSingleContent(source.cacheSrcModelForContent(sourceContent)!, realIndex) && canMoveSymbol; } private canMoveSingleContent(contentItem: ContentElement | Component, index: number): boolean { @@ -39,6 +77,13 @@ export default class CanvasNewComponentNode extends CanvasComponentNode { return this.addSingleChild(content, index, insertingTextableIntoText); } + /** + * Adds a single content item to the current node. + * @param {ContentType} content - The content to add. + * @param {number} index - The index where the content is to be added. + * @param {boolean} insertingTextableIntoText - Whether the operation involves textable content. + * @returns {CanvasNewComponentNode} - The newly added node. + */ private addSingleChild( content: ContentType, index: number, diff --git a/packages/core/src/utils/sorter/ComponentSorter.ts b/packages/core/src/utils/sorter/ComponentSorter.ts index dba5e9316..b71e9313c 100644 --- a/packages/core/src/utils/sorter/ComponentSorter.ts +++ b/packages/core/src/utils/sorter/ComponentSorter.ts @@ -1,3 +1,4 @@ +import { bindAll } from 'underscore'; import { CanvasSpotBuiltInTypes } from '../../canvas/model/CanvasSpot'; import Component from '../../dom_components/model/Component'; import EditorModel from '../../editor/model/Editor'; @@ -62,6 +63,8 @@ export default class ComponentSorter extends }, }, }); + + bindAll(this, 'handleScrollEvent'); } private onStartSort() { @@ -69,6 +72,22 @@ export default class ComponentSorter extends this.setAutoCanvasScroll(true); } + protected bindDragEventHandlers() { + this.em.on('frame:scroll', this.handleScrollEvent); + super.bindDragEventHandlers(); + } + + protected cleanupEventListeners(): void { + this.em.off('frame:scroll', this.handleScrollEvent); + super.cleanupEventListeners(); + } + + handleScrollEvent(...agrs: any[]) { + const frame = agrs?.[0]?.frame; + const canvasScroll = this.em.Canvas.getCanvasView().frame === frame; + if (canvasScroll) this.recalculateTargetOnScroll(); + } + private onMouseMove = (mouseEvent: MouseEvent) => { const insertingTextableIntoText = this.targetIsText && this.sourceNodes?.some((node) => node.isTextable()); if (insertingTextableIntoText) { diff --git a/packages/core/src/utils/sorter/Dimension.ts b/packages/core/src/utils/sorter/Dimension.ts new file mode 100644 index 000000000..0fbfcf9d3 --- /dev/null +++ b/packages/core/src/utils/sorter/Dimension.ts @@ -0,0 +1,116 @@ +import CanvasModule from '../../canvas'; +import { Placement } from './types'; + +/** + * A class representing dimensions of an element, including position, size, offsets, and other metadata. + * Provides functionality to calculate differences between current and previous dimensions and update them. + */ +export default class Dimension { + public top: number; + public left: number; + public height: number; + public width: number; + public offsets: ReturnType; + public dir?: boolean; + + /** + * Initializes the DimensionCalculator with the given initial dimensions. + * + * @param initialDimensions - The initial dimensions containing `top`, `left`, `height`, `width`, and other properties. + */ + constructor(initialDimensions: { + top: number; + left: number; + height: number; + width: number; + offsets: ReturnType; + dir?: boolean; + el?: HTMLElement; + indexEl?: number; + }) { + this.top = initialDimensions.top; + this.left = initialDimensions.left; + this.height = initialDimensions.height; + this.width = initialDimensions.width; + this.offsets = initialDimensions.offsets; + this.dir = initialDimensions.dir; + } + + /** + * Calculates the difference between the current and previous dimensions. + * If there are no previous dimensions, it will return zero differences. + * + * @returns An object containing the differences in `top` and `left` positions. + */ + public calculateDimensionDifference(dimension: Dimension): { topDifference: number; leftDifference: number } { + const topDifference = dimension.top - this.top; + const leftDifference = dimension.left - this.left; + + return { topDifference, leftDifference }; + } + + /** + * Updates the current dimensions by adding the given differences to the `top` and `left` values. + * + * @param topDifference - The difference to add to the current `top` value. + * @param leftDifference - The difference to add to the current `left` value. + */ + public adjustDimensions(difference: { topDifference: number; leftDifference: number }): Dimension { + this.top += difference.topDifference; + this.left += difference.leftDifference; + + return this; + } + + /** + * Determines the placement ('before' or 'after') based on the X and Y coordinates and center points. + * + * @param {number} mouseX X coordinate of the mouse + * @param {number} mouseY Y coordinate of the mouse + * @return {Placement} 'before' or 'after' + */ + public determinePlacement(mouseX: number, mouseY: number): Placement { + const xCenter = this.left + this.width / 2; + const yCenter = this.top + this.height / 2; + + if (this.dir) { + return mouseY < yCenter ? 'before' : 'after'; + } else { + return mouseX < xCenter ? 'before' : 'after'; + } + } + + /** + * Compares the current dimension object with another dimension to check equality. + * + * @param {Dimension} dimension - The dimension to compare against. + * @returns {boolean} True if the dimensions are equal, otherwise false. + */ + public equals(dimension: Dimension | undefined): boolean { + if (!dimension) return false; + return ( + this.top === dimension.top && + this.left === dimension.left && + this.height === dimension.height && + this.width === dimension.width && + this.dir === dimension.dir && + JSON.stringify(this.offsets) === JSON.stringify(dimension.offsets) + ); + } + + /** + * Creates a clone of the current Dimension object. + * + * @returns {Dimension} A new Dimension object with the same properties. + */ + public clone(): Dimension { + return new Dimension({ + top: this.top, + left: this.left, + height: this.height, + width: this.width, + offsets: { ...this.offsets }, // Shallow copy of offsets + dir: this.dir, + }); + } +} diff --git a/packages/core/src/utils/sorter/DropLocationDeterminer.ts b/packages/core/src/utils/sorter/DropLocationDeterminer.ts index fef8fb80c..c8cccc0ee 100644 --- a/packages/core/src/utils/sorter/DropLocationDeterminer.ts +++ b/packages/core/src/utils/sorter/DropLocationDeterminer.ts @@ -3,17 +3,11 @@ import { $, View } from '../../common'; import EditorModel from '../../editor/model/Editor'; import { isTextNode, off, on } from '../dom'; import { SortableTreeNode } from './SortableTreeNode'; -import { - Dimension, - Placement, - PositionOptions, - DragDirection, - SorterEventHandlers, - CustomTarget, - DragSource, -} from './types'; +import { Placement, PositionOptions, DragDirection, SorterEventHandlers, CustomTarget, DragSource } from './types'; import { bindAll, each } from 'underscore'; -import { matches, findPosition, offset, isInFlow } from './SorterUtils'; +import { matches, findPosition, offset, isStyleInFlow } from './SorterUtils'; +import { RateLimiter } from './RateLimiter'; +import Dimension from './Dimension'; type ContainerContext = { container: HTMLElement; @@ -35,19 +29,19 @@ interface DropLocationDeterminerOptions> * Represents the data related to the last move event during drag-and-drop sorting. * This type is discriminated by the presence or absence of a valid target node. */ -type LastMoveData = { +type lastMoveData = { /** The target node under the mouse pointer during the last move. */ - lastTargetNode?: NodeType; + targetNode?: NodeType; + /** The node under the mouse pointer during this move*/ + hoveredNode?: NodeType; /** The index where the placeholder or dragged element should be inserted. */ - lastIndex?: number; + index?: number; /** Placement relative to the target ('before' or 'after'). */ - lastPlacement?: Placement; - /** The dimensions of the target node. */ - lastTargetDimensions?: Dimension; - /** The dimensions of the child elements within the target node. */ - lastChildrenDimensions?: Dimension[]; + placement?: Placement; /** The mouse event, used if we want to move placeholder with scrolling. */ - lastMouseEvent?: MouseEvent; + mouseEvent?: MouseEvent; + + placeholderDimensions?: Dimension; }; export class DropLocationDeterminer> extends View { @@ -60,11 +54,13 @@ export class DropLocationDeterminer> ext eventHandlers: SorterEventHandlers; sourceNodes: NodeType[] = []; - lastMoveData!: LastMoveData; + lastMoveData!: lastMoveData; containerOffset = { top: 0, left: 0, }; + private moveThreshold: number = 20; + private rateLimiter: RateLimiter; // Rate limiter for onMove constructor(options: DropLocationDeterminerOptions) { super(); @@ -77,6 +73,7 @@ export class DropLocationDeterminer> ext bindAll(this, 'endDrag', 'cancelDrag', 'recalculateTargetOnScroll', 'startSort', 'onDragStart', 'onMove'); this.restLastMoveData(); + this.rateLimiter = new RateLimiter(this.moveThreshold); } /** @@ -101,24 +98,31 @@ export class DropLocationDeterminer> ext * to determine the new target. */ recalculateTargetOnScroll(): void { - const { lastTargetNode, lastMouseEvent } = this.lastMoveData; - - // recalculate dimensions when the canvas is scrolled - this.restLastMoveData(); - this.lastMoveData.lastTargetNode = lastTargetNode; + const { mouseEvent: lastMouseEvent } = this.lastMoveData; if (!lastMouseEvent) { return; } this.onMove(lastMouseEvent); - this.lastMoveData.lastMouseEvent = lastMouseEvent; } private onMove(mouseEvent: MouseEvent): void { + this.rateLimiter.updateArgs(mouseEvent); + this.rateLimiter.execute(this.handleMove.bind(this)); + } + + private handleMove(mouseEvent: MouseEvent): void { + this.adjustForScroll(); + + const { targetNode: lastTargetNode } = this.lastMoveData; this.eventHandlers.onMouseMove?.(mouseEvent); const { mouseXRelativeToContainer: mouseX, mouseYRelativeToContainer: mouseY } = this.getMousePositionRelativeToContainer(mouseEvent); const targetNode = this.getTargetNode(mouseEvent); + const targetChanged = !targetNode?.equals(lastTargetNode); + if (targetChanged) { + this.eventHandlers.onTargetChange?.(lastTargetNode, targetNode); + } if (!targetNode) { this.triggerLegacyOnMoveCallback(mouseEvent, 0); this.triggerMoveEvent(mouseX, mouseY); @@ -127,49 +131,77 @@ export class DropLocationDeterminer> ext } // Handle movement over the valid target node - const index = this.handleMovementOnTarget(targetNode, mouseX, mouseY); + const { index, placement, placeholderDimensions } = this.getDropPosition(targetNode, mouseX, mouseY); + + const placeHolderMoved = + !placeholderDimensions.equals(this.lastMoveData.placeholderDimensions) || + placement !== this.lastMoveData.placement; + if (placeHolderMoved) { + this.eventHandlers.onPlaceholderPositionChange?.(placeholderDimensions!, placement!); + } + + this.lastMoveData = { + ...this.lastMoveData, + targetNode, + mouseEvent, + index, + placement, + placeholderDimensions, + }; this.triggerMoveEvent(mouseX, mouseY); this.triggerLegacyOnMoveCallback(mouseEvent, index); - this.lastMoveData.lastMouseEvent = mouseEvent; + } + + private adjustForScroll() { + const lastTargetNode = this.lastMoveData.targetNode; + if (lastTargetNode?.element) { + const dims = this.getDim(lastTargetNode?.element); + const diff = lastTargetNode.nodeDimensions?.calculateDimensionDifference(dims); + if (diff) { + lastTargetNode.adjustDimensions(diff); + } + } + + const lastHoveredNode = this.lastMoveData.hoveredNode; + if (lastHoveredNode?.element) { + const dims = this.getDim(lastHoveredNode?.element); + const diff = lastHoveredNode.nodeDimensions?.calculateDimensionDifference(dims); + if (diff) { + lastHoveredNode.adjustDimensions(diff); + } + } } private restLastMoveData() { this.lastMoveData = { - lastTargetNode: undefined, - lastIndex: undefined, - lastPlacement: undefined, - lastTargetDimensions: undefined, - lastChildrenDimensions: undefined, - lastMouseEvent: undefined, + targetNode: undefined, + index: undefined, + placement: undefined, + mouseEvent: undefined, }; } - private triggerLegacyOnMoveCallback(mouseEvent: MouseEvent, index: number) { + private triggerLegacyOnMoveCallback(mouseEvent: MouseEvent, index?: number) { // For backward compatibility, leave it to a single node const model = this.sourceNodes[0]?.model; this.eventHandlers.legacyOnMoveClb?.({ event: mouseEvent, target: model, - parent: this.lastMoveData.lastTargetNode?.model, + parent: this.lastMoveData.targetNode?.model, index: index, }); } private triggerMoveEvent(mouseX: number, mouseY: number) { - const { - lastTargetNode: targetNode, - lastPlacement: placement, - lastIndex: index, - lastChildrenDimensions: childrenDimensions, - } = this.lastMoveData; + const { targetNode: targetNode, placement: placement, index: index } = this.lastMoveData; const legacyIndex = index ? index + (placement === 'after' ? -1 : 0) : 0; this.em.trigger('sorter:drag', { target: targetNode?.element || null, - targetModel: this.lastMoveData.lastTargetNode?.model, + targetModel: this.lastMoveData.targetNode?.model, sourceModel: this.sourceNodes[0].model, - dims: childrenDimensions || [], + dims: targetNode?.childrenDimensions || [], pos: { index: legacyIndex, indexEl: legacyIndex, @@ -184,64 +216,100 @@ export class DropLocationDeterminer> ext * Handles the movement of the dragged element over a target node. * Updates the placeholder position and triggers relevant events when necessary. * - * @param hoveredNode - The node currently being hovered over. + * @param node - The node currently being hovered over. * @param mouseX - The x-coordinate of the mouse relative to the container. * @param mouseY - The y-coordinate of the mouse relative to the container. - * @returns The index at which the placeholder should be positioned. */ - private handleMovementOnTarget(hoveredNode: NodeType, mouseX: number, mouseY: number): number { - const { lastTargetNode, lastChildrenDimensions } = this.lastMoveData; - - const targetChanged = !hoveredNode.equals(lastTargetNode); - if (targetChanged) { - this.eventHandlers.onTargetChange?.(lastTargetNode, hoveredNode); - } - - let placeholderDimensions, index, placement: Placement; - const children = hoveredNode.getChildren(); + private getDropPosition(node: NodeType, mouseX: number, mouseY: number) { + let { nodeDimensions, childrenDimensions } = node; + const children = node.getChildren(); const nodeHasChildren = children && children.length > 0; - const hoveredNodeDimensions = this.getDim(hoveredNode.element!); - const childrenDimensions = - targetChanged || !!!lastChildrenDimensions ? this.getChildrenDim(hoveredNode) : lastChildrenDimensions; + nodeDimensions = !nodeDimensions ? this.getDim(node.element!) : nodeDimensions; + node.nodeDimensions = nodeDimensions; + + childrenDimensions = !childrenDimensions ? this.getChildrenDim(node) : childrenDimensions; + node.childrenDimensions = childrenDimensions; + let placeholderDimensions = nodeDimensions.clone(), + index = 0, + placement = 'inside' as Placement; if (nodeHasChildren) { ({ index, placement } = findPosition(childrenDimensions, mouseX, mouseY)); - placeholderDimensions = childrenDimensions[index]; - } else { - placeholderDimensions = hoveredNodeDimensions; - index = 0; - placement = 'inside'; + placeholderDimensions = childrenDimensions[index].clone(); + index = index + (placement == 'after' ? 1 : 0); } - index = index + (placement == 'after' ? 1 : 0); - this.eventHandlers.onPlaceholderPositionChange?.(placeholderDimensions, placement); - - this.lastMoveData = { - lastTargetNode: hoveredNode, - lastTargetDimensions: hoveredNodeDimensions, - lastChildrenDimensions: childrenDimensions, - lastIndex: index, - lastPlacement: placement, + return { + index, + placement, + placeholderDimensions, }; - - return index; } - private getTargetNode(mouseEvent: MouseEvent) { - const customTarget = this.containerContext.customTarget; + /** + * 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); + // 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, mouseEvent.clientX, mouseEvent.clientY); + + // Determine the valid target node (or its valid parent) + let targetNode = this.getValidParent(hoveredNode, index, mouseEvent.clientX, mouseEvent.clientY); + + return this.getOrReuseTargetNode(targetNode); + } + + /** + * Creates a new hovered node or reuses the last hovered node if it is the same. + * + * @param hoveredModel - The model corresponding to the hovered element. + * @returns The new or reused hovered node. + */ + 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; + + return newHoveredNode; + } + + /** + * Checks if the target node has changed and returns the last one if they are identical. + * + * @param targetNode - The newly calculated target node. + * @returns The new or reused target node. + */ + private getOrReuseTargetNode(targetNode?: NodeType): NodeType | undefined { + const lastTargetNode = this.lastMoveData.targetNode; + return targetNode?.equals(lastTargetNode) ? lastTargetNode : targetNode; + } + + private getMouseTargetElement(mouseEvent: MouseEvent) { + const customTarget = this.containerContext.customTarget; let mouseTarget = this.containerContext.document.elementFromPoint( mouseEvent.clientX, mouseEvent.clientY, ) as HTMLElement; let mouseTargetEl: HTMLElement | null = customTarget ? customTarget({ event: mouseEvent }) : mouseTarget; - const targetEl = this.getFirstElementWithAModel(mouseTargetEl); - if (!targetEl) return; - const targetModel = $(targetEl)?.data('model'); - const mouseTargetNode = new this.treeClass(targetModel); - const targetNode = this.getValidParentNode(mouseTargetNode); - return targetNode; + + return mouseTargetEl; } private onDragStart(mouseEvent: MouseEvent): void { @@ -253,7 +321,7 @@ export class DropLocationDeterminer> ext } cancelDrag() { - const { lastTargetNode } = this.lastMoveData; + const { targetNode: lastTargetNode } = this.lastMoveData; this.eventHandlers.onTargetChange?.(lastTargetNode, undefined); this.finalizeMove(); } @@ -264,16 +332,17 @@ export class DropLocationDeterminer> ext this.eventHandlers.onEnd?.(); this.eventHandlers.legacyOnEnd?.(); this.restLastMoveData(); + this.rateLimiter.clearTimeout(); } private dropDragged() { - const { lastTargetNode, lastIndex } = this.lastMoveData; + const { targetNode: lastTargetNode, index: lastIndex } = this.lastMoveData; this.eventHandlers.onDrop?.(lastTargetNode, this.sourceNodes, lastIndex); this.finalizeMove(); } private triggerOnDragEndEvent() { - const { lastTargetNode: targetNode } = this.lastMoveData; + const { targetNode: targetNode } = this.lastMoveData; // For backward compatibility, leave it to a single node const firstSourceNode = this.sourceNodes[0]; @@ -317,25 +386,36 @@ export class DropLocationDeterminer> ext return null; } - private getValidParentNode(targetNode: NodeType) { - let finalNode = targetNode; - while (finalNode !== null) { - const canMove = this.sourceNodes.some((node) => finalNode.canMove(node, 0)); - - // For backward compatibility, leave it to a single node - const firstSource = this.sourceNodes[0]; - this.em.trigger('sorter:drag:validation', { - valid: canMove, - src: firstSource?.element, - srcModel: firstSource?.model, - trg: finalNode.element, - trgModel: finalNode.model, - }); - if (canMove) break; - finalNode = finalNode.getParent()! as NodeType; - } + private getValidParent(targetNode: NodeType, index: number, mouseX: number, mouseY: number): NodeType | undefined { + if (!targetNode) return; + const positionNotChanged = targetNode.equals(this.lastMoveData.targetNode) && index === this.lastMoveData.index; + if (positionNotChanged) return targetNode; + + const canMove = this.sourceNodes.some((node) => targetNode.canMove(node, index)); + this.triggerDragValidation(canMove, targetNode); + if (canMove) return targetNode; - return finalNode; + const parent = targetNode.getParent() as NodeType; + if (!parent) return; + + let indexInParent = parent?.indexOfChild(targetNode); + const nodeDimensions = this.getDim(targetNode.element!); + nodeDimensions.dir = this.getDirection(targetNode.element!, parent.element!); + indexInParent = indexInParent + (nodeDimensions.determinePlacement(mouseX, mouseY) == 'after' ? 1 : 0); + return this.getValidParent(parent, indexInParent, mouseX, mouseY); + } + + private triggerDragValidation(canMove: boolean, targetNode: NodeType) { + const firstSource = this.sourceNodes[0]; + this.em.trigger('sorter:drag:validation', { + valid: canMove, + src: firstSource?.element, + srcModel: firstSource?.model, + trg: targetNode.element, + trgModel: targetNode.model, + }); + + return firstSource; } /** @@ -352,6 +432,25 @@ export class DropLocationDeterminer> ext off(this.containerContext.document, 'mouseup dragend touchend', this.endDrag); } + /** + * Determines if an element is in the normal flow of the document. + * This checks whether the element is not floated or positioned in a way that removes it from the flow. + * + * @param {HTMLElement} el - The element to check. + * @param {HTMLElement} [parent=document.body] - The parent element for additional checks (defaults to `document.body`). + * @return {boolean} Returns `true` if the element is in flow, otherwise `false`. + * @private + */ + private getDirection(el: HTMLElement, parent: HTMLElement): boolean { + let dirValue: boolean; + + if (this.dragDirection === DragDirection.Vertical) dirValue = true; + else if (this.dragDirection === DragDirection.Horizontal) dirValue = false; + else dirValue = isStyleInFlow(el, parent); + + return dirValue; + } + /** * Get children dimensions * @param {NodeType} el Element root @@ -378,14 +477,7 @@ export class DropLocationDeterminer> ext } const dim = this.getDim(el); - let dir = this.dragDirection; - let dirValue: boolean; - - if (dir === DragDirection.Vertical) dirValue = true; - else if (dir === DragDirection.Horizontal) dirValue = false; - else dirValue = isInFlow(el, targetElement); - - dim.dir = dirValue; + dim.dir = this.getDirection(el, targetElement); dims.push(dim); }); @@ -463,6 +555,6 @@ export class DropLocationDeterminer> ext width = el.offsetWidth; } - return { top, left, height, width, offsets }; + return new Dimension({ top, left, height, width, offsets }); } } diff --git a/packages/core/src/utils/sorter/PlaceholderClass.ts b/packages/core/src/utils/sorter/PlaceholderClass.ts index f501ca541..b87f38ce6 100644 --- a/packages/core/src/utils/sorter/PlaceholderClass.ts +++ b/packages/core/src/utils/sorter/PlaceholderClass.ts @@ -1,5 +1,12 @@ import { View } from '../../common'; -import { Dimension, Placement } from './types'; +import { Placement } from './types'; +import Dimension from './Dimension'; +import { RateLimiter } from './RateLimiter'; + +type PlaceHolderPosition = { + elementDimension: Dimension; + placement: Placement; +}; export class PlaceholderClass extends View { pfx: string; @@ -10,6 +17,8 @@ export class PlaceholderClass extends View { top: number; left: number; }; + private moveLimiter: RateLimiter; + constructor(options: { container: HTMLElement; pfx?: string; @@ -29,6 +38,9 @@ export class PlaceholderClass extends View { top: options.offset.top || 0, left: options.offset.left || 0, }; + + // Initialize the RateLimiter with the moveThreshold + this.moveLimiter = new RateLimiter(100); } show() { @@ -36,15 +48,28 @@ export class PlaceholderClass extends View { } hide() { + this.moveLimiter.clearTimeout(); this.el.style.display = 'none'; } /** - * Updates the position of the placeholder. + * Updates the position of the placeholder with a movement threshold. * @param {Dimension} elementDimension element dimensions. - * @param {Position} placement either before or after the target. + * @param {Placement} placement either before or after the target. */ move(elementDimension: Dimension, placement: Placement) { + const position: PlaceHolderPosition = { elementDimension, placement }; + + // Update the position arguments in the RateLimiter + this.moveLimiter.updateArgs(position); + + // Execute the callback with a threshold + this.moveLimiter.execute(({ elementDimension, placement }) => { + this._move(elementDimension, placement); + }); + } + + private _move(elementDimension: Dimension, placement: Placement) { const marginOffset = 0; const unit = 'px'; let top = 0; diff --git a/packages/core/src/utils/sorter/RateLimiter.ts b/packages/core/src/utils/sorter/RateLimiter.ts new file mode 100644 index 000000000..d03edb1d4 --- /dev/null +++ b/packages/core/src/utils/sorter/RateLimiter.ts @@ -0,0 +1,31 @@ +export class RateLimiter { + private threshold: number; + private lastArgs: T | undefined; + private timeout: NodeJS.Timeout | null = null; + + constructor(threshold: number) { + this.threshold = threshold; + } + + updateArgs(args: T) { + this.lastArgs = args; + } + + execute(callback: (args: T) => void) { + if (!this.timeout) { + this.timeout = setTimeout(() => { + if (this.lastArgs) { + callback(this.lastArgs); + } + this.timeout = null; + }, this.threshold); + } + } + + clearTimeout() { + if (this.timeout) { + clearTimeout(this.timeout); + this.timeout = null; + } + } +} diff --git a/packages/core/src/utils/sorter/SortableTreeNode.ts b/packages/core/src/utils/sorter/SortableTreeNode.ts index 5bcdc12e5..ee18b619b 100644 --- a/packages/core/src/utils/sorter/SortableTreeNode.ts +++ b/packages/core/src/utils/sorter/SortableTreeNode.ts @@ -1,4 +1,5 @@ import { View } from '../../common'; +import Dimension from './Dimension'; import { DragSource } from './types'; /** @@ -9,6 +10,10 @@ import { DragSource } from './types'; export abstract class SortableTreeNode { protected _model: T; protected _dragSource: DragSource; + /** The dimensions of the node. */ + public nodeDimensions?: Dimension; + /** The dimensions of the child elements within the target node. */ + public childrenDimensions?: Dimension[]; constructor(model: T, dragSource: DragSource = {}) { this._model = model; this._dragSource = dragSource; @@ -87,7 +92,14 @@ export abstract class SortableTreeNode { return this._dragSource; } - equals(node?: SortableTreeNode): boolean { + equals(node?: SortableTreeNode): node is SortableTreeNode { return !!node?._model && this._model === node._model; } + + adjustDimensions(diff: { topDifference: number; leftDifference: number }) { + if (diff.topDifference === 0 && diff.leftDifference === 0) return; + + this.nodeDimensions?.adjustDimensions(diff); + this.childrenDimensions?.forEach((dims) => dims.adjustDimensions(diff)); + } } diff --git a/packages/core/src/utils/sorter/Sorter.ts b/packages/core/src/utils/sorter/Sorter.ts index 7c852f9b5..747c7d02f 100644 --- a/packages/core/src/utils/sorter/Sorter.ts +++ b/packages/core/src/utils/sorter/Sorter.ts @@ -12,9 +12,9 @@ import { PositionOptions, SorterDragBehaviorOptions, SorterEventHandlers, - Dimension, Placement, } from './types'; +import Dimension from './Dimension'; import { SorterOptions } from './types'; export default class Sorter> { @@ -107,7 +107,7 @@ export default class Sorter> { /** * This method is should be called when the user scrolls within the container. */ - recalculateTargetOnScroll(): void { + protected recalculateTargetOnScroll(): void { this.dropLocationDeterminer.recalculateTargetOnScroll(); } @@ -183,7 +183,7 @@ export default class Sorter> { return sourceElement; } - private bindDragEventHandlers() { + protected bindDragEventHandlers() { on(this.containerContext.document, 'keydown', this.rollback); } @@ -196,7 +196,7 @@ export default class Sorter> { * * @private */ - private cleanupEventListeners(): void { + protected cleanupEventListeners(): void { off(this.containerContext.document, 'keydown', this.rollback); } diff --git a/packages/core/src/utils/sorter/SorterUtils.ts b/packages/core/src/utils/sorter/SorterUtils.ts index 3c588a78a..4f1029683 100644 --- a/packages/core/src/utils/sorter/SorterUtils.ts +++ b/packages/core/src/utils/sorter/SorterUtils.ts @@ -3,7 +3,8 @@ import EditorModel from '../../editor/model/Editor'; import { isTextNode } from '../dom'; import { matches as matchesMixin } from '../mixins'; import { SortableTreeNode } from './SortableTreeNode'; -import { Dimension, Placement, DragDirection, SorterOptions } from './types'; +import { Placement, DragDirection, SorterOptions } from './types'; +import Dimension from './Dimension'; /** * Find the position based on passed dimensions and coordinates @@ -133,18 +134,6 @@ export function closest(el: HTMLElement, selector: string): HTMLElement | undefi elem = elem.parentNode; } } -/** - * Determines if an element is in the normal flow of the document. - * This checks whether the element is not floated or positioned in a way that removes it from the flow. - * - * @param {HTMLElement} el - The element to check. - * @param {HTMLElement} [parent=document.body] - The parent element for additional checks (defaults to `document.body`). - * @return {boolean} Returns `true` if the element is in flow, otherwise `false`. - * @private - */ -export function isInFlow(el: HTMLElement, parent: HTMLElement = document.body): boolean { - return !!el && isStyleInFlow(el, parent); -} /** * Checks if an element has styles that keep it in the document flow. @@ -155,7 +144,7 @@ export function isInFlow(el: HTMLElement, parent: HTMLElement = document.body): * @return {boolean} Returns `true` if the element is styled to be in flow, otherwise `false`. * @private */ -function isStyleInFlow(el: HTMLElement, parent: HTMLElement): boolean { +export function isStyleInFlow(el: HTMLElement, parent: HTMLElement): boolean { if (isTextNode(el)) return false; const elementStyles = el.style || {}; diff --git a/packages/core/src/utils/sorter/types.ts b/packages/core/src/utils/sorter/types.ts index 450c35ee2..04d33baae 100644 --- a/packages/core/src/utils/sorter/types.ts +++ b/packages/core/src/utils/sorter/types.ts @@ -1,6 +1,6 @@ -import CanvasModule from '../../canvas'; import { ComponentDefinition } from '../../dom_components/model/types'; import EditorModel from '../../editor/model/Editor'; +import Dimension from './Dimension'; import { SortableTreeNode } from './SortableTreeNode'; export type ContentElement = string | ComponentDefinition; @@ -24,17 +24,6 @@ export type DragSource = DraggableContent & { model?: T; }; -export interface Dimension { - top: number; - left: number; - height: number; - width: number; - offsets: ReturnType; - dir?: boolean; - el?: HTMLElement; - indexEl?: number; -} - export type Placement = 'inside' | 'before' | 'after'; export enum DragDirection {