Skip to content

Commit

Permalink
Preserve span when changing grid item position (#6633)
Browse files Browse the repository at this point in the history
**Problem:**

`span` values for grid child positioning props are wiped when using
`gridChangeElementLocationStrategy`.

**Fix:**

Rework the way grid element props are calculated when changing element
location pins, so that `spans` are respected and kept in place if
present.

The behavior is now so that:
- when it makes sense to do so, return shorthand versions of grid
positioning props
- when moving an element with `span` values, support moving them when
the span is in the starting prop as well as the ending prop

Examples for moving a item one cell to the right:

| Initial config | Result |
|------------|------------|
| `gridColumn: 'span 2'` | `gridColumn: 'span 2 / 4'` |
| `gridColumn: '2 / span 2'` | `gridColumn: '3 / span 2'` |
| `gridColumnStart: 'span 2'` | `gridColumn: 'span 2 / 4'` |
| `gridColumnEnd: 'span 2'` | `gridColumn: '3 / span 2'` |

**Note:** a subsequent PR will take care of also keepign `span` intact
for the reorder strategy.

Fixes #6639
  • Loading branch information
ruggi authored and liady committed Dec 13, 2024
1 parent 73db22b commit ffa24f6
Show file tree
Hide file tree
Showing 9 changed files with 415 additions and 175 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
strategyApplicationResult,
} from '../canvas-strategy-types'
import type { InteractionSession } from '../interaction-state'
import { setGridPropsCommands } from './grid-helpers'
import { getCommandsForGridItemPlacement } from './grid-helpers'
import { getGridChildCellCoordBoundsFromCanvas } from './grid-cell-bounds'
import { accumulatePresses } from './shared-keyboard-strategy-helpers'
import { gridItemIdentifier } from '../../../editor/store/editor-state'
Expand Down Expand Up @@ -121,7 +121,7 @@ export function gridChangeElementLocationResizeKeyboardStrategy(
}

