Skip to content

Commit

Permalink
Allow reparenting of components without style prop support (#6616)
Browse files Browse the repository at this point in the history
**Problem:**
Components without style prop support are not allowed to be reparented.

**Fix:**
- Remove the condition which disallows reparenting when the target does
not honours position style props.
- Fix condition in absolute reparenting that honouring style props is
necessary for absolute reparent
- Fix exception in grid rearrange animation when the selector does not
find an element
- Added test

**Problems to solve:**
This PR is a technical PR to support reparenting, but the experience is
quite bad:
1. Moving of these components is not allowed, so we show the
notpermitted cursor until you stay in the original parent (so most users
would probably give up before actually trying to reparent)
2. Reparent to flow is allowed, but that doesn't render the element
(only a blue line between siblings, if there are siblings), and does not
even render an outline (which is rendered inside the original parent)

**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 6, 2024
1 parent bee47ee commit 2c8aee3
Show file tree
Hide file tree
Showing 6 changed files with 286 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@ import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import { mapDropNulls } from '../../../../core/shared/array-utils'
import * as EP from '../../../../core/shared/element-path'
import type { ElementPathTrees } from '../../../../core/shared/element-path-tree'
import type { ElementInstanceMetadataMap } from '../../../../core/shared/element-template'
import {
metadataHasPositionAbsoluteOrNull,
type ElementInstanceMetadataMap,
} from '../../../../core/shared/element-template'
import type { ElementPath, NodeModules } from '../../../../core/shared/project-file-types'
import * as PP from '../../../../core/shared/property-path'
import { assertNever } from '../../../../core/shared/utils'
import type { IndexPosition } from '../../../../utils/utils'
import type { ProjectContentTreeRoot } from '../../../assets'
import type { AllElementProps } from '../../../editor/store/editor-state'
Expand Down Expand Up @@ -75,10 +79,18 @@ export function baseAbsoluteReparentStrategy(
element,
)

return (
elementMetadata?.specialSizeMeasurements.position === 'absolute' &&
honoursPropsPosition(canvasState, element)
)
const honoursPosition = honoursPropsPosition(canvasState, element)

switch (honoursPosition) {
case 'does-not-honour':
return false
case 'absolute-position-and-honours-numeric-props':
return metadataHasPositionAbsoluteOrNull(elementMetadata)
case 'honours-numeric-props-only':
return elementMetadata?.specialSizeMeasurements.position === 'absolute'
default:
assertNever(honoursPosition)
}
})
if (!isApplicable) {
return null
Expand Down Expand Up @@ -164,7 +176,6 @@ export function applyAbsoluteReparent(

const allowedToReparent = selectedElements.every((selectedElement) => {
return isAllowedToReparent(
canvasState.projectContents,
canvasState.startingMetadata,
selectedElement,
newParent.intendedParentPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,263 @@ describe('Flow Reparent To Flow Strategy', () => {
`),
)
})

it('reparents flow component without style prop support to flow parent', async () => {
const renderResult = await renderTestEditorWithCode(
`import * as React from 'react'
import { Scene, Storyboard, View, Group } from 'utopia-api'
export var App = (props) => {
return (
<div
style={{
position: 'absolute',
width: 700,
height: 600,
}}
data-uid='container'
data-testid='container'
>
<div
style={{
position: 'absolute',
width: 250,
height: 500,
left: 0,
top: 0,
backgroundColor: 'blue',
}}
data-uid='flowparent1'
data-testid='flowparent1'
>
<div
style={{
width: 100,
height: 100,
backgroundColor: 'purple',
}}
data-uid='flowchild1'
data-testid='flowchild1'
/>
<div
style={{
width: 100,
height: 100,
backgroundColor: 'pink',
}}
data-uid='flowchild2'
data-testid='flowchild2'
/>
</div>
<div
style={{
position: 'absolute',
width: 250,
height: 500,
left: 350,
top: 0,
backgroundColor: 'lightgreen',
}}
data-uid='flowparent2'
data-testid='flowparent2'
>
<CompNoStyle data-uid='flowchild3' />
<div
style={{
width: 100,
height: 100,
backgroundColor: 'red',
}}
data-uid='flowchild4'
data-testid='flowchild4'
/>
</div>
</div>
)
}
export var storyboard = (props) => {
return (
<Storyboard data-uid='utopia-storyboard-uid'>
<Scene
style={{
left: 0,
top: 0,
width: 2000,
height: 2000,
}}
data-uid='scene-aaa'
>
<App
data-uid='app-entity'
style={{
position: 'absolute',
bottom: 0,
left: 0,
right: 0,
top: 0,
}}
/>
</Scene>
</Storyboard>
)
}
function CompNoStyle(props) {
return (
<div
style={{
width: 100,
height: 100,
backgroundColor: 'teal',
}}
data-testid="compnostyle"
data-uid='compnostyle'
/>
)
}
`,
'await-first-dom-report',
)

const targetFlowParent = await renderResult.renderedDOM.findByTestId('flowparent1')
const targetFlowParentRect = targetFlowParent.getBoundingClientRect()
const targetFlowParentEnd = {
x: targetFlowParentRect.x + targetFlowParentRect.width / 2,
y: targetFlowParentRect.y + targetFlowParentRect.height - 15,
}
const flowChildToReparent = await renderResult.renderedDOM.findByTestId('compnostyle')
const flowChildToReparentRect = flowChildToReparent.getBoundingClientRect()
const flowChildToReparentCenter = {
x: flowChildToReparentRect.x + flowChildToReparentRect.width / 2,
y: flowChildToReparentRect.y + flowChildToReparentRect.height / 2,
}

await renderResult.getDispatchFollowUpActionsFinished()
const dragDelta = windowPoint({
x: targetFlowParentEnd.x - flowChildToReparentCenter.x + 5,
y: targetFlowParentEnd.y - flowChildToReparentCenter.y,
})

await dragElement(renderResult, 'compnostyle', dragDelta, cmdModifier)

await renderResult.getDispatchFollowUpActionsFinished()
expect(getPrintedUiJsCode(renderResult.getEditorState())).toEqual(
formatTestProjectCode(`import * as React from 'react'
import { Scene, Storyboard, View, Group } from 'utopia-api'
export var App = (props) => {
return (
<div
style={{
position: 'absolute',
width: 700,
height: 600,
}}
data-uid='container'
data-testid='container'
>
<div
style={{
position: 'absolute',
width: 250,
height: 500,
left: 0,
top: 0,
backgroundColor: 'blue',
}}
data-uid='flowparent1'
data-testid='flowparent1'
>
<div
style={{
width: 100,
height: 100,
backgroundColor: 'purple',
}}
data-uid='flowchild1'
data-testid='flowchild1'
/>
<div
style={{
width: 100,
height: 100,
backgroundColor: 'pink',
}}
data-uid='flowchild2'
data-testid='flowchild2'
/>
<CompNoStyle data-uid='flowchild3' />
</div>
<div
style={{
position: 'absolute',
width: 250,
height: 500,
left: 350,
top: 0,
backgroundColor: 'lightgreen',
}}
data-uid='flowparent2'
data-testid='flowparent2'
>
<div
style={{
width: 100,
height: 100,
backgroundColor: 'red',
}}
data-uid='flowchild4'
data-testid='flowchild4'
/>
</div>
</div>
)
}
export var storyboard = (props) => {
return (
<Storyboard data-uid='utopia-storyboard-uid'>
<Scene
style={{
left: 0,
top: 0,
width: 2000,
height: 2000,
}}
data-uid='scene-aaa'
>
<App
data-uid='app-entity'
style={{
position: 'absolute',
bottom: 0,
left: 0,
right: 0,
top: 0,
}}
/>
</Scene>
</Storyboard>
)
}
function CompNoStyle(props) {
return (
<div
style={{
width: 100,
height: 100,
backgroundColor: 'teal',
}}
data-testid="compnostyle"
data-uid='compnostyle'
/>
)
}`),
)
})

it('reparents flow element to flow parent in row layout', async () => {
const renderResult = await renderTestEditorWithCode(
makeTestProjectCodeWithSnippet(`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ export function applyGridReparent(

const allowedToReparent = selectedElements.every((selectedElement) => {
return isAllowedToReparent(
canvasState.projectContents,
canvasState.startingMetadata,
selectedElement,
newParent.intendedParentPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ import { assertNever } from '../../../../../core/shared/utils'
export type ShouldAddContainLayout = 'add-contain-layout' | 'dont-add-contain-layout'

export function isAllowedToReparent(
projectContents: ProjectContentTreeRoot,
startingMetadata: ElementInstanceMetadataMap,
elementToReparent: ElementPath,
targetParentPath: ElementPath,
Expand Down Expand Up @@ -114,15 +113,12 @@ export function isAllowedToReparent(

return foldEither(
(_) => true,
(elementFromMetadata) =>
!elementReferencesElsewhere(elementFromMetadata) &&
MetadataUtils.targetHonoursPropsPosition(projectContents, metadata) !== 'does-not-honour',
(elementFromMetadata) => !elementReferencesElsewhere(elementFromMetadata),
metadata.element,
)
}

export function isAllowedToNavigatorReparent(
projectContents: ProjectContentTreeRoot,
startingMetadata: ElementInstanceMetadataMap,
target: ElementPath,
): boolean {
Expand All @@ -137,14 +133,8 @@ export function isAllowedToNavigatorReparent(
return maybeBranchConditionalCase(parentPath, conditional, target) != null
}
return false
} else {
return foldEither(
(_) => true,
(elementFromMetadata) =>
MetadataUtils.targetHonoursPropsPosition(projectContents, metadata) !== 'does-not-honour',
metadata.element,
)
}
return true
}
}

Expand Down Expand Up @@ -241,12 +231,7 @@ export function ifAllowedToReparent(
ifAllowed: () => StrategyApplicationResult,
): StrategyApplicationResult {
const allowed = elementsToReparent.every((elementToReparent) => {
return isAllowedToReparent(
canvasState.projectContents,
startingMetadata,
elementToReparent,
targetParentPath,
)
return isAllowedToReparent(startingMetadata, elementToReparent, targetParentPath)
})
if (allowed) {
return ifAllowed()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,18 @@ export function useCanvasAnimation(paths: ElementPath[]) {
return uids.map((uid) => `[data-uid='${uid}']`).join(',')
}, [uids])

const elements = React.useMemo(
() => (selector == null || selector === '' ? [] : document.querySelectorAll(selector)),
[selector],
)

return React.useCallback(
(keyframes: DOMKeyframesDefinition, options?: DynamicAnimationOptions) => {
if (ctx.animate == null || selector == null) {
if (ctx.animate == null || elements.length === 0 || selector == null) {
return
}
void ctx.animate(selector, keyframes, options)
},
[ctx, selector],
[ctx, elements, selector],
)
}
Loading

0 comments on commit 2c8aee3

Please sign in to comment.