Skip to content

Commit

Permalink
Fix getGridRelativeContainingBlock for flow elements (#6713)
Browse files Browse the repository at this point in the history
**Problem:**

`getGridRelativeContainingBlock` doesn't correctly return the item
position for flow elements. It was temporarily patched with the
positioning coming from `getGridChildCellCoordBoundsFromCanvas`, but
this PR instead fixes it for good, which cascade-solves a bunch of other
small bugs.

**Fix:**

The flow positioning requires _all_ preceding grid items to be available
in the grid, so this PR does just that: inject every (ordered!) child
into the temporary grid, retrieve the wanted one, and return its
bounding box.

**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

Fixes #6712
  • Loading branch information
ruggi authored and liady committed Dec 13, 2024
1 parent b7ea4af commit 8859734
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,8 @@ import {
isAutoGridPin,
getCommandsForGridItemPlacement,
sortElementsByGridPosition,
getGridRelativeContainingBlock,
} from './grid-helpers'
import type { GridCellCoordinates } from './grid-cell-bounds'
import {
canvasRectangle,
offsetPoint,
offsetRect,
rectContainsPoint,
zeroCanvasRect,
} from '../../../../core/shared/math-utils'

export const gridChangeElementLocationStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from '../../../../core/shared/element-template'
import {
localRectangle,
zeroRectangle,
zeroRectIfNullOrInfinity,
type CanvasRectangle,
type LocalRectangle,
Expand Down Expand Up @@ -750,8 +751,8 @@ const TemporaryGridID = 'temporary-grid'

export function getGridRelativeContainingBlock(
gridMetadata: ElementInstanceMetadata,
cellMetadata: ElementInstanceMetadata,
cellProperties: GridElementProperties,
gridElements: ElementInstanceMetadata[],
targetElement: ElementPath,
options?: {
forcePositionRelative?: boolean
},
Expand Down Expand Up @@ -828,47 +829,68 @@ export function getGridRelativeContainingBlock(
}
offsetContainer.appendChild(gridElement)

// Create a child of the grid element with the appropriate properties.
const gridChildElement = document.createElement('div')
function addItemToGrid(measurements: SpecialSizeMeasurements) {
// Create a child of the grid element with the appropriate properties.
const gridChildElement = document.createElement('div')

if (cellProperties.gridColumnStart != null) {
gridChildElement.style.gridColumnStart = printPinAsString(
gridProperties,
cellProperties.gridColumnStart,
'column',
)
}
if (cellProperties.gridColumnEnd != null) {
gridChildElement.style.gridColumnEnd = printPinAsString(
gridProperties,
cellProperties.gridColumnEnd,
'column',
)
}
if (cellProperties.gridRowStart != null) {
gridChildElement.style.gridRowStart = printPinAsString(
gridProperties,
cellProperties.gridRowStart,
'row',
)
}
if (cellProperties.gridRowEnd != null) {
gridChildElement.style.gridRowEnd = printPinAsString(
gridProperties,
cellProperties.gridRowEnd,
'row',
)
const { elementGridProperties, position } = measurements

if (elementGridProperties.gridColumnStart != null) {
gridChildElement.style.gridColumnStart = printPinAsString(
gridProperties,
elementGridProperties.gridColumnStart,
'column',
)
}
if (elementGridProperties.gridColumnEnd != null) {
gridChildElement.style.gridColumnEnd = printPinAsString(
gridProperties,
elementGridProperties.gridColumnEnd,
'column',
)
}
if (elementGridProperties.gridRowStart != null) {
gridChildElement.style.gridRowStart = printPinAsString(
gridProperties,
elementGridProperties.gridRowStart,
'row',
)
}
if (elementGridProperties.gridRowEnd != null) {
gridChildElement.style.gridRowEnd = printPinAsString(
gridProperties,
elementGridProperties.gridRowEnd,
'row',
)
}
gridChildElement.style.position = options?.forcePositionRelative
? 'relative'
: position ?? 'initial'
// Fill out the entire space available.
gridChildElement.style.top = '0'
gridChildElement.style.left = '0'
gridChildElement.style.bottom = '0'
gridChildElement.style.right = '0'

return gridChildElement
}
gridChildElement.style.position = options?.forcePositionRelative
? 'relative'
: cellMetadata.specialSizeMeasurements.position ?? 'initial'
// Fill out the entire space available.
gridChildElement.style.top = '0'
gridChildElement.style.left = '0'
gridChildElement.style.bottom = '0'
gridChildElement.style.right = '0'

gridElement.appendChild(gridChildElement)
// add every element into the grid, so flow elements will be correctly positioned
const createdElements = gridElements.map((element) =>
addItemToGrid(element.specialSizeMeasurements),
)
createdElements.forEach((elem) => gridElement.appendChild(elem))

// get the index of the generated target element
const targetIndex = gridElements.findIndex((element) =>
EP.pathsEqual(element.elementPath, targetElement),
)
if (targetIndex < 0 || targetIndex >= createdElements.length) {
// this should never happen
return localRectangle(zeroRectangle)
}
// grab the generated element via its index
const gridChildElement = createdElements[targetIndex]

// Get the result and cleanup the temporary elements.
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,34 +1,29 @@
import type { ElementPath } from 'utopia-shared/src/types'
import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import * as EP from '../../../../core/shared/element-path'
import type { JSXElementChild } from '../../../../core/shared/element-template'
import type {
JSXElementChild,
SpecialSizeMeasurements,
} from '../../../../core/shared/element-template'
import {
isJSXElement,
type ElementInstanceMetadata,
type ElementInstanceMetadataMap,
type GridContainerProperties,
} from '../../../../core/shared/element-template'
import type { CanvasRectangle, CanvasVector } from '../../../../core/shared/math-utils'
import type { CanvasRectangle } from '../../../../core/shared/math-utils'
import {
canvasRectangle,
canvasRectangleToLocalRectangle,
isNotNullFiniteRectangle,
nullIfInfinity,
offsetPoint,
offsetRect,
rectangleContainsRectangleInclusive,
rectContainsPoint,
windowPoint,
zeroCanvasRect,
zeroRectIfNullOrInfinity,
zeroSize,
} from '../../../../core/shared/math-utils'
import type { CanvasCommand } from '../../commands/commands'
import { showGridControls } from '../../commands/show-grid-controls-command'
import {
controlsForGridPlaceholders,
GridElementChildContainingBlockKey,
} from '../../controls/grid-controls-for-strategies'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import type { CanvasStrategyFactory } from '../canvas-strategies'
import { onlyFitWhenDraggingThisControl } from '../canvas-strategies'
import type { InteractionCanvasState } from '../canvas-strategy-types'
Expand Down Expand Up @@ -57,8 +52,6 @@ import { getMoveCommandsForDrag } from './shared-move-strategies-helpers'
import { toFirst } from '../../../../core/shared/optics/optic-utilities'
import { eitherRight, fromTypeGuard } from '../../../../core/shared/optics/optic-creators'
import { defaultEither } from '../../../../core/shared/either'
import { forceNotNull } from '../../../..//core/shared/optional-utils'
import { windowToCanvasCoordinates } from '../../dom-lookup'

export const gridMoveAbsoluteStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
Expand Down Expand Up @@ -197,8 +190,8 @@ export function getNewGridElementPropsCheckingOriginalGrid(
// Identify the containing block position and size.
const originalContainingBlockRectangle = getGridRelativeContainingBlock(
originalGridMetadata,
selectedElementMetadata,
selectedElementMetadata.specialSizeMeasurements.elementGridProperties,
[selectedElementMetadata],
selectedElementMetadata.elementPath,
)

// Capture the original position of the grid child.
Expand Down Expand Up @@ -347,11 +340,21 @@ function runGridMoveAbsolute(
)

// Get the containing block of the grid child.
const patchedSpecialSizeMeasurements: SpecialSizeMeasurements = {
...selectedElementMetadata.specialSizeMeasurements,
elementGridProperties:
newGridElementProps?.gridElementProperties ??
selectedElementMetadata.specialSizeMeasurements.elementGridProperties,
}
const containingBlockRectangle = getGridRelativeContainingBlock(
originalGrid,
selectedElementMetadata,
newGridElementProps?.gridElementProperties ??
selectedElementMetadata.specialSizeMeasurements.elementGridProperties,
[
{
...selectedElementMetadata,
specialSizeMeasurements: patchedSpecialSizeMeasurements,
},
],
selectedElementMetadata.elementPath,
)

// Get the appropriately shifted and typed local frame value to use.
Expand Down
6 changes: 5 additions & 1 deletion editor/src/components/canvas/controls/grid-controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2261,7 +2261,11 @@ const RulerMarkers = React.memo((props: RulerMarkersProps) => {
}

const parentGrid = elementMetadata.specialSizeMeasurements.parentContainerGridProperties
const cellRect = calculateGridCellRectangle(store.editor.jsxMetadata, props.path)
const cellRect = calculateGridCellRectangle(
store.editor.jsxMetadata,
store.editor.elementPathTree,
props.path,
)
if (cellRect == null) {
return null
}
Expand Down
96 changes: 12 additions & 84 deletions editor/src/components/canvas/controls/grid-measurements.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { applicative4Either, defaultEither, isRight, mapEither } from '../../../
import * as EP from '../../../core/shared/element-path'
import type {
BorderWidths,
ElementInstanceMetadata,
ElementInstanceMetadataMap,
GridAutoOrTemplateBase,
GridElementProperties,
Expand All @@ -28,8 +27,6 @@ import type { GridIdentifier } from '../../editor/store/editor-state'
import { Substores, useEditorState } from '../../editor/store/store-hook'
import { useMonitorChangesToElements } from '../../editor/store/store-monitor'
import { isStaticGridRepeat, parseCSSLength } from '../../inspector/common/css-utils'
import { getGridChildCellCoordBoundsFromCanvas } from '../canvas-strategies/strategies/grid-cell-bounds'
import type { GridCellGlobalFrames } from '../canvas-strategies/strategies/grid-helpers'
import {
findOriginalGrid,
getGridRelativeContainingBlock,
Expand All @@ -42,6 +39,7 @@ import {
getGridElementProperties,
} from '../dom-walker'
import { addChangeCallback, removeChangeCallback } from '../observers'
import type { ElementPathTrees } from '../../../core/shared/element-path-tree'

export type GridElementMeasurementHelperData = {
frame: CanvasRectangle
Expand Down Expand Up @@ -357,42 +355,16 @@ export function useGridElementMeasurementHelperData(
return useKeepReferenceEqualityIfPossible(getGridElementMeasurementHelperData(elementPath, scale))
}

function getCellPositionFromCanvas(
elementMetadata: ElementInstanceMetadata,
parentGridCellGlobalFrames: GridCellGlobalFrames,
): { left: number; top: number } | null {
const cellCoordBoundsFromCanvas = getGridChildCellCoordBoundsFromCanvas(
elementMetadata,
parentGridCellGlobalFrames,
)
if (cellCoordBoundsFromCanvas == null) {
return null
}

if (parentGridCellGlobalFrames.length === 0) {
return null
}

const firstRow = parentGridCellGlobalFrames[0]
const left = firstRow[cellCoordBoundsFromCanvas.column - 1].x

const cellBoundsRowIndex = cellCoordBoundsFromCanvas.row - 1
if (
parentGridCellGlobalFrames.length <= cellBoundsRowIndex ||
parentGridCellGlobalFrames[cellBoundsRowIndex].length === 0
) {
export function calculateGridCellRectangle(
jsxMetadata: ElementInstanceMetadataMap,
pathTree: ElementPathTrees,
path: ElementPath,
): CanvasRectangle | null {
const cellItemMetadata = MetadataUtils.findElementByElementPath(jsxMetadata, path)
if (cellItemMetadata == null) {
return null
}
const firstColumn = parentGridCellGlobalFrames[cellBoundsRowIndex][0]
const top = firstColumn.y

return { left, top }
}

function getCellDimensions(
jsxMetadata: ElementInstanceMetadataMap,
cellItemMetadata: ElementInstanceMetadata,
): { width: number; height: number } | null {
const originalGrid = findOriginalGrid(jsxMetadata, EP.parentPath(cellItemMetadata.elementPath))
if (originalGrid == null) {
return null
Expand All @@ -406,54 +378,10 @@ function getCellDimensions(
const coordinateSystemBounds =
cellItemMetadata.specialSizeMeasurements.immediateParentBounds ?? zeroCanvasRect

const calculatedCellBounds = getGridRelativeContainingBlock(
gridMetadata,
cellItemMetadata,
cellItemMetadata.specialSizeMeasurements.elementGridProperties,
{
forcePositionRelative: true,
},
)
const cellRect = offsetRect(canvasRectangle(calculatedCellBounds), coordinateSystemBounds)
const { width, height } = cellRect

return { width, height }
}

/**
* Returns the _actual_ canvas rectangle for a grid _cell_ (not its contained item) that's rendered on the canvas.
* This is done by combining grabbing the position from the parent grid frames (which is required for flow elements!),
* with the HTML calculation of dimensions in getGridRelativeContainingBlock.
*/
export function calculateGridCellRectangle(
jsxMetadata: ElementInstanceMetadataMap,
path: ElementPath,
): CanvasRectangle | null {
const cellItemMetadata = MetadataUtils.findElementByElementPath(jsxMetadata, path)
if (cellItemMetadata == null) {
return null
}

const parentGridCellGlobalFrames =
cellItemMetadata.specialSizeMeasurements.parentGridCellGlobalFrames
if (parentGridCellGlobalFrames == null) {
return null
}

const position = getCellPositionFromCanvas(cellItemMetadata, parentGridCellGlobalFrames)
if (position == null) {
return null
}

const dimensions = getCellDimensions(jsxMetadata, cellItemMetadata)
if (dimensions == null) {
return null
}
const siblings = MetadataUtils.getSiblingsOrdered(jsxMetadata, pathTree, path)

return canvasRectangle({
x: position.left,
y: position.top,
width: dimensions.width,
height: dimensions.height,
const calculatedCellBounds = getGridRelativeContainingBlock(gridMetadata, siblings, path, {
forcePositionRelative: true,
})
return offsetRect(canvasRectangle(calculatedCellBounds), coordinateSystemBounds)
}

0 comments on commit 8859734

Please sign in to comment.