Skip to content

Commit

Permalink
Grid component support (#6640)
Browse files Browse the repository at this point in the history
**Problem:**
Followup to #6636

The goal is to support interaction with grid item in the following
scenario:
- the grid is in a component
- the grid renders the component's children
- the grid is not the root element of the component

**Fix:**
In #6636 I introduced a
way to indentify a grid using the element path of its item (which is
necessary because when the grid is in a non-focused component).

- `GridMeasurementHelper` collects the grids from the metadata. These
grids in non-focused components are not in the metadata, so they are not
collected. I fixed this by collecting component items from the grids
too.
- To get the measurement data for these grids which identified by their
items, I added a new parameter to `getGridMeasurementHelperData` so it
can get the styling of the parentElement of the item's element.
- The measurement helper was identified by its element path, and the
dom-sampler could retrieve the helper from the dom using that. However,
this is again not a good solution for these grids in non-focused
components, so I added global WeakMap which stores the pairing between
HTMLElements of grids and the id attribute of their
GridMeasurementHelper.
- I updated the `path` fiels names of `GridContainerIdentifier` and
`GridItemIdentifier`: it was not a good idea to use the same name in the
two paths, because it was possible to not check the type and use the
path field.

**Missing:**
`gridMoveStrategiesExtraCommands` still tries to set grid props on the
parent, and when it is a component, those can be absolutely useless. We
should detect when that is the case, and not generate these commands.

**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 authored Nov 18, 2024
1 parent 7b85cb3 commit 85df996
Show file tree
Hide file tree
Showing 12 changed files with 429 additions and 186 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,26 +97,28 @@ describe('grid element change location strategy', () => {
})
})

it('can change the location of elements in a grid component', async () => {
const editor = await renderTestEditorWithCode(
ProjectCodeGridComponent,
'await-first-dom-report',
)
describe('grid component', () => {
it('can change the location of elements in a grid component', async () => {
const editor = await renderTestEditorWithCode(
ProjectCodeGridComponent,
'await-first-dom-report',
)

const testId = 'aaa'
const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } = await runGridMoveTest(
editor,
{
scale: 1,
pathString: `sb/scene/grid/${testId}`,
testId: testId,
},
)
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: '7',
gridColumnStart: '3',
gridRowEnd: '4',
gridRowStart: '2',
const testId = 'aaa'
const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } = await runGridMoveTest(
editor,
{
scale: 1,
pathString: `sb/scene/grid/${testId}`,
testId: testId,
},
)
expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
gridColumnEnd: '7',
gridColumnStart: '3',
gridRowEnd: '4',
gridRowStart: '2',
})
})
})

