Better grid reparent (#6499)

**Problem:**
One of the root causes of some major issues is that the grid reparent
strategies were not created by the general reparent-metastrategy, but
the grid-rearrange-move strategy itself.

**Fix:**
- Removed reparent strategy creation from grid-rearrange-move. After
this grid-rearrange-move is always selectable when dragging from a grid,
even when outside of a grid (so you can manually switch to this strategy
and the child will jump back to the original grid, into the cell which
is closest to the mouse). This is a behavior which is similar to flex
reorder.
- reparent-metastrategy can create a reparent into grid strategy. To
achieve this I just had to allow `GRID_CELL_HANDLE` controls in the
strategy factory.
- Fortunately reparent-metastrategy does not allow reparenting into the
same parent, so this automagically solved the problem that reparenting
inside the original grid should just do a rearrange (because the
reparent strategy is not even selectable in that case)
- With this solution, when reparenting from one grid to a different
grid, the grid controls were shown on both the original and the new
parent grid too. Unfortunately, the grid controls do not support showing
the controls on multiple grid instances at the same time (e.g. they
shared the state that which cell was highlighted), so I had to switch
the grid control of the rearrange strategy to
`visible-only-while-active` (so it is not visible on the original grid
when the reparent strategy is active)
- I had to make sure the grid specific style props are removed during
all kinds of reparent (earlier this was manually handled in the grid
specific code)
- I could remove plenty of reparent specific code lines from
`grid-rearrange-move-strategy.ts`
- In the earlier solution reparent to flow was the default when
repareing into a flow target, but in our regular reparent flow reparent
is never the default. Because of this I had to rewrite one test to
select the flow strategy manually.

**Manual Tests:**
I hereby swear that:

- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Play mode

editor/src/components/canvas/canvas-strategies/strategies/drag-to-move-metastrategy.tsx

@@ -58,7 +58,8 @@ export const dragToMoveMetaStrategy: MetaCanvasStrategy = (
     selectedElements.length === 0 ||
     selectedElements.some(EP.isRootElementOfInstance) ||
     interactionSession == null ||
-    interactionSession.activeControl.type !== 'BOUNDING_AREA' ||
+    (interactionSession.activeControl.type !== 'BOUNDING_AREA' &&
+      interactionSession.activeControl.type !== 'GRID_CELL_HANDLE') ||
     interactionSession.interactionData.type !== 'DRAG' ||
     interactionSession.interactionData.modifiers.alt
   ) {

editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.ts

@@ -77,7 +77,6 @@ export const gridRearrangeMoveStrategy: CanvasStrategyFactory = (
   }
 
   const strategyToApply = getStrategyToApply(
-    canvasState,
     interactionSession.interactionData,
     parentGridPath,
     canvasState.startingMetadata,
@@ -130,23 +129,11 @@ export const gridRearrangeMoveStrategy: CanvasStrategyFactory = (
         ),
       ]
 
-      const { commands, patch, elementsToRerender } =
-        strategyToApply.type === 'GRID_REARRANGE'
-          ? getCommandsAndPatchForGridRearrange(
-              canvasState,
-              interactionSession.interactionData,
-              selectedElement,
-            )
-          : getCommandsAndPatchForReparent(
-              strategyToApply.strategy,
-              canvasState,
-              interactionSession.interactionData,
-              interactionSession,
-              customState,
-              selectedElement,
-              strategyLifecycle,
-              gridFrame,
-            )
+      const { commands, patch, elementsToRerender } = getCommandsAndPatchForGridRearrange(
+        canvasState,
+        interactionSession.interactionData,
+        selectedElement,
+      )
 
       if (commands.length === 0) {
         return emptyStrategyApplicationResult
@@ -188,102 +175,6 @@ function getCommandsAndPatchForGridRearrange(
   }
 }
 
-function getCommandsAndPatchForReparent(
-  strategy: FindReparentStrategyResult,
-  canvasState: InteractionCanvasState,
-  interactionData: DragInteractionData,
-  interactionSession: InteractionSession,
-  customState: CustomStrategyState,
-  targetElement: ElementPath,
-  strategyLifecycle: InteractionLifecycle,
-  gridFrame: CanvasRectangle,
-): {
-  commands: CanvasCommand[]
-  patch: CustomStrategyStatePatch
-  elementsToRerender: ElementPath[]
-} {
-  if (interactionData.drag == null) {
-    return { commands: [], patch: {}, elementsToRerender: [] }
-  }
-
-  function applyReparent() {
-    switch (strategy.strategy) {
-      case 'REPARENT_AS_ABSOLUTE':
-        return applyAbsoluteReparent(
-          canvasState,
-          interactionSession,
-          customState,
-          strategy.target,
-          [targetElement],
-        )(strategyLifecycle)
-      case 'REPARENT_AS_STATIC':
-        return applyStaticReparent(canvasState, interactionSession, customState, strategy.target)
-      case 'REPARENT_INTO_GRID':
-        return applyGridReparent(
-          canvasState,
-          interactionData,
-          interactionSession,
-          customState,
-          strategy.target,
-          [targetElement],
-          gridFrame,
-        )()
-      default:
-        assertNever(strategy.strategy)
-    }
-  }
-  const result = applyReparent()
-
-  let commands: CanvasCommand[] = []
-
-  const frame = MetadataUtils.getFrameOrZeroRect(targetElement, canvasState.startingMetadata)
-
-  if (strategy.strategy === 'REPARENT_AS_ABSOLUTE') {
-    // for absolute reparents, set positioning and size props
-    commands.push(
-      updateBulkProperties('always', targetElement, [
-        propertyToSet(PP.create('style', 'position'), 'absolute'),
-        propertyToSet(PP.create('style', 'top'), frame.y + interactionData.drag.y),
-        propertyToSet(PP.create('style', 'left'), frame.x + interactionData.drag.x),
-        propertyToSet(PP.create('style', 'width'), frame.width),
-        propertyToSet(PP.create('style', 'height'), frame.height),
-      ]),
-    )
-  } else if (strategy.strategy === 'REPARENT_AS_STATIC') {
-    // for static reparents, set size props
-    commands.push(
-      updateBulkProperties('always', targetElement, [
-        propertyToSet(PP.create('style', 'width'), frame.width),
-        propertyToSet(PP.create('style', 'height'), frame.height),
-      ]),
-    )
-  }
-
-  // for absolute, static, or non-same-grid reparents, remove cell placement props
-  if (
-    strategy.strategy !== 'REPARENT_INTO_GRID' ||
-    !EP.pathsEqual(strategy.target.newParent.intendedParentPath, EP.parentPath(targetElement))
-  ) {
-    commands.push(
-      updateBulkProperties('on-complete', targetElement, [
-        propertyToDelete(PP.create('style', 'gridRow')),
-        propertyToDelete(PP.create('style', 'gridColumn')),
-      ]),
-    )
-  }
-
-  commands.push(...result.commands)
-
-  return {
-    commands: commands,
-    patch: result.customStatePatch,
-    elementsToRerender: [
-      EP.parentPath(targetElement),
-      strategy.target.newParent.intendedParentPath,
-    ],
-  }
-}
-
 function restoreGridTemplateFromProps(params: {
   columns: GridAutoOrTemplateBase
   rows: GridAutoOrTemplateBase
@@ -342,21 +233,12 @@ function getGridTemplates(jsxMetadata: ElementInstanceMetadataMap, gridPath: Ele
   }
 }
 
-type StrategyToApply =
-  | {
-      type: 'GRID_REARRANGE'
-      controlsToRender: ControlWithProps<any>[]
-      name: string
-    }
-  | {
-      type: 'REPARENT'
-      controlsToRender: ControlWithProps<any>[]
-      name: string
-      strategy: FindReparentStrategyResult
-    }
+type StrategyToApply = {
+  controlsToRender: ControlWithProps<any>[]
+  name: string
+}
 
 function getStrategyToApply(
-  canvasState: InteractionCanvasState,
   interactionData: DragInteractionData,
   parentGridPath: ElementPath,
   jsxMetadata: ElementInstanceMetadataMap,
@@ -366,46 +248,6 @@ function getStrategyToApply(
     return null
   }
 
-  const shouldReparent = interactionData.modifiers.cmd
-  if (shouldReparent) {
-    const pointOnCanvas = offsetPoint(interactionData.originalDragStart, interactionData.drag)
-    const reparentStrategies = findReparentStrategies(
-      canvasState,
-      true,
-      pointOnCanvas,
-      'allow-smaller-parent',
-    )
-
-    const strategy = reparentStrategies[0]
-    if (strategy != null) {
-      switch (strategy.strategy) {
-        case 'REPARENT_AS_ABSOLUTE':
-          return {
-            type: 'REPARENT',
-            name: 'Reparent (Abs)',
-            controlsToRender: controlsForAbsoluteReparent(strategy.target),
-            strategy: strategy,
-          }
-        case 'REPARENT_AS_STATIC':
-          return {
-            type: 'REPARENT',
-            name: 'Reparent (Flex)',
-            controlsToRender: controlsForStaticReparent(strategy.target),
-            strategy: strategy,
-          }
-        case 'REPARENT_INTO_GRID':
-          return {
-            type: 'REPARENT',
-            name: 'Reparent (Grid)',
-            controlsToRender: controlsForGridReparent(strategy.target),
-            strategy: strategy,
-          }
-        default:
-          assertNever(strategy.strategy)
-      }
-    }
-  }
-
   const element = MetadataUtils.findElementByElementPath(jsxMetadata, cell)
 
   const name =
@@ -415,8 +257,7 @@ function getStrategyToApply(
       : 'Rearrange Grid (Move)'
 
   return {
-    type: 'GRID_REARRANGE',
     name: name,
-    controlsToRender: [controlsForGridPlaceholders(parentGridPath)],
+    controlsToRender: [controlsForGridPlaceholders(parentGridPath, 'visible-only-while-active')],
   }
 }

editor/src/components/canvas/canvas-strategies/strategies/grid-reparent-strategies.spec.browser2.tsx

@@ -14,6 +14,7 @@ import {
   mouseDragFromPointToPoint,
   mouseMoveToPoint,
   mouseUpAtPoint,
+  pressKey,
 } from '../../event-helpers.test-utils'
 import type { EditorRenderResult } from '../../ui-jsx.test-utils'
 import {
@@ -311,8 +312,8 @@ describe('grid reparent strategies', () => {
             width: 79,
             height: 86,
             position: 'absolute',
-            top: 934,
             left: 1627,
+            top: 934,
           }}
           data-uid='dragme'
           data-testid='dragme'
@@ -451,7 +452,7 @@ describe('grid reparent strategies', () => {
         ),
       )
     })
-    it('into a flow element', async () => {
+    it('into a flow element with flow strategy selected', async () => {
       const editor = await renderTestEditorWithCode(
         makeTestProjectCode({
           insideGrid: `
@@ -519,7 +520,9 @@ describe('grid reparent strategies', () => {
         y: fooRect.y + fooRect.height / 2,
       }
 
-      await dragOut(editor, EP.fromString('sb/grid/dragme'), endPoint)
+      await dragOut(editor, EP.fromString('sb/grid/dragme'), endPoint, async () => {
+        await pressKey('3', { modifiers: cmdModifier }) // this should select the Reparent (Flow) strategy
+      })
 
       expect(getPrintedUiJsCode(editor.getEditorState())).toEqual(
         formatTestProjectCode(
@@ -755,11 +758,11 @@ describe('grid reparent strategies', () => {
         <div
           style={{
             backgroundColor: '#f0f',
+            width: 87,
+            height: 135,
             position: 'absolute',
-            top: 934,
             left: 1627,
-            width: 86.5,
-            height: 135,
+            top: 934,
           }}
           data-uid='dragme'
           data-testid='dragme'
@@ -827,6 +830,7 @@ async function dragOut(
   renderResult: EditorRenderResult,
   cell: ElementPath,
   endPoint: Point,
+  midDragCallback?: () => Promise<void>,
 ): Promise<void> {
   const sourceGridCell = renderResult.renderedDOM.getByTestId(GridCellTestId(cell))
   const sourceRect = sourceGridCell.getBoundingClientRect()
@@ -857,6 +861,9 @@ async function dragOut(
     },
     modifiers: cmdModifier,
   })
+  if (midDragCallback != null) {
+    await midDragCallback()
+  }
   await mouseUpAtPoint(renderResult.renderedDOM.getByTestId(CanvasControlsContainerID), endPoint, {
     modifiers: cmdModifier,
   })

editor/src/components/canvas/canvas-strategies/strategies/grid-reparent-strategy.tsx

@@ -45,11 +45,7 @@ import { removeAbsolutePositioningProps } from './reparent-helpers/reparent-prop
 import type { ReparentTarget } from './reparent-helpers/reparent-strategy-helpers'
 import { getReparentOutcome, pathToReparent } from './reparent-utils'
 import { flattenSelection } from './shared-move-strategies-helpers'
-import {
-  getClosestGridCellToPointFromMetadata,
-  getGridCellUnderMouseFromMetadata,
-  type GridCellCoordinates,
-} from './grid-cell-bounds'
+import { getClosestGridCellToPointFromMetadata, type GridCellCoordinates } from './grid-cell-bounds'
 
 export function gridReparentStrategy(
   reparentTarget: ReparentTarget,

editor/src/components/canvas/canvas-strategies/strategies/reparent-helpers/reparent-property-changes.ts

@@ -73,6 +73,12 @@ const propertiesToRemove: Array<PropertyPath> = [
   PP.create('style', 'top'),
   PP.create('style', 'right'),
   PP.create('style', 'bottom'),
+  PP.create('style', 'gridRow'),
+  PP.create('style', 'gridColumn'),
+  PP.create('style', 'gridRowStart'),
+  PP.create('style', 'gridRowEnd'),
+  PP.create('style', 'gridColumnStart'),
+  PP.create('style', 'gridColumnEnd'),
 ]
 
 export type ForcePins = 'force-pins' | 'do-not-force-pins'

editor/src/components/canvas/canvas-strategies/strategies/reparent-metastrategy.tsx

@@ -274,7 +274,8 @@ export const reparentMetaStrategy: MetaCanvasStrategy = (
   if (
     reparentSubjects.length === 0 ||
     interactionSession == null ||
-    interactionSession.activeControl.type !== 'BOUNDING_AREA' ||
+    (interactionSession.activeControl.type !== 'BOUNDING_AREA' &&
+      interactionSession.activeControl.type !== 'GRID_CELL_HANDLE') ||
     interactionSession.interactionData.type !== 'DRAG' ||
     interactionSession.interactionData.drag == null ||
     interactionSession.interactionData.modifiers.alt

editor/src/components/canvas/controls/grid-controls.tsx

@@ -59,7 +59,10 @@ import { useColorTheme, UtopiaStyles } from '../../../uuiui'
 import { useDispatch } from '../../editor/store/dispatch-context'
 import { Substores, useEditorState, useRefEditorState } from '../../editor/store/store-hook'
 import CanvasActions from '../canvas-actions'
-import type { ControlWithProps } from '../canvas-strategies/canvas-strategy-types'
+import type {
+  ControlWithProps,
+  WhenToShowControl,
+} from '../canvas-strategies/canvas-strategy-types'
 import { controlForStrategyMemoized } from '../canvas-strategies/canvas-strategy-types'
 import type {
   GridResizeEdge,
@@ -1837,12 +1840,15 @@ function gridPlaceholderWidthOrHeight(scale: number): string {
   return `calc(100% + ${(placeholderBorderBaseWidth * 2) / scale}px)`
 }
 
-export function controlsForGridPlaceholders(gridPath: ElementPath): ControlWithProps<any> {
+export function controlsForGridPlaceholders(
+  gridPath: ElementPath,
+  whenToShow: WhenToShowControl = 'always-visible',
+): ControlWithProps<any> {
   return {
     control: GridControls,
     props: { targets: [gridPath] },
     key: GridControlsKey(gridPath),
-    show: 'always-visible',
+    show: whenToShow,
     priority: 'bottom',
   }
 }