Skip to content

Commit

Permalink
Set props through style plugin (#6599)
Browse files Browse the repository at this point in the history
See #6574 for a long-form
writeup on the full background of this PR.

## The problem addressed on this PR
The normalisation step cannot easily handle commands/actions removing
properties from elements.

## Fix
Instead of working around the limitation of the normalisation step, this
PR introduces an approach where the style plugins themselves implement
setting/removing style props on elements, and the `SetProperty` /
`DeleteProperty` command uses the style plugin API to set/remove style
props. The PR deals with patching props that are removed during a canvas
interaction (details in
#6574, TLDR: if `gap` is
removed with the gap control, it needs to be set to `0px` while the
interaction is in progress, otherwise the value from the Tailwind class
will be applied which would be janky).

This is an under-the-hood PR, no user-facing functionality is added.

### Details
- Updated the `StylePlugin` interface with the `updateStyles` function,
which can be used to set/delete element styles. The inline style plugin
and the Tailwind style plugin are updated accordingly
- Injected `StylePlugin.updateStyles` into `runSetProperty` and
`runDeleteProperties` via the new `runStyleUpdateForStrategy` function.
This function encapsulates a performance-related feature: if an
interaction is ongoing, it sets props in the inline style prop (and
patches removed props), so updates are real-time, and when an
interaction is done, it uses the active plugin. This trick works because
the active strategy is re-run on interaction finish, producing the right
patches for a style update.
- The machinery dealing with zeroing props is added. The entrypoint for
adding the zeroed props is `patchRemovedProperties` (called from
`dispatch-strategies.tsx`, and the entrypoint for recording which props
need to be zeroed is `runStyleUpdateMidInteraction`
- The code related to the normalisation step is removed
- `UpdateClassListCommand` is removed and turned into a helper function
(since it's not called as a command anymore, just as a helper in the
Tailwind style plugin)

**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
bkrmendy authored Nov 11, 2024
1 parent c162a94 commit 07b81de
Show file tree
Hide file tree
Showing 19 changed files with 545 additions and 597 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ import { reparentSubjectsForInteractionTarget } from './strategies/reparent-help
import { getReparentTargetUnified } from './strategies/reparent-helpers/reparent-strategy-parent-lookup'
import { gridChangeElementLocationResizeKeyboardStrategy } from './strategies/grid-change-element-location-keyboard-strategy'
import createCachedSelector from 're-reselect'
import { getActivePlugin } from '../plugins/style-plugins'
import { getActivePlugin, patchRemovedProperties } from '../plugins/style-plugins'
import {
controlsForGridPlaceholders,
GridControls,
Expand Down Expand Up @@ -496,6 +496,16 @@ export function applyCanvasStrategy(
return strategy.apply(strategyLifecycle)
}

export function applyElementsToRerenderFromStrategyResultAndPatchRemovedProps(
editorState: EditorState,
strategyResult: StrategyApplicationResult,
): EditorState {
return applyElementsToRerenderFromStrategyResult(
patchRemovedProperties(editorState),
strategyResult,
)
}

export function applyElementsToRerenderFromStrategyResult(
editorState: EditorState,
strategyResult: StrategyApplicationResult,
Expand Down
7 changes: 7 additions & 0 deletions editor/src/components/canvas/canvas-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,3 +555,10 @@ export interface StyleInfo {
gap: FlexGapInfo | null
flexDirection: FlexDirectionInfo | null
}

const emptyStyleInfo: StyleInfo = {
gap: null,
flexDirection: null,
}

export const isStyleInfoKey = (key: string): key is keyof StyleInfo => key in emptyStyleInfo
9 changes: 2 additions & 7 deletions editor/src/components/canvas/commands/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ import {
runShowGridControlsCommand,
type ShowGridControlsCommand,
} from './show-grid-controls-command'
import type { UpdateClassList } from './update-class-list-command'
import { runUpdateClassList } from './update-class-list-command'

export interface CommandFunctionResult {
editorStatePatches: Array<EditorStatePatch>
Expand Down Expand Up @@ -131,7 +129,6 @@ export type CanvasCommand =
| SetActiveFrames
| UpdateBulkProperties
| ShowGridControlsCommand
| UpdateClassList

export function runCanvasCommand(
editorState: EditorState,
Expand Down Expand Up @@ -174,9 +171,9 @@ export function runCanvasCommand(
case 'PUSH_INTENDED_BOUNDS_AND_UPDATE_HUGGING_ELEMENTS':
return runPushIntendedBoundsAndUpdateHuggingElements(editorState, command)
case 'DELETE_PROPERTIES':
return runDeleteProperties(editorState, command)
return runDeleteProperties(editorState, command, commandLifecycle)
case 'SET_PROPERTY':
return runSetProperty(editorState, command)
return runSetProperty(editorState, command, commandLifecycle)
case 'UPDATE_BULK_PROPERTIES':
return runBulkUpdateProperties(editorState, command)
case 'ADD_IMPORTS_TO_FILE':
Expand Down Expand Up @@ -211,8 +208,6 @@ export function runCanvasCommand(
return runSetActiveFrames(editorState, command)
case 'SHOW_GRID_CONTROLS':
return runShowGridControlsCommand(editorState, command)
case 'UPDATE_CLASS_LIST':
return runUpdateClassList(editorState, command)
default:
const _exhaustiveCheck: never = command
throw new Error(`Unhandled canvas command ${JSON.stringify(command)}`)
Expand Down
37 changes: 30 additions & 7 deletions editor/src/components/canvas/commands/delete-properties-command.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import type { EditorState } from '../../../components/editor/store/editor-state'
import type { ElementPath, PropertyPath } from '../../../core/shared/project-file-types'
import type { BaseCommand, CommandFunction, WhenToRun } from './commands'
import type { BaseCommand, WhenToRun } from './commands'
import * as EP from '../../../core/shared/element-path'
import * as PP from '../../../core/shared/property-path'
import { deleteValuesAtPath } from './utils/property-utils'
import { deleteValuesAtPath, maybeCssPropertyFromInlineStyle } from './utils/property-utils'
import type { InteractionLifecycle } from '../canvas-strategies/canvas-strategy-types'
import { runStyleUpdateForStrategy } from '../plugins/style-plugins'
import * as SU from '../plugins/style-plugins'

export interface DeleteProperties extends BaseCommand {
type: 'DELETE_PROPERTIES'
element: ElementPath
properties: Array<PropertyPath>
}

export function deleteProperties(
whenToRun: WhenToRun,
element: ElementPath,
Expand All @@ -24,19 +26,40 @@ export function deleteProperties(
}
}

export const runDeleteProperties: CommandFunction<DeleteProperties> = (
export const runDeleteProperties = (
editorState: EditorState,
command: DeleteProperties,
interactionLifecycle: InteractionLifecycle,
) => {
const propsToDelete = command.properties.reduce(
(acc: { styleProps: string[]; rest: PropertyPath[] }, p) => {
const prop = maybeCssPropertyFromInlineStyle(p)
if (prop == null) {
acc.rest.push(p)
} else {
acc.styleProps.push(prop)
}
return acc
},
{ styleProps: [], rest: [] },
)

// Apply the update to the properties.
const { editorStatePatch: propertyUpdatePatch } = deleteValuesAtPath(
const { editorStateWithChanges } = deleteValuesAtPath(
editorState,
command.element,
command.properties,
propsToDelete.rest,
)

const { editorStatePatches } = runStyleUpdateForStrategy(
interactionLifecycle,
editorStateWithChanges,
command.element,
propsToDelete.styleProps.map(SU.deleteCSSProp),
)

return {
editorStatePatches: [propertyUpdatePatch],
editorStatePatches: editorStatePatches,
commandDescription: `Delete Properties ${command.properties
.map(PP.toString)
.join(',')} on ${EP.toUid(command.element)}`,
Expand Down
57 changes: 36 additions & 21 deletions editor/src/components/canvas/commands/set-property-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,15 @@ import type {
} from '../../../core/shared/project-file-types'
import * as PP from '../../../core/shared/property-path'
import type { EditorState } from '../../editor/store/editor-state'
import type { BaseCommand, CommandFunction, WhenToRun } from './commands'
import { applyValuesAtPath, deleteValuesAtPath } from './utils/property-utils'
import type { InteractionLifecycle } from '../canvas-strategies/canvas-strategy-types'
import { runStyleUpdateForStrategy } from '../plugins/style-plugins'
import * as SU from '../plugins/style-plugins'
import type { BaseCommand, CommandFunction, CommandFunctionResult, WhenToRun } from './commands'
import {
applyValuesAtPath,
deleteValuesAtPath,
maybeCssPropertyFromInlineStyle,
} from './utils/property-utils'

type PositionProp = 'left' | 'top' | 'right' | 'bottom' | 'width' | 'height'

Expand All @@ -19,33 +26,27 @@ export interface SetProperty extends BaseCommand {
property: PropertyPath
value: string | number
}

export type PropertyToUpdate = PropertyToSet | PropertyToDelete

type PropertyToSet = { type: 'SET'; path: PropertyPath; value: string | number }
type PropertyToDelete = { type: 'DELETE'; path: PropertyPath }

export function propertyToSet(path: PropertyPath, value: string | number): PropertyToUpdate {
return {
type: 'SET',
path: path,
value: value,
}
}

export function propertyToDelete(path: PropertyPath): PropertyToUpdate {
return {
type: 'DELETE',
path: path,
}
}

export interface UpdateBulkProperties extends BaseCommand {
type: 'UPDATE_BULK_PROPERTIES'
element: ElementPath
properties: PropertyToUpdate[]
}

export function setProperty<T extends PropertyPathPart>(
whenToRun: WhenToRun,
element: ElementPath,
Expand All @@ -60,7 +61,6 @@ export function setProperty<T extends PropertyPathPart>(
value: value,
}
}

export function updateBulkProperties(
whenToRun: WhenToRun,
element: ElementPath,
Expand All @@ -73,7 +73,6 @@ export function updateBulkProperties(
properties: properties,
}
}

export function setPropertyOmitNullProp<T extends PropertyPathPart>(
whenToRun: WhenToRun,
element: ElementPath,
Expand All @@ -87,24 +86,40 @@ export function setPropertyOmitNullProp<T extends PropertyPathPart>(
}
}

export const runSetProperty: CommandFunction<SetProperty> = (
function getCommandDescription(command: SetProperty) {
return `Set Property ${PP.toString(command.property)}=${JSON.stringify(
command.property,
null,
2,
)} on ${EP.toUid(command.element)}`
}

export const runSetProperty = (
editorState: EditorState,
command: SetProperty,
) => {
// Apply the update to the properties.
const { editorStatePatch: propertyUpdatePatch } = applyValuesAtPath(
interactionLifecycle: InteractionLifecycle,
): CommandFunctionResult => {
const prop = maybeCssPropertyFromInlineStyle(command.property)
if (prop == null) {
const { editorStatePatch } = applyValuesAtPath(editorState, command.element, [
{ path: command.property, value: jsExpressionValue(command.value, emptyComments) },
])
return {
editorStatePatches: [editorStatePatch],
commandDescription: getCommandDescription(command),
}
}

const { editorStatePatches } = runStyleUpdateForStrategy(
interactionLifecycle,
editorState,
command.element,
[{ path: command.property, value: jsExpressionValue(command.value, emptyComments) }],
[SU.updateCSSProp(prop, command.value)],
)

return {
editorStatePatches: [propertyUpdatePatch],
commandDescription: `Set Property ${PP.toString(command.property)}=${JSON.stringify(
command.property,
null,
2,
)} on ${EP.toUid(command.element)}`,
editorStatePatches: editorStatePatches,
commandDescription: getCommandDescription(command),
}
}

Expand Down
101 changes: 0 additions & 101 deletions editor/src/components/canvas/commands/update-class-list-command.ts

This file was deleted.

Loading

0 comments on commit 07b81de

Please sign in to comment.