From b7687ff0c396bec53f68feabeaa7b7ea3a5ab4db Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:20:12 +0200 Subject: [PATCH] Grid resize: remove shorthand, serialize other longhand when not auto (#6581) **Problem:** If a grid is configured with the `gridTemplate` shorthand, resizing the grid does not handle that correctly. **Fix:** This PR handles `gridTemplate` shorthands during template changes via the canvas. **An incremental PR will do the same for the inspector template editing**. For example, if a grid is configured as follows: `gridTemplate: 1fr 2fr / 3fr 4fr`, changing its first column to `3.5fr` will result in the following style: ``` gridTemplateColumns: '3.5fr 4fr', gridTemplateRows: '1fr 2fr' ``` If a grid has a shorthand with an axis being the default (a single `auto` keyword), for example `gridTemplate: auto / 3fr 4fr`, the same change as above will result in: ``` gridTemplateColumns: '3.5fr 4fr', ``` Fixes #6580 --- .../strategies/grid-helpers.ts | 9 + .../resize-grid-strategy.spec.browser2.tsx | 388 +++++++++++++++++- .../strategies/resize-grid-strategy.ts | 56 ++- .../src/components/inspector/flex-section.tsx | 11 +- 4 files changed, 429 insertions(+), 35 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-helpers.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-helpers.ts index 8f5029506b21..079a12531a0c 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-helpers.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-helpers.ts @@ -788,3 +788,12 @@ export function findOriginalGrid( return findOriginalGrid(metadata, parentPath) } + +// Returns whether the given dimensions are made of just one item, being a CSS keyword with value "auto". +export function isJustAutoGridDimension(dimensions: GridDimension[]): boolean { + return ( + dimensions.length === 1 && + dimensions[0].type === 'KEYWORD' && + dimensions[0].value.value === 'auto' + ) +} diff --git a/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.spec.browser2.tsx index f086926e2bc6..a8bc6730953f 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.spec.browser2.tsx @@ -5,7 +5,12 @@ import { CanvasControlsContainerID } from '../../controls/new-canvas-controls' import { mouseDownAtPoint, mouseMoveToPoint, mouseUpAtPoint } from '../../event-helpers.test-utils' import { canvasPoint } from '../../../../core/shared/math-utils' -const makeTestProject = (columns: string, rows: string) => ` +const makeTestProject = (params: { + columns: string | null + rows: string | null + shorthand: string | null + height?: number | string +}) => ` import * as React from 'react' import { Storyboard } from 'utopia-api' @@ -21,10 +26,10 @@ export var storyboard = ( display: 'grid', gap: 10, width: 600, - height: 600, - gridTemplateColumns: '${columns}', - gridTemplateRows: '${rows}', - height: 'max-content', + height: ${params.height ?? "'max-content'"}, + ${params.columns != null ? `gridTemplateColumns: '${params.columns}',` : ''} + ${params.rows != null ? `gridTemplateRows: '${params.rows}',` : ''} + ${params.shorthand != null ? `gridTemplate: '${params.shorthand}',` : ''} }} >
{ it('update a fractionally sized column', async () => { const renderResult = await renderTestEditorWithCode( - makeTestProject('2.4fr 1fr 1fr', '99px 109px 90px'), + makeTestProject({ columns: '2.4fr 1fr 1fr', rows: '99px 109px 90px', shorthand: null }), 'await-first-dom-report', ) const target = EP.fromString(`sb/grid/row-1-column-2`) @@ -171,10 +176,9 @@ export var storyboard = ( display: 'grid', gap: 10, width: 600, - height: 600, + height: 'max-content', gridTemplateColumns: '2.4fr 1.8fr 1fr', gridTemplateRows: '99px 109px 90px', - height: 'max-content', }} >
{[1, 2, 3].map((n) => { @@ -310,7 +313,7 @@ export var storyboard = ( it('update a pixel sized row', async () => { const renderResult = await renderTestEditorWithCode( - makeTestProject('2.4fr 1fr 1fr', '99px 109px 90px'), + makeTestProject({ columns: '2.4fr 1fr 1fr', rows: '99px 109px 90px', shorthand: null }), 'await-first-dom-report', ) const target = EP.fromString(`sb/grid/row-1-column-2`) @@ -349,10 +352,9 @@ export var storyboard = ( display: 'grid', gap: 10, width: 600, - height: 600, + height: 'max-content', gridTemplateColumns: '2.4fr 1fr 1fr', gridTemplateRows: '99px 129px 90px', - height: 'max-content', }} >
{ const renderResult = await renderTestEditorWithCode( - makeTestProject('repeat(3, 1fr)', '99px 109px 90px'), + makeTestProject({ columns: 'repeat(3, 1fr)', rows: '99px 109px 90px', shorthand: null }), 'await-first-dom-report', ) const target = EP.fromString(`sb/grid/row-1-column-2`) @@ -465,10 +467,247 @@ export var storyboard = ( display: 'grid', gap: 10, width: 600, - height: 600, + height: 'max-content', gridTemplateColumns: 'repeat(3, 1.5fr)', gridTemplateRows: '99px 109px 90px', + }} + > +
+
+
+
+
+
+
+
+
+
+ +) +`) + }) + + describe('shorthand handling', () => { + it('removes the shorthand and adds the longhand for both the other axii', async () => { + const renderResult = await renderTestEditorWithCode( + makeTestProject({ + shorthand: '99px 109px 90px / 2.4fr 1fr 1fr', + columns: null, + rows: null, + }), + 'await-first-dom-report', + ) + const target = EP.fromString(`sb/grid/row-1-column-2`) + await renderResult.dispatch(selectComponents([target], false), true) + await renderResult.getDispatchFollowUpActionsFinished() + const canvasControlsLayer = renderResult.renderedDOM.getByTestId(CanvasControlsContainerID) + const resizeControl = renderResult.renderedDOM.getByTestId(`grid-column-handle-1`) + const resizeControlRect = resizeControl.getBoundingClientRect() + const startPoint = canvasPoint({ + x: resizeControlRect.x + resizeControlRect.width / 2, + y: resizeControlRect.y + resizeControlRect.height / 2, + }) + const endPoint = canvasPoint({ + x: startPoint.x + 100, + y: startPoint.y, + }) + await mouseMoveToPoint(resizeControl, startPoint) + await mouseDownAtPoint(resizeControl, startPoint) + await mouseMoveToPoint(canvasControlsLayer, endPoint) + await mouseUpAtPoint(canvasControlsLayer, endPoint) + await renderResult.getDispatchFollowUpActionsFinished() + + expect(getPrintedUiJsCode(renderResult.getEditorState())) + .toEqual(`import * as React from 'react' +import { Storyboard } from 'utopia-api' + +export var storyboard = ( + +
+
+
+
+
+
+
+
+
+
+
+ +) +`) + }) + it("removes the shorthand without adding the longhand if it's auto", async () => { + const renderResult = await renderTestEditorWithCode( + makeTestProject({ + shorthand: 'auto / 2.4fr 1fr 1fr', + columns: null, + rows: null, + height: 600, + }), + 'await-first-dom-report', + ) + const target = EP.fromString(`sb/grid/row-1-column-2`) + await renderResult.dispatch(selectComponents([target], false), true) + await renderResult.getDispatchFollowUpActionsFinished() + const canvasControlsLayer = renderResult.renderedDOM.getByTestId(CanvasControlsContainerID) + const resizeControl = renderResult.renderedDOM.getByTestId(`grid-column-handle-1`) + const resizeControlRect = resizeControl.getBoundingClientRect() + const startPoint = canvasPoint({ + x: resizeControlRect.x + resizeControlRect.width / 2, + y: resizeControlRect.y + resizeControlRect.height / 2, + }) + const endPoint = canvasPoint({ + x: startPoint.x + 100, + y: startPoint.y, + }) + await mouseMoveToPoint(resizeControl, startPoint) + await mouseDownAtPoint(resizeControl, startPoint) + await mouseMoveToPoint(canvasControlsLayer, endPoint) + await mouseUpAtPoint(canvasControlsLayer, endPoint) + await renderResult.getDispatchFollowUpActionsFinished() + + expect(getPrintedUiJsCode(renderResult.getEditorState())) + .toEqual(`import * as React from 'react' +import { Storyboard } from 'utopia-api' + +export var storyboard = ( + +
) `) + }) + it("does not add the other longhand if the shorthand is not defined and the other longhand it's auto", async () => { + const renderResult = await renderTestEditorWithCode( + makeTestProject({ + shorthand: 'auto / 2.4fr 1fr 1fr', + columns: null, + rows: null, + height: 600, + }), + 'await-first-dom-report', + ) + const target = EP.fromString(`sb/grid/row-1-column-2`) + await renderResult.dispatch(selectComponents([target], false), true) + await renderResult.getDispatchFollowUpActionsFinished() + const canvasControlsLayer = renderResult.renderedDOM.getByTestId(CanvasControlsContainerID) + const resizeControl = renderResult.renderedDOM.getByTestId(`grid-column-handle-1`) + const resizeControlRect = resizeControl.getBoundingClientRect() + const startPoint = canvasPoint({ + x: resizeControlRect.x + resizeControlRect.width / 2, + y: resizeControlRect.y + resizeControlRect.height / 2, + }) + const endPoint = canvasPoint({ + x: startPoint.x + 100, + y: startPoint.y, + }) + await mouseMoveToPoint(resizeControl, startPoint) + await mouseDownAtPoint(resizeControl, startPoint) + await mouseMoveToPoint(canvasControlsLayer, endPoint) + await mouseUpAtPoint(canvasControlsLayer, endPoint) + await renderResult.getDispatchFollowUpActionsFinished() + + expect(getPrintedUiJsCode(renderResult.getEditorState())) + .toEqual(`import * as React from 'react' +import { Storyboard } from 'utopia-api' + +export var storyboard = ( + +
+
+
+
+
+
+
+
+
+
+
+ +) +`) + }) }) }) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts index 2df2477034eb..e3242fdd3f97 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/resize-grid-strategy.ts @@ -7,7 +7,12 @@ import { import { MetadataUtils } from '../../../../core/model/element-metadata-utils' import * as EP from '../../../../core/shared/element-path' import * as PP from '../../../../core/shared/property-path' -import { setProperty } from '../../commands/set-property-command' +import type { PropertyToUpdate } from '../../commands/set-property-command' +import { + propertyToDelete, + propertyToSet, + updateBulkProperties, +} from '../../commands/set-property-command' import { controlsForGridPlaceholders, GridRowColumnResizingControls, @@ -30,17 +35,19 @@ import { printArrayGridDimensions, } from '../../../../components/inspector/common/css-utils' import { toFirst } from '../../../../core/shared/optics/optic-utilities' -import type { Either } from '../../../../core/shared/either' -import { foldEither, isLeft, isRight } from '../../../../core/shared/either' -import { roundTo, roundToNearestWhole } from '../../../../core/shared/math-utils' +import { isLeft } from '../../../../core/shared/either' +import { roundToNearestWhole } from '../../../../core/shared/math-utils' import type { GridAutoOrTemplateBase } from '../../../../core/shared/element-template' import { expandGridDimensions, findOriginalGrid, + isJustAutoGridDimension, replaceGridTemplateDimensionAtIndex, } from './grid-helpers' import { setCursorCommand } from '../../commands/set-cursor-command' import { CSSCursor } from '../../canvas-types' +import type { CanvasCommand } from '../../commands/commands' +import type { Axis } from '../../gap-utils' export const resizeGridStrategy: CanvasStrategyFactory = ( canvasState: InteractionCanvasState, @@ -116,6 +123,12 @@ export const resizeGridStrategy: CanvasStrategyFactory = ( ? gridSpecialSizeMeasurements.containerGridProperties.gridTemplateColumns : gridSpecialSizeMeasurements.containerGridProperties.gridTemplateRows + const otherAxis: Axis = control.axis === 'column' ? 'row' : 'column' + const otherAxisValues = + otherAxis === 'column' + ? gridSpecialSizeMeasurements.containerGridPropertiesFromProps.gridTemplateColumns + : gridSpecialSizeMeasurements.containerGridPropertiesFromProps.gridTemplateRows + if ( calculatedValues == null || calculatedValues.type !== 'DIMENSIONS' || @@ -186,16 +199,31 @@ export const resizeGridStrategy: CanvasStrategyFactory = ( const propertyValueAsString = printArrayGridDimensions(newDimensions) - const commands = [ - setProperty( - 'always', - originalGridPath, - PP.create( - 'style', - control.axis === 'column' ? 'gridTemplateColumns' : 'gridTemplateRows', - ), - propertyValueAsString, - ), + const propertyAxis = PP.create( + 'style', + control.axis === 'column' ? 'gridTemplateColumns' : 'gridTemplateRows', + ) + let propertiesToUpdate: PropertyToUpdate[] = [ + propertyToSet(propertyAxis, propertyValueAsString), + propertyToDelete(PP.create('style', 'gridTemplate')), // delete the shorthand in favor of the longhands + ] + + if ( + otherAxisValues?.type === 'DIMENSIONS' && + otherAxisValues.dimensions.length > 0 && + !isJustAutoGridDimension(otherAxisValues.dimensions) + ) { + // if the other axis has dimensions, serialize them in their longhand + const propertyOtherAxis = PP.create( + 'style', + otherAxis === 'column' ? 'gridTemplateColumns' : 'gridTemplateRows', + ) + const otherAxisValueAsString = printArrayGridDimensions(otherAxisValues.dimensions) + propertiesToUpdate.push(propertyToSet(propertyOtherAxis, otherAxisValueAsString)) + } + + let commands: CanvasCommand[] = [ + updateBulkProperties('always', originalGridPath, propertiesToUpdate), setCursorCommand(control.axis === 'column' ? CSSCursor.ColResize : CSSCursor.RowResize), ] diff --git a/editor/src/components/inspector/flex-section.tsx b/editor/src/components/inspector/flex-section.tsx index 34f9aaf4cf29..5b5410823dc0 100644 --- a/editor/src/components/inspector/flex-section.tsx +++ b/editor/src/components/inspector/flex-section.tsx @@ -66,7 +66,10 @@ import { type ElementInstanceMetadata, type GridElementProperties, } from '../../core/shared/element-template' -import { setGridPropsCommands } from '../canvas/canvas-strategies/strategies/grid-helpers' +import { + isJustAutoGridDimension, + setGridPropsCommands, +} from '../canvas/canvas-strategies/strategies/grid-helpers' import { type CanvasCommand } from '../canvas/commands/commands' import type { DropdownMenuItem } from '../../uuiui/radix-components' import { @@ -480,11 +483,7 @@ const TemplateDimensionControl = React.memo( template.dimensions.length === 0 || (autoTemplate?.type === 'DIMENSIONS' && autoTemplate.dimensions.length > 0 && - !( - autoTemplate.dimensions.length === 1 && - autoTemplate.dimensions[0].type === 'KEYWORD' && - autoTemplate.dimensions[0].value.value === 'auto' - )) + !isJustAutoGridDimension(autoTemplate.dimensions)) ) }, [template, autoTemplate])