Skip to content

Commit

Permalink
Trim dynamic grids' collapsed extra rows/cols from the computed templ…
Browse files Browse the repository at this point in the history
…ate (#6496)

**Problem:**

In grid templates when using `repeat` functions and
`auto-fit`/`auto-fill`, the computed style might contain extra
rows/cols, collapsed to a `0px` size. Normally this is not an issue, but
since for dynamic templates we display the grid wireframe based on the
computed dimensions, those extra dimensions would appear visible on the
canvas as zombies. This behavior of appending extra collapsed tracks is
explained in https://www.w3.org/TR/css-grid-1/#repeat-notation.

<img width="928" alt="Screenshot 2024-10-08 at 13 55 48"
src="https://github.com/user-attachments/assets/042ab509-8e82-4f7a-842c-cbb6a8ef7267">

**Fix:**

When parsing the templates, first parse the `fromProps` template.
Then, after parsing the computed template, if the `fromProps` template
it contains dynamic tracks, trim any dangling collapsed elements in the
computed template dimensions array.
This is not necessarily optimal (if you have an explicit `0px` sized
column in your template, it will be hidden from the canvas), but
concretely it will do "the right thing" and you can always use the
inspector to target any 0-sized cols/rows (which you would have to do
anyways).

<img width="928" alt="Screenshot 2024-10-08 at 14 00 03"
src="https://github.com/user-attachments/assets/9bd5d76c-9eae-406b-956d-f1f300b19ba7">


Fixes #6495
  • Loading branch information
ruggi authored and liady committed Dec 13, 2024
1 parent d8cc824 commit 3660d86
Showing 1 changed file with 52 additions and 12 deletions.
64 changes: 52 additions & 12 deletions editor/src/components/canvas/dom-walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import type {
GridContainerProperties,
GridElementProperties,
DomElementMetadata,
GridAutoOrTemplateBase,
} from '../../core/shared/element-template'
import {
specialSizeMeasurements,
gridContainerProperties,
gridElementProperties,
gridAutoOrTemplateFallback,
domElementMetadata,
gridAutoOrTemplateDimensions,
} from '../../core/shared/element-template'
import type { ElementPath } from '../../core/shared/project-file-types'
import type { ElementCanvasRectangleCache } from '../../core/shared/dom-utils'
Expand Down Expand Up @@ -52,6 +54,7 @@ import {
parseGridAutoOrTemplateBase,
parseGridAutoFlow,
isCSSKeyword,
isDynamicGridRepeat,
} from '../inspector/common/css-utils'
import type { UtopiaStoreAPI } from '../editor/store/store-hook'
import {
Expand Down Expand Up @@ -555,6 +558,10 @@ export function collectDomElementMetadataForElement(

function getGridContainerProperties(
elementStyle: CSSStyleDeclaration | null,
options?: {
dynamicCols: boolean
dynamicRows: boolean
},
): GridContainerProperties {
if (elementStyle == null) {
return {
Expand All @@ -565,14 +572,23 @@ function getGridContainerProperties(
gridAutoFlow: null,
}
}
const gridTemplateColumns = defaultEither(
gridAutoOrTemplateFallback(elementStyle.gridTemplateColumns),
parseGridAutoOrTemplateBase(elementStyle.gridTemplateColumns),

const gridTemplateColumns = trimDynamicEmptyDimensions(
defaultEither(
gridAutoOrTemplateFallback(elementStyle.gridTemplateColumns),
parseGridAutoOrTemplateBase(elementStyle.gridTemplateColumns),
),
options?.dynamicCols === true,
)
const gridTemplateRows = defaultEither(
gridAutoOrTemplateFallback(elementStyle.gridTemplateRows),
parseGridAutoOrTemplateBase(elementStyle.gridTemplateRows),

const gridTemplateRows = trimDynamicEmptyDimensions(
defaultEither(
gridAutoOrTemplateFallback(elementStyle.gridTemplateRows),
parseGridAutoOrTemplateBase(elementStyle.gridTemplateRows),
),
options?.dynamicRows === true,
)

const gridAutoColumns = defaultEither(
gridAutoOrTemplateFallback(elementStyle.gridAutoColumns),
parseGridAutoOrTemplateBase(elementStyle.gridAutoColumns),
Expand All @@ -590,6 +606,23 @@ function getGridContainerProperties(
)
}

function trimDynamicEmptyDimensions(
template: GridAutoOrTemplateBase,
isDynamic: boolean,
): GridAutoOrTemplateBase {
if (!isDynamic) {
return template
}
if (template.type !== 'DIMENSIONS') {
return template
}

const lastNonEmptyColumn = template.dimensions.findLastIndex(
(d) => d.type === 'KEYWORD' || (d.type === 'NUMBER' && d.value.value !== 0),
)
return gridAutoOrTemplateDimensions(template.dimensions.slice(0, lastNonEmptyColumn + 1))
}

function getGridElementProperties(
container: GridContainerProperties,
elementStyle: CSSStyleDeclaration,
Expand Down Expand Up @@ -882,8 +915,6 @@ function getSpecialMeasurements(

const parentContainerGridProperties = getGridContainerProperties(parentElementStyle)

const containerGridProperties = getGridContainerProperties(elementStyle)

const paddingValue = isRight(padding)
? padding.value
: sides(undefined, undefined, undefined, undefined)
Expand All @@ -898,15 +929,20 @@ function getSpecialMeasurements(
)
: null

const containerElementProperties = getGridElementProperties(
parentContainerGridProperties,
elementStyle,
)
const containerGridPropertiesFromProps = getGridContainerProperties(element.style)
const containerGridProperties = getGridContainerProperties(elementStyle, {
dynamicCols: isDynamicGridTemplate(containerGridPropertiesFromProps.gridTemplateColumns),
dynamicRows: isDynamicGridTemplate(containerGridPropertiesFromProps.gridTemplateRows),
})

const containerElementPropertiesFromProps = getGridElementProperties(
parentContainerGridProperties,
element.style,
)
const containerElementProperties = getGridElementProperties(
parentContainerGridProperties,
elementStyle,
)

return specialSizeMeasurements(
offset,
Expand Down Expand Up @@ -965,6 +1001,10 @@ function getSpecialMeasurements(
)
}

function isDynamicGridTemplate(template: GridAutoOrTemplateBase | null) {
return template?.type === 'DIMENSIONS' && template.dimensions.some((d) => isDynamicGridRepeat(d))
}

function elementContainsOnlyText(element: HTMLElement): boolean {
if (element.childNodes.length === 0) {
return false
Expand Down

0 comments on commit 3660d86

Please sign in to comment.