Remove TargetGridCell from grid move (#6033)

**Problem:**

The target grid cell during the move strategy relies on a super ugly
singleton variable.

**Fix:**

Get rid of `TargetGridCell` and instead do the proper adjustments inside
the move strategy.

**Notes:**
- No behavioral changes
- The code can be improved further by moving hovering cell data into the
editor state (I added a comment in the code) but I left it out of this
PR and potentially do it incrementally.

editor/src/components/canvas/canvas-strategies/canvas-strategy-types.ts

@@ -11,6 +11,7 @@ import type { StrategyApplicationStatus } from './interaction-state'
 import type { ElementPathTrees } from '../../../core/shared/element-path-tree'
 import type { ActiveFrameAction } from '../commands/set-active-frames-command'
 import type { PropertyControlsInfo } from '../../custom-code/code-file'
+import type { GridCellCoordinates } from '../controls/grid-controls'
 
 // TODO: fill this in, maybe make it an ADT for different strategies
 export interface CustomStrategyState {
@@ -20,6 +21,7 @@ export interface CustomStrategyState {
   strategyGeneratedUidsCache: { [elementPath: string]: string | undefined }
   elementsToRerender: Array<ElementPath>
   action: ActiveFrameAction | null
+  targetGridCell: GridCellCoordinates | null
 }
 
 export type CustomStrategyStatePatch = Partial<CustomStrategyState>
@@ -32,6 +34,7 @@ export function defaultCustomStrategyState(): CustomStrategyState {
     strategyGeneratedUidsCache: {},
     elementsToRerender: [],
     action: null,
+    targetGridCell: null,
   }
 }
 

editor/src/components/canvas/canvas-strategies/strategies/grid-helpers.ts

@@ -0,0 +1,24 @@
+import type { WindowPoint } from '../../../../core/shared/math-utils'
+import type { GridCellCoordinates } from '../../controls/grid-controls'
+import { gridCellCoordinates } from '../../controls/grid-controls'
+
+export function getGridCellUnderMouse(
+  windowPoint: WindowPoint,
+): { id: string; coordinates: GridCellCoordinates } | null {
+  const cellsUnderMouse = document
+    .elementsFromPoint(windowPoint.x, windowPoint.y)
+    .filter((el) => el.id.startsWith(`gridcell-`))
+  if (cellsUnderMouse.length > 0) {
+    const cellUnderMouse = cellsUnderMouse[0]
+    const row = cellUnderMouse.getAttribute('data-grid-row')
+    const column = cellUnderMouse.getAttribute('data-grid-column')
+    return {
+      id: cellsUnderMouse[0].id,
+      coordinates: gridCellCoordinates(
+        row == null ? 0 : parseInt(row),
+        column == null ? 0 : parseInt(column),
+      ),
+    }
+  }
+  return null
+}

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

@@ -1,23 +1,27 @@
 import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
 import * as EP from '../../../../core/shared/element-path'
 import type { GridElementProperties } from '../../../../core/shared/element-template'
+import { offsetPoint } from '../../../../core/shared/math-utils'
 import { create } from '../../../../core/shared/property-path'
 import type { CanvasCommand } from '../../commands/commands'
 import { setProperty } from '../../commands/set-property-command'
-import { GridControls, TargetGridCell } from '../../controls/grid-controls'
+import { GridControls } from '../../controls/grid-controls'
+import { canvasPointToWindowPoint } from '../../dom-lookup'
 import type { CanvasStrategyFactory } from '../canvas-strategies'
 import { onlyFitWhenDraggingThisControl } from '../canvas-strategies'
-import type { InteractionCanvasState } from '../canvas-strategy-types'
+import type { CustomStrategyState, InteractionCanvasState } from '../canvas-strategy-types'
 import {
   getTargetPathsFromInteractionTarget,
   emptyStrategyApplicationResult,
   strategyApplicationResult,
 } from '../canvas-strategy-types'
 import type { InteractionSession } from '../interaction-state'
+import { getGridCellUnderMouse } from './grid-helpers'
 
 export const gridRearrangeMoveStrategy: CanvasStrategyFactory = (
   canvasState: InteractionCanvasState,
   interactionSession: InteractionSession | null,
+  customState: CustomStrategyState,
 ) => {
   const selectedElements = getTargetPathsFromInteractionTarget(canvasState.interactionTarget)
   if (selectedElements.length !== 1) {
@@ -52,7 +56,7 @@ export const gridRearrangeMoveStrategy: CanvasStrategyFactory = (
       },
     ],
     fitness: onlyFitWhenDraggingThisControl(interactionSession, 'GRID_CELL_HANDLE', 2),
-    apply: () => {
+    apply: (lc) => {
       if (
         interactionSession == null ||
         interactionSession.interactionData.type !== 'DRAG' ||
@@ -62,9 +66,24 @@ export const gridRearrangeMoveStrategy: CanvasStrategyFactory = (
         return emptyStrategyApplicationResult
       }
 
+      const mouseWindowPoint = canvasPointToWindowPoint(
+        offsetPoint(
+          interactionSession.interactionData.dragStart,
+          interactionSession.interactionData.drag,
+        ),
+        canvasState.scale,
+        canvasState.canvasOffset,
+      )
+
+      let targetCell = customState.targetGridCell ?? null
+      const cellUnderMouse = getGridCellUnderMouse(mouseWindowPoint)
+      if (cellUnderMouse != null) {
+        targetCell = cellUnderMouse.coordinates
+      }
+
       let commands: CanvasCommand[] = []
 
-      if (TargetGridCell.current.row > 0 && TargetGridCell.current.column > 0) {
+      if (targetCell != null && targetCell.row > 0 && targetCell.column > 0) {
         const metadata = MetadataUtils.findElementByElementPath(
           canvasState.startingMetadata,
           selectedElement,
@@ -88,31 +107,20 @@ export const gridRearrangeMoveStrategy: CanvasStrategyFactory = (
               'always',
               selectedElement,
               create('style', 'gridColumnStart'),
-              TargetGridCell.current.column,
+              targetCell.column,
             ),
             setProperty(
               'always',
               selectedElement,
               create('style', 'gridColumnEnd'),
-              Math.max(
-                TargetGridCell.current.column,
-                TargetGridCell.current.column + (gridColumnEnd - gridColumnStart),
-              ),
-            ),
-            setProperty(
-              'always',
-              selectedElement,
-              create('style', 'gridRowStart'),
-              TargetGridCell.current.row,
+              Math.max(targetCell.column, targetCell.column + (gridColumnEnd - gridColumnStart)),
             ),
+            setProperty('always', selectedElement, create('style', 'gridRowStart'), targetCell.row),
             setProperty(
               'always',
               selectedElement,
               create('style', 'gridRowEnd'),
-              Math.max(
-                TargetGridCell.current.row,
-                TargetGridCell.current.row + (gridRowEnd - gridRowStart),
-              ),
+              Math.max(targetCell.row, targetCell.row + (gridRowEnd - gridRowStart)),
             ),
           )
         }
@@ -122,7 +130,7 @@ export const gridRearrangeMoveStrategy: CanvasStrategyFactory = (
         return emptyStrategyApplicationResult
       }
 
-      return strategyApplicationResult(commands)
+      return strategyApplicationResult(commands, { targetGridCell: targetCell })
     },
   }
 }

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

@@ -46,17 +46,16 @@ import {
 import { windowToCanvasCoordinates } from '../dom-lookup'
 import { CanvasOffsetWrapper } from './canvas-offset-wrapper'
 import { useColorTheme } from '../../../uuiui'
+import { getGridCellUnderMouse } from '../canvas-strategies/strategies/grid-helpers'
 
 export const GridCellTestId = (elementPath: ElementPath) => `grid-cell-${EP.toString(elementPath)}`
 
-type GridCellCoordinates = { row: number; column: number }
+export type GridCellCoordinates = { row: number; column: number }
 
-function emptyGridCellCoordinates(): GridCellCoordinates {
-  return { row: 0, column: 0 }
+export function gridCellCoordinates(row: number, column: number): GridCellCoordinates {
+  return { row: row, column: column }
 }
 
-export let TargetGridCell = { current: emptyGridCellCoordinates() }
-
 function getCellsCount(template: GridAutoOrTemplateBase | null): number {
   if (template == null) {
     return 0
@@ -271,8 +270,6 @@ export const GridControls = controlForStrategyMemoized(() => {
   const startInteractionWithUid = React.useCallback(
     (params: { uid: string; row: number; column: number; frame: CanvasRectangle }) =>
       (event: React.MouseEvent) => {
-        TargetGridCell.current = emptyGridCellCoordinates()
-
         setInitialShadowFrame(params.frame)
 
         const start = windowToCanvasCoordinates(
@@ -763,16 +760,12 @@ function useMouseMove(activelyDraggingOrResizingCell: string | null) {
         setHoveringStart(null)
         return
       }
-      const cellsUnderMouse = document
-        .elementsFromPoint(e.clientX, e.clientY)
-        .filter((el) => el.id.startsWith(`gridcell-`))
-
-      if (cellsUnderMouse.length > 0) {
-        const cellUnderMouse = cellsUnderMouse[0]
-        const row = cellUnderMouse.getAttribute('data-grid-row')
-        const column = cellUnderMouse.getAttribute('data-grid-column')
-        TargetGridCell.current.row = row == null ? 0 : parseInt(row)
-        TargetGridCell.current.column = column == null ? 0 : parseInt(column)
+
+      // TODO most of this logic can be simplified consistently
+      // by moving the hovering cell info to the editor state, dispatched
+      // from the grid-rearrange-move-strategy logic.
+      const cellUnderMouse = getGridCellUnderMouse(windowPoint({ x: e.clientX, y: e.clientY }))
+      if (cellUnderMouse != null) {
         setHoveringCell(cellUnderMouse.id)
 
         const newMouseCanvasPosition = windowToCanvasCoordinates(

editor/src/components/editor/store/dispatch-strategies.spec.tsx

@@ -198,6 +198,7 @@ describe('interactionStart', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": null,
         "startingAllElementProps": Object {},
@@ -260,6 +261,7 @@ describe('interactionStart', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": null,
         "startingAllElementProps": Object {},
@@ -342,6 +344,7 @@ describe('interactionUpdate', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": Array [
           Object {
@@ -424,6 +427,7 @@ describe('interactionUpdate', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": null,
         "startingAllElementProps": Object {},
@@ -500,6 +504,7 @@ describe('interactionHardReset', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": Array [
           Object {
@@ -584,6 +589,7 @@ describe('interactionHardReset', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": null,
         "startingAllElementProps": Object {},
@@ -674,6 +680,7 @@ describe('interactionUpdate with user changed strategy', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": Array [
           Object {
@@ -759,6 +766,7 @@ describe('interactionUpdate with user changed strategy', () => {
           "escapeHatchActivated": false,
           "lastReorderIdx": null,
           "strategyGeneratedUidsCache": Object {},
+          "targetGridCell": null,
         },
         "sortedApplicableStrategies": null,
         "startingAllElementProps": Object {},