From 0bc349e29778b53e53d83659810c53acf5cdd290 Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Fri, 27 Oct 2023 18:43:11 +0200 Subject: [PATCH] Hide constraints section for non-absolute children (#4437) --- ...spector-end-to-end-tests.spec.browser2.tsx | 17 +-- .../constraints-section.spec.browser2.tsx | 82 ++++++++++++-- .../inspector/constraints-section.tsx | 17 ++- .../inspector/fill-hug-fixed-control.tsx | 102 +++++++++--------- ...performance-regression-tests.spec.tsx.snap | 22 ++++ .../performance-regression-tests.spec.tsx | 2 +- 6 files changed, 163 insertions(+), 79 deletions(-) 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 34b806d88aba..9b5abf44e115 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 @@ -1806,6 +1806,7 @@ describe('inspector tests with real metadata', () => {
) @@ -2129,9 +2130,6 @@ describe('inspector tests with real metadata', () => { `"simple"`, ) - expectSelectControlValue(renderResult, 'frame-child-constraint-width-popuplist', 'Mixed') // TODO this should be Left - expectSelectControlValue(renderResult, 'frame-child-constraint-height-popuplist', 'Mixed') // TODO this should be Top - matchInlineSnapshotBrowser(metadata.computedStyle?.['opacity'], `"1"`) matchInlineSnapshotBrowser(opacityControl.value, `"1"`) matchInlineSnapshotBrowser( @@ -2152,7 +2150,7 @@ describe('inspector tests with real metadata', () => { }} data-uid={'aaa'} > -
hello
+
hello
`), 'await-first-dom-report', @@ -2193,7 +2191,7 @@ describe('inspector tests with real metadata', () => { }} data-uid={'aaa'} > -
hello
+
hello
`), 'await-first-dom-report', @@ -2237,9 +2235,6 @@ describe('inspector tests with real metadata', () => { // flexShrink.attributes.getNamedItemNS(null, 'data-controlstatus')?.value, // `"simple"`, // ) - - expectSelectControlValue(renderResult, 'frame-child-constraint-width-popuplist', 'Mixed') // TODO this should be Left - expectSelectControlValue(renderResult, 'frame-child-constraint-height-popuplist', 'Mixed') // TODO this should be Top }) it('Flex longhand properties', async () => { @@ -2302,9 +2297,6 @@ describe('inspector tests with real metadata', () => { // flexShrink.attributes.getNamedItemNS(null, 'data-controlstatus')?.value, // `"simple"`, // ) - - expectSelectControlValue(renderResult, 'frame-child-constraint-width-popuplist', 'Mixed') // TODO this should be Left - expectSelectControlValue(renderResult, 'frame-child-constraint-height-popuplist', 'Mixed') // TODO this should be Top }) it('Flex longhand in style props using a simple expression', async () => { const renderResult = await renderTestEditorWithCode( @@ -2353,9 +2345,6 @@ describe('inspector tests with real metadata', () => { heightControl.attributes.getNamedItemNS(null, 'data-controlstatus')?.value, `"simple"`, ) - - expectSelectControlValue(renderResult, 'frame-child-constraint-width-popuplist', 'Mixed') // TODO this should be Left - expectSelectControlValue(renderResult, 'frame-child-constraint-height-popuplist', 'Mixed') // TODO this should be Top }) it('Shows multifile selected element properties', async () => { let projectContents: ProjectContents = { diff --git a/editor/src/components/inspector/constraints-section.spec.browser2.tsx b/editor/src/components/inspector/constraints-section.spec.browser2.tsx index 1734377d000c..ed2b64225c53 100644 --- a/editor/src/components/inspector/constraints-section.spec.browser2.tsx +++ b/editor/src/components/inspector/constraints-section.spec.browser2.tsx @@ -167,7 +167,7 @@ describe('Constraints Section', () => { it( 'Span without size shows up as Scale / Scale constrained', testGroupChild({ - snippet: `An implicitly constrained span`, + snippet: `An implicitly constrained span`, expectedWidthConstraintDropdownOption: 'Scale', expectedHeightConstraintDropdownOption: 'Scale', }), @@ -176,7 +176,7 @@ describe('Constraints Section', () => { it( 'Span with only width shows up as Scale / Scale', testGroupChild({ - snippet: `An implicitly constrained span`, + snippet: `An implicitly constrained span`, expectedWidthConstraintDropdownOption: 'Scale', expectedHeightConstraintDropdownOption: 'Scale', }), @@ -185,7 +185,7 @@ describe('Constraints Section', () => { it( 'Span with max-content width shows up as Width / Scale', testGroupChild({ - snippet: `An implicitly constrained span`, + snippet: `An implicitly constrained span`, expectedWidthConstraintDropdownOption: 'Width', expectedHeightConstraintDropdownOption: 'Scale', }), @@ -194,7 +194,7 @@ describe('Constraints Section', () => { it( 'Element with max-content width does not change if selecting the same option again', testGroupChild({ - snippet: `An implicitly constrained span`, + snippet: `An implicitly constrained span`, expectedWidthConstraintDropdownOption: 'Width', expectedHeightConstraintDropdownOption: 'Scale', after: async (renderResult) => { @@ -222,7 +222,7 @@ describe('Constraints Section', () => { it( 'Span with max-content width height shows up as Width / Height', testGroupChild({ - snippet: `An implicitly constrained span`, + snippet: `An implicitly constrained span`, expectedWidthConstraintDropdownOption: 'Width', expectedHeightConstraintDropdownOption: 'Height', }), @@ -231,7 +231,7 @@ describe('Constraints Section', () => { it( 'Span with size shows up as Scale', testGroupChild({ - snippet: `An implicitly constrained span`, + snippet: `An implicitly constrained span`, expectedWidthConstraintDropdownOption: 'Scale', expectedHeightConstraintDropdownOption: 'Scale', }), @@ -240,7 +240,7 @@ describe('Constraints Section', () => { it( 'Div with right and bottom pins and size shows up as Right / Bottom', testGroupChild({ - snippet: `An implicitly constrained span`, + snippet: `An implicitly constrained span`, expectedWidthConstraintDropdownOption: 'Right', expectedHeightConstraintDropdownOption: 'Bottom', }), @@ -273,7 +273,6 @@ describe('Constraints Section', () => { const section = screen.queryByTestId(InspectorSectionConstraintsTestId) expect(section).toBeNull() }) - it('is hidden when the selection contains groups', async () => { const renderResult = await renderTestEditorWithCode( formatTestProjectCode(` @@ -329,6 +328,73 @@ describe('Constraints Section', () => { true, ) + const section = screen.queryByTestId(InspectorSectionConstraintsTestId) + expect(section).toBeNull() + }) + it('is hidden when the selection contains non-absolute elements', async () => { + const renderResult = await renderTestEditorWithCode( + formatTestProjectCode(` + import * as React from 'react' + import { Group, Storyboard } from 'utopia-api' + + var storyboard = () => { + return ( + + +
+
+ +
+
+
+
+
+ + ) + } + `), + 'await-first-dom-report', + ) + + await renderResult.dispatch( + [selectComponents([EP.fromString('sb/group'), EP.fromString('sb/flex/foo2')], true)], + true, + ) + + const section = screen.queryByTestId(InspectorSectionConstraintsTestId) + expect(section).toBeNull() + }) + it('is hidden when the selection contains non-absolute elements (bad group child)', async () => { + // most likely this will never happen in reality, because group children are always absolute, but + // this is to validate the intersection with the other conditions. + const renderResult = await renderTestEditorWithCode( + formatTestProjectCode(` + import * as React from 'react' + import { Group, Storyboard } from 'utopia-api' + + var storyboard = () => { + return ( + + +
+
+ + +
+
+ + + ) + } + `), + 'await-first-dom-report', + ) + + await renderResult.dispatch( + [selectComponents([EP.fromString('sb/group'), EP.fromString('sb/grop2/bad')], true)], + true, + ) + const section = screen.queryByTestId(InspectorSectionConstraintsTestId) expect(section).toBeNull() }) diff --git a/editor/src/components/inspector/constraints-section.tsx b/editor/src/components/inspector/constraints-section.tsx index bc54ddeeece7..e2c8e5f24c29 100644 --- a/editor/src/components/inspector/constraints-section.tsx +++ b/editor/src/components/inspector/constraints-section.tsx @@ -17,6 +17,7 @@ import { InspectorPropsContext } from './common/property-path-hooks' import { PinControl, PinHeightControl, PinWidthControl } from './controls/pin-control' import { allElementsAreGroupChildren, + allElementsArePositionedAbsolutelySelector, anySelectedElementGroupOrChildOfGroup, } from './fill-hug-fixed-control' import { selectedViewsSelector } from './inpector-selectors' @@ -51,10 +52,22 @@ export const ConstraintsSection = React.memo(() => { allElementsAreGroupChildren, 'ConstraintsSection onlyGroupChildrenSelected', ) + const allElementsArePositionedAbsolutely = useEditorState( + Substores.metadata, + allElementsArePositionedAbsolutelySelector, + 'ConstraintsSection allElementsArePositionedAbsolutely', + ) const showSection = React.useMemo(() => { - return noGroupOrGroupChildrenSelected || onlyGroupChildrenSelected - }, [noGroupOrGroupChildrenSelected, onlyGroupChildrenSelected]) + return ( + allElementsArePositionedAbsolutely && + (noGroupOrGroupChildrenSelected || onlyGroupChildrenSelected) + ) + }, [ + noGroupOrGroupChildrenSelected, + onlyGroupChildrenSelected, + allElementsArePositionedAbsolutely, + ]) if (!showSection) { return null diff --git a/editor/src/components/inspector/fill-hug-fixed-control.tsx b/editor/src/components/inspector/fill-hug-fixed-control.tsx index d96b90fef7e5..ddf01dac28ee 100644 --- a/editor/src/components/inspector/fill-hug-fixed-control.tsx +++ b/editor/src/components/inspector/fill-hug-fixed-control.tsx @@ -1,30 +1,49 @@ /** @jsxRuntime classic */ /** @jsx jsx */ import { jsx } from '@emotion/react' +import createCachedSelector from 're-reselect' import React from 'react' import { createSelector } from 'reselect' +import { FlexCol } from 'utopia-api' +import type { LayoutPinnedPropIncludingCenter } from '../../core/layout/layout-helpers-new' +import { type LayoutPinnedProp } from '../../core/layout/layout-helpers-new' +import { MetadataUtils } from '../../core/model/element-metadata-utils' +import { mapDropNulls, safeIndex, strictEvery } from '../../core/shared/array-utils' +import * as EP from '../../core/shared/element-path' +import type { ElementInstanceMetadataMap } from '../../core/shared/element-template' +import { emptyComments, jsExpressionValue } from '../../core/shared/element-template' import { optionalMap } from '../../core/shared/optional-utils' +import type { ElementPath } from '../../core/shared/project-file-types' +import * as PP from '../../core/shared/property-path' import { intersection } from '../../core/shared/set-utils' import { assertNever } from '../../core/shared/utils' -import { - FlexRow, - InspectorSubsectionHeader, - NumberInput, - PopupList, - useColorTheme, - UtopiaTheme, -} from '../../uuiui' -import type { - ControlStatus, - ControlStyles, - OnSubmitValueOrUnknownOrEmpty, - SelectOption, -} from '../../uuiui-deps' +import { NumberInput, PopupList, useColorTheme, UtopiaTheme } from '../../uuiui' +import type { SelectOption } from '../../uuiui-deps' import { getControlStyles, InspectorRowHoverCSS } from '../../uuiui-deps' +import { MixedPlaceholder } from '../../uuiui/inputs/base-input' +import { + convertGroupToFrameCommands, + getInstanceForGroupToFrameConversion, + isConversionForbidden, +} from '../canvas/canvas-strategies/strategies/group-conversion-helpers' +import { treatElementAsGroupLike } from '../canvas/canvas-strategies/strategies/group-helpers' +import { notice } from '../common/notice' +import type { EditorAction, EditorDispatch } from '../editor/action-types' +import { + applyCommandsAction, + setProp_UNSAFE, + showToast, + unsetProperty, +} from '../editor/actions/action-creators' import { useDispatch } from '../editor/store/dispatch-context' +import type { AllElementProps } from '../editor/store/editor-state' import { Substores, useEditorState, useRefEditorState } from '../editor/store/store-hook' +import type { MetadataSubstate } from '../editor/store/store-hook-substore-types' +import { fixedSizeDimensionHandlingText } from '../text-editor/text-handling' import type { CSSNumber, CSSNumberType, UnknownOrEmptyInput } from './common/css-utils' import { cssNumber } from './common/css-utils' +import type { FramePinsInfo } from './common/layout-property-path-hooks' +import { PinControl } from './controls/pin-control' import { metadataSelector, pathTreesSelector, @@ -45,40 +64,8 @@ import { } from './inspector-strategies/inspector-strategies' import type { InspectorStrategy } from './inspector-strategies/inspector-strategy' import { executeFirstApplicableStrategy } from './inspector-strategies/inspector-strategy' -import type { MetadataSubstate } from '../editor/store/store-hook-substore-types' -import type { ElementPath } from '../../core/shared/project-file-types' -import { treatElementAsGroupLike } from '../canvas/canvas-strategies/strategies/group-helpers' -import * as EP from '../../core/shared/element-path' -import * as PP from '../../core/shared/property-path' import type { GridRowVariant } from './widgets/ui-grid-row' import { UIGridRow } from './widgets/ui-grid-row' -import { mapDropNulls, safeIndex, uniqBy } from '../../core/shared/array-utils' -import { fixedSizeDimensionHandlingText } from '../text-editor/text-handling' -import { when } from '../../utils/react-conditionals' -import type { LayoutPinnedPropIncludingCenter } from '../../core/layout/layout-helpers-new' -import { isLayoutPinnedProp, type LayoutPinnedProp } from '../../core/layout/layout-helpers-new' -import type { AllElementProps, EditorState } from '../editor/store/editor-state' -import type { ElementInstanceMetadataMap } from '../../core/shared/element-template' -import { jsExpressionValue, emptyComments } from '../../core/shared/element-template' -import type { EditorDispatch, EditorAction } from '../editor/action-types' -import { - unsetProperty, - setProp_UNSAFE, - applyCommandsAction, - showToast, -} from '../editor/actions/action-creators' -import { FlexCol } from 'utopia-api' -import { PinControl } from './controls/pin-control' -import type { FramePinsInfo } from './common/layout-property-path-hooks' -import { MixedPlaceholder } from '../../uuiui/inputs/base-input' -import { PinHeightSVG, PinWidthSVG } from './utility-controls/pin-control' -import { - convertGroupToFrameCommands, - getInstanceForGroupToFrameConversion, - isConversionForbidden, -} from '../canvas/canvas-strategies/strategies/group-conversion-helpers' -import { notice } from '../common/notice' -import createCachedSelector from 're-reselect' export const FillFixedHugControlId = (segment: 'width' | 'height'): string => `hug-fixed-fill-${segment}` @@ -730,16 +717,23 @@ export const anySelectedElementGroupOrChildOfGroup = createSelector( export const allElementsAreGroupChildren = createSelector( metadataSelector, - (store: MetadataSubstate) => store.editor.elementPathTree, selectedViewsSelector, - (metadata, pathTrees, selectedViews): boolean => { - if (selectedViews.length === 0) { - return false - } - function elementOrAnyChildGroup(path: ElementPath) { + (metadata, selectedViews): boolean => { + return strictEvery(selectedViews, (path: ElementPath) => { return treatElementAsGroupLike(metadata, EP.parentPath(path)) - } - return selectedViews.every(elementOrAnyChildGroup) + }) + }, +) + +export const allElementsArePositionedAbsolutelySelector = createSelector( + metadataSelector, + selectedViewsSelector, + (metadata, selectedViews): boolean => { + return strictEvery(selectedViews, (path) => { + return MetadataUtils.isPositionAbsolute( + MetadataUtils.findElementByElementPath(metadata, path), + ) + }) }, ) diff --git a/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap b/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap index 1d834c3a5ec9..f8dc6d98b4df 100644 --- a/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap +++ b/editor/src/core/performance/__snapshots__/performance-regression-tests.spec.tsx.snap @@ -2438,5 +2438,27 @@ Array [ "/UtopiaSpiedFunctionComponent(ComponentRenderer(App))/Symbol(react.forward_ref)(SpyWrapper)/UtopiaSpiedFunctionComponent(View)/div", "/UtopiaSpiedFunctionComponent(View)/div/Symbol(react.forward_ref)(SpyWrapper)/UtopiaSpiedFunctionComponent(View)", "/div/Symbol(react.forward_ref)(SpyWrapper)/UtopiaSpiedFunctionComponent(View)/div", + "/Symbol(react.forward_ref)(Styled(div))/div//Symbol(react.memo)()", + "/div///span", + "/div///span", + "/div///Symbol(react.memo)()", + "/div///Symbol(react.memo)()", + "/div///Symbol(react.forward_ref)(Styled(div))", + "/div///UtopiaSpiedClass(Resizable)", + "///UtopiaSpiedClass(Resizable)/UtopiaSpiedClass(Resizer)", + "///UtopiaSpiedClass(Resizable)/div", + "///UtopiaSpiedClass(Resizable)/div", + "/div/null/Symbol(react.forward_ref)(Styled(div))/div", + "/Symbol(react.forward_ref)(Styled(div))/div//UtopiaSpiedFunctionComponent(SimpleFlexRow)", + "/Symbol(react.forward_ref)(Styled(div))/div//UtopiaSpiedFunctionComponent(SimpleFlexRow)", + "/Symbol(react.forward_ref)(Styled(div))/div//UtopiaSpiedFunctionComponent(SimpleFlexRow)", + "/Symbol(react.forward_ref)(Styled(div))/div//Symbol(react.forward_ref)(EmotionCssPropInternal)", + "/Symbol(react.forward_ref)(Styled(div))/div//UtopiaSpiedFunctionComponent(SimpleFlexRow)", + "/div//UtopiaSpiedFunctionComponent(SimpleFlexRow)/div", + "/UtopiaSpiedFunctionComponent(SimpleFlexRow)/div/UtopiaSpiedFunctionComponent(SimpleFlexRow)/div", + "/UtopiaSpiedFunctionComponent(SimpleFlexRow)/div/UtopiaSpiedFunctionComponent(SimpleFlexRow)/div", + "/UtopiaSpiedFunctionComponent(SimpleFlexRow)/div/UtopiaSpiedFunctionComponent(SimpleFlexRow)/div", + "/UtopiaSpiedFunctionComponent(SimpleFlexRow)/div/Symbol(react.forward_ref)(EmotionCssPropInternal)/Symbol(react.forward_ref)(Styled(div))", + "/div/Symbol(react.forward_ref)(EmotionCssPropInternal)/Symbol(react.forward_ref)(Styled(div))/div", ] `; diff --git a/editor/src/core/performance/performance-regression-tests.spec.tsx b/editor/src/core/performance/performance-regression-tests.spec.tsx index c9e9b8441e1d..29c2306e27cb 100644 --- a/editor/src/core/performance/performance-regression-tests.spec.tsx +++ b/editor/src/core/performance/performance-regression-tests.spec.tsx @@ -65,7 +65,7 @@ describe('React Render Count Tests -', () => { const renderCountAfter = renderResult.getNumberOfRenders() // if this breaks, GREAT NEWS but update the test please :) - expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`698`) + expect(renderCountAfter - renderCountBefore).toMatchInlineSnapshot(`720`) expect(renderResult.getRenderInfo()).toMatchSnapshot() })