Skip to content

Commit c162a94

Browse files
authored
Keep shorthand values when changing grid item location (#6626)
**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
1 parent 004cc05 commit c162a94

File tree

3 files changed

+135
-23
lines changed

3 files changed

+135
-23
lines changed

editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts

+26-17
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@ import type {
88
GridContainerProperties,
99
GridElementProperties,
1010
} from '../../../../core/shared/element-template'
11-
import {
12-
getJSXAttribute,
13-
gridPositionValue,
14-
isJSXElement,
15-
} from '../../../../core/shared/element-template'
11+
import { gridPositionValue, isJSXElement } from '../../../../core/shared/element-template'
1612
import { isInfinityRectangle } from '../../../../core/shared/math-utils'
1713
import { absolute } from '../../../../utils/utils'
1814
import type { CanvasCommand } from '../../commands/commands'
@@ -241,9 +237,9 @@ export function runGridChangeElementLocation(
241237

242238
const gridProps: Partial<GridElementProperties> = {
243239
gridColumnStart: gridPositionValue(targetRootCell.column),
244-
gridColumnEnd: gridPositionValue(targetRootCell.column + originalCellBounds.height),
240+
gridColumnEnd: gridPositionValue(targetRootCell.column + originalCellBounds.width),
245241
gridRowStart: gridPositionValue(targetRootCell.row),
246-
gridRowEnd: gridPositionValue(targetRootCell.row + originalCellBounds.width),
242+
gridRowEnd: gridPositionValue(targetRootCell.row + originalCellBounds.height),
247243
}
248244

249245
// TODO: Remove this logic once there is a fix for the handling of the track end fields.
@@ -256,17 +252,30 @@ export function runGridChangeElementLocation(
256252
.compose(fromField('props')),
257253
selectedElementMetadata,
258254
(elementProperties) => {
259-
const gridColumnEnd = getJSXAttributesAtPath(
260-
elementProperties,
261-
PP.create('style', 'gridColumnEnd'),
262-
)
263-
if (gridColumnEnd.attribute.type === 'ATTRIBUTE_NOT_FOUND') {
264-
keepGridColumnEnd = false
265-
}
266-
const gridRowEnd = getJSXAttributesAtPath(elementProperties, PP.create('style', 'gridRowEnd'))
267-
if (gridRowEnd.attribute.type === 'ATTRIBUTE_NOT_FOUND') {
268-
keepGridRowEnd = false
255+
function shouldKeep(shorthandProp: 'gridColumn' | 'gridRow'): boolean {
256+
const longhandProp = shorthandProp === 'gridColumn' ? 'gridColumnEnd' : 'gridRowEnd'
257+
258+
const shorthand = getJSXAttributesAtPath(
259+
elementProperties,
260+
PP.create('style', shorthandProp),
261+
)
262+
if (
263+
shorthand.attribute.type === 'ATTRIBUTE_VALUE' &&
264+
typeof shorthand.attribute.value === 'string' &&
265+
shorthand.attribute.value.includes('/')
266+
) {
267+
return true
268+
}
269+
270+
const longhand = getJSXAttributesAtPath(elementProperties, PP.create('style', longhandProp))
271+
if (longhand.attribute.type !== 'ATTRIBUTE_NOT_FOUND') {
272+
return true
273+
}
274+
275+
return false
269276
}
277+
keepGridColumnEnd = shouldKeep('gridColumn')
278+
keepGridRowEnd = shouldKeep('gridRow')
270279
},
271280
)
272281

editor/src/components/canvas/canvas-strategies/strategies/grid-element-change-location-strategy.spec.browser2.tsx

+106-4
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,29 @@ describe('grid element change location strategy', () => {
163163
})
164164
})
165165

166+
it('can change the location of a multicell element (shorthand)', async () => {
167+
const editor = await renderTestEditorWithCode(
168+
ProjectCodeReorderwithMultiCellChildShorthand,
169+
'await-first-dom-report',
170+
)
171+
172+
const testId = 'orange'
173+
const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } = await runMoveTest(editor, {
174+
scale: 1,
175+
pathString: `sb/scene/grid/${testId}`,
176+
testId: testId,
177+
tab: true,
178+
targetCell: { row: 3, column: 1 },
179+
})
180+
181+
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
182+
gridColumnStart: '1',
183+
gridColumnEnd: '3',
184+
gridRowStart: '3',
185+
gridRowEnd: 'auto',
186+
})
187+
})
188+
166189
it('can change the location of elements on a grid (zoom out)', async () => {
167190
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')
168191

@@ -1271,7 +1294,7 @@ export var storyboard = (
12711294
height: '100%',
12721295
}}
12731296
data-uid='pink'
1274-
data-testid='pink'
1297+
data-testid='pink'
12751298
data-label='pink'
12761299
/>
12771300
<div
@@ -1281,7 +1304,7 @@ export var storyboard = (
12811304
height: '100%',
12821305
}}
12831306
data-uid='orange'
1284-
data-testid='orange'
1307+
data-testid='orange'
12851308
data-label='orange'
12861309
/>
12871310
<div
@@ -1291,7 +1314,7 @@ export var storyboard = (
12911314
height: '100%',
12921315
}}
12931316
data-uid='cyan'
1294-
data-testid='cyan'
1317+
data-testid='cyan'
12951318
data-label='cyan'
12961319
/>
12971320
<div
@@ -1301,7 +1324,7 @@ export var storyboard = (
13011324
height: '100%',
13021325
}}
13031326
data-uid='blue'
1304-
data-testid='blue'
1327+
data-testid='blue'
13051328
data-label='blue'
13061329
/>
13071330
</div>
@@ -1466,3 +1489,82 @@ export var storyboard = (
14661489
</Storyboard>
14671490
)
14681491
`
1492+
1493+
const ProjectCodeReorderwithMultiCellChildShorthand = `import * as React from 'react'
1494+
import { Scene, Storyboard } from 'utopia-api'
1495+
1496+
export var storyboard = (
1497+
<Storyboard data-uid='sb'>
1498+
<Scene
1499+
id='playground-scene'
1500+
commentId='playground-scene'
1501+
style={{
1502+
width: 600,
1503+
height: 600,
1504+
position: 'absolute',
1505+
left: 0,
1506+
top: 0,
1507+
}}
1508+
data-label='Playground'
1509+
data-uid='scene'
1510+
>
1511+
<div
1512+
style={{
1513+
backgroundColor: '#aaaaaa33',
1514+
width: 500,
1515+
height: 500,
1516+
display: 'grid',
1517+
gridTemplateColumns: '1fr 1fr 1fr',
1518+
gridTemplateRows: '1fr 1fr 1fr',
1519+
gridGap: 10,
1520+
padding: 10,
1521+
}}
1522+
data-testid='grid'
1523+
data-uid='grid'
1524+
>
1525+
<div
1526+
style={{
1527+
backgroundColor: '#f09',
1528+
width: '100%',
1529+
height: '100%',
1530+
}}
1531+
data-uid='pink'
1532+
data-testid='pink'
1533+
data-label='pink'
1534+
/>
1535+
<div
1536+
style={{
1537+
backgroundColor: '#f90',
1538+
width: '100%',
1539+
height: '100%',
1540+
gridColumn: '2 / 4',
1541+
}}
1542+
data-uid='orange'
1543+
data-testid='orange'
1544+
data-label='orange'
1545+
/>
1546+
<div
1547+
style={{
1548+
backgroundColor: '#0f9',
1549+
width: '100%',
1550+
height: '100%',
1551+
}}
1552+
data-uid='cyan'
1553+
data-testid='cyan'
1554+
data-label='cyan'
1555+
/>
1556+
<div
1557+
style={{
1558+
backgroundColor: '#09f',
1559+
width: '100%',
1560+
height: '100%',
1561+
}}
1562+
data-uid='blue'
1563+
data-testid='blue'
1564+
data-label='blue'
1565+
/>
1566+
</div>
1567+
</Scene>
1568+
</Storyboard>
1569+
)
1570+
`

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,11 @@ function getGridChildCellCoordBoundsFromProps(
154154
}
155155
return propValue.numericalPosition ?? innerFallback
156156
}
157+
157158
const column = getGridProperty('gridColumnStart', fallback.column)
158-
const height = getGridProperty('gridColumnEnd', fallback.column + (fallback.width ?? 1)) - column
159+
const width = getGridProperty('gridColumnEnd', fallback.column + (fallback.width ?? 1)) - column
159160
const row = getGridProperty('gridRowStart', fallback.row)
160-
const width = getGridProperty('gridRowEnd', fallback.row + (fallback.height ?? 1)) - row
161+
const height = getGridProperty('gridRowEnd', fallback.row + (fallback.height ?? 1)) - row
161162

162163
return {
163164
row,

0 commit comments

Comments
 (0)