From 67abcdd1d9fa11c0bf42be255cf655bba45a5a38 Mon Sep 17 00:00:00 2001 From: Balint Gabor <127662+gbalint@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:33:11 +0200 Subject: [PATCH] Modifiers for reparenting and convert to absolute strategies (#6054) **Description:** To make our canvas interactions more predictable I put reparenting and conversion to absolute behind modifier keys: - cmd is needed for reparenting - ctrl is needed for conversion to absolute **Notes:** - Nearly 70 tests were broken (nearly all reparenting and convert-to-absolute tests). In most of the cases I just had to add the necessary modifiers to make the tests work - Insertion relies on reparenting, I needed to modify the draw-to-insert strategy too - cmd disables snapping, so now it is not possible to have snapping during reparenting (I needed to disable the tests for this) - space forces convert to absolute, this is in partial conflict with ctrl, but they are not really the same: without ctrl convert-to-absolute strategies are disabled, but even with ctrl other strategies are still possible, while space really forces convert-to-absolute strategies - when you have a selection, cmd mouse down on a descendant element will change the selection to the descendant. If you don't want that, you need to press cmd after the mouse down. - with ctrl, you always need to press it after mouse down, otherwise the context menu will open - earlier cmd had a special meaning in case of reparenting: it allowed the element to be reparented into a parent which is smaller than the element itself. This distinction doesn't exist anymore, because without pressing cmd there is no reparent at all **Manual Tests:** I hereby swear that: - [x] I opened a hydrogen project and it loaded - [x] I could navigate to various routes in Preview mode Fixes https://github.com/concrete-utopia/utopia/issues/6047 --- ...eparent-strategy-helpers.spec.browser2.tsx | 105 ++++-- ...solute-reparent-strategy.spec.browser2.tsx | 47 +-- ...eparent-to-flex-strategy.spec.browser2.tsx | 6 +- .../ancestor-metastrategy.spec.browser2.tsx | 26 +- ...solute-and-move-strategy.spec.browser2.tsx | 304 +++++++++++++++--- .../convert-to-absolute-and-move-strategy.tsx | 69 ++-- .../drag-to-insert-metastrategy.tsx | 2 +- .../draw-to-insert-metastrategy.tsx | 2 +- .../draw-to-insert-text-strategy.tsx | 2 +- ...eparent-to-flex-strategy.spec.browser2.tsx | 4 +- ...lute-reparent-strategies.spec.browser2.tsx | 3 +- .../strategies/reparent-metastrategy.tsx | 39 ++- .../canvas/event-helpers.test-utils.tsx | 2 +- .../remix/remix-rendering.spec.browser2.tsx | 5 +- 14 files changed, 468 insertions(+), 148 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/absolute-flex-reparent-strategy-helpers.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/absolute-flex-reparent-strategy-helpers.spec.browser2.tsx index 6fa59da246da..2878285b03f9 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/absolute-flex-reparent-strategy-helpers.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/absolute-flex-reparent-strategy-helpers.spec.browser2.tsx @@ -65,6 +65,62 @@ async function dragElement( const defaultMouseDownOffset = windowPoint({ x: 20, y: 20 }) describe('Unified Reparent Fitness Function Tests', () => { + it('if cmd is not pressed no reparent strategies have nonzero fitness', async () => { + const renderResult = await renderTestEditorWithCode( + makeTestProjectCodeWithSnippet(` +
+
+
+
+
+ `), + 'await-first-dom-report', + ) + + const dragDelta = windowPoint({ x: 120, y: 0 }) + + const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'bbb', 'ccc']) + await renderResult.dispatch([selectComponents([targetPath], false)], false) + await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, emptyModifiers, false) + + const strategies = renderResult.getEditorState().strategyState.sortedApplicableStrategies + + if (strategies == null) { + // here for type assertion + throw new Error('`strategies` should not be null') + } + expect(strategies[0].strategy.id.includes('REPARENT')).toBeFalsy() + }) + it('if an element is larger than its parent, we still allow reparent to its grandparent, if the reparenting starts from the area of the original parent', async () => { const renderResult = await renderTestEditorWithCode( makeTestProjectCodeWithSnippet(` @@ -110,7 +166,7 @@ describe('Unified Reparent Fitness Function Tests', () => { const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'bbb', 'ccc']) await renderResult.dispatch([selectComponents([targetPath], false)], false) - await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, emptyModifiers, true) + await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, cmdModifier, true) await renderResult.getDispatchFollowUpActionsFinished() @@ -198,7 +254,7 @@ describe('Unified Reparent Fitness Function Tests', () => { const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'bbb', 'ccc']) await renderResult.dispatch([selectComponents([targetPath], false)], false) - await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, emptyModifiers, true) + await dragElement(renderResult, 'ccc', defaultMouseDownOffset, dragDelta, cmdModifier, true) await renderResult.getDispatchFollowUpActionsFinished() @@ -291,7 +347,7 @@ describe('Unified Reparent Fitness Function Tests', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, ) @@ -486,7 +542,7 @@ describe('Unified Reparent Fitness Function Tests', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, ) @@ -582,7 +638,7 @@ describe('Unified Reparent Fitness Function Tests', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, ) @@ -949,7 +1005,7 @@ describe('Target parents with flow layout', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, ) @@ -1092,7 +1148,7 @@ describe('Target parents with flow layout', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, ) @@ -1200,7 +1256,7 @@ describe('Target parents with flow layout', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, ) @@ -1297,7 +1353,7 @@ describe('Target parents with flow layout', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, () => pressKey('2', { modifiers: cmdModifier }), // Switch to flow reparenting strategy ) @@ -1423,7 +1479,7 @@ describe('Target parents with flow layout', () => { 'draggedElement', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, true, () => pressKey('2', { modifiers: cmdModifier }), // Switch to flow reparenting strategy ) @@ -1918,7 +1974,8 @@ describe('Target parent filtering', () => { ) }) - it('Dragging without cmd with cursor over the smaller element prevents reparenting into the larger element', async () => { + // now we need cmd for any kind of reparent, so this test is outdated + xit('Dragging without cmd with cursor over the smaller element prevents reparenting into the larger element', async () => { const renderResult = await renderTestEditorWithCode(startingCode, 'await-first-dom-report') const targetPath = EP.elementPath([[BakedInStoryboardUID, TestSceneUID, 'dragme']]) @@ -1929,7 +1986,7 @@ describe('Target parent filtering', () => { // drag to the left so that the cursor is over the larger element const dragDelta = windowPoint({ x: -10, y: -10 }) - await dragElement(renderResult, 'dragme', mouseDownOffset, dragDelta, emptyModifiers, true) + await dragElement(renderResult, 'dragme', mouseDownOffset, dragDelta, cmdModifier, true) await renderResult.getDispatchFollowUpActionsFinished() @@ -2272,7 +2329,7 @@ describe('Reparent indicators', () => { 'seconddiv', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2318,7 +2375,7 @@ describe('Reparent indicators', () => { 'seconddiv', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2361,7 +2418,7 @@ describe('Reparent indicators', () => { 'seconddiv', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2404,7 +2461,7 @@ describe('Reparent indicators', () => { 'seconddiv', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2448,7 +2505,7 @@ describe('Reparent indicators', () => { 'absolutechild', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2492,7 +2549,7 @@ describe('Reparent indicators', () => { 'absolutechild', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2536,7 +2593,7 @@ describe('Reparent indicators', () => { 'absolutechild', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2580,7 +2637,7 @@ describe('Reparent indicators', () => { 'absolutechild', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) @@ -2660,11 +2717,11 @@ describe('Reparent indicators', () => { 'absolutechild', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) - await pressKey('2') // Switch to flow reparenting strategy + await pressKey('2', { modifiers: cmdModifier }) // Switch to flow reparenting strategy await renderResult.getDispatchFollowUpActionsFinished() @@ -2758,11 +2815,11 @@ describe('Reparent indicators', () => { 'absolutechild', defaultMouseDownOffset, dragDelta, - emptyModifiers, + cmdModifier, false, ) - await pressKey('2') // Switch to flow reparenting strategy + await pressKey('2', { modifiers: cmdModifier }) // Switch to flow reparenting strategy await renderResult.getDispatchFollowUpActionsFinished() diff --git a/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-strategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-strategy.spec.browser2.tsx index 8fdb02a89c2d..4c8f7a1d08bd 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-strategy.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-strategy.spec.browser2.tsx @@ -17,7 +17,7 @@ import { windowPoint, } from '../../../../core/shared/math-utils' import type { Modifiers } from '../../../../utils/modifiers' -import { cmdModifier, emptyModifiers } from '../../../../utils/modifiers' +import { cmdModifier } from '../../../../utils/modifiers' import { selectComponents } from '../../../editor/actions/meta-actions' import { RightMenuTab, navigatorEntryToKey } from '../../../editor/store/editor-state' import { CSSCursor } from '../../canvas-types' @@ -191,8 +191,8 @@ describe('Absolute Reparent Strategy', () => { y: targetElementBounds.y + 50, } - await mouseMoveToPoint(canvasControlsLayer, firstInsertionPoint) - await mouseClickAtPoint(canvasControlsLayer, firstInsertionPoint) + await mouseMoveToPoint(canvasControlsLayer, firstInsertionPoint, { modifiers: cmdModifier }) + await mouseClickAtPoint(canvasControlsLayer, firstInsertionPoint, { modifiers: cmdModifier }) await renderResult.getDispatchFollowUpActionsFinished() const afterFirstInsertMetadataKeys = new Set( @@ -216,8 +216,8 @@ describe('Absolute Reparent Strategy', () => { y: targetElementBounds.y + 200, } - await mouseMoveToPoint(canvasControlsLayer, secondInsertionPoint) - await mouseClickAtPoint(canvasControlsLayer, secondInsertionPoint) + await mouseMoveToPoint(canvasControlsLayer, secondInsertionPoint, { modifiers: cmdModifier }) + await mouseClickAtPoint(canvasControlsLayer, secondInsertionPoint, { modifiers: cmdModifier }) await renderResult.getDispatchFollowUpActionsFinished() const afterSecondInsertMetadataKeys = new Set( @@ -511,7 +511,8 @@ describe('Absolute Reparent Strategy', () => { ), ) }) - it('reparents to the canvas root when target parent on the canvas is small', async () => { + // this test is outdated, we only reparent with cmd pressed, and then we reparent to smaller parents too + xit('reparents to the canvas root when target parent on the canvas is small', async () => { const renderResult = await renderTestEditorWithCode( formatTestProjectCode(` import * as React from 'react' @@ -552,9 +553,12 @@ export var ${BakedInStoryboardVariableName} = (props) => { ) const dragDelta = windowPoint({ x: -1000, y: -1000 }) - await dragElement(renderResult, 'bbb', dragDelta, emptyModifiers, null, async () => - renderResult.dispatch([CanvasActions.setUsersPreferredStrategy('ABSOLUTE_REPARENT')], true), - ) + await dragElement(renderResult, 'bbb', dragDelta, cmdModifier, null, async () => { + void renderResult.dispatch( + [CanvasActions.setUsersPreferredStrategy('ABSOLUTE_REPARENT')], + true, + ) + }) await renderResult.getDispatchFollowUpActionsFinished() @@ -598,7 +602,8 @@ export var ${BakedInStoryboardVariableName} = (props) => { ), ) }) - it('does not reparent to ancestor outside of the containing component when the mouse is inside the containing component bounds', async () => { + // Outdated test, cmd is necessary for reparenting, and it forces reparenting to ancestor outside of the containing component + xit('does not reparent to ancestor outside of the containing component when the mouse is inside the containing component bounds', async () => { const renderResult = await renderTestEditorWithCode( makeTestProjectCodeWithSnippet(` <> @@ -620,7 +625,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { ) const dragDelta = windowPoint({ x: -1000, y: -1000 }) - await dragElement(renderResult, 'bbb', dragDelta, emptyModifiers, null, null) + await dragElement(renderResult, 'bbb', dragDelta, cmdModifier, null, null) await renderResult.getDispatchFollowUpActionsFinished() @@ -762,7 +767,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { ) const dragDelta = windowPoint({ x: -1000, y: -1000 }) - await dragElement(renderResult, 'bbb', dragDelta, emptyModifiers, null, null) + await dragElement(renderResult, 'bbb', dragDelta, cmdModifier, null, null) await renderResult.getDispatchFollowUpActionsFinished() @@ -1040,7 +1045,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { } const dragDelta = windowPoint({ x: bbbCenter.x - cccCenter.x, y: bbbCenter.y - cccCenter.y }) - await dragElement(renderResult, 'ccc', dragDelta, emptyModifiers, null, async () => { + await dragElement(renderResult, 'ccc', dragDelta, cmdModifier, null, async () => { const draggedElement = await renderResult.renderedDOM.findByTestId('ccc') const draggedElementBounds = draggedElement.getBoundingClientRect() const draggedElementCanvasBounds = boundingClientRectToCanvasRectangle( @@ -1161,7 +1166,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { } const dragDelta = windowPoint({ x: bbbCenter.x - cccCenter.x, y: bbbCenter.y - cccCenter.y }) - await dragElement(renderResult, 'ccc', dragDelta, emptyModifiers, null, async () => { + await dragElement(renderResult, 'ccc', dragDelta, cmdModifier, null, async () => { const draggedElement = await renderResult.renderedDOM.findByTestId('ccc') const draggedElementBounds = draggedElement.getBoundingClientRect() const draggedElementCanvasBounds = boundingClientRectToCanvasRectangle( @@ -1526,7 +1531,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { ) const dragDelta = windowPoint({ x: -150, y: -150 }) - await dragElement(renderResult, 'bbb', dragDelta, emptyModifiers, null, null) + await dragElement(renderResult, 'bbb', dragDelta, cmdModifier, null, null) await renderResult.getDispatchFollowUpActionsFinished() @@ -1575,7 +1580,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { ) const dragDelta = windowPoint({ x: -150, y: -150 }) - await dragElement(renderResult, 'bbb', dragDelta, emptyModifiers, null, null) + await dragElement(renderResult, 'bbb', dragDelta, cmdModifier, null, null) await renderResult.getDispatchFollowUpActionsFinished() @@ -1629,7 +1634,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { ) const dragDelta = windowPoint({ x: -150, y: -150 }) - await dragElement(renderResult, 'bbb', dragDelta, emptyModifiers, null, null) + await dragElement(renderResult, 'bbb', dragDelta, cmdModifier, null, null) await renderResult.getDispatchFollowUpActionsFinished() @@ -1754,6 +1759,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { { x: dragme.x + 10, y: dragme.y + 10 }, { x: -100, y: 0 }, { + modifiers: cmdModifier, midDragCallback: async () => renderResult.dispatch( [CanvasActions.setUsersPreferredStrategy('ABSOLUTE_REPARENT')], @@ -1809,6 +1815,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { { x: dragme.x + 10, y: dragme.y + 10 }, { x: -100, y: 0 }, { + modifiers: cmdModifier, midDragCallback: async () => renderResult.dispatch( [CanvasActions.setUsersPreferredStrategy('ABSOLUTE_REPARENT')], @@ -1844,7 +1851,8 @@ export var ${BakedInStoryboardVariableName} = (props) => { ) }) }) - describe('snapping', () => { + // we don't have snapping with reparenting anymore, because cmd enables reparenting and disables snapping at the same time + xdescribe('snapping', () => { const NewParentTestId = 'new-parent' const NewSiblingTestId = 'new-sibling' const project = (innards: string) => `
{ elementToDragCenter, windowPoint({ x: newParentCenterX, y: newSiblingCenterY }), { + modifiers: cmdModifier, midDragCallback: async () => { const guidelines = renderResult.getEditorState().editor.canvas.controls.snappingGuidelines @@ -1992,6 +2001,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { elementToDragCenter, windowPoint({ x: newParentCenterX, y: newSiblingCenterY }), { + modifiers: cmdModifier, midDragCallback: async () => { const guidelines = renderResult.getEditorState().editor.canvas.controls.snappingGuidelines @@ -2051,6 +2061,7 @@ export var ${BakedInStoryboardVariableName} = (props) => { elementToDragCenter, windowPoint({ x: newParentCenterX, y: newSiblingCenterY }), { + modifiers: cmdModifier, midDragCallback: async () => { const guidelines = renderResult.getEditorState().editor.canvas.controls.snappingGuidelines diff --git a/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-to-flex-strategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-to-flex-strategy.spec.browser2.tsx index 367df5bed913..69411fd0d3e0 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-to-flex-strategy.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-to-flex-strategy.spec.browser2.tsx @@ -64,6 +64,7 @@ async function dragElement( movementY: dragDelta.y, buttons: 1, }, + modifiers: cmdModifier, }, ) } else { @@ -857,7 +858,7 @@ describe('Absolute Reparent To Flex Strategy with more complex flex layouts', () x: parentRightEdge.x - childCenter.x, y: parentRightEdge.y - childCenter.y, }) - await dragElement(renderResult, 'innerchild2', dragDelta, emptyModifiers) + await dragElement(renderResult, 'innerchild2', dragDelta, cmdModifier) await renderResult.getDispatchFollowUpActionsFinished() @@ -900,7 +901,7 @@ describe('Absolute Reparent To Flex Strategy with more complex flex layouts', () x: parentRightEdge.x - childCenter.x, y: parentRightEdge.y - childCenter.y, }) - await dragElement(renderResult, 'innerchild2', dragDelta, emptyModifiers) + await dragElement(renderResult, 'innerchild2', dragDelta, cmdModifier) await renderResult.getDispatchFollowUpActionsFinished() @@ -943,7 +944,6 @@ describe('Absolute Reparent To Flex Strategy with more complex flex layouts', () x: newParentTargetCenter.x - childCenter.x, y: newParentTargetCenter.y - childCenter.y, }) - await dragElement(renderResult, 'innerchild2', dragDelta, emptyModifiers, true) await renderResult.getDispatchFollowUpActionsFinished() diff --git a/editor/src/components/canvas/canvas-strategies/strategies/ancestor-metastrategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/ancestor-metastrategy.spec.browser2.tsx index 1cd699ef9c4d..3ca445909622 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/ancestor-metastrategy.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/ancestor-metastrategy.spec.browser2.tsx @@ -2,7 +2,8 @@ import * as EP from '../../../../core/shared/element-path' import type { WindowPoint } from '../../../../core/shared/math-utils' import { windowPoint } from '../../../../core/shared/math-utils' import type { ElementPath } from '../../../../core/shared/project-file-types' -import { shiftModifier } from '../../../../utils/modifiers' +import type { Modifiers } from '../../../../utils/modifiers' +import { ctrlModifier, shiftModifier } from '../../../../utils/modifiers' import { selectComponents } from '../../../editor/actions/meta-actions' import { CanvasControlsContainerID } from '../../controls/new-canvas-controls' import { @@ -498,6 +499,7 @@ async function dragElement( startPoint: WindowPoint, dragDelta: WindowPoint, midDragCallback?: () => Promise, + modifiers?: Modifiers, ): Promise { await mouseDownAtPoint(canvasControlsLayer, startPoint) await mouseDragFromPointWithDelta(canvasControlsLayer, startPoint, dragDelta, { @@ -526,12 +528,22 @@ async function runTest( const { x, y } = startPoint const canvasControlsLayer = editor.renderedDOM.getByTestId(CanvasControlsContainerID) - await dragElement(canvasControlsLayer, startPoint, windowPoint({ x: 20, y: 0 }), async () => { - await mouseDownAtPoint(divToBeDragged, { x: x, y: y }) - await mouseMoveToPoint(divToBeDragged, { x: x + 20, y: y }, { eventOptions: { buttons: 1 } }) - - check(editor) - }) + await dragElement( + canvasControlsLayer, + startPoint, + windowPoint({ x: 20, y: 0 }), + async () => { + await mouseDownAtPoint(divToBeDragged, { x: x, y: y }, { modifiers: ctrlModifier }) + await mouseMoveToPoint( + divToBeDragged, + { x: x + 20, y: y }, + { eventOptions: { buttons: 1 }, modifiers: ctrlModifier }, + ) + + check(editor) + }, + ctrlModifier, + ) } describe('finds an applicable strategy for the nearest ancestor', () => { diff --git a/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.spec.browser2.tsx index be4ac5d716a7..e711710d7e34 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.spec.browser2.tsx @@ -42,7 +42,6 @@ import { mouseUpAtPoint, pressKey, } from '../../event-helpers.test-utils' -import { cmdModifier } from '../../../../utils/modifiers' import type { FragmentLikeType } from './fragment-like-helpers' import { AllFragmentLikeNonDomElementTypes, @@ -683,7 +682,14 @@ describe('Convert to Absolute', () => { x: elementBounds.x + elementBounds.width / 2, y: elementBounds.y + elementBounds.width / 2, }, - { modifiers: cmdModifier }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) // move so that the bottom right corner snaps to the center of the parent @@ -747,12 +753,30 @@ describe('Convert to absolute/escape hatch', () => { x: elementBounds.x + 10, y: elementBounds.y + 10, }, - { modifiers: cmdModifier }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) + await mouseMoveToPoint( + canvasControlsLayer, + { + x: elementBounds.x + 50, + y: elementBounds.y + 50, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) - await mouseMoveToPoint(canvasControlsLayer, { - x: elementBounds.x + 50, - y: elementBounds.y + 50, - }) const strategyBeforeSpacePressed = renderResult.getEditorState().strategyState.currentStrategy expect(strategyBeforeSpacePressed).toEqual('FLEX_REORDER') @@ -811,7 +835,14 @@ describe('Convert to absolute/escape hatch', () => { x: elementBounds.x + 10, y: elementBounds.y + 10, }, - { modifiers: cmdModifier }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) await mouseMoveToPoint(canvasControlsLayer, { x: elementBounds.x + 50, @@ -858,7 +889,14 @@ describe('Convert to absolute/escape hatch', () => { x: elementBounds.x + 10, y: elementBounds.y + 10, }, - { modifiers: cmdModifier }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) // Drag without going outside the sibling bounds @@ -918,24 +956,53 @@ describe('Convert to absolute/escape hatch', () => { x: elementBounds.x + 10, y: elementBounds.y + 10, }, - { modifiers: cmdModifier }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) // Drag without going outside the sibling bounds - await mouseMoveToPoint(canvasControlsLayer, { - x: elementBounds.x + 50, - y: elementBounds.y + 10, - }) + await mouseMoveToPoint( + canvasControlsLayer, + { + x: elementBounds.x + 50, + y: elementBounds.y + 10, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) const midDragStrategy = renderResult.getEditorState().strategyState.currentStrategy expect(midDragStrategy).not.toBeNull() expect(midDragStrategy).not.toEqual(ConvertToAbsoluteAndMoveStrategyID) // Now drag until we have passed the sibling bounds - await mouseMoveToPoint(canvasControlsLayer, { - x: elementBounds.x + 110, - y: elementBounds.y + 10, - }) + await mouseMoveToPoint( + canvasControlsLayer, + { + x: elementBounds.x + 110, + y: elementBounds.y + 10, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) const endDragStrategy = renderResult.getEditorState().strategyState.currentStrategy expect(endDragStrategy).not.toBeNull() @@ -981,24 +1048,53 @@ describe('Convert to absolute/escape hatch', () => { x: elementBounds.x + 10, y: elementBounds.y + 10, }, - { modifiers: cmdModifier }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) // Drag without going outside the sibling bounds - await mouseMoveToPoint(canvasControlsLayer, { - x: elementBounds.x + 50, - y: elementBounds.y + 10, - }) + await mouseMoveToPoint( + canvasControlsLayer, + { + x: elementBounds.x + 50, + y: elementBounds.y + 10, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) const midDragStrategy = renderResult.getEditorState().strategyState.currentStrategy expect(midDragStrategy).not.toBeNull() expect(midDragStrategy).not.toEqual(ConvertToAbsoluteAndMoveStrategyID) // Now drag until we have passed the sibling bounds - await mouseMoveToPoint(canvasControlsLayer, { - x: elementBounds.x + 110, - y: elementBounds.y + 10, - }) + await mouseMoveToPoint( + canvasControlsLayer, + { + x: elementBounds.x + 110, + y: elementBounds.y + 10, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) const endDragStrategy = renderResult.getEditorState().strategyState.currentStrategy expect(endDragStrategy).not.toBeNull() @@ -1043,33 +1139,73 @@ describe('Convert to absolute/escape hatch', () => { x: elementBounds.x + 10, y: elementBounds.y + 10, }, - { modifiers: cmdModifier }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) // Drag without going outside the sibling bounds - await mouseMoveToPoint(canvasControlsLayer, { - x: elementBounds.x + 50, - y: elementBounds.y + 10, - }) + await mouseMoveToPoint( + canvasControlsLayer, + { + x: elementBounds.x + 50, + y: elementBounds.y + 10, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) const midDragStrategy = renderResult.getEditorState().strategyState.currentStrategy expect(midDragStrategy).not.toBeNull() expect(midDragStrategy).not.toEqual(ConvertToAbsoluteAndMoveStrategyID) // Now drag until we have passed the sibling bounds - await mouseMoveToPoint(canvasControlsLayer, { - x: elementBounds.x + 110, - y: elementBounds.y + 10, - }) + await mouseMoveToPoint( + canvasControlsLayer, + { + x: elementBounds.x + 110, + y: elementBounds.y + 10, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) const endDragStrategy = renderResult.getEditorState().strategyState.currentStrategy expect(endDragStrategy).not.toBeNull() expect(endDragStrategy).toEqual(ConvertToAbsoluteAndMoveStrategyID) - await mouseUpAtPoint(canvasControlsLayer, { - x: elementBounds.x + 110, - y: elementBounds.y + 10, - }) + await mouseUpAtPoint( + canvasControlsLayer, + { + x: elementBounds.x + 110, + y: elementBounds.y + 10, + }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, + ) const jsxMetadataAfter = renderResult.getEditorState().editor.jsxMetadata const allElementPropsAfter = renderResult.getEditorState().editor.allElementProps @@ -1261,6 +1397,14 @@ describe('Convert to absolute/escape hatch', () => { canvas, viewCenter, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 })), + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) expect(getPrintedUiJsCode(initialEditor.getEditorState())).toEqual( @@ -1366,15 +1510,36 @@ describe('Convert to absolute/escape hatch', () => { y: viewBounds.top + viewBounds.height / 2, }) - await mouseDownAtPoint(canvas, viewCenter) - await mouseMoveToPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 }))) + await mouseDownAtPoint(canvas, viewCenter, { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) + await mouseMoveToPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 })), { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) await initialEditor.dispatch( [CanvasActions.setUsersPreferredStrategy(ConvertToAbsoluteAndMoveStrategyID)], true, ) - await mouseUpAtPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 }))) + await mouseUpAtPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 })), { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) expect(getPrintedUiJsCode(initialEditor.getEditorState())).toEqual( makeTestProjectCodeWithSnippet( @@ -1427,15 +1592,36 @@ describe('Convert to absolute/escape hatch', () => { y: viewBounds.top + viewBounds.height / 2, }) - await mouseDownAtPoint(canvas, viewCenter) - await mouseMoveToPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 }))) + await mouseDownAtPoint(canvas, viewCenter, { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) + await mouseMoveToPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 })), { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) await initialEditor.dispatch( [CanvasActions.setUsersPreferredStrategy(ConvertToAbsoluteAndMoveStrategyID)], true, ) - await mouseUpAtPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 }))) + await mouseUpAtPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 })), { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) expect(getPrintedUiJsCode(initialEditor.getEditorState())).toEqual( makeTestProjectCodeWithSnippet( @@ -1495,8 +1681,22 @@ describe('Convert to absolute/escape hatch', () => { y: viewBounds.top + viewBounds.height / 2, }) - await mouseDownAtPoint(canvas, viewCenter) - await mouseMoveToPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 }))) + await mouseDownAtPoint(canvas, viewCenter, { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) + await mouseMoveToPoint(canvas, offsetPoint(viewCenter, canvasPoint({ x: 15, y: 15 })), { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }) await initialEditor.dispatch( [CanvasActions.setUsersPreferredStrategy(ConvertToAbsoluteAndMoveStrategyID)], @@ -1604,6 +1804,14 @@ describe('Escape hatch strategy on awkward project', () => { x: 15, y: 15, }, + { + modifiers: { + alt: false, + cmd: true, + ctrl: true, + shift: false, + }, + }, ) expect(getPrintedUiJsCode(renderResult.getEditorState())).toEqual( diff --git a/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.tsx index 5170949f1561..663e6dd22159 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.tsx @@ -198,12 +198,13 @@ function convertToAbsoluteAndMoveStrategyFactory(setHuggingParentToFixed: SetHug ] : [] - const baseFitness = getFitness( + const fitness = getFitness( interactionSession, hasAutoLayoutSiblings, autoLayoutSiblingsBounds, originalTargets.length > 1, isPositionRelative, + setHuggingParentToFixed, ) return { @@ -227,8 +228,7 @@ function convertToAbsoluteAndMoveStrategyFactory(setHuggingParentToFixed: SetHug }), ...autoLayoutSiblingsControl, ], // Uses existing hooks in select-mode-hooks.tsx - fitness: - setHuggingParentToFixed === 'set-hugging-parent-to-fixed' ? baseFitness + 0.1 : baseFitness, // by default we set the parent to fixed size + fitness: fitness, apply: () => { if ( interactionSession != null && @@ -285,6 +285,7 @@ function convertToAbsoluteAndMoveStrategyFactory(setHuggingParentToFixed: SetHug } } +const VeryHighWeight = 100 const BaseWeight = 0.5 const DragConversionWeight = 1.5 // needs to be higher then FlexReorderFitness in flex-reorder-strategy @@ -294,42 +295,56 @@ function getFitness( autoLayoutSiblingsBounds: CanvasRectangle | null, multipleTargets: boolean, isPositionRelative: boolean, + setHuggingParentToFixed: SetHuggingParentToFixed, ): number { if ( interactionSession != null && interactionSession.interactionData.type === 'DRAG' && - interactionSession.activeControl.type === 'BOUNDING_AREA' + !( + interactionSession.interactionData.modifiers.ctrl || + interactionSession.interactionData.spacePressed + ) ) { - if (interactionSession.interactionData.spacePressed) { - // If space is pressed, this should happening! - return 100 - } - - if (interactionSession.interactionData.drag == null) { - return BaseWeight - } + return 0 + } + const baseFitness = (() => { + if ( + interactionSession != null && + interactionSession.interactionData.type === 'DRAG' && + interactionSession.activeControl.type === 'BOUNDING_AREA' + ) { + if (interactionSession.interactionData.spacePressed) { + // If space is pressed, this should happening! + return VeryHighWeight + } - if (!hasAutoLayoutSiblings) { - if (multipleTargets || isPositionRelative) { - // multi-selection should require a spacebar press to activate - // position relative can be just moved with relative move, no need to convert to absolute when relative move is applicable + if (interactionSession.interactionData.drag == null) { return BaseWeight } - return DragConversionWeight - } - const pointOnCanvas = offsetPoint( - interactionSession.interactionData.dragStart, - interactionSession.interactionData.drag, - ) + if (!hasAutoLayoutSiblings) { + if (multipleTargets || isPositionRelative) { + // multi-selection should require a spacebar press to activate + // position relative can be just moved with relative move, no need to convert to absolute when relative move is applicable + return BaseWeight + } + return DragConversionWeight + } - const isInsideBoundingBoxOfSiblings = - autoLayoutSiblingsBounds != null && rectContainsPoint(autoLayoutSiblingsBounds, pointOnCanvas) + const pointOnCanvas = offsetPoint( + interactionSession.interactionData.dragStart, + interactionSession.interactionData.drag, + ) - return isInsideBoundingBoxOfSiblings || isPositionRelative ? BaseWeight : DragConversionWeight - } + const isInsideBoundingBoxOfSiblings = + autoLayoutSiblingsBounds != null && + rectContainsPoint(autoLayoutSiblingsBounds, pointOnCanvas) - return 0 + return isInsideBoundingBoxOfSiblings || isPositionRelative ? BaseWeight : DragConversionWeight + } + return 0 + })() + return setHuggingParentToFixed === 'set-hugging-parent-to-fixed' ? baseFitness + 0.1 : baseFitness // by default we set the parent to fixed size } const getAutoLayoutSiblingsBounds = memoize(getAutoLayoutSiblingsBoundsInner, { maxSize: 1 }) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/drag-to-insert-metastrategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/drag-to-insert-metastrategy.tsx index d98c44b2dd47..425afc32cdb4 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/drag-to-insert-metastrategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/drag-to-insert-metastrategy.tsx @@ -92,7 +92,7 @@ export const dragToInsertMetaStrategy: MetaCanvasStrategy = ( const applicableReparentFactories = getApplicableReparentFactories( canvasState, pointOnCanvas, - interactionData.modifiers.cmd, + true, // cmd is necessary to allow reparenting true, 'allow-smaller-parent', customStrategyState, diff --git a/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-metastrategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-metastrategy.tsx index 423ab0bd4d15..1dab244f3209 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-metastrategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-metastrategy.tsx @@ -102,7 +102,7 @@ export const drawToInsertMetaStrategy: MetaCanvasStrategy = ( const applicableReparentFactories = getApplicableReparentFactories( canvasState, pointOnCanvas, - interactionSession.interactionData.modifiers.cmd, + true, // cmd is necessary to allow reparenting true, 'allow-smaller-parent', customStrategyState, diff --git a/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-text-strategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-text-strategy.tsx index 738eb8a9ec5e..4605386024b8 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-text-strategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/draw-to-insert-text-strategy.tsx @@ -67,7 +67,7 @@ export const drawToInsertTextStrategy: MetaCanvasStrategy = ( const applicableReparentFactories = getApplicableReparentFactories( canvasState, pointOnCanvas, - false, + true, // cmd is necessary to allow reparenting true, 'allow-smaller-parent', customStrategyState, diff --git a/editor/src/components/canvas/canvas-strategies/strategies/flex-reparent-to-flex-strategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/flex-reparent-to-flex-strategy.spec.browser2.tsx index d0ca163123c5..94be253e1d43 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/flex-reparent-to-flex-strategy.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/flex-reparent-to-flex-strategy.spec.browser2.tsx @@ -397,7 +397,9 @@ describe('Flex Reparent To Flex Strategy', () => { y: targetFlexParentEnd.y - flexChildToReparentCenter.y, }) - await mouseDragFromPointWithDelta(canvasControlsLayer, flexChildToReparentCenter, dragDelta) + await mouseDragFromPointWithDelta(canvasControlsLayer, flexChildToReparentCenter, dragDelta, { + modifiers: cmdModifier, + }) await renderResult.getDispatchFollowUpActionsFinished() diff --git a/editor/src/components/canvas/canvas-strategies/strategies/forced-absolute-reparent-strategies.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/forced-absolute-reparent-strategies.spec.browser2.tsx index 7ca699db0af7..7c9ec0953a84 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/forced-absolute-reparent-strategies.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/forced-absolute-reparent-strategies.spec.browser2.tsx @@ -328,10 +328,9 @@ describe('Fallback Absolute Reparent Strategies', () => { dragDelta, cmdModifier, async function midDragCallback() { - await pressKey('2') // this should select the Reparent (Abs) strategy + await pressKey('2', { modifiers: cmdModifier }) // this should select the Reparent (Flow) strategy }, ) - await renderResult.getDispatchFollowUpActionsFinished() expect(getPrintedUiJsCode(renderResult.getEditorState())).toEqual( diff --git a/editor/src/components/canvas/canvas-strategies/strategies/reparent-metastrategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/reparent-metastrategy.tsx index b1a414c5c26f..5b6a8876c7a3 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/reparent-metastrategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/reparent-metastrategy.tsx @@ -69,11 +69,18 @@ export function getApplicableReparentFactories( const factories: Array = reparentStrategies.map((result) => { switch (result.strategy) { case 'REPARENT_AS_ABSOLUTE': { - const fitness = result.isReparentingOutFromScene - ? OutOfSceneReparentWeight - : result.isFallback - ? FallbackReparentWeight - : DefaultReparentWeight + const fitness = (() => { + if (!cmdPressed) { + return 0 + } + if (result.isReparentingOutFromScene) { + return OutOfSceneReparentWeight + } + if (result.isFallback) { + return FallbackReparentWeight + } + return DefaultReparentWeight + })() if (allDraggedElementsAbsolute) { return { @@ -106,13 +113,21 @@ export function getApplicableReparentFactories( const targetParentDisplayType = parentLayoutSystem === 'flex' ? 'flex' : 'flow' // We likely never want flow insertion or re-parenting to be the default - const fitness = result.isReparentingOutFromScene - ? OutOfSceneReparentWeight - : targetParentDisplayType === 'flow' - ? FlowReparentWeight - : result.isFallback - ? FallbackReparentWeight - : DefaultReparentWeight + const fitness = (() => { + if (!cmdPressed) { + return 0 + } + if (result.isReparentingOutFromScene) { + return OutOfSceneReparentWeight + } + if (targetParentDisplayType === 'flow') { + return FlowReparentWeight + } + if (result.isFallback) { + return FallbackReparentWeight + } + return DefaultReparentWeight + })() return { targetParent: result.target.newParent, diff --git a/editor/src/components/canvas/event-helpers.test-utils.tsx b/editor/src/components/canvas/event-helpers.test-utils.tsx index b012c595f520..d05cdd786d87 100644 --- a/editor/src/components/canvas/event-helpers.test-utils.tsx +++ b/editor/src/components/canvas/event-helpers.test-utils.tsx @@ -217,7 +217,7 @@ export async function mouseDragFromPointToPoint( await mouseMoveToPoint(eventSourceElement, startPoint, options) } if (options.realMouseDown) { - dispatchMouseDownEventAtPoint(startPoint) + dispatchMouseDownEventAtPoint(startPoint, options) } else { await mouseDownAtPoint(eventSourceElement, startPoint, options) } diff --git a/editor/src/components/canvas/remix/remix-rendering.spec.browser2.tsx b/editor/src/components/canvas/remix/remix-rendering.spec.browser2.tsx index 7e9b390d3498..f2c34dcf69d0 100644 --- a/editor/src/components/canvas/remix/remix-rendering.spec.browser2.tsx +++ b/editor/src/components/canvas/remix/remix-rendering.spec.browser2.tsx @@ -2129,7 +2129,7 @@ export default function Index() { renderResult, windowPoint({ x: absoluteDivBounds.x + 1, y: absoluteDivBounds.y + 1 }), windowPoint({ x: 10, y: -77 }), - emptyModifiers, + cmdModifier, async () => renderResult.dispatch( [CanvasActions.setUsersPreferredStrategy('ABSOLUTE_REPARENT')], @@ -2159,11 +2159,12 @@ export default function Index() { AbsoluteDivTestId, ) const absoluteDivBounds = absoluteElement.getBoundingClientRect() + await dragMouse( renderResult, windowPoint({ x: absoluteDivBounds.x + 1, y: absoluteDivBounds.y + 1 }), windowPoint({ x: -10, y: 77 }), - emptyModifiers, + cmdModifier, async () => renderResult.dispatch( [CanvasActions.setUsersPreferredStrategy('ABSOLUTE_REPARENT')],