Skip to content

Commit

Permalink
Keep shorthand values when changing grid item location (#6626)
Browse files Browse the repository at this point in the history
**Problem:**

Changing a grid item location that spans multiple cells via a shorthand
(e.g. `gridColumn: '3 / 5'`) regressed and now the interaction causes
the element to shrink to 1-sized dimensions.

**Fix:**

When checking whether to keep the end pins, check whether they come from
a shorthand prop too.

Also, fix naming of variables when calculating the cell bounds so width
is related to cols and height is related to rows.

Fixes #6625
  • Loading branch information
ruggi authored Nov 8, 2024
1 parent 004cc05 commit c162a94
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ import type {
GridContainerProperties,
GridElementProperties,
} from '../../../../core/shared/element-template'
import {
getJSXAttribute,
gridPositionValue,
isJSXElement,
} from '../../../../core/shared/element-template'
import { gridPositionValue, isJSXElement } from '../../../../core/shared/element-template'
import { isInfinityRectangle } from '../../../../core/shared/math-utils'
import { absolute } from '../../../../utils/utils'
import type { CanvasCommand } from '../../commands/commands'
Expand Down Expand Up @@ -241,9 +237,9 @@ export function runGridChangeElementLocation(

const gridProps: Partial<GridElementProperties> = {
gridColumnStart: gridPositionValue(targetRootCell.column),
gridColumnEnd: gridPositionValue(targetRootCell.column + originalCellBounds.height),
gridColumnEnd: gridPositionValue(targetRootCell.column + originalCellBounds.width),
gridRowStart: gridPositionValue(targetRootCell.row),
gridRowEnd: gridPositionValue(targetRootCell.row + originalCellBounds.width),
gridRowEnd: gridPositionValue(targetRootCell.row + originalCellBounds.height),
}

// TODO: Remove this logic once there is a fix for the handling of the track end fields.
Expand All @@ -256,17 +252,30 @@ export function runGridChangeElementLocation(
.compose(fromField('props')),
selectedElementMetadata,
(elementProperties) => {
const gridColumnEnd = getJSXAttributesAtPath(
elementProperties,
PP.create('style', 'gridColumnEnd'),
)
if (gridColumnEnd.attribute.type === 'ATTRIBUTE_NOT_FOUND') {
keepGridColumnEnd = false
}
const gridRowEnd = getJSXAttributesAtPath(elementProperties, PP.create('style', 'gridRowEnd'))
if (gridRowEnd.attribute.type === 'ATTRIBUTE_NOT_FOUND') {
keepGridRowEnd = false
function shouldKeep(shorthandProp: 'gridColumn' | 'gridRow'): boolean {
const longhandProp = shorthandProp === 'gridColumn' ? 'gridColumnEnd' : 'gridRowEnd'

const shorthand = getJSXAttributesAtPath(
elementProperties,
PP.create('style', shorthandProp),
)
if (
shorthand.attribute.type === 'ATTRIBUTE_VALUE' &&
typeof shorthand.attribute.value === 'string' &&
shorthand.attribute.value.includes('/')
) {
return true
}

const longhand = getJSXAttributesAtPath(elementProperties, PP.create('style', longhandProp))
if (longhand.attribute.type !== 'ATTRIBUTE_NOT_FOUND') {
return true
}

return false
}
keepGridColumnEnd = shouldKeep('gridColumn')
keepGridRowEnd = shouldKeep('gridRow')
},
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,29 @@ describe('grid element change location strategy', () => {
})
})

it('can change the location of a multicell element (shorthand)', async () => {
const editor = await renderTestEditorWithCode(
ProjectCodeReorderwithMultiCellChildShorthand,
'await-first-dom-report',
)

const testId = 'orange'
const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } = await runMoveTest(editor, {
scale: 1,
pathString: `sb/scene/grid/${testId}`,
testId: testId,
tab: true,
targetCell: { row: 3, column: 1 },
})

expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnStart: '1',
gridColumnEnd: '3',
gridRowStart: '3',
gridRowEnd: 'auto',
})
})

it('can change the location of elements on a grid (zoom out)', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')

Expand Down Expand Up @@ -1271,7 +1294,7 @@ export var storyboard = (
height: '100%',
}}
data-uid='pink'
data-testid='pink'
data-testid='pink'
data-label='pink'
/>
<div
Expand All @@ -1281,7 +1304,7 @@ export var storyboard = (
height: '100%',
}}
data-uid='orange'
data-testid='orange'
data-testid='orange'
data-label='orange'
/>
<div
Expand All @@ -1291,7 +1314,7 @@ export var storyboard = (
height: '100%',
}}
data-uid='cyan'
data-testid='cyan'
data-testid='cyan'
data-label='cyan'
/>
<div
Expand All @@ -1301,7 +1324,7 @@ export var storyboard = (
height: '100%',
}}
data-uid='blue'
data-testid='blue'
data-testid='blue'
data-label='blue'
/>
</div>
Expand Down Expand Up @@ -1466,3 +1489,82 @@ export var storyboard = (
</Storyboard>
)
`

const ProjectCodeReorderwithMultiCellChildShorthand = `import * as React from 'react'
import { Scene, Storyboard } from 'utopia-api'
export var storyboard = (
<Storyboard data-uid='sb'>
<Scene
id='playground-scene'
commentId='playground-scene'
style={{
width: 600,
height: 600,
position: 'absolute',
left: 0,
top: 0,
}}
data-label='Playground'
data-uid='scene'
>
<div
style={{
backgroundColor: '#aaaaaa33',
width: 500,
height: 500,
display: 'grid',
gridTemplateColumns: '1fr 1fr 1fr',
gridTemplateRows: '1fr 1fr 1fr',
gridGap: 10,
padding: 10,
}}
data-testid='grid'
data-uid='grid'
>
<div
style={{
backgroundColor: '#f09',
width: '100%',
height: '100%',
}}
data-uid='pink'
data-testid='pink'
data-label='pink'
/>
<div
style={{
backgroundColor: '#f90',
width: '100%',
height: '100%',
gridColumn: '2 / 4',
}}
data-uid='orange'
data-testid='orange'
data-label='orange'
/>
<div
style={{
backgroundColor: '#0f9',
width: '100%',
height: '100%',
}}
data-uid='cyan'
data-testid='cyan'
data-label='cyan'
/>
<div
style={{
backgroundColor: '#09f',
width: '100%',
height: '100%',
}}
data-uid='blue'
data-testid='blue'
data-label='blue'
/>
</div>
</Scene>
</Storyboard>
)
`
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,11 @@ function getGridChildCellCoordBoundsFromProps(
}
return propValue.numericalPosition ?? innerFallback
}

const column = getGridProperty('gridColumnStart', fallback.column)
const height = getGridProperty('gridColumnEnd', fallback.column + (fallback.width ?? 1)) - column
const width = getGridProperty('gridColumnEnd', fallback.column + (fallback.width ?? 1)) - column
const row = getGridProperty('gridRowStart', fallback.row)
const width = getGridProperty('gridRowEnd', fallback.row + (fallback.height ?? 1)) - row
const height = getGridProperty('gridRowEnd', fallback.row + (fallback.height ?? 1)) - row

return {
row,
Expand Down

0 comments on commit c162a94

Please sign in to comment.