Skip to content

Commit

Permalink
Fix Grid Controls HMR (#6521)
Browse files Browse the repository at this point in the history
**Problem:**
The strategy controls were wrapped in `controlForStrategyMemoized` which
[defeats Vite's
HMR.](https://github.com/vitejs/vite-plugin-react/tree/main/packages/plugin-react#consistent-components-exports)

**Fix:**
Move all the component internals into a dedicated file which doesn't
export other functions to prevent breaking the rules of hmr.

**Commit Details:**

- Moved all grid control components and their helper code into
`grid-controls-components`
- `grid-controls` now imports these components
- No code was changed, only moved

I verified that with this change, HMR started working!

## Bonus

At @liady 's suggestion, I added `eslint-plugin-react-refresh` to the
list of our ESLint rules. It is not rock solid, but definitely helps in
remembering that there _are_ rules for HMR / Fast Refresh to work
correctly.
  • Loading branch information
balazsbajorics authored Oct 11, 2024
1 parent 5eb72f0 commit d366ed5
Show file tree
Hide file tree
Showing 24 changed files with 820 additions and 466 deletions.
1 change: 1 addition & 0 deletions editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@
"eslint-plugin-jsx-a11y": "6.5.1",
"eslint-plugin-react": "7.23.2",
"eslint-plugin-react-hooks": "4.2.0",
"eslint-plugin-react-refresh": "0.4.12",
"eslint4b": "6.6.0",
"fast-deep-equal": "2.0.1",
"fontfaceobserver": "2.1.0",
Expand Down
10 changes: 10 additions & 0 deletions editor/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { queueTrueUpElement } from '../../commands/queue-true-up-command'
import { setCursorCommand } from '../../commands/set-cursor-command'

import { updateHighlightedViews } from '../../commands/update-highlighted-views-command'
import { controlsForGridPlaceholders } from '../../controls/grid-controls'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import { ImmediateParentBounds } from '../../controls/parent-bounds'
import { ImmediateParentOutlines } from '../../controls/parent-outlines'
import { AbsoluteResizeControl } from '../../controls/select-mode/absolute-resize-control'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { insertElementInsertionSubject } from '../../commands/insert-element-ins
import { showGridControls } from '../../commands/show-grid-controls-command'
import { updateHighlightedViews } from '../../commands/update-highlighted-views-command'
import { wildcardPatch } from '../../commands/wildcard-patch-command'
import { controlsForGridPlaceholders } from '../../controls/grid-controls'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import { canvasPointToWindowPoint } from '../../dom-lookup'
import {
getWrapperWithGeneratedUid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import * as EP from '../../../../core/shared/element-path'
import type { GridPositionValue } from '../../../../core/shared/element-template'
import { gridPositionValue } from '../../../../core/shared/element-template'
import { controlsForGridPlaceholders } from '../../controls/grid-controls'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import type {
CanvasStrategy,
CustomStrategyState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { setCursorCommand } from '../../commands/set-cursor-command'

import { updateHighlightedViews } from '../../commands/update-highlighted-views-command'
import { updateSelectedViews } from '../../commands/update-selected-views-command'
import { controlsForGridPlaceholders } from '../../controls/grid-controls'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import type { CanvasStrategyFactory } from '../canvas-strategies'
import { onlyFitWhenDraggingThisControl } from '../canvas-strategies'
import type { CustomStrategyState, InteractionCanvasState } from '../canvas-strategy-types'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '../../../../core/shared/math-utils'
import { selectComponentsForTest } from '../../../../utils/utils.test-utils'
import CanvasActions from '../../canvas-actions'
import { GridCellTestId } from '../../controls/grid-controls'
import { GridCellTestId } from '../../controls/grid-controls-for-strategies'
import { mouseDragFromPointToPoint } from '../../event-helpers.test-utils'
import type { EditorRenderResult } from '../../ui-jsx.test-utils'
import { renderTestEditorWithCode } from '../../ui-jsx.test-utils'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ElementPath } from 'utopia-shared/src/types'
import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import * as EP from '../../../../core/shared/element-path'
import { controlsForGridPlaceholders } from '../../controls/grid-controls'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import type {
ElementInstanceMetadataMap,
GridAutoOrTemplateBase,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { offsetPoint, windowPoint } from '../../../../core/shared/math-utils'
import type { Modifiers } from '../../../../utils/modifiers'
import { cmdModifier } from '../../../../utils/modifiers'
import { selectComponentsForTest } from '../../../../utils/utils.test-utils'
import { GridCellTestId } from '../../controls/grid-controls'
import { GridCellTestId } from '../../controls/grid-controls-for-strategies'
import { CanvasControlsContainerID } from '../../controls/new-canvas-controls'
import type { Point } from '../../event-helpers.test-utils'
import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ 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'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import { ParentBounds } from '../../controls/parent-bounds'
import { ParentOutlines } from '../../controls/parent-outlines'
import { ZeroSizedElementControls } from '../../controls/zero-sized-element-controls'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
offsetPoint,
} from '../../../../core/shared/math-utils'
import { selectComponentsForTest } from '../../../../utils/utils.test-utils'
import { GridResizeEdgeTestId } from '../../controls/grid-controls'
import { GridResizeEdgeTestId } from '../../controls/grid-controls-for-strategies'
import { mouseDragFromPointToPoint } from '../../event-helpers.test-utils'
import type { EditorRenderResult } from '../../ui-jsx.test-utils'
import { renderTestEditorWithCode } from '../../ui-jsx.test-utils'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
controlsForGridPlaceholders,
gridEdgeToEdgePosition,
GridResizeControls,
} from '../../controls/grid-controls'
} from '../../controls/grid-controls-for-strategies'
import type { CanvasStrategyFactory } from '../canvas-strategies'
import { onlyFitWhenDraggingThisControl } from '../canvas-strategies'
import type { InteractionCanvasState } from '../canvas-strategy-types'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
pressKey,
} from '../../event-helpers.test-utils'
import { canvasPoint } from '../../../../core/shared/math-utils'
import { GridCellTestId } from '../../controls/grid-controls'
import { GridCellTestId } from '../../controls/grid-controls-for-strategies'