return strategyApplicationResult(
setGridPropsCommands(target, gridTemplate, {
getCommandsForGridItemPlacement(target, gridTemplate, {
gridColumnStart,
gridColumnEnd,
gridRowStart,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
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 * as PP from '../../../../core/shared/property-path'
import type {
ElementInstanceMetadata,
ElementInstanceMetadataMap,
GridContainerProperties,
GridElementProperties,
GridPositionOrSpan,
} from '../../../../core/shared/element-template'
import { gridPositionValue, isJSXElement } from '../../../../core/shared/element-template'
import { isInfinityRectangle } from '../../../../core/shared/math-utils'
import { gridPositionValue, isGridSpan } from '../../../../core/shared/element-template'
import { absolute } from '../../../../utils/utils'
import { gridItemIdentifier } from '../../../editor/store/editor-state'
import { cssKeyword } from '../../../inspector/common/css-utils'
import { getTargetGridCellData } from '../../../inspector/grid-helpers'
import type { CanvasCommand } from '../../commands/commands'
import { reorderElement } from '../../commands/reorder-element-command'
import { showGridControls } from '../../commands/show-grid-controls-command'
Expand All @@ -29,18 +31,10 @@ import {
getOriginalElementGridConfiguration,
getParentGridTemplatesFromChildMeasurements,
gridMoveStrategiesExtraCommands,
setGridPropsCommands,
isAutoGridPin,
getCommandsForGridItemPlacement,
sortElementsByGridPosition,
} from './grid-helpers'
import { getTargetGridCellData } from '../../../inspector/grid-helpers'
import { forEachOf } from '../../../../core/shared/optics/optic-utilities'
import {
eitherRight,
fromField,
fromTypeGuard,
} from '../../../../core/shared/optics/optic-creators'
import { getJSXAttributesAtPath } from '../../../..//core/shared/jsx-attribute-utils'
import { gridItemIdentifier } from '../../../editor/store/editor-state'

export const gridChangeElementLocationStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
Expand Down Expand Up @@ -217,67 +211,83 @@ export function runGridChangeElementLocation(
}
const { targetCellCoords, targetRootCell } = targetGridCellData

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

// TODO: Remove this logic once there is a fix for the handling of the track end fields.
let keepGridColumnEnd: boolean = true
let keepGridRowEnd: boolean = true
forEachOf(
fromField<ElementInstanceMetadata, 'element'>('element')
.compose(eitherRight())
.compose(fromTypeGuard(isJSXElement))
.compose(fromField('props')),
selectedElementMetadata,
(elementProperties) => {
function shouldKeep(shorthandProp: 'gridColumn' | 'gridRow'): boolean {
const longhandProp = shorthandProp === 'gridColumn' ? 'gridColumnEnd' : 'gridRowEnd'
const elementGridProperties =
selectedElementMetadata.specialSizeMeasurements.elementGridProperties

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
function getUpdatedPins(
start: GridPositionOrSpan | null,
end: GridPositionOrSpan | null,
axis: 'row' | 'column',
dimension: number,
): {
start: GridPositionOrSpan
end: GridPositionOrSpan
} {
const isSpanning = isGridSpan(start) || isGridSpan(end)
if (isSpanning) {
if (isGridSpan(start)) {
const isEndGridSpanArea = isGridSpan(end) || start.type === 'SPAN_AREA'
if (!isEndGridSpanArea) {
return {
start: start,
end: gridPositionValue(start.value + targetRootCell[axis]),
}
}

const longhand = getJSXAttributesAtPath(elementProperties, PP.create('style', longhandProp))
if (longhand.attribute.type !== 'ATTRIBUTE_NOT_FOUND') {
return true
} else if (isGridSpan(end)) {
return {
start: gridPositionValue(targetRootCell[axis]),
end: end,
}

return false
}
keepGridColumnEnd = shouldKeep('gridColumn')
keepGridRowEnd = shouldKeep('gridRow')
},
} else {
const shouldSetEndPosition = end != null && !isAutoGridPin(end)
if (shouldSetEndPosition) {
return {
start: gridPositionValue(targetRootCell[axis]),
end:
dimension === 1
? cssKeyword('auto')
: gridPositionValue(targetRootCell[axis] + dimension),
}
}
return {
start: gridPositionValue(targetRootCell[axis]),
end: cssKeyword('auto'),
}
}

return {
start: cssKeyword('auto'),
end: cssKeyword('auto'),
}
}

const columnBounds = getUpdatedPins(
elementGridProperties.gridColumnStart,
elementGridProperties.gridColumnEnd,
'column',
originalCellBounds.width,
)

const rowBounds = getUpdatedPins(
elementGridProperties.gridRowStart,
elementGridProperties.gridRowEnd,
'row',
originalCellBounds.height,
)

const gridCellMoveCommands = setGridPropsCommands(
const gridProps: GridElementProperties = {
gridColumnStart: columnBounds.start,
gridColumnEnd: columnBounds.end,
gridRowStart: rowBounds.start,
gridRowEnd: rowBounds.end,
}

const gridCellMoveCommands = getCommandsForGridItemPlacement(
pathForCommands,
gridTemplate,
gridProps,
).filter((command) => {
if (command.type === 'SET_PROPERTY') {
if (PP.pathsEqual(command.property, PP.create('style', 'gridColumnEnd'))) {
return keepGridColumnEnd
} else if (PP.pathsEqual(command.property, PP.create('style', 'gridRowEnd'))) {
return keepGridRowEnd
} else {
return true
}
} else {
return true
}
})
)

// The siblings of the grid element being moved
const siblings = MetadataUtils.getSiblingsUnordered(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import {
getStyleAttributesForFrameInAbsolutePosition,
updateInsertionSubjectWithAttributes,
} from './draw-to-insert-metastrategy'
import { setGridPropsCommands } from './grid-helpers'
import { getCommandsForGridItemPlacement } from './grid-helpers'
import { newReparentSubjects } from './reparent-helpers/reparent-strategy-helpers'
import { getReparentTargetUnified } from './reparent-helpers/reparent-strategy-parent-lookup'
import { getGridCellUnderMouseFromMetadata } from './grid-cell-bounds'
Expand Down Expand Up @@ -212,7 +212,7 @@ const gridDrawToInsertStrategyInner =
[
insertionCommand,
...nukeAllAbsolutePositioningPropsCommands(insertedElementPath), // do not use absolute positioning in grid cells
...setGridPropsCommands(insertedElementPath, gridTemplate, {
...getCommandsForGridItemPlacement(insertedElementPath, gridTemplate, {
gridRowStart: { numericalPosition: gridCellCoordinates.row },
gridColumnStart: { numericalPosition: gridCellCoordinates.column },
gridRowEnd: { numericalPosition: gridCellCoordinates.row + 1 },
Expand Down
Loading

0 comments on commit ffa24f6

Please sign in to comment.