Skip to content

Commit

Permalink
Better grid resize element strategy (#6457)
Browse files Browse the repository at this point in the history
**Problem:**
The original grid-resize-element strategy was written in a way, that it
resized based on the data that which cell is currently under the mouse.
This made it impossible to resize with the mouse e.g. going in between
cells over the gap, or outside of the grid.

**Fix:**
Let's rely on the resize bounding box coming from
`resizeBoundingBoxFromSide`, and we can resize to those cells which have
an intersection with the resize bounding box.

As an extra, I refactored the `getMetadataWithGridCellBounds` function,
so that it returns the updated custom strategy state, and the strategies
don't have to construct that (this strategy state update guarantees that
if the grid cell bounds were only found in the latestmetadata, then we
store them in the custom strategy state, so they are kept until the end
of the interaction). For the note, some grid strategies add further
changes to the strategy state, after the subsequent strategy refactors
these will be removed.

**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
  • Loading branch information
gbalint authored Oct 3, 2024
1 parent b7c7f9e commit 0a538b7
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 161 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,13 @@ const gridDrawToInsertStrategyInner =
canvasState.propertyControlsInfo,
)?.newParent.intendedParentPath

const { metadata: parent, foundIn } = getMetadataWithGridCellBounds(
targetParent,
canvasState.startingMetadata,
interactionSession.latestMetadata,
customStrategyState,
)
const { metadata: parent, customStrategyState: updatedCustomState } =
getMetadataWithGridCellBounds(
targetParent,
canvasState.startingMetadata,
interactionSession.latestMetadata,
customStrategyState,
)

