Skip to content

Commit

Permalink
grid-specific hug strategy (#6087)
Browse files Browse the repository at this point in the history
## Problem
The basic hug contents strategy doesn't really make sense for grids.

## Fix
Create a hug contents strategy tailored to grids. When setting a grid to
hug along an axis, if the grid doesn't use any `fr` units in
`gridTemplate{Row | Column}` along that axis, the width/height prop is
removed from that axis, so the grid is sized by its template rows or
columns. If there's an `fr` unit in`gridTemplate{Row | Column}`, the
strategy is not applicable because the `fr` only works if the grid is
sized
  • Loading branch information
bkrmendy authored and liady committed Dec 13, 2024
1 parent e9aa99a commit dc9dc4c
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3124,6 +3124,96 @@ describe('Double click on resize edge', () => {
expect(div.style.width).toEqual(MaxContent)
expect(div.style.height).toEqual('445px')
})

describe('grids', () => {
const project = ({
rows: rows,
columns: columns,
}: {
rows: string
columns: string
}) => `import * as React from 'react'
import { Scene, Storyboard } from 'utopia-api'
export var storyboard = (
<Storyboard data-uid='sb'>
<Scene
id='playground-scene'
commentId='playground-scene'
style={{
width: 700,
height: 759,
position: 'absolute',
left: 212,
top: 128,
}}
data-label='Playground'
data-uid='sc'
>
<div
style={{
display: 'grid',
gridTemplateRows: '${rows}',
gridTemplateColumns: '${columns}',
backgroundColor: '#0074ff',
position: 'absolute',
left: 135,
top: 55,
width: 400,
height: 400,
opacity: '30%',
}}
data-uid='grid'
data-testid='mydiv'
>
<img
src='https://github.com/concrete-utopia/utopia/blob/master/editor/resources/editor/[email protected]?raw=true'
alt='Utopia logo'
style={{
objectFit: 'contain',
display: 'inline-block',
width: 100,
height: 100,
gridColumnStart: 2,
gridColumnEnd: 2,
gridRowStart: 2,
gridRowEnd: 2,
}}
data-uid='smyramid'
/>
</div>
</Scene>
</Storyboard>
)
`
it('removes width when right edge is clicked', async () => {
const editor = await renderTestEditorWithCode(
project({ rows: '66px 66px 66px 66px', columns: '50px 81px 96px 85px' }),
'await-first-dom-report',
)
const div = await doDblClickTest(editor, edgeResizeControlTestId(EdgePositionRight))
expect(div.style.width).toEqual('') // width is removed
expect(div.style.height).toEqual('400px')
})
it('removes height when bottom edge is clicked', async () => {
const editor = await renderTestEditorWithCode(
project({ rows: '66px 66px 66px 66px', columns: '50px 81px 96px 85px' }),
'await-first-dom-report',
)
const div = await doDblClickTest(editor, edgeResizeControlTestId(EdgePositionBottom))
expect(div.style.width).toEqual('400px')
expect(div.style.height).toEqual('') // height is removed
})
it("isn't applicable when the selected grid uses fr along the affected axis", async () => {
const editor = await renderTestEditorWithCode(
project({ rows: '66px 1fr 66px 66px', columns: '50px 81px 96px 85px' }),
'await-first-dom-report',
)
const div = await doDblClickTest(editor, edgeResizeControlTestId(EdgePositionBottom))
expect(div.style.width).toEqual('400px')
expect(div.style.height).toEqual('400px')
})
})
})

