From f1cfaf185195fa4de7e235a4f0e978107c393913 Mon Sep 17 00:00:00 2001 From: Sean Parsons Date: Tue, 3 Oct 2023 14:09:11 +0100 Subject: [PATCH] feature(groups) Completely disallow percentage based group children. - Change `InvalidGroupState` value `'child-has-percentage-pins-without-group-size'` to `'child-has-percentage-pins'`. - Remove a handful of checks to see if the groups have an explicit size. - Add a test that checks for a child with percentage pins but explicit dimensions. --- .../strategies/group-helpers.ts | 45 ++++----------- ...spector-end-to-end-tests.spec.browser2.tsx | 56 ++++++++++++++++++- .../common/layout-property-path-hooks.ts | 7 +-- .../fill-container-basic-strategy.ts | 4 +- .../fixed-size-basic-strategy.ts | 4 +- 5 files changed, 69 insertions(+), 47 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/group-helpers.ts b/editor/src/components/canvas/canvas-strategies/strategies/group-helpers.ts index 06cb8639aaf4..79546d760296 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/group-helpers.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/group-helpers.ts @@ -94,7 +94,7 @@ export type GroupValidity = 'valid' | 'warning' | 'error' export type InvalidGroupState = | 'child-not-position-absolute' - | 'child-has-percentage-pins-without-group-size' + | 'child-has-percentage-pins' | 'child-has-missing-pins' | 'child-does-not-honour-props-position' | 'child-does-not-honour-props-size' @@ -103,7 +103,7 @@ export type InvalidGroupState = export function groupValidityFromInvalidGroupState(groupState: InvalidGroupState): GroupValidity { switch (groupState) { - case 'child-has-percentage-pins-without-group-size': + case 'child-has-percentage-pins': case 'child-has-missing-pins': case 'group-has-percentage-pins': return 'warning' @@ -139,8 +139,8 @@ export function invalidGroupStateToString(state: InvalidGroupState): string { // children state case 'child-not-position-absolute': return 'All children of group should have absolute position' - case 'child-has-percentage-pins-without-group-size': - return 'Group needs dimensions to use children with % pins' + case 'child-has-percentage-pins': + return 'Children of a group should not have percentage pins' case 'child-has-missing-pins': return 'All children of group should have valid pins' case 'child-does-not-honour-props-position': @@ -261,7 +261,6 @@ function maybeInvalidGroupChildren( allElementProps: AllElementProps, projectContents: ProjectContentTreeRoot, ): InvalidGroupState | 'valid' { - const groupHasExplicitSize = checkGroupHasExplicitSize(group) const childPaths = MetadataUtils.getChildrenUnordered(metadata, path).map( (child) => child.elementPath, ) @@ -274,11 +273,7 @@ function maybeInvalidGroupChildren( for (const childPath of flattenedChildPaths) { const childMetadata = MetadataUtils.findElementByElementPath(metadata, childPath) if (childMetadata != null) { - const childGroupState = getGroupChildState( - projectContents, - childMetadata, - groupHasExplicitSize, - ) + const childGroupState = getGroupChildState(projectContents, childMetadata) if (isInvalidGroupState(childGroupState)) { return childGroupState } @@ -287,13 +282,10 @@ function maybeInvalidGroupChildren( return 'valid' } -export function getGroupChildStateFromJSXElement( - jsxElement: JSXElement, - groupHasExplicitSize: boolean, -): GroupState { +export function getGroupChildStateFromJSXElement(jsxElement: JSXElement): GroupState { return ( maybeGroupChildNotPositionAbsolutely(jsxElement) ?? - maybeGroupChildHasPercentagePinsWithoutGroupSize(jsxElement, groupHasExplicitSize) ?? + maybeGroupChildHasPercentagePinsWithoutGroupSize(jsxElement) ?? maybeGroupChildHasMissingPins(jsxElement) ?? 'valid' ) @@ -313,7 +305,6 @@ export function getGroupValidity( function getGroupChildState( projectContents: ProjectContentTreeRoot, elementMetadata: ElementInstanceMetadata | null, - groupHasExplicitSize: boolean, ): GroupState { if (elementMetadata == null) { return 'unknown' @@ -332,7 +323,7 @@ function getGroupChildState( return 'child-does-not-honour-props-size' } - return getGroupChildStateFromJSXElement(jsxElement, groupHasExplicitSize) + return getGroupChildStateFromJSXElement(jsxElement) } export function getGroupChildStateWithGroupMetadata( @@ -345,11 +336,7 @@ export function getGroupChildStateWithGroupMetadata( return 'unknown' } - return getGroupChildState( - projectContents, - elementMetadata, - checkGroupHasExplicitSize(groupElement), - ) + return getGroupChildState(projectContents, elementMetadata) } export function isMaybeGroupForWrapping(element: JSXElementChild, imports: Imports): boolean { @@ -415,20 +402,11 @@ export function maybeGroupChildNotPositionAbsolutely( export function maybeGroupChildHasPercentagePinsWithoutGroupSize( jsxElement: JSXElement | null, - groupHasExplicitSize: boolean, ): InvalidGroupState | null { if (jsxElement == null) { return 'unknown' } - return !groupHasExplicitSize && elementHasPercentagePins(jsxElement) - ? 'child-has-percentage-pins-without-group-size' - : null -} - -export function maybeGroupChildWithoutFixedSizeForFill( - group: JSXElement | null, -): InvalidGroupState | null { - return !checkGroupHasExplicitSize(group) ? 'child-has-percentage-pins-without-group-size' : null + return elementHasPercentagePins(jsxElement) ? 'child-has-percentage-pins' : null } export function groupErrorToastCommand(state: InvalidGroupState): ShowToastCommand { @@ -500,8 +478,7 @@ export function groupStateFromJSXElement( ) } else if (treatElementAsGroupLike(metadata, EP.parentPath(path))) { // group child - const group = MetadataUtils.getJSXElementFromMetadata(metadata, EP.parentPath(path)) - return getGroupChildStateFromJSXElement(element, checkGroupHasExplicitSize(group)) + return getGroupChildStateFromJSXElement(element) } else { // not a group return null diff --git a/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx b/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx index 8234727854f9..53cded87e221 100644 --- a/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx +++ b/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx @@ -3379,7 +3379,61 @@ describe('inspector tests with real metadata', () => { await setControlValue('position-left-number-input', '25%', renderResult.renderedDOM) expect(getFrame(targetPath, renderResult)).toBe(elementFrame) - expectGroupToast(renderResult, 'child-has-percentage-pins-without-group-size') + expectGroupToast(renderResult, 'child-has-percentage-pins') + }) + it('ignores settings percentage pins on a group child if the parent has explicit width and height', async () => { + const renderResult = await renderTestEditorWithCode( + makeTestProjectCodeWithSnippetStyledComponents(` +
+ +
+ +
+ `), + 'await-first-dom-report', + ) + + const targetPath = EP.appendNewElementPath(TestScenePath, ['aaa', 'group', 'foo']) + + await act(async () => { + const dispatchDone = renderResult.getDispatchFollowUpActionsFinished() + await renderResult.dispatch([selectComponents([targetPath], false)], false) + await dispatchDone + }) + + const leftControl = (await renderResult.renderedDOM.findByTestId( + 'position-left-number-input', + )) as HTMLInputElement + + expect(leftControl.value).toBe('30') + + const elementFrame = getFrame(targetPath, renderResult) + + await setControlValue('position-left-number-input', '25%', renderResult.renderedDOM) + + expect(getFrame(targetPath, renderResult)).toBe(elementFrame) + expectGroupToast(renderResult, 'child-has-percentage-pins') }) describe('group children', () => { const tests: { diff --git a/editor/src/components/inspector/common/layout-property-path-hooks.ts b/editor/src/components/inspector/common/layout-property-path-hooks.ts index 5c8e11c6b497..b50ff9c84d72 100644 --- a/editor/src/components/inspector/common/layout-property-path-hooks.ts +++ b/editor/src/components/inspector/common/layout-property-path-hooks.ts @@ -46,7 +46,6 @@ import { getFramePointsFromMetadata, MaxContent } from '../inspector-common' import { mapDropNulls } from '../../../core/shared/array-utils' import { maybeInvalidGroupState, - maybeGroupChildWithoutFixedSizeForFill, groupErrorToastAction, } from '../../canvas/canvas-strategies/strategies/group-helpers' @@ -416,11 +415,7 @@ export function usePinToggling(): UsePinTogglingResult { : null }, onGroupChild: (path) => { - const group = MetadataUtils.getJSXElementFromMetadata( - jsxMetadataRef.current, - EP.parentPath(path), - ) - return maybeGroupChildWithoutFixedSizeForFill(group) ?? null + return 'child-has-percentage-pins' }, }, ) diff --git a/editor/src/components/inspector/inspector-strategies/fill-container-basic-strategy.ts b/editor/src/components/inspector/inspector-strategies/fill-container-basic-strategy.ts index 4e34651785e2..759e8db7ffd9 100644 --- a/editor/src/components/inspector/inspector-strategies/fill-container-basic-strategy.ts +++ b/editor/src/components/inspector/inspector-strategies/fill-container-basic-strategy.ts @@ -23,7 +23,6 @@ import { } from '../../canvas/commands/set-css-length-command' import { groupErrorToastCommand, - maybeGroupChildWithoutFixedSizeForFill, maybeInvalidGroupState, } from '../../canvas/canvas-strategies/strategies/group-helpers' @@ -45,8 +44,7 @@ export const fillContainerStrategyFlow = ( const invalidGroupState = maybeInvalidGroupState(elements, metadata, { onGroup: () => 'group-has-percentage-pins', onGroupChild: (path) => { - const group = MetadataUtils.getJSXElementFromMetadata(metadata, EP.parentPath(path)) - return maybeGroupChildWithoutFixedSizeForFill(group) ?? null + return 'child-has-percentage-pins' }, }) if (invalidGroupState != null) { diff --git a/editor/src/components/inspector/inspector-strategies/fixed-size-basic-strategy.ts b/editor/src/components/inspector/inspector-strategies/fixed-size-basic-strategy.ts index 28617b24ce30..6f9f9552a49f 100644 --- a/editor/src/components/inspector/inspector-strategies/fixed-size-basic-strategy.ts +++ b/editor/src/components/inspector/inspector-strategies/fixed-size-basic-strategy.ts @@ -13,7 +13,6 @@ import type { InspectorStrategy } from './inspector-strategy' import { queueGroupTrueUp } from '../../canvas/commands/queue-group-true-up-command' import { groupErrorToastCommand, - maybeGroupChildWithoutFixedSizeForFill, maybeInvalidGroupState, } from '../../canvas/canvas-strategies/strategies/group-helpers' import { trueUpElementChanged } from '../../../components/editor/store/editor-state' @@ -32,8 +31,7 @@ export const fixedSizeBasicStrategy = ( const invalidGroupState = maybeInvalidGroupState(elementPaths, metadata, { onGroup: () => (value.unit === '%' ? 'group-has-percentage-pins' : null), onGroupChild: (path) => { - const group = MetadataUtils.getJSXElementFromMetadata(metadata, EP.parentPath(path)) - return value.unit === '%' ? maybeGroupChildWithoutFixedSizeForFill(group) : null + return value.unit === '%' ? 'child-has-percentage-pins' : null }, }) if (invalidGroupState != null) {