if (targetParent == null || parent == null || !MetadataUtils.isGridLayoutedContainer(parent)) {
return null
Expand All @@ -160,32 +161,17 @@ const gridDrawToInsertStrategyInner =
const newTargetCell = getGridCellUnderMouseFromMetadata(parent, canvasPointToUse)

if (strategyLifecycle === 'mid-interaction' && interactionData.type === 'HOVER') {
const customStatePatch =
foundIn === 'latestMetadata'
? {
...customStrategyState,
grid: {
...customStrategyState.grid,
// this is added here during the hover interaction so that
// `GridControls` can render the hover highlight based on the
// coordinates in `targetCellData`
targetCellData: newTargetCell ?? customStrategyState.grid.targetCellData,
metadataCacheForGrids: {
...customStrategyState.grid.metadataCacheForGrids,
[EP.toString(targetParent)]: parent,
},
},
}
: {
...customStrategyState,
grid: {
...customStrategyState.grid,
// this is added here during the hover interaction so that
// `GridControls` can render the hover highlight based on the
// coordinates in `targetCellData`
targetCellData: newTargetCell ?? customStrategyState.grid.targetCellData,
},
}
const baseCustomState = updatedCustomState ?? customStrategyState
const customStatePatch = {
...baseCustomState,
grid: {
...baseCustomState.grid,
// this is added here during the hover interaction so that
// `GridControls` can render the hover highlight based on the
// coordinates in `targetCellData`
targetCellData: newTargetCell ?? customStrategyState.grid.targetCellData,
},
}
return strategyApplicationResult(
[
wildcardPatch('mid-interaction', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -774,12 +774,12 @@ export function getMetadataWithGridCellBounds(
customStrategyState: CustomStrategyState,
): {
metadata: ElementInstanceMetadata | null
foundIn: 'startingMetadata' | 'latestMetadata' | 'strategyState' | null
customStrategyState: CustomStrategyState | null
} {
if (path == null) {
return {
metadata: null,
foundIn: null,
customStrategyState: null,
}
}

Expand All @@ -788,28 +788,37 @@ export function getMetadataWithGridCellBounds(
if (fromStartingMetadata?.specialSizeMeasurements.gridCellGlobalFrames != null) {
return {
metadata: fromStartingMetadata,
foundIn: 'startingMetadata',
customStrategyState: null,
}
}

const fromStrategyState = customStrategyState.grid.metadataCacheForGrids[EP.toString(path)]
if (fromStrategyState != null) {
return {
metadata: fromStrategyState,
foundIn: 'strategyState',
customStrategyState: null,
}
}

const fromLatestMetadata = MetadataUtils.findElementByElementPath(latestMetadata, path)
if (fromLatestMetadata?.specialSizeMeasurements.gridCellGlobalFrames != null) {
return {
metadata: fromLatestMetadata,
foundIn: 'latestMetadata',
customStrategyState: {
...customStrategyState,
grid: {
...customStrategyState.grid,
metadataCacheForGrids: {
...customStrategyState.grid.metadataCacheForGrids,
[EP.toString(path)]: fromLatestMetadata,
},
},
},
}
}

return {
metadata: fromStartingMetadata,
foundIn: 'startingMetadata',
customStrategyState: null,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,13 @@ export function applyGridReparent(
return emptyStrategyApplicationResult
}

const { metadata: grid, foundIn } = getMetadataWithGridCellBounds(
newParent.intendedParentPath,
canvasState.startingMetadata,
interactionSession.latestMetadata,
customStrategyState,
)
const { metadata: grid, customStrategyState: updatedCustomState } =
getMetadataWithGridCellBounds(
newParent.intendedParentPath,
canvasState.startingMetadata,
interactionSession.latestMetadata,
customStrategyState,
)

if (grid == null) {
return emptyStrategyApplicationResult
Expand Down Expand Up @@ -241,26 +242,15 @@ export function applyGridReparent(
...selectedElements.map(EP.parentPath),
])

const customStrategyStatePatch =
foundIn === 'latestMetadata'
? {
elementsToRerender: elementsToRerender,
grid: {
...customStrategyState.grid,
targetCellData: targetCellData,
metadataCacheForGrids: {
...customStrategyState.grid.metadataCacheForGrids,
[EP.toString(newParent.intendedParentPath)]: grid,
},
},
}
: {
elementsToRerender: elementsToRerender,
grid: {
...customStrategyState.grid,
targetCellData: targetCellData,
},
}
const baseCustomState = updatedCustomState ?? customStrategyState
const customStrategyStatePatch = {
...baseCustomState,
elementsToRerender: elementsToRerender,
grid: {
...baseCustomState.grid,
targetCellData: targetCellData,
},
}

return strategyApplicationResult(
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import type {
} from 'utopia-shared/src/types'
import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import * as EP from '../../../../core/shared/element-path'
import { getRectCenter, localRectangle } from '../../../../core/shared/math-utils'
import {
getRectCenter,
localPoint,
type LocalPoint,
localRectangle,
offsetPoint,
} from '../../../../core/shared/math-utils'
import { selectComponentsForTest } from '../../../../utils/utils.test-utils'
import { GridResizeEdgeTestId } from '../../controls/grid-controls'
import { mouseDragFromPointToPoint } from '../../event-helpers.test-utils'
Expand Down Expand Up @@ -47,6 +53,34 @@ async function runCellResizeTest(
)
}

async function runCellResizeTestWithDragVector(
editor: EditorRenderResult,
edge: GridResizeEdge,
dragVector: LocalPoint,
elementPathToDrag: ElementPath = EP.fromString('sb/scene/grid/ddd'),
) {
await selectComponentsForTest(editor, [elementPathToDrag])

const resizeControl = editor.renderedDOM.getByTestId(GridResizeEdgeTestId(edge))

const resizeControlCenter = getRectCenter(
localRectangle({
x: resizeControl.getBoundingClientRect().x,
y: resizeControl.getBoundingClientRect().y,
width: resizeControl.getBoundingClientRect().width,
height: resizeControl.getBoundingClientRect().height,
}),
)
await mouseDragFromPointToPoint(
resizeControl,
resizeControlCenter,
offsetPoint(resizeControlCenter, dragVector),
{
moveBeforeMouseDown: true,
},
)
}

describe('grid resize element strategy', () => {
it('cannot resize non-filling cells', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')
Expand Down Expand Up @@ -215,6 +249,47 @@ describe('grid resize element strategy', () => {
})
})

it('can resize element with mouse move outside of grid cells', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')
await runCellResizeTest(
editor,
'column-end',
gridCellTargetId(EP.fromString('sb/scene/grid'), 1, 8),
)

{
const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } =
editor.renderedDOM.getByTestId('grid-child').style
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: '9',
gridColumnStart: '7',
gridRowEnd: 'auto',
gridRowStart: '2',
})
}

{
// moving a 2 cell wide element in the middle, over the gap between 2 cells
await runCellResizeTestWithDragVector(
editor,
'row-start',
localPoint({
x: 0,
y: -50,
}),
)

const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } =
editor.renderedDOM.getByTestId('grid-child').style
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: '9',
gridColumnStart: '7',
gridRowEnd: '3',
gridRowStart: '1',
})
}
})

it('removes the grid-area prop on resize', async () => {
const editor = await renderTestEditorWithCode(ProjectCodeWithGridArea, 'await-first-dom-report')

Expand Down
Loading

0 comments on commit 0a538b7

Please sign in to comment.