Relative move is not the default strategy when TLBR is not set (#4382)

* Relative move is not the default strategy when TLBR is not set * Balance convert to absolute, relative move and flex reorder strategies * Adding test * Added some comments
concrete-utopia · Oct 18, 2023 · 09cbc1e · 09cbc1e
1 parent e7fe49a
commit 09cbc1e
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 12 deletions.
diff --git a/.../components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.tsx b/.../components/canvas/canvas-strategies/strategies/convert-to-absolute-and-move-strategy.tsx
@@ -141,6 +141,14 @@ function convertToAbsoluteAndMoveStrategyFactory(setHuggingParentToFixed: SetHug
       return null
     }
 
+    const isPositionRelative = retargetedTargets.every((element) => {
+      const elementMetadata = MetadataUtils.findElementByElementPath(
+        canvasState.startingMetadata,
+        element,
+      )
+      return elementMetadata?.specialSizeMeasurements.position === 'relative'
+    })
+
     // When the parent is not hugging, don't offer the strategy which sets it to fixed size
     if (setHuggingParentToFixed === 'set-hugging-parent-to-fixed') {
       const setParentToFixedSizeCommands = createSetParentsToFixedSizeCommands(
@@ -183,6 +191,7 @@ function convertToAbsoluteAndMoveStrategyFactory(setHuggingParentToFixed: SetHug
       hasAutoLayoutSiblings,
       autoLayoutSiblingsBounds,
       originalTargets.length > 1,
+      isPositionRelative,
     )
 
     return {
@@ -264,13 +273,14 @@ function convertToAbsoluteAndMoveStrategyFactory(setHuggingParentToFixed: SetHug
 }
 
 const BaseWeight = 0.5
-const DragConversionWeight = 1.5
+const DragConversionWeight = 1.5 // needs to be higher then FlexReorderFitness in flex-reorder-strategy
 
 function getFitness(
   interactionSession: InteractionSession | null,
   hasAutoLayoutSiblings: boolean,
   autoLayoutSiblingsBounds: CanvasRectangle | null,
   multipleTargets: boolean,
+  isPositionRelative: boolean,
 ): number {
   if (
     interactionSession != null &&
@@ -287,8 +297,9 @@ function getFitness(
     }
 
     if (!hasAutoLayoutSiblings) {
-      if (multipleTargets) {
+      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
@@ -302,7 +313,7 @@ function getFitness(
     const isInsideBoundingBoxOfSiblings =
       autoLayoutSiblingsBounds != null && rectContainsPoint(autoLayoutSiblingsBounds, pointOnCanvas)
 
-    return isInsideBoundingBoxOfSiblings ? BaseWeight : DragConversionWeight
+    return isInsideBoundingBoxOfSiblings || isPositionRelative ? BaseWeight : DragConversionWeight
   }
 
   return 0

diff --git a/editor/src/components/canvas/canvas-strategies/strategies/flex-reorder-strategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/flex-reorder-strategy.tsx
@@ -53,6 +53,8 @@ export function flexReorderStrategy(
     return null
   }
 
+  const FlexReorderFitness = 1.4 // needs to be higher than WeightWithoutExistingOffset in relative-move-strategy, but lower than DragConversionWeight in convert-to-absolute-and-move-strategy
+
   const parentFlexDirection = element?.specialSizeMeasurements.parentFlexDirection
   const reorderDirection = parentFlexDirection === 'column' ? 'vertical' : 'horizontal'
   return {
@@ -88,7 +90,7 @@ export function flexReorderStrategy(
         interactionSession != null &&
         interactionSession.interactionData.type === 'DRAG' &&
         interactionSession.activeControl.type === 'BOUNDING_AREA'
-          ? 1
+          ? FlexReorderFitness
           : 0,
       apply: () => {
         return interactionSession == null

diff --git a/...c/components/canvas/canvas-strategies/strategies/relative-move-strategy.spec.browser2.tsx b/...c/components/canvas/canvas-strategies/strategies/relative-move-strategy.spec.browser2.tsx
@@ -1,7 +1,12 @@
 import type { Modifiers } from '../../../../utils/modifiers'
 import { cmdModifier, shiftModifier } from '../../../../utils/modifiers'
 import { CanvasControlsContainerID } from '../../controls/new-canvas-controls'
-import { mouseClickAtPoint, mouseDragFromPointWithDelta } from '../../event-helpers.test-utils'
+import {
+  mouseClickAtPoint,
+  mouseDownAtPoint,
+  mouseDragFromPointWithDelta,
+  mouseMoveToPoint,
+} from '../../event-helpers.test-utils'
 import type { EditorRenderResult } from '../../ui-jsx.test-utils'
 import {
   getPrintedUiJsCode,
@@ -108,6 +113,51 @@ describe('Relative move', () => {
         `),
         )
       })
+
+      it('in flex parent flex reorder wins', async () => {
+        const project = makeTestProjectCodeWithSnippet(`
+        <div style={{ display: 'flex', width: '100%', height: '100%' }} data-uid='foo'>
+          <div
+            style={{
+              backgroundColor: '#f0f',
+              position: 'relative',
+              width: 200,
+              height: 200,
+            }}
+            data-uid='bar'
+            data-testid='bar'
+          />
+          <div
+            style={{
+              backgroundColor: '#f0f',
+              position: 'relative',
+              width: 200,
+              height: 200,
+            }}
+            data-uid='bar2'
+            data-testid='bar2'
+          />
+        </div>
+      `)
+
+        const renderResult = await renderTestEditorWithCode(project, 'await-first-dom-report')
+
+        const targetElement = renderResult.renderedDOM.getByTestId('bar')
+        const targetElementBounds = targetElement.getBoundingClientRect()
+        const canvasControlsLayer = renderResult.renderedDOM.getByTestId(CanvasControlsContainerID)
+
+        const startPoint = { x: targetElementBounds.x + 5, y: targetElementBounds.y + 5 }
+
+        await mouseDownAtPoint(canvasControlsLayer, startPoint, { modifiers: cmdModifier })
+
+        await mouseMoveToPoint(canvasControlsLayer, {
+          x: startPoint.x + 10,
+          y: startPoint.y + 10,
+        })
+
+        const activeStrategy = renderResult.getEditorState().strategyState.currentStrategy
+        expect(activeStrategy).toEqual('FLEX_REORDER')
+      })
     })
 
     describe('when the element has offsets', () => {

diff --git a/editor/src/components/canvas/canvas-strategies/strategies/relative-move-strategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/relative-move-strategy.tsx
@@ -45,6 +45,20 @@ export function relativeMoveStrategy(
     offsets != null &&
     (offsets.left != null || offsets.top != null || offsets.right != null || offsets.bottom != null)
 
+  const WeightWithExistingOffset = 4
+  const WeightWithoutExistingOffset = 1.3 // this needs to be lower than DragConversionWeight in convert-to-absolute-and-move-strategy.tsx and FlexReorderFitness in flex-reorder-strategy.tsx
+
+  const fitness = (() => {
+    if (
+      interactionSession == null ||
+      interactionSession.interactionData.type !== 'DRAG' ||
+      interactionSession.activeControl.type !== 'BOUNDING_AREA'
+    ) {
+      return 0
+    }
+    return hasOffsets ? WeightWithExistingOffset : WeightWithoutExistingOffset
+  })()
+
   return {
     strategy: {
       id: 'RELATIVE_MOVE',
@@ -68,13 +82,7 @@ export function relativeMoveStrategy(
           show: 'visible-only-while-active',
         }),
       ],
-      fitness:
-        interactionSession != null &&
-        interactionSession.interactionData.type === 'DRAG' &&
-        interactionSession.activeControl.type === 'BOUNDING_AREA'
-          ? 4
-          : 0,
-
+      fitness: fitness,
       apply: () => {
         if (
           interactionSession != null &&