Skip to content

Commit 2c8aee3

Browse files
gbalintgbalint
authored
Allow reparenting of components without style prop support (#6616)
**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
1 parent bee47ee commit 2c8aee3

File tree

6 files changed

+286
-32
lines changed

6 files changed

+286
-32
lines changed

‎editor/src/components/canvas/canvas-strategies/strategies/absolute-reparent-strategy.tsx

+17-6
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@ import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
33
import { mapDropNulls } from '../../../../core/shared/array-utils'
44
import * as EP from '../../../../core/shared/element-path'
55
import type { ElementPathTrees } from '../../../../core/shared/element-path-tree'
6-
import type { ElementInstanceMetadataMap } from '../../../../core/shared/element-template'
6+
import {
7+
metadataHasPositionAbsoluteOrNull,
8+
type ElementInstanceMetadataMap,
9+
} from '../../../../core/shared/element-template'
710
import type { ElementPath, NodeModules } from '../../../../core/shared/project-file-types'
811
import * as PP from '../../../../core/shared/property-path'
12+
import { assertNever } from '../../../../core/shared/utils'
913
import type { IndexPosition } from '../../../../utils/utils'
1014
import type { ProjectContentTreeRoot } from '../../../assets'
1115
import type { AllElementProps } from '../../../editor/store/editor-state'
@@ -75,10 +79,18 @@ export function baseAbsoluteReparentStrategy(
7579
element,
7680
)
7781

78-
return (
79-
elementMetadata?.specialSizeMeasurements.position === 'absolute' &&
80-
honoursPropsPosition(canvasState, element)
81-
)
82+
const honoursPosition = honoursPropsPosition(canvasState, element)
83+
84+
switch (honoursPosition) {
85+
case 'does-not-honour':
86+
return false
87+
case 'absolute-position-and-honours-numeric-props':
88+
return metadataHasPositionAbsoluteOrNull(elementMetadata)
89+
case 'honours-numeric-props-only':
90+
return elementMetadata?.specialSizeMeasurements.position === 'absolute'
91+
default:
92+
assertNever(honoursPosition)
93+
}
8294
})
8395
if (!isApplicable) {
8496
return null
@@ -164,7 +176,6 @@ export function applyAbsoluteReparent(
164176

165177
const allowedToReparent = selectedElements.every((selectedElement) => {
166178
return isAllowedToReparent(
167-
canvasState.projectContents,
168179
canvasState.startingMetadata,
169180
selectedElement,
170181
newParent.intendedParentPath,

‎editor/src/components/canvas/canvas-strategies/strategies/flow-reparent-to-flow-strategy.spec.browser2.tsx

+257
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,263 @@ describe('Flow Reparent To Flow Strategy', () => {
259259
`),
260260
)
261261
})
262+
263+
it('reparents flow component without style prop support to flow parent', async () => {
264+
const renderResult = await renderTestEditorWithCode(
265+
`import * as React from 'react'
266+
import { Scene, Storyboard, View, Group } from 'utopia-api'
267+
268+
export var App = (props) => {
269+
return (
270+
<div
271+
style={{
272+
position: 'absolute',
273+
width: 700,
274+
height: 600,
275+
}}
276+
data-uid='container'
277+
data-testid='container'
278+
>
279+
<div
280+
style={{
281+
position: 'absolute',
282+
width: 250,
283+
height: 500,
284+
left: 0,
285+
top: 0,
286+
backgroundColor: 'blue',
287+
}}
288+
data-uid='flowparent1'
289+
data-testid='flowparent1'
290+
>
291+
<div
292+
style={{
293+
width: 100,
294+
height: 100,
295+
backgroundColor: 'purple',
296+
}}
297+
data-uid='flowchild1'
298+
data-testid='flowchild1'
299+
/>
300+
<div
301+
style={{
302+
width: 100,
303+
height: 100,
304+
backgroundColor: 'pink',
305+
}}
306+
data-uid='flowchild2'
307+
data-testid='flowchild2'
308+
/>
309+
</div>
310+
<div
311+
style={{
312+
position: 'absolute',
313+
width: 250,
314+
height: 500,
315+
left: 350,
316+
top: 0,
317+
backgroundColor: 'lightgreen',
318+
}}
319+
data-uid='flowparent2'
320+
data-testid='flowparent2'
321+
>
322+
<CompNoStyle data-uid='flowchild3' />
323+
<div
324+
style={{
325+
width: 100,
326+
height: 100,
327+
backgroundColor: 'red',
328+
}}
329+
data-uid='flowchild4'
330+
data-testid='flowchild4'
331+
/>
332+
</div>
333+
</div>
334+
)
335+
}
336+
337+
export var storyboard = (props) => {
338+
return (
339+
<Storyboard data-uid='utopia-storyboard-uid'>
340+
<Scene
341+
style={{
342+
left: 0,
343+
top: 0,
344+
width: 2000,
345+
height: 2000,
346+
}}
347+
data-uid='scene-aaa'
348+
>
349+
<App
350+
data-uid='app-entity'
351+
style={{
352+
position: 'absolute',
353+
bottom: 0,
354+
left: 0,
355+
right: 0,
356+
top: 0,
357+
}}
358+
/>
359+
</Scene>
360+
</Storyboard>
361+
)
362+
}
363+
364+
function CompNoStyle(props) {
365+
return (
366+
<div
367+
style={{
368+
width: 100,
369+
height: 100,
370+
backgroundColor: 'teal',
371+
}}
372+
data-testid="compnostyle"
373+
data-uid='compnostyle'
374+
/>
375+
)
376+
}
377+
`,
378+
'await-first-dom-report',
379+
)
380+
381+
const targetFlowParent = await renderResult.renderedDOM.findByTestId('flowparent1')
382+
const targetFlowParentRect = targetFlowParent.getBoundingClientRect()
383+
const targetFlowParentEnd = {
384+
x: targetFlowParentRect.x + targetFlowParentRect.width / 2,
385+
y: targetFlowParentRect.y + targetFlowParentRect.height - 15,
386+
}
387+
const flowChildToReparent = await renderResult.renderedDOM.findByTestId('compnostyle')
388+
const flowChildToReparentRect = flowChildToReparent.getBoundingClientRect()
389+
const flowChildToReparentCenter = {
390+
x: flowChildToReparentRect.x + flowChildToReparentRect.width / 2,
391+
y: flowChildToReparentRect.y + flowChildToReparentRect.height / 2,
392+
}
393+
394+
await renderResult.getDispatchFollowUpActionsFinished()
395+
const dragDelta = windowPoint({
396+
x: targetFlowParentEnd.x - flowChildToReparentCenter.x + 5,
397+
y: targetFlowParentEnd.y - flowChildToReparentCenter.y,
398+
})
399+
400+
await dragElement(renderResult, 'compnostyle', dragDelta, cmdModifier)
401+
402+
await renderResult.getDispatchFollowUpActionsFinished()
403+
expect(getPrintedUiJsCode(renderResult.getEditorState())).toEqual(
404+
formatTestProjectCode(`import * as React from 'react'
405+
import { Scene, Storyboard, View, Group } from 'utopia-api'
406+
407+
export var App = (props) => {
408+
return (
409+
<div
410+
style={{
411+
position: 'absolute',
412+
width: 700,
413+
height: 600,
414+
}}
415+
data-uid='container'
416+
data-testid='container'
417+
>
418+
<div
419+
style={{
420+
position: 'absolute',
421+
width: 250,
422+
height: 500,
423+
left: 0,
424+
top: 0,
425+
backgroundColor: 'blue',
426+
}}
427+
data-uid='flowparent1'
428+
data-testid='flowparent1'
429+
>
430+
<div
431+
style={{
432+
width: 100,
433+
height: 100,
434+
backgroundColor: 'purple',
435+
}}
436+
data-uid='flowchild1'
437+
data-testid='flowchild1'
438+
/>
439+
<div
440+
style={{
441+
width: 100,
442+
height: 100,
443+
backgroundColor: 'pink',
444+
}}
445+
data-uid='flowchild2'
446+
data-testid='flowchild2'
447+
/>
448+
<CompNoStyle data-uid='flowchild3' />
449+
</div>
450+
<div
451+
style={{
452+
position: 'absolute',
453+
width: 250,
454+
height: 500,
455+
left: 350,
456+
top: 0,
457+
backgroundColor: 'lightgreen',
458+
}}
459+
data-uid='flowparent2'
460+
data-testid='flowparent2'
461+
>
462+
<div
463+
style={{
464+
width: 100,
465+
height: 100,
466+
backgroundColor: 'red',
467+
}}
468+
data-uid='flowchild4'
469+
data-testid='flowchild4'
470+
/>
471+
</div>
472+
</div>
473+
)
474+
}
475+
476+
export var storyboard = (props) => {
477+
return (
478+
<Storyboard data-uid='utopia-storyboard-uid'>
479+
<Scene
480+
style={{
481+
left: 0,
482+
top: 0,
483+
width: 2000,
484+
height: 2000,
485+
}}
486+
data-uid='scene-aaa'
487+
>
488+
<App
489+
data-uid='app-entity'
490+
style={{
491+
position: 'absolute',
492+
bottom: 0,
493+
left: 0,
494+
right: 0,
495+
top: 0,
496+
}}
497+
/>
498+
</Scene>
499+
</Storyboard>
500+
)
501+
}
502+
503+
function CompNoStyle(props) {
504+
return (
505+
<div
506+
style={{
507+
width: 100,
508+
height: 100,
509+
backgroundColor: 'teal',
510+
}}
511+
data-testid="compnostyle"
512+
data-uid='compnostyle'
513+
/>
514+
)
515+
}`),
516+
)
517+
})
518+
262519
it('reparents flow element to flow parent in row layout', async () => {
263520
const renderResult = await renderTestEditorWithCode(
264521
makeTestProjectCodeWithSnippet(`

‎editor/src/components/canvas/canvas-strategies/strategies/grid-reparent-strategy.tsx

-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ export function applyGridReparent(
162162

163163
const allowedToReparent = selectedElements.every((selectedElement) => {
164164
return isAllowedToReparent(
165-
canvasState.projectContents,
166165
canvasState.startingMetadata,
167166
selectedElement,
168167
newParent.intendedParentPath,

‎editor/src/components/canvas/canvas-strategies/strategies/reparent-helpers/reparent-helpers.ts

+3-18
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ import { assertNever } from '../../../../../core/shared/utils'
7878
export type ShouldAddContainLayout = 'add-contain-layout' | 'dont-add-contain-layout'
7979

8080
export function isAllowedToReparent(
81-
projectContents: ProjectContentTreeRoot,
8281
startingMetadata: ElementInstanceMetadataMap,
8382
elementToReparent: ElementPath,
8483
targetParentPath: ElementPath,
@@ -114,15 +113,12 @@ export function isAllowedToReparent(
114113

115114
return foldEither(
116115
(_) => true,
117-
(elementFromMetadata) =>
118-
!elementReferencesElsewhere(elementFromMetadata) &&
119-
MetadataUtils.targetHonoursPropsPosition(projectContents, metadata) !== 'does-not-honour',
116+
(elementFromMetadata) => !elementReferencesElsewhere(elementFromMetadata),
120117
metadata.element,
121118
)
122119
}
123120

124121
export function isAllowedToNavigatorReparent(
125-
projectContents: ProjectContentTreeRoot,
126122
startingMetadata: ElementInstanceMetadataMap,
127123
target: ElementPath,
128124
): boolean {
@@ -137,14 +133,8 @@ export function isAllowedToNavigatorReparent(
137133
return maybeBranchConditionalCase(parentPath, conditional, target) != null
138134
}
139135
return false
140-
} else {
141-
return foldEither(
142-
(_) => true,
143-
(elementFromMetadata) =>
144-
MetadataUtils.targetHonoursPropsPosition(projectContents, metadata) !== 'does-not-honour',
145-
metadata.element,
146-
)
147136
}
137+
return true
148138
}
149139
}
150140

@@ -241,12 +231,7 @@ export function ifAllowedToReparent(
241231
ifAllowed: () => StrategyApplicationResult,
242232
): StrategyApplicationResult {
243233
const allowed = elementsToReparent.every((elementToReparent) => {
244-
return isAllowedToReparent(
245-
canvasState.projectContents,
246-
startingMetadata,
247-
elementToReparent,
248-
targetParentPath,
249-
)
234+
return isAllowedToReparent(startingMetadata, elementToReparent, targetParentPath)
250235
})
251236
if (allowed) {
252237
return ifAllowed()

‎editor/src/components/canvas/ui-jsx-canvas-renderer/animation-context.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,18 @@ export function useCanvasAnimation(paths: ElementPath[]) {
5252
return uids.map((uid) => `[data-uid='${uid}']`).join(',')
5353
}, [uids])
5454

55+
const elements = React.useMemo(
56+
() => (selector == null || selector === '' ? [] : document.querySelectorAll(selector)),
57+
[selector],
58+
)
59+
5560
return React.useCallback(
5661
(keyframes: DOMKeyframesDefinition, options?: DynamicAnimationOptions) => {
57-
if (ctx.animate == null || selector == null) {
62+
if (ctx.animate == null || elements.length === 0 || selector == null) {
5863
return
5964
}
6065
void ctx.animate(selector, keyframes, options)
6166
},
62-
[ctx, selector],
67+
[ctx, elements, selector],
6368
)
6469
}

0 commit comments

Comments
 (0)