From c27b787192cf07410b73458e0edfece950a5d609 Mon Sep 17 00:00:00 2001 From: Garvish Panchal <85312938+Gxrvish@users.noreply.github.com> Date: Fri, 31 Jul 2026 17:42:59 +0530 Subject: [PATCH] Fix stale drop target on slow drags in sorter (#6798) The stale-target cache in DropLocationDeterminer#getValidParent compared the current drop-bounds check (container-relative coords) against one computed from the last mouse event's clientX/clientY (viewport coords), making the comparison meaningless. On slow drags, where the hovered node and index stay the same between move samples, the cached target was returned even after the pointer entered an edge band that should escalate the target to the parent. Store the drop-bounds result of each move in lastMoveData and compare against it, keeping the whole check in a single coordinate space. Introduced in #6542 (0.22.10). --- .../utils/sorter/DropLocationDeterminer.ts | 10 +-- .../test/specs/utils/sorter/sorterLayers.ts | 67 +++++++++++++++++++ 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/packages/core/src/utils/sorter/DropLocationDeterminer.ts b/packages/core/src/utils/sorter/DropLocationDeterminer.ts index 32c887d32..8308529d0 100644 --- a/packages/core/src/utils/sorter/DropLocationDeterminer.ts +++ b/packages/core/src/utils/sorter/DropLocationDeterminer.ts @@ -42,6 +42,8 @@ type lastMoveData = { placement?: Placement; /** The mouse event, used if we want to move placeholder with scrolling. */ mouseEvent?: MouseEvent; + /** Whether the mouse was within the hovered node's drop bounds during the last move. */ + hoveredWithinBounds?: boolean; placeholderDimensions?: Dimension; }; @@ -161,6 +163,7 @@ export class DropLocationDeterminer> ext mouseEvent, index, hoveredIndex, + hoveredWithinBounds: hoveredNode.isWithinDropBounds(mouseX, mouseY), placement, placeholderDimensions, }; @@ -372,7 +375,7 @@ export class DropLocationDeterminer> ext targetNode: lastTargetNode, hoveredNode: lastHoveredNode, hoveredIndex: lastHoveredIndex, - mouseEvent: lastMouseEvent, + hoveredWithinBounds: lastHoveredWithinBounds, } = this.lastMoveData; const sameHoveredNode = targetNode.equals(lastHoveredNode); @@ -380,10 +383,7 @@ export class DropLocationDeterminer> ext const hoverIndex = this.getIndexInParent(targetNode, targetNode.nodeDimensions!, mouseX, mouseY); const sameHoveredIndex = hoverIndex === lastHoveredIndex; const isWithinDropArea = targetNode.isWithinDropBounds(mouseX, mouseY); - const sameHoverPosition = - sameHoveredNode && - sameHoveredIndex && - isWithinDropArea === targetNode.isWithinDropBounds(lastMouseEvent?.clientX ?? 0, lastMouseEvent?.clientY ?? 0); + const sameHoverPosition = sameHoveredNode && sameHoveredIndex && isWithinDropArea === lastHoveredWithinBounds; if (sameHoverPosition && lastTargetNode) return lastTargetNode; diff --git a/packages/core/test/specs/utils/sorter/sorterLayers.ts b/packages/core/test/specs/utils/sorter/sorterLayers.ts index 7734a7619..b9a7480cf 100644 --- a/packages/core/test/specs/utils/sorter/sorterLayers.ts +++ b/packages/core/test/specs/utils/sorter/sorterLayers.ts @@ -1,5 +1,6 @@ import Component from '../../../../src/dom_components/model/Component'; import Editor from '../../../../src/editor'; +import Dimension from '../../../../src/utils/sorter/Dimension'; import LayersComponentNode from '../../../../src/utils/sorter/LayersComponentNode'; import { setupTestEditor } from '../../../common'; @@ -69,4 +70,70 @@ describe('Layers sorter', () => { expect(getChildIds(wrapper)).toEqual(['visible-a', 'hidden-a', 'visible-b', 'hidden-b']); }); + + test('escalates the drop target to the parent when a slow drag leaves the drop bounds without changing node or index', () => { + editor.setComponents(` +
Block A
+
Block B
+
Child C1
+ `); + + const wrapper = editor.getWrapper()!; + const source = wrapper.find('#block-a')[0]; + const blockB = wrapper.find('#block-b')[0]; + const sectionC = wrapper.find('#section-c')[0]; + + editor.select(source); + editor.Layers.setRoot(wrapper); + fixtures.appendChild(editor.Layers.render()); + + const sorter = source.viewLayer!.sorter; + const determiner: any = sorter.dropLocationDeterminer; + const sectionEl = sectionC.viewLayer!.el; + + // Container-relative boxes for a vertical layer list. The section's drop + // area (ratio 0.4, min 3px, max 20px) shrinks 76-124 to 90.4-109.6. + const box = (top: number, height: number) => + new Dimension({ top, left: 0, height, width: 200, offsets: {} as any, dir: true }); + const boxes = new Map([ + [source.viewLayer!.el, box(28, 24)], + [blockB.viewLayer!.el, box(52, 24)], + [sectionEl, box(76, 48)], + ]); + const wrapperBox = () => box(0, 300); + + // The container offset makes viewport and container coordinates diverge, + // as they do in a real page where the layer panel is not at the origin. + const offsetTop = 200; + jest.spyOn(determiner, 'cacheContainerPosition').mockImplementation(() => { + determiner.containerOffset = { top: offsetTop, left: 0 }; + }); + jest.spyOn(determiner, 'getDim').mockImplementation((el: any) => boxes.get(el)?.clone() ?? wrapperBox()); + jest.spyOn(determiner, 'getDirection').mockReturnValue(true); + jest.spyOn(determiner, 'getMouseTargetElement').mockReturnValue(sectionEl); + jest.spyOn(determiner, 'getFirstElementWithAModel').mockReturnValue(sectionEl); + // Placeholder placement is downstream of target selection and needs + // rendered child layers, which jsdom cannot lay out. + jest.spyOn(determiner, 'getDropPosition').mockImplementation(() => ({ + index: 0, + placement: 'before', + placeholderDimensions: box(0, 0), + })); + + sorter.startSort([{ element: source.viewLayer!.el }]); + const move = (containerY: number) => + determiner.handleMove(new MouseEvent('mousemove', { clientX: 100, clientY: containerY + offsetTop })); + + // Inside the section's drop area: the section itself is the target. + move(95); + expect(determiner.lastMoveData.targetNode?.model).toBe(sectionC); + + // Slow drag into the edge band above the drop area: same hovered node, + // same index ('before' in both samples), but the target must escalate + // to the wrapper instead of sticking to the cached section. + move(85); + expect(determiner.lastMoveData.targetNode?.model).toBe(wrapper); + + determiner.cancelDrag(); + }); });