Skip to content

Commit

Permalink
GridMeasurementHelper component to continuously measure grid cells (#…
Browse files Browse the repository at this point in the history
…6517)

**Problem:**
When you reparent from a grid to another grid, for a single frame the
element blinks at the same cell position as it was in the original grid:
https://screenshot.click/09-31-10wrd-ysw94.mp4

**Root cause:**
As you hover over the target grid, the grid-reparent-strategy is
selected. It triggers a showGridControl command, so the grid control is
shown and the cells can be measured. However, it won't be able to
position to the real target cell in this frame yet, because the control
is not enabled yet, so we don't have the cell bounds in the metadata.

**Fix:**
Instead of using grid controls to measure let show permanent grid
measurement helpers so grids can always be measured.

**Commit Details:** (< vv pls delete this section if's not relevant)

- Added memoized function `getAllGrids` which returns all the grids from
the metadata
- Added `GridMeasurementHelper` component which renders a grid with
placeholder divs in its cells (similarly to `GridControl`, but much-much
simpler)
- Rendering `GridMeasurementHelper` for all grids
- Use rendered grid measurement helpers in dom-walker to measure the
size of grid cell bounds
- Remove all the hacks (most notably `getMetadataWithGridCellBounds`)
which made it possible to use latestMetadata in interaction when
startingMetadata did not include grid cell bounds. This is not needed
anymore, because the grid cells are always measured using the always-on
`GridMeasurementHelper` components

**Manual Tests:**
I hereby swear that:

- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Play mode
  • Loading branch information
gbalint authored Oct 15, 2024
1 parent a4487d5 commit 66fc4c8
Show file tree
Hide file tree
Showing 11 changed files with 249 additions and 146 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import {
getStyleAttributesForFrameInAbsolutePosition,
updateInsertionSubjectWithAttributes,
} from './draw-to-insert-metastrategy'
import { getMetadataWithGridCellBounds, setGridPropsCommands } from './grid-helpers'
import { setGridPropsCommands } from './grid-helpers'
import { newReparentSubjects } from './reparent-helpers/reparent-strategy-helpers'
import { getReparentTargetUnified } from './reparent-helpers/reparent-strategy-parent-lookup'
import { getGridCellUnderMouseFromMetadata } from './grid-cell-bounds'
Expand Down Expand Up @@ -132,13 +132,10 @@ const gridDrawToInsertStrategyInner =
canvasState.propertyControlsInfo,
)?.newParent.intendedParentPath

const { metadata: parent, customStrategyState: updatedCustomState } =
getMetadataWithGridCellBounds(
targetParent,
canvasState.startingMetadata,
interactionSession.latestMetadata,
customStrategyState,
)
const parent = MetadataUtils.findElementByElementPath(
canvasState.startingMetadata,
targetParent,
)

