feature(groups) Convert group child percentage pins to pixel based pi…

…ns. (#4314) - Added `whenReplacingPercentageValues` property to `SetCssLengthProperty`. - `runSetCssLengthProperty` now adds an appropriate toast when replacing a percentage based property and when it is instructed to warn in this particular case. - `setElementPins` now replaces the children's pin values with pixel based ones in all cases regardless of what was there before. - `runPushIntendedBoundsAndUpdateGroups` ensures that any toasts are carried forward in the editor state patch. - The patching part of `runShowToastCommand` has been extracted into a utility function `addToastPatch`.
concrete-utopia · Oct 4, 2023 · 0c140fb · 0c140fb
1 parent 682d89d
commit 0c140fb
Show file tree

Hide file tree

Showing 6 changed files with 153 additions and 20 deletions.
diff --git a/editor/src/components/canvas/canvas-strategies/strategies/groups.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/groups.spec.browser2.tsx
@@ -32,6 +32,7 @@ import { resizeElement } from './absolute-resize.test-utils'
 import { changeInspectorNumberControl } from '../../../inspector/common/inspector.test-utils'
 import { ParentOutlinesTestIdSuffix } from '../../controls/parent-outlines'
 import { ParentBoundsTestIdSuffix } from '../../controls/parent-bounds'
+import { safeIndex } from '../../../../core/shared/array-utils'
 
 const GroupPath = `${BakedInStoryboardUID}/${TestSceneUID}/${TestAppUID}:root-div/group`
 
@@ -2156,6 +2157,11 @@ describe('Groups behaviors', () => {
           await selectComponentsForTest(editor, [fromString(GroupPath)])
           await resizeElement(editor, { x: 100, y: 50 }, EdgePositionBottomRight, emptyModifiers)
 
+          const toasts = editor.getEditorState().editor.toasts
+          expect(toasts).toHaveLength(1)
+          const firstToast = safeIndex(toasts, 0)
+          expect(firstToast?.id).toEqual('percentage-pin-replaced')
+
           expect(groupDiv.style.width).toBe('300px')
           expect(groupDiv.style.height).toBe('300px')
 
@@ -2178,8 +2184,8 @@ describe('Groups behaviors', () => {
           assertStylePropsSet(editor, `${GroupPath}/child-2`, {
             left: 150,
             top: 120,
-            width: '50%',
-            height: '60%',
+            width: 150,
+            height: 180,
             right: undefined,
             bottom: undefined,
           })
@@ -2212,8 +2218,8 @@ describe('Groups behaviors', () => {
           assertStylePropsSet(editor, `${GroupPath}/child-2`, {
             left: 175,
             top: 140,
-            width: '50%',
-            height: '60%',
+            width: 175,
+            height: 210,
             right: undefined,
             bottom: undefined,
           })

diff --git a/...nents/canvas/canvas-strategies/strategies/keyboard-absolute-move-resize.spec.browser2.tsx b/...nents/canvas/canvas-strategies/strategies/keyboard-absolute-move-resize.spec.browser2.tsx
@@ -265,7 +265,7 @@ describe('Keyboard Absolute Resize E2E', () => {
     })
   })
 
-  it('keeps trueuing up groups as directions change', async () => {
+  it('keeps trueing up groups as directions change', async () => {
     const renderResult = await renderTestEditorWithCode(
       formatTestProjectCode(
         makeTestProjectCodeWithSnippet(`
@@ -349,7 +349,7 @@ describe('Keyboard Absolute Resize E2E', () => {
             >
               <div
                 style={{
-                  height: '100%',
+                  height: 255, 
                   position: 'absolute',
                   left: 0,
                   top: 0,

diff --git a/editor/src/components/canvas/commands/push-intended-bounds-and-update-groups-command.ts b/editor/src/components/canvas/commands/push-intended-bounds-and-update-groups-command.ts
@@ -23,7 +23,7 @@ import type {
 } from '../../editor/store/editor-state'
 import { deriveState, trueUpElementChanged } from '../../editor/store/editor-state'
 import { patchedCreateRemixDerivedDataMemo } from '../../editor/store/remix-derived-data'
-import type { FlexDirection } from '../../inspector/common/css-utils'
+import { cssPixelLength, type FlexDirection } from '../../inspector/common/css-utils'
 import {
   isHugFromStyleAttribute,
   isHugFromStyleAttributeOrNull,
@@ -39,7 +39,11 @@ import type { CreateIfNotExistant } from './adjust-css-length-command'
 import { adjustCssLengthProperties, lengthPropertyToAdjust } from './adjust-css-length-command'
 import type { BaseCommand, CanvasCommand, CommandFunctionResult } from './commands'
 import { foldAndApplyCommandsSimple } from './commands'
-import { setCssLengthProperty, setValueKeepingOriginalUnit } from './set-css-length-command'
+import {
+  setCssLengthProperty,
+  setExplicitCssValue,
+  setValueKeepingOriginalUnit,
+} from './set-css-length-command'
 import type { FrameWithAllPoints } from './utils/group-resize-utils'
 import { rectangleToSixFramePoints } from './utils/group-resize-utils'
 import { sixFramePointsToCanvasRectangle } from './utils/group-resize-utils'
@@ -99,7 +103,10 @@ export const runPushIntendedBoundsAndUpdateGroups = (
   )
 
   // TODO this is the worst editor patch in history, this should be much more fine grained, only patching the elements that changed
-  const editorPatch = { projectContents: { $set: editorAfterResizingAncestors.projectContents } }
+  const editorPatch = {
+    projectContents: { $set: editorAfterResizingAncestors.projectContents },
+    toasts: { $set: editorAfterResizingAncestors.toasts },
+  }
 
   const intendedBoundsPatch =
     commandLifecycle === 'mid-interaction'
@@ -453,33 +460,37 @@ function setElementPins(
       'always',
       target,
       PP.create('style', 'left'),
-      setValueKeepingOriginalUnit(framePoints.left, parentSize.width),
+      setExplicitCssValue(cssPixelLength(framePoints.left)),
       parentFlexDirection,
       'do-not-create-if-doesnt-exist',
+      'warn-about-replacement',
     ),
     setCssLengthProperty(
       'always',
       target,
       PP.create('style', 'top'),
-      setValueKeepingOriginalUnit(framePoints.top, parentSize.height),
+      setExplicitCssValue(cssPixelLength(framePoints.top)),
       parentFlexDirection,
       'do-not-create-if-doesnt-exist',
+      'warn-about-replacement',
     ),
     setCssLengthProperty(
       'always',
       target,
       PP.create('style', 'right'),
-      setValueKeepingOriginalUnit(framePoints.right, parentSize.width),
+      setExplicitCssValue(cssPixelLength(framePoints.right)),
       parentFlexDirection,
       'do-not-create-if-doesnt-exist',
+      'warn-about-replacement',
     ),
     setCssLengthProperty(
       'always',
       target,
       PP.create('style', 'bottom'),
-      setValueKeepingOriginalUnit(framePoints.bottom, parentSize.height),
+      setExplicitCssValue(cssPixelLength(framePoints.bottom)),
       parentFlexDirection,
       'do-not-create-if-doesnt-exist',
+      'warn-about-replacement',
     ),
   ]
 
@@ -489,9 +500,10 @@ function setElementPins(
         'always',
         target,
         PP.create('style', 'width'),
-        setValueKeepingOriginalUnit(framePoints.width, parentSize.width),
+        setExplicitCssValue(cssPixelLength(framePoints.width)),
         parentFlexDirection,
         'do-not-create-if-doesnt-exist',
+        'warn-about-replacement',
       ),
     )
   }
@@ -501,9 +513,10 @@ function setElementPins(
         'always',
         target,
         PP.create('style', 'height'),
-        setValueKeepingOriginalUnit(framePoints.height, parentSize.height),
+        setExplicitCssValue(cssPixelLength(framePoints.height)),
         parentFlexDirection,
         'do-not-create-if-doesnt-exist',
+        'warn-about-replacement',
       ),
     )
   }

diff --git a/editor/src/components/canvas/commands/set-css-length-command.ts b/editor/src/components/canvas/commands/set-css-length-command.ts
@@ -1,3 +1,4 @@
+import { notice } from '../../../components/common/notice'
 import { isLeft, isRight, left } from '../../../core/shared/either'
 import * as EP from '../../../core/shared/element-path'
 import {
@@ -12,7 +13,7 @@ import {
 } from '../../../core/shared/jsx-attributes'
 import type { ElementPath, PropertyPath } from '../../../core/shared/project-file-types'
 import * as PP from '../../../core/shared/property-path'
-import type { DerivedState, EditorState } from '../../editor/store/editor-state'
+import type { DerivedState, EditorState, EditorStatePatch } from '../../editor/store/editor-state'
 import { deriveState, withUnderlyingTargetFromEditorState } from '../../editor/store/editor-state'
 import { patchedCreateRemixDerivedDataMemo } from '../../editor/store/remix-derived-data'
 import type { CSSKeyword, CSSNumber, FlexDirection } from '../../inspector/common/css-utils'
@@ -26,6 +27,7 @@ import type { CreateIfNotExistant } from './adjust-css-length-command'
 import { deleteConflictingPropsForWidthHeight } from './adjust-css-length-command'
 import { applyValuesAtPath } from './adjust-number-command'
 import type { BaseCommand, CommandFunction, WhenToRun } from './commands'
+import { addToastPatch } from './show-toast-command'
 
 type CssNumberOrKeepOriginalUnit =
   | { type: 'EXPLICIT_CSS_NUMBER'; value: CSSNumber | CSSKeyword }
@@ -49,6 +51,7 @@ export interface SetCssLengthProperty extends BaseCommand {
   value: CssNumberOrKeepOriginalUnit
   parentFlexDirection: FlexDirection | null
   createIfNonExistant: CreateIfNotExistant
+  whenReplacingPercentageValues: 'do-not-warn' | 'warn-about-replacement'
 }
 
 export function setCssLengthProperty(
@@ -58,6 +61,7 @@ export function setCssLengthProperty(
   value: CssNumberOrKeepOriginalUnit,
   parentFlexDirection: FlexDirection | null,
   createIfNonExistant: CreateIfNotExistant = 'create-if-not-existing', // TODO remove the default value and set it explicitly everywhere
+  whenReplacingPercentageValues: SetCssLengthProperty['whenReplacingPercentageValues'] = 'do-not-warn',
 ): SetCssLengthProperty {
   return {
     type: 'SET_CSS_LENGTH_PROPERTY',
@@ -67,6 +71,7 @@ export function setCssLengthProperty(
     value: value,
     parentFlexDirection: parentFlexDirection,
     createIfNonExistant: createIfNonExistant,
+    whenReplacingPercentageValues: whenReplacingPercentageValues,
   }
 }
 
@@ -122,6 +127,8 @@ export const runSetCssLengthProperty: CommandFunction<SetCssLengthProperty> = (
 
   let propsToUpdate: Array<ValueAtPath> = []
 
+  let percentageValueWasReplaced: boolean = false
+
   const parsePercentResult = parseCSSPercent(simpleValueResult.value)
   if (
     isRight(parsePercentResult) &&
@@ -146,6 +153,13 @@ export const runSetCssLengthProperty: CommandFunction<SetCssLengthProperty> = (
         ? command.value.value
         : cssPixelLength(command.value.valuePx)
 
+    if (
+      command.whenReplacingPercentageValues === 'warn-about-replacement' &&
+      isRight(parsePercentResult)
+    ) {
+      percentageValueWasReplaced = true
+    }
+
     const printedValue = printCSSNumberOrKeyword(newCssValue, 'px')
 
     propsToUpdate.push({
@@ -161,8 +175,25 @@ export const runSetCssLengthProperty: CommandFunction<SetCssLengthProperty> = (
     propsToUpdate,
   )
 
+  // Always include the property update patch, but potentially also include a warning
+  // that a percentage based property was replaced with a pixel based one.
+  let editorStatePatches: Array<EditorStatePatch> = [propertyUpdatePatch]
+  if (percentageValueWasReplaced) {
+    editorStatePatches.push(
+      addToastPatch(
+        editorStateWithPropsDeleted.toasts,
+        notice(
+          'One or more percentage based style properties were replaced with a pixel based one.',
+          'INFO',
+          false,
+          'percentage-pin-replaced',
+        ),
+      ),
+    )
+  }
+
   return {
-    editorStatePatches: [propertyUpdatePatch],
+    editorStatePatches: editorStatePatches,
     commandDescription: `Set Css Length Prop: ${EP.toUid(command.target)}/${PP.toString(
       command.property,
     )} by ${

diff --git a/editor/src/components/canvas/commands/show-toast-command.ts b/editor/src/components/canvas/commands/show-toast-command.ts
@@ -1,6 +1,6 @@
 import type { Notice, NoticeLevel } from '../../common/notice'
 import { notice } from '../../common/notice'
-import type { EditorState } from '../../editor/store/editor-state'
+import type { EditorState, EditorStatePatch } from '../../editor/store/editor-state'
 import type { InteractionLifecycle } from '../canvas-strategies/canvas-strategy-types'
 import type { CommandFunctionResult, WhenToRun } from './commands'
 
@@ -22,6 +22,14 @@ export function showToastCommand(
   }
 }
 
+export function addToastPatch(
+  currentToasts: ReadonlyArray<Notice>,
+  noticeToAdd: Notice,
+): EditorStatePatch {
+  const updatedToasts = [...currentToasts.filter((t) => t.id !== noticeToAdd.id), noticeToAdd]
+  return { toasts: { $set: updatedToasts } }
+}
+
 export function runShowToastCommand(
   editorState: EditorState,
   command: ShowToastCommand,
@@ -34,9 +42,8 @@ export function runShowToastCommand(
     }
   }
 
-  const toasts = [...editorState.toasts.filter((t) => t.id !== command.notice.id), command.notice]
   return {
     commandDescription: 'Show a toast',
-    editorStatePatches: [{ toasts: { $set: toasts } }],
+    editorStatePatches: [addToastPatch(editorState.toasts, command.notice)],
   }
 }
diff --git a/editor/src/components/editor/shortcuts.spec.browser2.tsx b/editor/src/components/editor/shortcuts.spec.browser2.tsx
@@ -1,3 +1,4 @@
+import { safeIndex } from '../../core/shared/array-utils'
 import { BakedInStoryboardUID, BakedInStoryboardVariableName } from '../../core/model/scene-utils'
 import * as EP from '../../core/shared/element-path'
 import type { CanvasRectangle } from '../../core/shared/math-utils'
@@ -1263,6 +1264,81 @@ describe('group selection', () => {
       )
     })
 
+    it('wraps selected elements with percentage dimensions in a Group', async () => {
+      const renderResult = await renderTestEditorWithCode(
+        makeTestProjectCodeWithSnippet(
+          `<div style={{ ...props.style }} data-uid='container'>
+          <div
+          style={{
+            backgroundColor: '#aaaaaa33',
+            position: 'absolute',
+            left: '5%',
+            top: '10%',
+            width: '50%',
+            height: '20%',
+          }}
+          data-uid='aaa'
+        />
+        <div
+          style={{
+            backgroundColor: '#aaaaaa33',
+            position: 'absolute',
+            left: '10%',
+            top: '15%',
+            width: '70%',
+            height: '80%',
+          }}
+          data-uid='bbb'
+        />
+          </div>`,
+        ),
+        'await-first-dom-report',
+      )
+
+      await selectComponentsForTest(renderResult, [
+        EP.fromString(`${BakedInStoryboardUID}/${TestSceneUID}/${TestAppUID}:container/aaa`),
+        EP.fromString(`${BakedInStoryboardUID}/${TestSceneUID}/${TestAppUID}:container/bbb`),
+      ])
+
+      await expectSingleUndo2Saves(renderResult, async () =>
+        pressKey('g', { modifiers: cmdModifier }),
+      )
+
+      const toasts = renderResult.getEditorState().editor.toasts
+      expect(toasts).toHaveLength(1)
+      const firstToast = safeIndex(toasts, 0)
+      expect(firstToast?.id).toEqual('percentage-pin-replaced')
+
+      expect(getPrintedUiJsCodeWithoutUIDs(renderResult.getEditorState())).toEqual(
+        makeTestProjectCodeWithSnippetWithoutUIDs(
+          `<div style={{ ...props.style }}>
+          <Group style={{ position: 'absolute', left: 20, top: 40, width: 300, height: 340 }}>
+            <div
+              style={{
+                backgroundColor: '#aaaaaa33',
+                position: 'absolute',
+                left: 0,
+                top: 0,
+                width: 200,
+                height: 80,
+              }}
+            />
+            <div
+              style={{
+                backgroundColor: '#aaaaaa33',
+                position: 'absolute',
+                left: 20,
+                top: 20,
+                width: 280,
+                height: 320,
+              }}
+            />
+          </Group>
+        </div>`,
+        ),
+      )
+    })
+
     it('if Group is not imported, it is added to the imports after the Group has been inserted', async () => {
       const editor = await renderTestEditorWithCode(
         `import { Scene, Storyboard } from 'utopia-api'