Expand Down Expand Up @@ -1013,13 +1015,6 @@ export var storyboard = (
>
<Grid
style={{
display: 'grid',
gridTemplateRows: '75px 75px 75px 75px',
gridTemplateColumns:
'50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px',
gridGap: 16,
height: 482,
width: 786,
position: 'absolute',
left: 31,
top: 0,
Expand All @@ -1029,22 +1024,20 @@ export var storyboard = (
<div
style={{
minHeight: 0,
backgroundColor: '#f3785f',
gridColumnEnd: 5,
gridColumnStart: 1,
gridRowEnd: 3,
gridRowStart: 1,
backgroundColor: '#23565b',
}}
data-uid='aaa'
data-testid='aaa'
data-uid='bbb'
data-testid='bbb'
/>
<div
style={{
minHeight: 0,
backgroundColor: '#23565b',
backgroundColor: '#f3785f',
gridColumn: '1 / 5',
gridRow: '1 / 3',
}}
data-uid='bbb'
data-testid='bbb'
data-uid='aaa'
data-testid='aaa'
/>
<Placeholder
style={{
Expand Down Expand Up @@ -1075,18 +1068,26 @@ export var storyboard = (
export function Grid(props) {
return (
<div
{...props}
style={{
...props.style,
display: 'grid',
gridTemplateRows: '75px 75px 75px 75px',
gridTemplateColumns:
'50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px',
gridGap: 16,
display: 'flex',
flexDirection: 'column',
}}
data-uid='f84914f31dbc6c5d9b44c11ae54139ef'
>
{props.children}
<div
style={{
display: 'grid',
gridTemplateRows: '75px 75px 75px 75px',
gridTemplateColumns:
'50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px',
gridGap: 16,
}}
>
{props.children}
</div>
<div
style={{ height: 100 }}
/>
</div>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
getGridChildCellCoordBoundsFromCanvas,
gridCellCoordinates,
} from './grid-cell-bounds'
import type { GridIdentifier } from '../../../editor/store/editor-state'

export function gridPositionToValue(
p: GridPositionOrSpan | null | undefined,
Expand Down Expand Up @@ -684,3 +685,40 @@ export function gridMoveStrategiesExtraCommands(

return { midInteractionCommands, onCompleteCommands }
}

export function getGridIdentifierContainerOrComponentPath(identifier: GridIdentifier): ElementPath {
switch (identifier.type) {
case 'GRID_CONTAINER':
return identifier.container
case 'GRID_ITEM':
return EP.parentPath(identifier.item)
default:
assertNever(identifier)
}
}

export function gridIdentifiersSimilar(a: GridIdentifier, b: GridIdentifier): boolean {
switch (a.type) {
case 'GRID_CONTAINER':
return b.type === 'GRID_CONTAINER'
? EP.pathsEqual(a.container, b.container)
: EP.isParentOf(a.container, b.item)
case 'GRID_ITEM':
return b.type === 'GRID_ITEM'
? EP.pathsEqual(a.item, b.item)
: EP.isParentOf(b.container, a.item)
default:
assertNever(a)
}
}

export function gridIdentifierToString(identifier: GridIdentifier): string {
switch (identifier.type) {
case 'GRID_CONTAINER':
return `${identifier.type}-${EP.toString(identifier.container)}`
case 'GRID_ITEM':
return `${identifier.type}-${EP.toString(identifier.item)}`
default:
assertNever(identifier)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ describe('grid reorder', () => {

expect(cells).toEqual(['pink', 'cyan', 'orange', 'blue'])
})
it('reorders an element in a grid component without setting positioning (inside contiguous area)', async () => {
const editor = await renderTestEditorWithCode(
ProjectCodeReorderGridComponent,
'await-first-dom-report',
)
const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd, cells } =
await runReorderTest(editor, 'sb/scene/grid', 'orange', { row: 1, column: 3 })

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

expect(cells).toEqual(['pink', 'cyan', 'orange', 'blue'])
})
it('reorders a component (which does not take style props) inside contiguous area', async () => {
const editor = await renderTestEditorWithCode(
ProjectCodeReorderWithComponentItem,
Expand Down Expand Up @@ -339,6 +356,109 @@ export var storyboard = (
)
`

const ProjectCodeReorderGridComponent = `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'
>
<Grid
style={{
backgroundColor: '#aaaaaa33',
}}
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%',
}}
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'
/>
</Grid>
</Scene>
</Storyboard>
)
export function Grid(props) {
return (
<div
style={{
...props.style,
display: 'flex',
flexDirection: 'column',
}}
data-uid='43adc9c0cf5a0bbf66c67e2e37466c18'
>
<div
style={{
display: 'grid',
gridTemplateColumns: '1fr 1fr 1fr',
gridTemplateRows: '1fr 1fr 1fr',
gridGap: 10,
width: 500,
height: 500,
padding: 10,
}}
data-uid='f84914f31dbc6c5d9b44c11ae54139ef'
>
{props.children}
</div>
<div
style={{ height: 100 }}
data-uid='f5827ce0f04916c792a1da2bd69b6cad'
/>
</div>
)
}
`

const ProjectCodeReorderWithComponentItem = `import * as React from 'react'
import { Scene, Storyboard } from 'utopia-api'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -867,18 +867,26 @@ export var storyboard = (
export function Grid(props) {
return (
<div
{...props}
style={{
...props.style,
display: 'grid',
gridTemplateRows: '75px 75px 75px 75px',
gridTemplateColumns:
'50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px',
gridGap: 16,
display: 'flex',
flexDirection: 'column',
}}
data-uid='f84914f31dbc6c5d9b44c11ae54139ef'
>
{props.children}
<div
style={{
display: 'grid',
gridTemplateRows: '75px 75px 75px 75px',
gridTemplateColumns:
'50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px 50px',
gridGap: 16,
}}
>
{props.children}
</div>
<div
style={{ height: 100 }}
/>
</div>
)
}
Expand Down
Loading

0 comments on commit 85df996

Please sign in to comment.