Skip to content

Commit

Permalink
Reorder in grid reparent (#6552)
Browse files Browse the repository at this point in the history 
**Problem:**
We use grid-rearrange-move code after reparenting into a grid to
position the element into the target cell.
However, `reorder` mode of grid-rearrange-move is never activated, so we
set explicit gridRow/gridColumn props even when unnecessary.

**Fix:**
- Run the grid rearrange move commands after the reparent commands
- Add new optional parameter to `runGridRearrangeMove` called
`newPathAfterReparent` so it knows about the new path (end can use it in
the commands)
- Make the code to decide which mode (rearrange or reorder) work even
when the target is not originally in the new parent.

**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 @liady
gbalint authored and liady committed Dec 13, 2024
1 parent 03e697a commit 25c3ce8
Show file tree
Hide file tree
Showing 3 changed files with 221 additions and 89 deletions.
58 changes: 32 additions & 26 deletions editor/src/components/canvas/canvas-strategies/strategies/grid-helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ export function runGridRearrangeMove(
jsxMetadata: ElementInstanceMetadataMap,
interactionData: DragInteractionData,
grid: ElementInstanceMetadata,
newPathAfterReparent?: ElementPath,
): CanvasCommand[] {
if (interactionData.drag == null) {
return []
}

const isReparent = !EP.isParentOf(grid.elementPath, selectedElement)
const isReparent = newPathAfterReparent != null

const { gridCellGlobalFrames, containerGridProperties: gridTemplate } =
grid.specialSizeMeasurements
Expand Down Expand Up @@ -97,7 +98,9 @@ export function runGridRearrangeMove(
// get the new adjusted column
const column = Math.max(targetCellCoords.column - mouseCellPosInOriginalElement.column, 1)

const gridCellMoveCommands = setGridPropsCommands(targetElement, gridTemplate, {
const pathForCommands = newPathAfterReparent ?? targetElement // when reparenting, we want to use the new path for commands

const gridCellMoveCommands = setGridPropsCommands(pathForCommands, gridTemplate, {
gridColumnStart: gridPositionValue(column),
gridColumnEnd: gridPositionValue(column + originalCellBounds.height),
gridRowStart: gridPositionValue(row),
Expand Down Expand Up @@ -147,14 +150,12 @@ export function runGridRearrangeMove(
EP.pathsEqual(selectedElement, s.path),
)

const moveType =
originalElementMetadata == null
? 'rearrange'
: getGridMoveType({
originalElementMetadata: originalElementMetadata,
possiblyReorderIndex: possiblyReorderIndex,
cellsSortedByPosition: cellsSortedByPosition,
})
const moveType = getGridMoveType({
elementPath: targetElement,
originalElementMetadata: originalElementMetadata,
possiblyReorderIndex: possiblyReorderIndex,
cellsSortedByPosition: cellsSortedByPosition,
})

const updateGridControlsCommand = showGridControls(
'mid-interaction',
Expand All @@ -165,6 +166,9 @@ export function runGridRearrangeMove(

switch (moveType) {
case 'absolute': {
if (isReparent) {
return []
}
const absoluteMoveCommands = gridChildAbsoluteMoveCommands(
MetadataUtils.findElementByElementPath(jsxMetadata, targetElement),
MetadataUtils.getFrameOrZeroRectInCanvasCoords(grid.elementPath, jsxMetadata),
Expand All @@ -176,7 +180,7 @@ export function runGridRearrangeMove(
const targetRootCell = gridCellCoordinates(row, column)
const canvasRect = getGlobalFrameOfGridCell(grid, targetRootCell)
const absoluteMoveCommands =
canvasRect == null
canvasRect == null || isReparent
? []
: gridChildAbsoluteMoveCommands(
MetadataUtils.findElementByElementPath(jsxMetadata, targetElement),
Expand All @@ -188,16 +192,16 @@ export function runGridRearrangeMove(
...absoluteMoveCommands,
reorderElement(
'always',
selectedElement,
pathForCommands,
absolute(Math.max(indexInSortedCellsForRearrange, 0)),
),
updateGridControlsCommand,
]
}
case 'reorder': {
return [
reorderElement('always', selectedElement, absolute(possiblyReorderIndex)),
deleteProperties('always', selectedElement, [
reorderElement('always', pathForCommands, absolute(possiblyReorderIndex)),
deleteProperties('always', pathForCommands, [
PP.create('style', 'gridColumn'),
PP.create('style', 'gridRow'),
PP.create('style', 'gridColumnStart'),
Expand Down Expand Up @@ -451,12 +455,16 @@ type GridMoveType =
| 'absolute' // a regular absolute move, relative to the grid

function getGridMoveType(params: {
originalElementMetadata: ElementInstanceMetadata
elementPath: ElementPath
originalElementMetadata: ElementInstanceMetadata | null
possiblyReorderIndex: number
cellsSortedByPosition: SortableGridElementProperties[]
}): GridMoveType {
const specialSizeMeasurements = params.originalElementMetadata.specialSizeMeasurements
if (MetadataUtils.isPositionAbsolute(params.originalElementMetadata)) {
const specialSizeMeasurements = params.originalElementMetadata?.specialSizeMeasurements
if (
specialSizeMeasurements != null &&
MetadataUtils.isPositionAbsolute(params.originalElementMetadata)
) {
return MetadataUtils.hasNoGridCellPositioning(specialSizeMeasurements)
? 'absolute'
: 'rearrange'
Expand All @@ -465,11 +473,11 @@ function getGridMoveType(params: {
return 'rearrange'
}

const elementGridProperties = specialSizeMeasurements.elementGridProperties
const gridRowStart = gridPositionNumberValue(elementGridProperties.gridRowStart)
const gridColumnStart = gridPositionNumberValue(elementGridProperties.gridColumnStart)
const gridRowEnd = gridPositionNumberValue(elementGridProperties.gridRowEnd)
const gridColumnEnd = gridPositionNumberValue(elementGridProperties.gridColumnEnd)
const elementGridProperties = specialSizeMeasurements?.elementGridProperties
const gridRowStart = gridPositionNumberValue(elementGridProperties?.gridRowStart ?? null)
const gridColumnStart = gridPositionNumberValue(elementGridProperties?.gridColumnStart ?? null)
const gridRowEnd = gridPositionNumberValue(elementGridProperties?.gridRowEnd ?? null)
const gridColumnEnd = gridPositionNumberValue(elementGridProperties?.gridColumnEnd ?? null)

const isMultiCellChild =
(gridRowEnd != null && gridRowStart != null && gridRowEnd > gridRowStart + 1) ||
Expand All @@ -482,10 +490,8 @@ function getGridMoveType(params: {
// The first element is intrinsically in order, so try to adjust for that
if (params.possiblyReorderIndex === 0) {
const isTheOnlyChild = params.cellsSortedByPosition.length === 1
const isAlreadyTheFirstChild = EP.pathsEqual(
params.cellsSortedByPosition[0].path,
params.originalElementMetadata.elementPath,
)
const isAlreadyTheFirstChild =
EP.toUid(params.cellsSortedByPosition[0].path) === EP.toUid(params.elementPath)

const isAlreadyAtOrigin = gridRowStart === 1 && gridColumnStart === 1

Expand Down
236 changes: 179 additions & 57 deletions ...components/canvas/canvas-strategies/strategies/grid-reparent-strategies.spec.browser2.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { WindowPoint } from '../../../../core/shared/math-utils'
import { offsetPoint, windowPoint } from '../../../../core/shared/math-utils'
import type { Modifiers } from '../../../../utils/modifiers'
import { cmdModifier } from '../../../../utils/modifiers'
import { selectComponentsForTest } from '../../../../utils/utils.test-utils'
import { selectComponentsForTest, wait } from '../../../../utils/utils.test-utils'
import { GridCellTestId } from '../../controls/grid-controls-for-strategies'
import { CanvasControlsContainerID } from '../../controls/new-canvas-controls'
import type { Point } from '../../event-helpers.test-utils'
Expand Down Expand Up @@ -665,65 +665,187 @@ describe('grid reparent strategies', () => {
formatTestProjectCode(
makeTestProjectCode({
extraCode: `
<div
style={{
backgroundColor: '#aaaaaa33',
position: 'absolute',
left: 500,
top: 500,
padding: 10,
display: 'grid',
gap: 10,
gridTemplateRows: '1fr 1fr',
gridTemplateColumns: '1fr 1fr 1fr 1fr',
}}
data-uid='another-grid'
data-testid='another-grid'
>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
gridRow: 1,
gridColumn: 1,
}}
data-uid='foo'
data-testid='foo'
/>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
gridRow: 2,
gridColumn: 3,
}}
data-uid='bar'
data-testid='bar'
/>
<div
style={{
backgroundColor: '#f0f',
width: 79,
height: 86,
gridColumn: 1,
gridRow: 2,
}}
data-uid='dragme'
data-testid='dragme'
/>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
gridColumn: 2,
gridColumn: 2,
}}
data-uid='baz'
/>
</div>
`,
}),
),
)
})
it('into a grid container with reorder (no explicit gridRow/gridColumn props', async () => {
const editor = await renderTestEditorWithCode(
makeTestProjectCode({
insideGrid: `
<div
style={{
backgroundColor: '#aaaaaa33',
position: 'absolute',
left: 500,
top: 500,
padding: 10,
display: 'grid',
gap: 10,
gridTemplateRows: '1fr 1fr',
gridTemplateColumns: '1fr 1fr 1fr 1fr',
backgroundColor: '#f0f',
width: 79,
height: 86,
gridRow: 2,
gridColumn: 2,
}}
data-uid='another-grid'
data-testid='another-grid'
>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
gridRow: 1,
gridColumn: 1,
}}
data-uid='foo'
data-testid='foo'
/>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
gridRow: 2,
gridColumn: 3,
}}
data-uid='bar'
data-testid='bar'
/>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
gridColumn: 2,
gridColumn: 2,
}}
data-uid='baz'
/>
<div
style={{
backgroundColor: '#f0f',
width: 79,
height: 86,
gridColumn: 1,
gridRow: 2,
}}
data-uid='dragme'
data-testid='dragme'
/>
</div>
data-uid='dragme'
data-testid='dragme'
/>
`,
extraCode: `
<div
style={{
backgroundColor: '#aaaaaa33',
position: 'absolute',
left: 500,
top: 500,
padding: 10,
display: 'grid',
gap: 10,
gridTemplateRows: '1fr 1fr',
gridTemplateColumns: '1fr 1fr 1fr',
width: 317,
}}
data-uid='91c5676d-d826-423e-86d5-10b80717'
data-testid='another-grid'
>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
}}
data-uid='3995ebba-85b5-4293-964c-0c78fbe6'
data-testid='foo'
/>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
}}
data-uid='eacc03a2-05bd-4f77-a8b2-3ab490ec'
data-testid='bar'
/>
</div>
`,
}),
'await-first-dom-report',
)

await selectComponentsForTest(editor, [EP.fromString('sb/grid/dragme')])

await dragOutToAnotherGrid(
editor,
'another-grid',
{
x: 300,
y: 20,
},
EP.fromString('sb/grid/dragme'),
)

expect(getPrintedUiJsCode(editor.getEditorState())).toEqual(
formatTestProjectCode(
makeTestProjectCode({
extraCode: `
<div
style={{
backgroundColor: '#aaaaaa33',
position: 'absolute',
left: 500,
top: 500,
padding: 10,
display: 'grid',
gap: 10,
gridTemplateRows: '1fr 1fr',
gridTemplateColumns: '1fr 1fr 1fr',
width: 317,
}}
data-uid='91c5676d-d826-423e-86d5-10b80717'
data-testid='another-grid'
>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
}}
data-uid='3995ebba-85b5-4293-964c-0c78fbe6'
data-testid='foo'
/>
<div
style={{
backgroundColor: '#0ff',
width: 79,
height: 86,
}}
data-uid='eacc03a2-05bd-4f77-a8b2-3ab490ec'
data-testid='bar'
/>
<div
style={{
backgroundColor: '#f0f',
width: 79,
height: 86,
}}
data-uid='dragme'
data-testid='dragme'
/>
</div>
`,
}),
),
Expand Down
Loading

0 comments on commit 25c3ce8

Please sign in to comment.