describe('double click on resize corner', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import {
onlyChildIsSpan,
sizeToVisualDimensions,
} from '../../inspector/inspector-common'
import { setHugContentForAxis } from '../../inspector/inspector-strategies/hug-contents-basic-strategy'
import { setHugContentForAxis } from '../../inspector/inspector-strategies/hug-contents-strategy'

type FlexDirectionRowColumn = 'row' | 'column' // a limited subset as we won't never guess row-reverse or column-reverse
type FlexAlignItems = 'center' | 'flex-end'
Expand Down
13 changes: 9 additions & 4 deletions editor/src/components/inspector/inspector-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ import {
import { fixedSizeDimensionHandlingText } from '../text-editor/text-handling'
import { convertToAbsolute } from '../canvas/commands/convert-to-absolute-command'
import { hugPropertiesFromStyleMap } from '../../core/shared/dom-utils'
import { setHugContentForAxis } from './inspector-strategies/hug-contents-basic-strategy'
import { setHugContentForAxis } from './inspector-strategies/hug-contents-strategy'

export type StartCenterEnd = 'flex-start' | 'center' | 'flex-end'

Expand Down Expand Up @@ -255,12 +255,12 @@ export function detectAreElementsFlexContainers(
export const isFlexColumn = (flexDirection: FlexDirection): boolean =>
flexDirection.startsWith('column')

export const hugContentsApplicableForContainer = (
export const basicHugContentsApplicableForContainer = (
metadata: ElementInstanceMetadataMap,
pathTrees: ElementPathTrees,
elementPath: ElementPath,
): boolean => {
return (
const isNonFixStickOrAbsolute =
mapDropNulls(
(path) => MetadataUtils.findElementByElementPath(metadata, path),
MetadataUtils.getChildrenPathsOrdered(metadata, pathTrees, elementPath),
Expand All @@ -272,7 +272,12 @@ export const hugContentsApplicableForContainer = (
MetadataUtils.isPositionAbsolute(element)
),
).length > 0

const isGrid = MetadataUtils.isGridLayoutedContainer(
MetadataUtils.findElementByElementPath(metadata, elementPath),
)

return isNonFixStickOrAbsolute && !isGrid
}

export const hugContentsApplicableForText = (
Expand Down Expand Up @@ -999,7 +1004,7 @@ export function getFixedFillHugOptionsForElement(
isGroup ? 'hug-group' : null,
'fixed',
hugContentsApplicableForText(metadata, selectedView) ||
(!isGroup && hugContentsApplicableForContainer(metadata, pathTrees, selectedView))
(!isGroup && basicHugContentsApplicableForContainer(metadata, pathTrees, selectedView))
? 'hug'
: null,
fillContainerApplicable(metadata, selectedView) ? 'fill' : null,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ElementPathTrees } from '../../../core/shared/element-path-tree'
import { MetadataUtils } from '../../../core/model/element-metadata-utils'
import type { GridTemplate } from '../../../core/shared/element-template'
import { type ElementInstanceMetadataMap } from '../../../core/shared/element-template'
import type { ElementPath } from '../../../core/shared/project-file-types'
import * as PP from '../../../core/shared/property-path'
Expand All @@ -15,7 +16,7 @@ import { cssKeyword } from '../common/css-utils'
import type { Axis } from '../inspector-common'
import {
detectFillHugFixedState,
hugContentsApplicableForContainer,
basicHugContentsApplicableForContainer,
hugContentsApplicableForText,
MaxContent,
nukeSizingPropsForAxisCommand,
Expand All @@ -28,6 +29,8 @@ import { queueTrueUpElement } from '../../canvas/commands/queue-true-up-command'
import { trueUpGroupElementChanged } from '../../../components/editor/store/editor-state'
import type { AllElementProps } from '../../../components/editor/store/editor-state'
import { convertSizelessDivToFrameCommands } from '../../canvas/canvas-strategies/strategies/group-conversion-helpers'
import { deleteProperties } from '../../canvas/commands/delete-properties-command'
import { assertNever } from '../../../core/shared/utils'

const CHILDREN_CONVERTED_TOAST_ID = 'CHILDREN_CONVERTED_TOAST_ID'

Expand Down Expand Up @@ -87,6 +90,70 @@ function hugContentsSingleElement(
]
}

function gridTemplateUsesFr(template: GridTemplate | null) {
return (
template != null &&
template.type === 'DIMENSIONS' &&
template.dimensions.some((d) => d.unit === 'fr')
)
}

function elementUsesFrAlongAxis(
axis: Axis,
metadata: ElementInstanceMetadataMap,
elementPath: ElementPath,
) {
const instance = MetadataUtils.findElementByElementPath(metadata, elementPath)
if (instance == null) {
return false
}
const { containerGridProperties, containerGridPropertiesFromProps } =
instance.specialSizeMeasurements

switch (axis) {
case 'horizontal':
return (
gridTemplateUsesFr(containerGridProperties.gridTemplateColumns) ||
gridTemplateUsesFr(containerGridPropertiesFromProps.gridTemplateColumns)
)
case 'vertical':
return (
gridTemplateUsesFr(containerGridProperties.gridTemplateRows) ||
gridTemplateUsesFr(containerGridPropertiesFromProps.gridTemplateRows)
)
default:
assertNever(axis)
}
}

export const hugContentsGridStrategy = (
metadata: ElementInstanceMetadataMap,
elementPaths: ElementPath[],
axis: Axis,
): InspectorStrategy => ({
name: 'Set Grid to Hug',
strategy: () => {
// only run the strategy if all selected elements are grids
const allSelectedElementsGrids = elementPaths.every((e) =>
MetadataUtils.isGridLayoutedContainer(MetadataUtils.findElementByElementPath(metadata, e)),
)

// only run the strategy if no selected element uses fr along the affected
// axis, because that implies that the container needs to be sized
const anyElementUsesFrAlongAxis = elementPaths.some((e) =>
elementUsesFrAlongAxis(axis, metadata, e),
)

if (!allSelectedElementsGrids || anyElementUsesFrAlongAxis) {
return null
}

return elementPaths.flatMap((elementPath) =>
deleteProperties('always', elementPath, [PP.create('style', widthHeightFromAxis(axis))]),
)
},
})

export const hugContentsBasicStrategy = (
metadata: ElementInstanceMetadataMap,
elementPaths: ElementPath[],
Expand All @@ -97,7 +164,7 @@ export const hugContentsBasicStrategy = (
strategy: () => {
const elements = elementPaths.filter(
(path) =>
hugContentsApplicableForContainer(metadata, pathTrees, path) ||
basicHugContentsApplicableForContainer(metadata, pathTrees, path) ||
hugContentsApplicableForText(metadata, path),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import type { WhenToRun } from '../../../components/canvas/commands/commands'
import {
hugContentsAbsoluteStrategy,
hugContentsBasicStrategy,
} from './hug-contents-basic-strategy'
hugContentsGridStrategy,
} from './hug-contents-strategy'
import {
fillContainerStrategyFlexParent,
fillContainerStrategyFlow,
Expand Down Expand Up @@ -204,7 +205,10 @@ export const setPropHugStrategies = (
elementPaths: ElementPath[],
pathTrees: ElementPathTrees,
axis: Axis,
): Array<InspectorStrategy> => [hugContentsBasicStrategy(metadata, elementPaths, pathTrees, axis)]
): Array<InspectorStrategy> => [
hugContentsGridStrategy(metadata, elementPaths, axis),
hugContentsBasicStrategy(metadata, elementPaths, pathTrees, axis),
]

export const setPropHugAbsoluteStrategies = (
metadata: ElementInstanceMetadataMap,
Expand Down

0 comments on commit dc9dc4c

Please sign in to comment.