From 004cc05d14e7082cdd82fc5f5da199ea5c60e0da Mon Sep 17 00:00:00 2001
From: Balint Gabor <127662+gbalint@users.noreply.github.com>
Date: Fri, 8 Nov 2024 13:51:00 +0100
Subject: [PATCH] Disallow changing location for components which do not
support style props (#6624)
**Problem:**
When a grid contains component items which do not support style props:
- changing location should be disabled, because we can not attach the
gridRow*, gridColumn* props on the component
- reorder should still work, because that just modifies the order of the
grid items as siblings
**Fix:**
I used MetadataUtils.targetRegisteredStyleControlsOrHonoursStyleProps to
decide if a component supports the necessary grid placement style props:
- if `layout` inspector section is enabled in the component annotations,
we should allow changing location
- if gridRow/gridColumn/etc props are taken by the component, then we
should allow changing location
- reorder does not need any constrains, it does not depend on style
props
**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
---
...nge-element-location-duplicate-strategy.ts | 12 +-
.../grid-change-element-location-strategy.ts | 13 +-
...change-location-strategy.spec.browser2.tsx | 246 ++++++++++++++++++
3 files changed, 269 insertions(+), 2 deletions(-)
diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts
index 66296d761c51..a60713a30bd5 100644
--- a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts
+++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts
@@ -51,7 +51,17 @@ export const gridChangeElementLocationDuplicateStrategy: CanvasStrategyFactory =
canvasState.startingMetadata,
selectedElement,
)
- if (selectedElementMetadata == null) {
+ if (
+ selectedElementMetadata == null ||
+ !MetadataUtils.targetRegisteredStyleControlsOrHonoursStyleProps(
+ canvasState.projectContents,
+ selectedElementMetadata,
+ canvasState.propertyControlsInfo,
+ 'layout',
+ ['gridRow', 'gridColumn', 'gridRowStart', 'gridColumnStart', 'gridRowEnd', 'gridColumnEnd'],
+ 'some',
+ )
+ ) {
return null
}
diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts
index 1adc1f4f739d..98dec9e736a4 100644
--- a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts
+++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts
@@ -71,9 +71,20 @@ export const gridChangeElementLocationStrategy: CanvasStrategyFactory = (
canvasState.startingMetadata,
selectedElement,
)
- if (selectedElementMetadata == null) {
+ if (
+ selectedElementMetadata == null ||
+ !MetadataUtils.targetRegisteredStyleControlsOrHonoursStyleProps(
+ canvasState.projectContents,
+ selectedElementMetadata,
+ canvasState.propertyControlsInfo,
+ 'layout',
+ ['gridRow', 'gridColumn', 'gridRowStart', 'gridColumnStart', 'gridRowEnd', 'gridColumnEnd'],
+ 'every',
+ )
+ ) {
return null
}
+
const initialTemplates = getParentGridTemplatesFromChildMeasurements(
selectedElementMetadata.specialSizeMeasurements,
)
diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-element-change-location-strategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/grid-element-change-location-strategy.spec.browser2.tsx
index d1a11cc8377a..ed21826d1673 100644
--- a/editor/src/components/canvas/canvas-strategies/strategies/grid-element-change-location-strategy.spec.browser2.tsx
+++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-element-change-location-strategy.spec.browser2.tsx
@@ -41,6 +41,69 @@ describe('grid element change location strategy', () => {
})
})
+ describe('component items', () => {
+ it('can change the location of components on a grid when component takes style prop', async () => {
+ const editor = await renderTestEditorWithCode(
+ makeProjectCodeWithItemComponent(`export function Item(props) {
+ return (
+
+ )
+}`),
+ 'await-first-dom-report',
+ )
+ const testId = 'aaa'
+ const { gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd } = await runMoveTest(
+ editor,
+ {
+ scale: 1,
+ pathString: `sb/scene/grid/${testId}`,
+ testId: testId,
+ },
+ )
+ expect({ gridRowStart, gridRowEnd, gridColumnStart, gridColumnEnd }).toEqual({
+ gridColumnEnd: '7',
+ gridColumnStart: '3',
+ gridRowEnd: '4',
+ gridRowStart: '2',
+ })
+ })
+
+ it('can not change the location of components on a grid when component doesnt take style prop', async () => {
+ const editor = await renderTestEditorWithCode(
+ makeProjectCodeWithItemComponent(`export function Item(props) {
+ return (
+
+ )
+}`),
+ 'await-first-dom-report',
+ )
+
+ const testId = 'aaa'
+ await runMoveTest(
+ editor,
+ {
+ scale: 1,
+ pathString: `sb/scene/grid/${testId}`,
+ testId: testId,
+ },
+ (ed) => {
+ const strategies = ed.getEditorState().strategyState.sortedApplicableStrategies
+ const changeLocationStrategy = strategies?.find(
+ (s) => s.strategy.id === 'grid-change-element-location-strategy',
+ )
+ expect(changeLocationStrategy).toBeUndefined()
+ const reorderStrategy = strategies?.find(
+ (s) => s.strategy.id === 'reorder-grid-move-strategy',
+ )
+ expect(reorderStrategy).not.toBeUndefined()
+ },
+ )
+ })
+ })
+
it('can change the location of elements in a grid component', async () => {
const editor = await renderTestEditorWithCode(
ProjectCodeGridComponent,
@@ -626,6 +689,23 @@ export var storyboard = (
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,
+ '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 an element without setting positioning (edge of contiguous area)', async () => {
const editor = await renderTestEditorWithCode(ProjectCodeReorder, 'await-first-dom-report')
@@ -797,6 +877,7 @@ async function runMoveTest(
draggedCell?: GridCellCoordinates
tab?: boolean
},
+ midDragCallback?: (editor: EditorRenderResult) => void,
) {
const elementPathToDrag = EP.fromString(props.pathString)
@@ -844,6 +925,10 @@ async function runMoveTest(
if (props.tab) {
await keyDown('Tab')
}
+ if (midDragCallback != null) {
+ midDragCallback(editor)
+ }
+
await mouseUpAtPoint(editor.renderedDOM.getByTestId(CanvasControlsContainerID), endPoint)
return editor.renderedDOM.getByTestId(props.testId).style
@@ -964,6 +1049,90 @@ export var storyboard = (
)
`
+const makeProjectCodeWithItemComponent = (
+ itemComponentCode: string,
+) => `import * as React from 'react'
+import { Scene, Storyboard, Placeholder } from 'utopia-api'
+
+export var storyboard = (
+
+
+
+
+
+)
+
+${itemComponentCode}
+`
+
const ProjectCodeGridComponent = `import * as React from 'react'
import { Scene, Storyboard, Placeholder } from 'utopia-api'
@@ -1141,6 +1310,83 @@ export var storyboard = (
)
`
+const ProjectCodeReorderWithComponentItem = `import * as React from 'react'
+import { Scene, Storyboard } from 'utopia-api'
+
+export var storyboard = (
+
+
+
+
+
+
+)
+
+export function Item(props) {
+ return (
+
+ )
+}
+`
+
const ProjectCodeReorderwithMultiCellChild = `import * as React from 'react'
import { Scene, Storyboard } from 'utopia-api'