Skip to content

Commit

Permalink
Prefer span notation when resizing flow grid cells (#6687)
Browse files Browse the repository at this point in the history
**Problem:**

When resizing a grid cell for an in-flow element, `span` notation should
be prefered over explicit pins.

**Fix:**

If the resize happens on a right/bottom edge and the element is in flow,
apply the `span` notation.

**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

Fixes #6686
  • Loading branch information
ruggi authored and liady committed Dec 13, 2024
1 parent 161906b commit f61fa48
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,107 @@ export var storyboard = (
})
}
})
it('uses spans for flow elements', async () => {
const editor = await renderTestEditorWithCode(
makeProjectCodeWithCustomPlacement({ gridColumn: 'auto', gridRow: 'auto' }),
'await-first-dom-report',
)

// enlarge to the right, spans in flow
{
await runCellResizeTest(
editor,
'column-end',
gridCellTargetId(EP.fromString('sb/grid'), 1, 3),
EP.fromString('sb/grid/cell'),
)

const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } =
editor.renderedDOM.getByTestId('cell').style
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: 'auto',
gridColumnStart: 'span 3',
gridRowEnd: 'auto',
gridRowStart: 'auto',
})
}

// enlarge to the bottom, still spans in flow
{
await runCellResizeTest(
editor,
'row-end',
gridCellTargetId(EP.fromString('sb/grid'), 4, 3),
EP.fromString('sb/grid/cell'),
)

const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } =
editor.renderedDOM.getByTestId('cell').style
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: 'auto',
gridColumnStart: 'span 3',
gridRowEnd: 'auto',
gridRowStart: 'span 4',
})
}

// shrink from the right, still spans in flow
{
await runCellResizeTest(
editor,
'column-end',
gridCellTargetId(EP.fromString('sb/grid'), 4, 2),
EP.fromString('sb/grid/cell'),
)

const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } =
editor.renderedDOM.getByTestId('cell').style
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: 'auto',
gridColumnStart: 'span 2',
gridRowEnd: 'auto',
gridRowStart: 'span 4',
})
}

// shrink from the bottom, still spans in flow
{
await runCellResizeTest(
editor,
'row-end',
gridCellTargetId(EP.fromString('sb/grid'), 3, 2),
EP.fromString('sb/grid/cell'),
)

const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } =
editor.renderedDOM.getByTestId('cell').style
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: 'auto',
gridColumnStart: 'span 2',
gridRowEnd: 'auto',
gridRowStart: 'span 3',
})
}

// shrink from the top, it spans but now is pinned
{
await runCellResizeTest(
editor,
'row-start',
gridCellTargetId(EP.fromString('sb/grid'), 2, 2),
EP.fromString('sb/grid/cell'),
)

const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } =
editor.renderedDOM.getByTestId('cell').style
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: 'auto',
gridColumnStart: 'span 2',
gridRowEnd: '4',
gridRowStart: 'span 2',
})
}
})
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import {
emptyStrategyApplicationResult,
strategyApplicationResult,
} from '../canvas-strategy-types'
import type { InteractionSession } from '../interaction-state'
import { getCommandsForGridItemPlacement } from './grid-helpers'
import type { GridResizeEdge, InteractionSession } from '../interaction-state'
import { getCommandsForGridItemPlacement, isAutoGridPin } from './grid-helpers'
import { resizeBoundingBoxFromSide } from './resize-helpers'

export const gridResizeElementStrategy: CanvasStrategyFactory = (
Expand Down Expand Up @@ -138,6 +138,7 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = (
'start',
elementGridPropertiesFromProps.gridColumnEnd,
gridPropsNumeric.gridColumnEnd,
interactionSession.activeControl.edge,
),
gridColumnEnd: normalizePositionAfterResize(
elementGridPropertiesFromProps.gridColumnEnd,
Expand All @@ -146,6 +147,7 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = (
'end',
elementGridPropertiesFromProps.gridColumnStart,
gridPropsNumeric.gridColumnStart,
interactionSession.activeControl.edge,
),
gridRowStart: normalizePositionAfterResize(
elementGridPropertiesFromProps.gridRowStart,
Expand All @@ -154,6 +156,7 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = (
'start',
elementGridPropertiesFromProps.gridRowEnd,
gridPropsNumeric.gridRowEnd,
interactionSession.activeControl.edge,
),
gridRowEnd: normalizePositionAfterResize(
elementGridPropertiesFromProps.gridRowEnd,
Expand All @@ -162,6 +165,7 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = (
'end',
elementGridPropertiesFromProps.gridRowStart,
gridPropsNumeric.gridRowStart,
interactionSession.activeControl.edge,
),
}

Expand Down Expand Up @@ -211,23 +215,47 @@ function getNewGridPropsFromResizeBox(
}
}

// After a resize happens and we know the numerical grid positioning of the new bounds,
// return a normalized version of the new position so that it respects any spans that
// may have been there before the resize, and/or default it to 'auto' when it would become redundant.
/*
After a resize happens and we know the numerical grid positioning of the new bounds,
return a normalized version of the new position so that it respects any spans that
may have been there before the resize, and/or default it to 'auto' when it would become redundant.
If the positions match a flow configuration, give priority to span notation.
*/
function normalizePositionAfterResize(
position: GridPositionOrSpan | null,
resizedPosition: GridPositionValue,
size: number, // the number of cols/rows the cell occupies
bound: 'start' | 'end',
counterpart: GridPositionOrSpan | null,
counterpartResizedPosition: GridPositionValue,
edge: GridResizeEdge,
): GridPositionOrSpan | null {
if (isGridSpan(position)) {
function isFlowResizeOnBound(
wantedBound: 'start' | 'end',
flowStart: GridPositionOrSpan | null,
flowEnd: GridPositionOrSpan | null,
): boolean {
return (
(edge === 'column-end' || edge === 'row-end') &&
bound === wantedBound &&
(isGridSpan(flowStart) || isAutoGridPin(flowStart) || flowStart == null) &&
(isAutoGridPin(flowEnd) || flowEnd == null)
)
}

const isFlowStart = isFlowResizeOnBound('start', position, counterpart)
if (isFlowStart || isGridSpan(position)) {
if (size === 1) {
return cssKeyword('auto')
}
return gridSpanNumeric(size)
}

const isFlowEnd = isFlowResizeOnBound('end', counterpart, position)
if (isFlowEnd) {
return cssKeyword('auto')
}

if (
isGridSpan(counterpart) &&
counterpartResizedPosition.numericalPosition === 1 &&
Expand Down

0 comments on commit f61fa48

Please sign in to comment.