Skip to content

Commit

Permalink
fix(canvas) Fixed handling of parent frames to get the right position…
Browse files Browse the repository at this point in the history
…ing. (#6586)

- Split `maybeAddContainLayout` into that and another function
`shouldAddContainLayout`.
- Corrected two tests which were inserting into the second flex child
and were also offset similar to how the grid items were.
- `getAbsoluteReparentPropertyChanges` now takes a parameter which
specifies if `contain: 'layout'` will be added to the container.
- `getMoveCommandsForSelectedElement` now has an additional fallback for
`elementParentBounds` when the closest non fragment parent provides
bounds for absolute children.
- `getLocalFrame` now has an optional parameter to specify the parent to
target, to allow for handling the lookups where the element has yet to
be reparented.
  • Loading branch information
seanparsons authored Oct 25, 2024
1 parent fc4cfb4 commit 7f377c9
Show file tree
Hide file tree
Showing 21 changed files with 297 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import type { InteractionSession, UpdatedPathMap } from '../interaction-state'
import { absoluteMoveStrategy } from './absolute-move-strategy'
import { honoursPropsPosition, shouldKeepMovingDraggedGroupChildren } from './absolute-utils'
import { replaceFragmentLikePathsWithTheirChildrenRecursive } from './fragment-like-helpers'
import type { ShouldAddContainLayout } from './reparent-helpers/reparent-helpers'
import { ifAllowedToReparent, isAllowedToReparent } from './reparent-helpers/reparent-helpers'
import type { ForcePins } from './reparent-helpers/reparent-property-changes'
import { getAbsoluteReparentPropertyChanges } from './reparent-helpers/reparent-property-changes'
Expand Down Expand Up @@ -184,6 +185,12 @@ export function applyAbsoluteReparent(
projectContents,
nodeModules,
'force-pins',
shouldAddContainLayout(
canvasState.startingMetadata,
canvasState.startingAllElementProps,
canvasState.startingElementPathTree,
newParent.intendedParentPath,
),
),
selectedElements,
)
Expand Down Expand Up @@ -254,6 +261,7 @@ export function createAbsoluteReparentAndOffsetCommands(
projectContents: ProjectContentTreeRoot,
nodeModules: NodeModules,
forcePins: ForcePins,
willContainLayoutBeAdded: ShouldAddContainLayout,
) {
const reparentResult = getReparentOutcome(
metadata,
Expand All @@ -271,19 +279,21 @@ export function createAbsoluteReparentAndOffsetCommands(
if (reparentResult == null) {
return null
} else {
const offsetCommands = replaceFragmentLikePathsWithTheirChildrenRecursive(
const replacedPaths = replaceFragmentLikePathsWithTheirChildrenRecursive(
metadata,
elementProps,
pathTree,
[target],
).flatMap((p) => {
)
const offsetCommands = replacedPaths.flatMap((p) => {
return getAbsoluteReparentPropertyChanges(
p,
newParent.intendedParentPath,
metadata,
metadata,
projectContents,
forcePins,
willContainLayoutBeAdded,
)
})

Expand All @@ -296,12 +306,12 @@ export function createAbsoluteReparentAndOffsetCommands(
}
}

function maybeAddContainLayout(
export function shouldAddContainLayout(
metadata: ElementInstanceMetadataMap,
allElementProps: AllElementProps,
pathTrees: ElementPathTrees,
path: ElementPath,
): CanvasCommand[] {
): ShouldAddContainLayout {
const closestNonFragmentParent = MetadataUtils.getClosestNonFragmentParent(
metadata,
allElementProps,
Expand All @@ -310,14 +320,23 @@ function maybeAddContainLayout(
)

if (EP.isStoryboardPath(closestNonFragmentParent)) {
return []
return 'dont-add-contain-layout'
}

const parentProvidesBoundsForAbsoluteChildren =
MetadataUtils.findElementByElementPath(metadata, closestNonFragmentParent)
?.specialSizeMeasurements.providesBoundsForAbsoluteChildren === true

return parentProvidesBoundsForAbsoluteChildren
? []
: [setProperty('always', path, PP.create('style', 'contain'), 'layout')]
return parentProvidesBoundsForAbsoluteChildren ? 'dont-add-contain-layout' : 'add-contain-layout'
}

function maybeAddContainLayout(
metadata: ElementInstanceMetadataMap,
allElementProps: AllElementProps,
pathTrees: ElementPathTrees,
path: ElementPath,
): CanvasCommand[] {
return shouldAddContainLayout(metadata, allElementProps, pathTrees, path) === 'add-contain-layout'
? [setProperty('always', path, PP.create('style', 'contain'), 'layout')]
: []
}
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ function getConstrainedSizes(
constraints.right ||
constraints.width

const localFrame = MetadataUtils.getLocalFrame(element.elementPath, jsxMetadata)
const localFrame = MetadataUtils.getLocalFrame(element.elementPath, jsxMetadata, null)
if (
isConstrained &&
localFrame != null &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,15 @@ import {
import { RightMenuTab } from '../../../editor/store/editor-state'
import { FOR_TESTS_setNextGeneratedUid } from '../../../../core/model/element-template-utils.test-utils'
import type { WindowPoint } from '../../../../core/shared/math-utils'
import { windowPoint } from '../../../../core/shared/math-utils'
import {
nullIfInfinity,
rectangleContainsRectangle,
windowPoint,
} from '../../../../core/shared/math-utils'
import { selectComponentsForTest } from '../../../../utils/utils.test-utils'
import CanvasActions from '../../canvas-actions'
import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import { forceNotNull } from '../../../../core/shared/optional-utils'

function slightlyOffsetWindowPointBecauseVeryWeirdIssue(point: { x: number; y: number }) {
// FIXME when running in headless chrome, the result of getBoundingClientRect will be slightly
Expand Down Expand Up @@ -201,6 +207,109 @@ export var storyboard = (
</Scene>
</Storyboard>
)
`

const projectWithGridContent = `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: 700,
height: 759,
position: 'absolute',
left: 0,
top: 0,
}}
data-label='Playground'
data-uid='scene'
>
<div
style={{
position: 'absolute',
left: 10,
top: 10,
width: 500,
height: 300,
display: 'grid',
gap: 2,
gridTemplateColumns: '1fr 1fr 1fr',
gridTemplateRows: '1fr 1fr 1fr',
backgroundColor: '#9dc1ea',
}}
data-uid='grid'
data-testid='grid'
>
<div
style={{
backgroundColor: 'red',
}}
data-uid='grid-item-1'
data-testid='grid-item-1'
/>
<div
style={{
backgroundColor: 'green',
}}
data-uid='grid-item-2'
data-testid='grid-item-2'
/>
<div
style={{
backgroundColor: 'blue',
}}
data-uid='grid-item-3'
data-testid='grid-item-3'
/>
<div
style={{
backgroundColor: 'red',
}}
data-uid='grid-item-4'
data-testid='grid-item-4'
/>
<div
style={{
backgroundColor: 'green',
}}
data-uid='grid-item-5'
data-testid='grid-item-5'
/>
<div
style={{
backgroundColor: 'blue',
}}
data-uid='grid-item-6'
data-testid='grid-item-6'
/>
<div
style={{
backgroundColor: 'red',
}}
data-uid='grid-item-7'
data-testid='grid-item-7'
/>
<div
style={{
backgroundColor: 'green',
}}
data-uid='grid-item-8'
data-testid='grid-item-8'
/>
<div
style={{
backgroundColor: 'blue',
}}
data-uid='grid-item-9'
data-testid='grid-item-9'
/>
</div>
</Scene>
</Storyboard>
)
`

it('can click to insert into a grid', async () => {
Expand Down Expand Up @@ -377,5 +486,66 @@ export var storyboard = (
width: '20px',
})
})

it('can draw to insert into an element in a grid', async () => {
const editor = await renderTestEditorWithCode(
projectWithGridContent,
'await-first-dom-report',
)

await selectComponentsForTest(editor, [EP.fromString('sb/scene/grid/grid-item-8')])

await pressKey('d')
ensureInInsertMode(editor)

const gridItem = editor.renderedDOM.getByTestId('grid-item-8')
const gridBB = gridItem.getBoundingClientRect()

const target: WindowPoint = windowPoint({
x: gridBB.x + 5,
y: gridBB.y + 5,
})

const canvasControlsLayer = editor.renderedDOM.getByTestId(CanvasControlsContainerID)

await mouseMoveToPoint(canvasControlsLayer, target)
await mouseDragFromPointToPoint(canvasControlsLayer, target, {
x: target.x + 40,
y: target.y + 60,
})
await editor.getDispatchFollowUpActionsFinished()

const child = gridItem.firstChild
if (child == null) {
throw new Error('Draw to insert should be able to insert an element')
}

const { position, top, left, width, height } = (child as HTMLElement).style

expect({ position, top, left, width, height }).toEqual({
position: 'absolute',
left: '6px',
top: '6px',
width: '40px',
height: '60px',
})

const gridItem8Metadata =
editor.getEditorState().editor.jsxMetadata['sb/scene/grid/grid-item-8']
const gridItem8GlobalFrame = forceNotNull(
'Item 8 should have a global frame.',
nullIfInfinity(gridItem8Metadata.globalFrame),
)
const gridItem8Children = MetadataUtils.getChildrenUnordered(
editor.getEditorState().editor.jsxMetadata,
EP.fromString('sb/scene/grid/grid-item-8'),
)
const gridItem8Child = forceNotNull('Could not find child.', gridItem8Children.at(0))
const gridItem8ChildGlobalFrame = forceNotNull(
'Child of item 8 should have a global frame.',
nullIfInfinity(gridItem8Child.globalFrame),
)
expect(rectangleContainsRectangle(gridItem8GlobalFrame, gridItem8ChildGlobalFrame)).toBe(true)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ describe('Fallback Absolute Reparent Strategies', () => {
<div
style={{
position: 'absolute',
left: 100,
left: 0,
top: 0,
width: 100,
height: 100,
Expand Down Expand Up @@ -511,7 +511,7 @@ describe('Fallback Absolute Reparent Strategies', () => {
<div
style={{
position: 'absolute',
left: 100,
left: 0,
top: 0,
width: 100,
height: 100,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ import type { ReparentTargetForPaste } from '../reparent-utils'
import { cleanSteganoTextData } from '../../../../../core/shared/stegano-text'
import { assertNever } from '../../../../../core/shared/utils'

export type ShouldAddContainLayout = 'add-contain-layout' | 'dont-add-contain-layout'

export function isAllowedToReparent(
projectContents: ProjectContentTreeRoot,
startingMetadata: ElementInstanceMetadataMap,
Expand Down Expand Up @@ -531,7 +533,7 @@ export function absolutePositionForReparent(
}

const localFrame = zeroRectIfNullOrInfinity(
MetadataUtils.getLocalFrame(targetParent, metadata.currentMetadata) ?? null,
MetadataUtils.getLocalFrame(targetParent, metadata.currentMetadata, null) ?? null,
)

// offset the element with the target parent's offset, since the target parent doesn't
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import {
treatElementAsFragmentLike,
} from '../fragment-like-helpers'
import type { OldPathToNewPathMapping } from '../../post-action-options/post-action-paste'
import type { ShouldAddContainLayout } from './reparent-helpers'

const propertiesToRemove: Array<PropertyPath> = [
PP.create('style', 'left'),
Expand All @@ -90,6 +91,7 @@ export function getAbsoluteReparentPropertyChanges(
newParentStartingMetadata: ElementInstanceMetadataMap,
projectContents: ProjectContentTreeRoot,
forcePins: ForcePins,
willContainLayoutBeAdded: ShouldAddContainLayout,
): Array<AdjustCssLengthProperties | ConvertCssPercentToPx> {
const element: JSXElement | null = getJSXElementFromProjectContents(target, projectContents)

Expand All @@ -108,7 +110,10 @@ export function getAbsoluteReparentPropertyChanges(

const currentParentContentBox =
MetadataUtils.getGlobalContentBoxForChildren(originalParentInstance)
const newParentContentBox = MetadataUtils.getGlobalContentBoxForChildren(newParentInstance)
const newParentContentBox =
willContainLayoutBeAdded === 'add-contain-layout'
? nullIfInfinity(newParentInstance.globalFrame)
: MetadataUtils.getGlobalContentBoxForChildren(newParentInstance)

if (currentParentContentBox == null || newParentContentBox == null) {
return []
Expand Down Expand Up @@ -349,6 +354,7 @@ export function getReparentPropertyChanges(
currentMetadata,
projectContents,
'force-pins',
'dont-add-contain-layout',
)

const strategyCommands = runReparentPropertyStrategies([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ export const convertFragmentLikeChildrenToVisualSize =
metadata.currentMetadata,
projectContents,
'force-pins',
'dont-add-contain-layout',
)
} else {
const directions = singleAxisAutoLayoutContainerDirections(
Expand Down
Loading

0 comments on commit 7f377c9

Please sign in to comment.