Skip to content

Commit

Permalink
Disable grid control strategies for components which don't support st…
Browse files Browse the repository at this point in the history
…yle props (#6610)

**Problem:**
Now it is possible to use the gap and the grid resize controls on
components which do not support style props.

**Fix:**
We somehow have to decide whether a component supports style props or
not. I implemented the following heuristics, but we can improve that
later if necessary:
1. If a component has a descriptor and it allows 'layout-system' section
to be shown then it supports style props.
2. If it doesn't have a descriptor then we check if the component
implementation takes the necessary style props (similarly to the already
existing `targetSupportsPropsSize` function). This returns true when the
component implementation can not be found (if it is an external
component)

Note: When the component does not support the necessary style props:
- I don't show the gap control, but it is just misleading if it is there
- I show the grid resize controls, because they show information
(row/column size), which is meaningful even if the control is not
interactive.
These are just draft solution, so a followup task is necessary to
design/implement the the non-interactive state of the controls

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

-`targetRegisteredStyleControlsOrHonoursStyleProps` is the main function
which decides whether a component supports style props for the actual
control. You can give an inspector section to check (if that is enabled
from the annotation, it returns true), and a list of props (if they are
referred inside the component we return true).

**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 31, 2024
1 parent 277abce commit 71e3817
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { setCursorCommand } from '../../commands/set-cursor-command'
import { CSSCursor } from '../../canvas-types'
import type { CanvasCommand } from '../../commands/commands'
import type { Axis } from '../../gap-utils'
import { getComponentDescriptorForTarget } from '../../../../core/property-controls/property-controls-utils'

export const resizeGridStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
Expand All @@ -59,6 +60,22 @@ export const resizeGridStrategy: CanvasStrategyFactory = (
}

const selectedElement = selectedElements[0]
const selectedElementMetadata = MetadataUtils.findElementByElementPath(
canvasState.startingMetadata,
selectedElement,
)
if (selectedElementMetadata == null) {
return null
}

const supportsStyleProp = MetadataUtils.targetRegisteredStyleControlsOrHonoursStyleProps(
canvasState.projectContents,
selectedElementMetadata,
canvasState.propertyControlsInfo,
'layout-system',
['gridTemplateColumns', 'gridTemplateRows'],
'some',
)

const isGridCell = MetadataUtils.isGridCell(canvasState.startingMetadata, selectedElement)
const isGrid = MetadataUtils.isGridLayoutedContainer(
Expand Down Expand Up @@ -94,7 +111,9 @@ export const resizeGridStrategy: CanvasStrategyFactory = (
},
controlsForGridPlaceholders(gridPath),
],
fitness: onlyFitWhenDraggingThisControl(interactionSession, 'GRID_AXIS_HANDLE', 1),
fitness: supportsStyleProp
? onlyFitWhenDraggingThisControl(interactionSession, 'GRID_AXIS_HANDLE', 1)
: 0,
apply: () => {
if (
interactionSession == null ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import type { GridGapControlProps } from '../../controls/select-mode/grid-gap-co
import { GridGapControl } from '../../controls/select-mode/grid-gap-control'
import type { GridControlsProps } from '../../controls/grid-controls-for-strategies'
import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies'
import { getComponentDescriptorForTarget } from '../../../../core/property-controls/property-controls-utils'

const SetGridGapStrategyId = 'SET_GRID_GAP_STRATEGY'

Expand All @@ -63,22 +64,31 @@ export const setGridGapStrategy: CanvasStrategyFactory = (
}

const selectedElement = selectedElements[0]
const selectedElementMetadata = MetadataUtils.findElementByElementPath(
canvasState.startingMetadata,
selectedElement,
)
if (
!MetadataUtils.isGridLayoutedContainer(
MetadataUtils.findElementByElementPath(canvasState.startingMetadata, selectedElement),
)
selectedElementMetadata == null ||
!MetadataUtils.isGridLayoutedContainer(selectedElementMetadata)
) {
return null
}

const children = recurseIntoChildrenOfMapOrFragment(
canvasState.startingMetadata,
canvasState.startingAllElementProps,
canvasState.startingElementPathTree,
selectedElement,
const supportsStyleProp = MetadataUtils.targetRegisteredStyleControlsOrHonoursStyleProps(
canvasState.projectContents,
selectedElementMetadata,
canvasState.propertyControlsInfo,
'layout-system',
['gap', 'rowGap', 'columnGap'],
'some',
)
if (!supportsStyleProp) {
return null
}

const gridGap = maybeGridGapData(canvasState.startingMetadata, selectedElement)

if (gridGap == null) {
return null
}
Expand Down
6 changes: 3 additions & 3 deletions editor/src/components/custom-code/code-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export interface ShownInspectorSpec {
sections: Styling[]
}

export type TypedInpsectorSpec = { type: 'hidden' } | ShownInspectorSpec
export type TypedInspectorSpec = { type: 'hidden' } | ShownInspectorSpec

export interface ComponentDescriptor {
properties: PropertyControls
Expand All @@ -163,7 +163,7 @@ export interface ComponentDescriptor {
variants: ComponentInfo[]
source: ComponentDescriptorSource
focus: Focus
inspector: TypedInpsectorSpec
inspector: TypedInspectorSpec
emphasis: Emphasis
icon: Icon
label: string | null
Expand All @@ -187,7 +187,7 @@ export function componentDescriptor(
preferredChildComponents: Array<PreferredChildComponentDescriptor>,
source: ComponentDescriptorSource,
focus: Focus,
inspector: TypedInpsectorSpec,
inspector: TypedInspectorSpec,
emphasis: Emphasis,
icon: Icon,
label: string | null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ import type {
CurriedUtopiaRequireFn,
PropertyControlsInfo,
ComponentDescriptorFromDescriptorFile,
TypedInpsectorSpec,
TypedInspectorSpec,
ShownInspectorSpec,
StyleSectionState,
} from '../../custom-code/code-file'
Expand Down Expand Up @@ -3935,7 +3935,7 @@ export function ComponentDescriptorSourceKeepDeepEquality(): KeepDeepEqualityCal
}
}

const InspectorSpecKeepDeepEquality: KeepDeepEqualityCall<TypedInpsectorSpec> = (
const InspectorSpecKeepDeepEquality: KeepDeepEqualityCall<TypedInspectorSpec> = (
oldValue,
newValue,
) => {
Expand Down
67 changes: 66 additions & 1 deletion editor/src/core/model/element-metadata-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as OPI from 'object-path-immutable'
import type { Emphasis, FlexLength, Icon, Sides } from 'utopia-api/core'
import type { Emphasis, FlexLength, Icon, Sides, Styling } from 'utopia-api/core'
import { sides } from 'utopia-api/core'
import { getReorderDirection } from '../../components/canvas/controls/select-mode/yoga-utils'
import { getImageSize, scaleImageDimensions } from '../../components/images'
Expand Down Expand Up @@ -113,6 +113,7 @@ import {
componentUsesProperty,
findJSXElementChildAtPath,
elementChildSupportsChildrenAlsoText,
componentHonoursStyleProps,
} from './element-template-utils'
import {
isImportedComponent,
Expand Down Expand Up @@ -1212,6 +1213,70 @@ export const MetadataUtils = {
return componentHonoursPropsSize(underlyingComponent)
}
},
targetHonoursStyleProps(
projectContents: ProjectContentTreeRoot,
metadata: ElementInstanceMetadata | null,
props: Array<string>,
everyOrSome: 'every' | 'some',
): boolean {
if (metadata == null) {
return false
}
if (
isLeft(metadata.element) ||
(isRight(metadata.element) && !isJSXElement(metadata.element.value))
) {
return false
}
const underlyingComponent = findUnderlyingTargetComponentImplementationFromImportInfo(
projectContents,
metadata.importInfo,
)
if (underlyingComponent == null) {
// Could be an external third party component, assuming true for now.
return true
} else {
return componentHonoursStyleProps(underlyingComponent, props, everyOrSome)
}
},
targetRegisteredStyleControlsOrHonoursStyleProps(
projectContents: ProjectContentTreeRoot,
metadata: ElementInstanceMetadata,
propertyControlsInfo: PropertyControlsInfo,
inspectorSection: Styling, // if this inspector section is registered then the target honours the style props
props: Array<string>,
everyOrSome: 'every' | 'some',
): boolean {
const inspectorSectionRegistered = (() => {
const descriptor = getComponentDescriptorForTarget(
{ propertyControlsInfo, projectContents },
metadata.elementPath,
)

if (descriptor?.inspector == null) {
return 'no-annotation'
}

if (
descriptor.inspector.type === 'hidden' ||
!descriptor.inspector.sections.includes(inspectorSection)
) {
return 'registered-to-hide'
}
return 'registered-to-show'
})()

switch (inspectorSectionRegistered) {
case 'registered-to-show': //
return true
case 'registered-to-hide':
return false
case 'no-annotation':
return this.targetHonoursStyleProps(projectContents, metadata, props, everyOrSome)
default:
assertNever(inspectorSectionRegistered)
}
},
targetHonoursPropsPosition(
projectContents: ProjectContentTreeRoot,
metadata: ElementInstanceMetadata | null,
Expand Down
40 changes: 40 additions & 0 deletions editor/src/core/model/element-template-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,46 @@ export function componentHonoursPropsSize(component: UtopiaJSXComponent): boolea
}
}

export function componentHonoursStyleProps(
component: UtopiaJSXComponent,
props: Array<string>,
everyOrSome: 'every' | 'some',
): boolean {
if (component.params == null) {
return false
} else {
const nonNullParams = component.params
function checkElements(rootElement: JSXElementChild): boolean {
switch (rootElement.type) {
case 'JSX_ELEMENT':
if (propsStyleIsSpreadInto(nonNullParams, rootElement.props)) {
return true
}
const styleAttrs = props.map((prop) => ({
propName: prop,
attr: getJSXAttributesAtPath(rootElement.props, PP.create('style', prop)),
}))
const filterFn = (styleAttr: { propName: string; attr: GetJSXAttributeResult }) =>
propertyComesFromPropsStyle(nonNullParams, styleAttr.attr, styleAttr.propName)
switch (everyOrSome) {
case 'some':
return styleAttrs.some(filterFn)
case 'every':
return styleAttrs.every(filterFn)
default:
assertNever(everyOrSome)
}
break
case 'JSX_FRAGMENT':
return rootElement.children.every(checkElements)
default:
return false
}
}
return checkElements(component.rootElement)
}
}

function checkJSReferencesVariable(
jsExpression: JSExpressionOtherJavaScript,
variableName: string,
Expand Down
4 changes: 2 additions & 2 deletions editor/src/core/property-controls/property-controls-local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import type {
ComponentDescriptorWithName,
ComponentInfo,
PropertyControlsInfo,
TypedInpsectorSpec,
TypedInspectorSpec,
} from '../../components/custom-code/code-file'
import { dependenciesFromPackageJson } from '../../components/editor/npm-dependency/npm-dependency'
import { parseControlDescription } from './property-controls-parser'
Expand Down Expand Up @@ -998,7 +998,7 @@ async function parseComponentVariants(
return parsedVariants
}

function parseInspectorSpec(inspector: InspectorSpec | undefined): TypedInpsectorSpec {
function parseInspectorSpec(inspector: InspectorSpec | undefined): TypedInspectorSpec {
if (inspector == null) {
return ComponentDescriptorDefaults.inspector
}
Expand Down

0 comments on commit 71e3817

Please sign in to comment.