const testProject = `
import * as React from 'react'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { create } from '../../../../core/shared/property-path'
import type { CanvasCommand } from '../../commands/commands'
import { deleteProperties } from '../../commands/delete-properties-command'
import { rearrangeChildren } from '../../commands/rearrange-children-command'
import { controlsForGridPlaceholders } from '../../controls/grid-controls'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import { recurseIntoChildrenOfMapOrFragment } from '../../gap-utils'
import type { CanvasStrategyFactory } from '../canvas-strategies'
import { onlyFitWhenDraggingThisControl } from '../canvas-strategies'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { setProperty } from '../../commands/set-property-command'
import {
controlsForGridPlaceholders,
GridRowColumnResizingControls,
} from '../../controls/grid-controls'
} from '../../controls/grid-controls-for-strategies'
import type { CanvasStrategyFactory } from '../canvas-strategies'
import { onlyFitWhenDraggingThisControl } from '../canvas-strategies'
import type { InteractionCanvasState } from '../canvas-strategy-types'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import { colorTheme } from '../../../../uuiui'
import { activeFrameTargetPath, setActiveFrames } from '../../commands/set-active-frames-command'
import type { GridGapControlProps } from '../../controls/select-mode/grid-gap-control'
import { GridGapControl } from '../../controls/select-mode/grid-gap-control'
import type { GridControlsProps } from '../../controls/grid-controls'
import { controlsForGridPlaceholders } from '../../controls/grid-controls'
import type { GridControlsProps } from '../../controls/grid-controls-for-strategies'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'

const SetGridGapStrategyId = 'SET_GRID_GAP_STRATEGY'

Expand Down
206 changes: 206 additions & 0 deletions editor/src/components/canvas/controls/grid-controls-for-strategies.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
/** @jsxRuntime classic */
/** @jsx jsx */
import type { Sides } from 'utopia-api/core'
import type { ElementPath } from 'utopia-shared/src/types'
import { isStaticGridRepeat, printGridAutoOrTemplateBase } from '../../inspector/common/css-utils'
import { MetadataUtils } from '../../../core/model/element-metadata-utils'
import { mapDropNulls } from '../../../core/shared/array-utils'
import * as EP from '../../../core/shared/element-path'
import type { ElementInstanceMetadata } from '../../../core/shared/element-template'
import { type GridAutoOrTemplateBase } from '../../../core/shared/element-template'
import type { CanvasRectangle } from '../../../core/shared/math-utils'
import { isFiniteRectangle } from '../../../core/shared/math-utils'
import { assertNever } from '../../../core/shared/utils'
import { Substores, useEditorState } from '../../editor/store/store-hook'
import type {
ControlWithProps,
WhenToShowControl,
} from '../canvas-strategies/canvas-strategy-types'
import { controlForStrategyMemoized } from '../canvas-strategies/canvas-strategy-types'
import type { GridResizeEdge } from '../canvas-strategies/interaction-state'
import type { EdgePosition } from '../canvas-types'
import {
CSSCursor,
EdgePositionBottom,
EdgePositionLeft,
EdgePositionRight,
EdgePositionTop,
} from '../canvas-types'
import {
GridControlsComponent,
GridResizeControlsComponent,
GridRowColumnResizingControlsComponent,
} from './grid-controls'

export const GridCellTestId = (elementPath: ElementPath) => `grid-cell-${EP.toString(elementPath)}`

function getCellsCount(template: GridAutoOrTemplateBase | null): number {
if (template == null) {
return 0
}

switch (template.type) {
case 'DIMENSIONS':
return template.dimensions.reduce((acc, cur) => {
return acc + (isStaticGridRepeat(cur) ? cur.times : 1)
}, 0)
case 'FALLBACK':
return 0
default:
assertNever(template)
}
}

export function getNullableAutoOrTemplateBaseString(
template: GridAutoOrTemplateBase | null,
): string | undefined {
if (template == null) {
return undefined
} else {
return printGridAutoOrTemplateBase(template)
}
}

export type GridData = {
elementPath: ElementPath
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
padding: Sides
rows: number
columns: number
cells: number
metadata: ElementInstanceMetadata
}

export function useGridData(elementPaths: ElementPath[]): GridData[] {
const grids = useEditorState(
Substores.metadata,
(store) => {
return mapDropNulls((view) => {
const element = MetadataUtils.findElementByElementPath(store.editor.jsxMetadata, view)

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

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

const gap = targetGridContainer.specialSizeMeasurements.gap
const rowGap = targetGridContainer.specialSizeMeasurements.rowGap
const columnGap = targetGridContainer.specialSizeMeasurements.columnGap
const padding = targetGridContainer.specialSizeMeasurements.padding

const gridTemplateColumns =
targetGridContainer.specialSizeMeasurements.containerGridProperties.gridTemplateColumns
const gridTemplateRows =
targetGridContainer.specialSizeMeasurements.containerGridProperties.gridTemplateRows
const gridTemplateColumnsFromProps =
targetGridContainer.specialSizeMeasurements.containerGridPropertiesFromProps
.gridTemplateColumns
const gridTemplateRowsFromProps =
targetGridContainer.specialSizeMeasurements.containerGridPropertiesFromProps
.gridTemplateRows

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

return {
elementPath: targetGridContainer.elementPath,
metadata: targetGridContainer,
frame: targetGridContainer.globalFrame,
gridTemplateColumns: gridTemplateColumns,
gridTemplateRows: gridTemplateRows,
gridTemplateColumnsFromProps: gridTemplateColumnsFromProps,
gridTemplateRowsFromProps: gridTemplateRowsFromProps,
gap: gap,
rowGap: rowGap,
columnGap: columnGap,
justifyContent: targetGridContainer.specialSizeMeasurements.justifyContent,
alignContent: targetGridContainer.specialSizeMeasurements.alignContent,
padding: padding,
rows: rows,
columns: columns,
cells: rows * columns,
}
}, elementPaths)
},
'useGridData',
)

return grids
}

interface GridRowColumnResizingControlsProps {
target: ElementPath
}

export const GridRowColumnResizingControls =
controlForStrategyMemoized<GridRowColumnResizingControlsProps>(
GridRowColumnResizingControlsComponent,
)

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

export interface GridControlProps {
grid: GridData
}

export interface GridControlsProps {
targets: ElementPath[]
}

export const GridControls = controlForStrategyMemoized<GridControlsProps>(GridControlsComponent)

export const GridResizeEdgeTestId = (edge: GridResizeEdge) => `grid-resize-edge-${edge}`

interface GridResizeControlProps {
target: ElementPath
}

export const GridResizeControls = controlForStrategyMemoized<GridResizeControlProps>(
GridResizeControlsComponent,
)

export function gridEdgeToEdgePosition(edge: GridResizeEdge): EdgePosition {
switch (edge) {
case 'column-end':
return EdgePositionRight
case 'column-start':
return EdgePositionLeft
case 'row-end':
return EdgePositionBottom
case 'row-start':
return EdgePositionTop
default:
assertNever(edge)
}
}

export function controlsForGridPlaceholders(
gridPath: ElementPath,
whenToShow: WhenToShowControl = 'always-visible',
): ControlWithProps<any> {
return {
control: GridControls,
props: { targets: [gridPath] },
key: GridControlsKey(gridPath),
show: whenToShow,
priority: 'bottom',
}
}
Loading

0 comments on commit d366ed5

Please sign in to comment.