if (targetParent == null || parent == null || !MetadataUtils.isGridLayoutedContainer(parent)) {
return null
Expand Down Expand Up @@ -175,7 +172,6 @@ const gridDrawToInsertStrategyInner =
updateHighlightedViews('mid-interaction', [targetParent]),
],
[targetParent],
updatedCustomState ?? undefined,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,62 +710,6 @@ export function getGridRelatedIndexes(params: {
return expandedRelatedIndexes[params.index] ?? []
}

export function getMetadataWithGridCellBounds(
path: ElementPath | null | undefined,
startingMetadata: ElementInstanceMetadataMap,
latestMetadata: ElementInstanceMetadataMap,
customStrategyState: CustomStrategyState,
): {
metadata: ElementInstanceMetadata | null
customStrategyState: CustomStrategyState | null
} {
if (path == null) {
return {
metadata: null,
customStrategyState: null,
}
}

const fromStartingMetadata = MetadataUtils.findElementByElementPath(startingMetadata, path)

if (fromStartingMetadata?.specialSizeMeasurements.gridCellGlobalFrames != null) {
return {
metadata: fromStartingMetadata,
customStrategyState: null,
}
}

const fromStrategyState = customStrategyState.grid.metadataCacheForGrids[EP.toString(path)]
if (fromStrategyState != null) {
return {
metadata: fromStrategyState,
customStrategyState: null,
}
}

const fromLatestMetadata = MetadataUtils.findElementByElementPath(latestMetadata, path)
if (fromLatestMetadata?.specialSizeMeasurements.gridCellGlobalFrames != null) {
return {
metadata: fromLatestMetadata,
customStrategyState: {
...customStrategyState,
grid: {
...customStrategyState.grid,
metadataCacheForGrids: {
...customStrategyState.grid.metadataCacheForGrids,
[EP.toString(path)]: fromLatestMetadata,
},
},
},
}
}

return {
metadata: fromStartingMetadata,
customStrategyState: null,
}
}

function getOriginalElementGridConfiguration(
gridCellGlobalFrames: GridCellGlobalFrames,
interactionData: DragInteractionData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import type { InsertionPath } from '../../../editor/store/insertion-path'
import { CSSCursor } from '../../canvas-types'
import { setCursorCommand } from '../../commands/set-cursor-command'
import { propertyToSet, updateBulkProperties } from '../../commands/set-property-command'
import { showGridControls } from '../../commands/show-grid-controls-command'
import { updateSelectedViews } from '../../commands/update-selected-views-command'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import { ParentBounds } from '../../controls/parent-bounds'
Expand All @@ -39,7 +38,7 @@ import {
import type { DragInteractionData, InteractionSession, UpdatedPathMap } from '../interaction-state'
import { honoursPropsPosition, shouldKeepMovingDraggedGroupChildren } from './absolute-utils'
import { replaceFragmentLikePathsWithTheirChildrenRecursive } from './fragment-like-helpers'
import { getMetadataWithGridCellBounds, runGridRearrangeMove } from './grid-helpers'
import { runGridRearrangeMove } from './grid-helpers'
import { ifAllowedToReparent, isAllowedToReparent } from './reparent-helpers/reparent-helpers'
import { removeAbsolutePositioningProps } from './reparent-helpers/reparent-property-changes'
import type { ReparentTarget } from './reparent-helpers/reparent-strategy-helpers'
Expand Down Expand Up @@ -164,13 +163,10 @@ export function applyGridReparent(
return emptyStrategyApplicationResult
}

const { metadata: grid, customStrategyState: updatedCustomState } =
getMetadataWithGridCellBounds(
newParent.intendedParentPath,
canvasState.startingMetadata,
interactionSession.latestMetadata,
customStrategyState,
)
const grid = MetadataUtils.findElementByElementPath(
canvasState.startingMetadata,
newParent.intendedParentPath,
)

if (grid == null) {
return strategyApplicationResult([], [newParent.intendedParentPath])
Expand Down Expand Up @@ -231,12 +227,6 @@ export function applyGridReparent(
newParent.intendedParentPath,
])

const baseCustomState = updatedCustomState ?? customStrategyState
const customStrategyStatePatch = {
...baseCustomState,
elementsToRerender: elementsToRerender,
}

return strategyApplicationResult(
[
...outcomes.flatMap((c) => c.commands),
Expand All @@ -245,7 +235,9 @@ export function applyGridReparent(
setCursorCommand(CSSCursor.Reparent),
],
elementsToRerender,
customStrategyStatePatch,
{
elementsToRerender: elementsToRerender,
},
)
},
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import * as EP from '../../../../core/shared/element-path'
import type { GridElementProperties, GridPosition } from '../../../../core/shared/element-template'
import {
type CanvasRectangle,
isInfinityRectangle,
rectangleIntersection,
} from '../../../../core/shared/math-utils'
import { isCSSKeyword } from '../../../inspector/common/css-utils'
import { isFillOrStretchModeApplied } from '../../../inspector/inspector-common'
import {
controlsForGridPlaceholders,
Expand All @@ -22,13 +20,12 @@ import {
strategyApplicationResult,
} from '../canvas-strategy-types'
import type { InteractionSession } from '../interaction-state'
import { getMetadataWithGridCellBounds, setGridPropsCommands } from './grid-helpers'
import { setGridPropsCommands } from './grid-helpers'
import { resizeBoundingBoxFromSide } from './resize-helpers'

export const gridResizeElementStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
interactionSession: InteractionSession | null,
customState,
) => {
const selectedElements = getTargetPathsFromInteractionTarget(canvasState.interactionTarget)
if (selectedElements.length !== 1) {
Expand Down Expand Up @@ -86,13 +83,10 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = (
return emptyStrategyApplicationResult
}

const { metadata: container, customStrategyState: updatedCustomState } =
getMetadataWithGridCellBounds(
EP.parentPath(selectedElement),
canvasState.startingMetadata,
interactionSession.latestMetadata,
customState,
)
const container = MetadataUtils.findElementByElementPath(
canvasState.startingMetadata,
EP.parentPath(selectedElement),
)

if (container == null) {
return emptyStrategyApplicationResult
Expand Down Expand Up @@ -123,7 +117,6 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = (
return strategyApplicationResult(
setGridPropsCommands(selectedElement, gridTemplate, gridProps),
[parentGridPath],
updatedCustomState ?? undefined,
)
},
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/** @jsxRuntime classic */
/** @jsx jsx */
import fastDeepEqual from 'fast-deep-equal'
import type { Sides } from 'utopia-api/core'
import type { ElementPath } from 'utopia-shared/src/types'
import { isStaticGridRepeat, printGridAutoOrTemplateBase } from '../../inspector/common/css-utils'
Expand Down Expand Up @@ -61,22 +62,72 @@ export function getNullableAutoOrTemplateBaseString(
}
}

export type GridData = {
elementPath: ElementPath
export type GridMeasurementHelperData = {
frame: CanvasRectangle
gridTemplateColumns: GridAutoOrTemplateBase | null
gridTemplateRows: GridAutoOrTemplateBase | null
gridTemplateColumnsFromProps: GridAutoOrTemplateBase | null
gridTemplateRowsFromProps: GridAutoOrTemplateBase | null
gap: number | null
justifyContent: string | null
alignContent: string | null
rowGap: number | null
columnGap: number | null
justifyContent: string | null
alignContent: string | null
padding: Sides
rows: number
columns: number
cells: number
}

export function useGridMeasurentHelperData(
elementPath: ElementPath,
): GridMeasurementHelperData | null {
return useEditorState(
Substores.metadata,
(store) => {
const element = MetadataUtils.findElementByElementPath(store.editor.jsxMetadata, elementPath)

const targetGridContainer = MetadataUtils.isGridLayoutedContainer(element) ? element : null

if (
targetGridContainer == null ||
targetGridContainer.globalFrame == null ||
!isFiniteRectangle(targetGridContainer.globalFrame)
) {
return null
}

const columns = getCellsCount(
targetGridContainer.specialSizeMeasurements.containerGridProperties.gridTemplateColumns,
)
const rows = getCellsCount(
targetGridContainer.specialSizeMeasurements.containerGridProperties.gridTemplateRows,
)

return {
frame: targetGridContainer.globalFrame,
gridTemplateColumns:
targetGridContainer.specialSizeMeasurements.containerGridProperties.gridTemplateColumns,
gridTemplateRows:
targetGridContainer.specialSizeMeasurements.containerGridProperties.gridTemplateRows,
gap: targetGridContainer.specialSizeMeasurements.gap,
rowGap: targetGridContainer.specialSizeMeasurements.rowGap,
columnGap: targetGridContainer.specialSizeMeasurements.columnGap,
justifyContent: targetGridContainer.specialSizeMeasurements.justifyContent,
alignContent: targetGridContainer.specialSizeMeasurements.alignContent,
padding: targetGridContainer.specialSizeMeasurements.padding,
columns: columns,
cells: rows * columns,
}
},
'useGridMeasurentHelperData',
fastDeepEqual, //TODO: this should not be needed, but it seems EditorStateKeepDeepEquality is not running, and metadata reference is always updated
)
}

export type GridData = GridMeasurementHelperData & {
elementPath: ElementPath
gridTemplateColumnsFromProps: GridAutoOrTemplateBase | null
gridTemplateRowsFromProps: GridAutoOrTemplateBase | null
rows: number
cells: number
metadata: ElementInstanceMetadata
}

Expand Down Expand Up @@ -156,6 +207,12 @@ export const GridRowColumnResizingControls =
)

export const GridControlsKey = (gridPath: ElementPath) => `grid-controls-${EP.toString(gridPath)}`
export const GridControlKey = (gridPath: ElementPath) => `grid-control-${EP.toString(gridPath)}`

export const GridMeasurementHelpersKey = (gridPath: ElementPath) =>
`grid-measurement-helpers-${EP.toString(gridPath)}`
export const GridMeasurementHelperKey = (gridPath: ElementPath) =>
`grid-measurement-helper-${EP.toString(gridPath)}`

export interface GridControlProps {
grid: GridData
Expand Down
Loading

0 comments on commit 66fc4c8

Please sign in to comment.