From aa2934220f5a2b4976eac5252fdf756570100168 Mon Sep 17 00:00:00 2001 From: Sean Parsons <217400+seanparsons@users.noreply.github.com> Date: Wed, 16 Oct 2024 14:13:26 +0100 Subject: [PATCH] fix(grids) Handle Nested Grid Dragging (#6526) - Implemented `combineApplicableControls` to collate disparate instances of `GridControls`. - `controlsForGridPlaceholders` has been slightly tweaked to cater for the change in the type of `GridControlsProps` and also to handle a new option suffix. - Removed `pointerEvents` setting which was bizarrely the cause of the original issue. - Removed spurious property. - `GridControlsComponent` now includes ancestor paths. - `GridControlsComponent` now sorts the grid paths by their depth, so that the bottommost event triggers fire over those that are ancestors of that element. --- .../canvas-strategies/canvas-strategies.tsx | 85 +++++++++--- .../canvas-strategies/interaction-state.ts | 6 +- ...-rearrange-move-strategy.spec.browser2.tsx | 32 +++-- .../rearrange-grid-swap-strategy.ts | 4 +- .../controls/grid-controls-for-strategies.tsx | 15 +- .../canvas/controls/grid-controls.tsx | 130 +++++++++++++----- .../store/store-deep-equality-instances.ts | 6 +- editor/src/core/shared/array-utils.ts | 2 +- 8 files changed, 198 insertions(+), 82 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx b/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx index f95c64cacb59..c14f07eb91c0 100644 --- a/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx +++ b/editor/src/components/canvas/canvas-strategies/canvas-strategies.tsx @@ -19,6 +19,7 @@ import type { StrategyApplicationResult, InteractionLifecycle, CustomStrategyState, + WhenToShowControl, } from './canvas-strategy-types' import { ControlDelay, @@ -83,6 +84,11 @@ import { getReparentTargetUnified } from './strategies/reparent-helpers/reparent import { gridRearrangeResizeKeyboardStrategy } from './strategies/grid-rearrange-keyboard-strategy' import createCachedSelector from 're-reselect' import { getActivePlugin } from '../plugins/style-plugins' +import { + controlsForGridPlaceholders, + GridControls, + isGridControlsProps, +} from '../controls/grid-controls-for-strategies' export type CanvasStrategyFactory = ( canvasState: InteractionCanvasState, @@ -648,6 +654,42 @@ function controlPriorityToNumber(prio: ControlWithProps['priority']): numbe } } +export function combineApplicableControls( + strategyControls: Array>, +): Array> { + // Separate out the instances of `GridControls`. + let result: Array> = [] + let gridControlsInstances: Array> = [] + for (const control of strategyControls) { + if (control.control === GridControls) { + gridControlsInstances.push(control) + } else { + result.push(control) + } + } + + // Sift the instances of `GridControls`, storing their targets by when they should be shown. + let gridControlsTargets: Map> = new Map() + for (const control of gridControlsInstances) { + if (isGridControlsProps(control.props)) { + const possibleTargets = gridControlsTargets.get(control.show) + if (possibleTargets == null) { + gridControlsTargets.set(control.show, control.props.targets) + } else { + possibleTargets.push(...control.props.targets) + } + } + } + + // Create new instances of `GridControls` with the combined targets. + for (const [show, targets] of gridControlsTargets) { + result.push(controlsForGridPlaceholders(targets, show, `-${show}`)) + } + + // Return the newly created controls with the combined entries. + return result +} + const controlEquals = (l: ControlWithProps, r: ControlWithProps) => { return l.control === r.control && l.key === r.key } @@ -667,34 +709,37 @@ export function useGetApplicableStrategyControls(localSelectedViews: Array { + let strategyControls: Array> = [] let isResizable: boolean = false - const bottomStrategyControls: Array> = [] - const middleStrategyControls: Array> = [] - const topStrategyControls: Array> = [] // Add the controls for currently applicable strategies. for (const strategy of applicableStrategies) { if (isResizableStrategy(strategy)) { isResizable = true } - const strategyControls = getApplicableControls(currentStrategy, strategy) - - // uniquely add the strategyControls to the bottom, middle, and top arrays - for (const control of strategyControls) { - switch (control.priority) { - case 'bottom': - pushUniquelyBy(bottomStrategyControls, control, controlEquals) - break - case undefined: - pushUniquelyBy(middleStrategyControls, control, controlEquals) - break - case 'top': - pushUniquelyBy(topStrategyControls, control, controlEquals) - break - default: - assertNever(control.priority) - } + strategyControls.push(...getApplicableControls(currentStrategy, strategy)) + } + const combinedControls = combineApplicableControls(strategyControls) + const bottomStrategyControls: Array> = [] + const middleStrategyControls: Array> = [] + const topStrategyControls: Array> = [] + + // uniquely add the strategyControls to the bottom, middle, and top arrays + for (const control of combinedControls) { + switch (control.priority) { + case 'bottom': + pushUniquelyBy(bottomStrategyControls, control, controlEquals) + break + case undefined: + pushUniquelyBy(middleStrategyControls, control, controlEquals) + break + case 'top': + pushUniquelyBy(topStrategyControls, control, controlEquals) + break + default: + assertNever(control.priority) } } + // Special case controls. if (!isResizable && !currentlyInProgress) { middleStrategyControls.push(notResizableControls) diff --git a/editor/src/components/canvas/canvas-strategies/interaction-state.ts b/editor/src/components/canvas/canvas-strategies/interaction-state.ts index 8be4107e03b3..d781e4a7f9a6 100644 --- a/editor/src/components/canvas/canvas-strategies/interaction-state.ts +++ b/editor/src/components/canvas/canvas-strategies/interaction-state.ts @@ -646,13 +646,13 @@ export function reorderSlider(): ReorderSlider { export interface GridCellHandle { type: 'GRID_CELL_HANDLE' - id: string + path: ElementPath } -export function gridCellHandle(params: { id: string }): GridCellHandle { +export function gridCellHandle(params: { path: ElementPath }): GridCellHandle { return { type: 'GRID_CELL_HANDLE', - id: params.id, + path: params.path, } } diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.spec.browser2.tsx b/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.spec.browser2.tsx index 62683a10f419..31d979dd8cc7 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.spec.browser2.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-rearrange-move-strategy.spec.browser2.tsx @@ -11,7 +11,8 @@ import { import { selectComponentsForTest } from '../../../../utils/utils.test-utils' import CanvasActions from '../../canvas-actions' import { GridCellTestId } from '../../controls/grid-controls-for-strategies' -import { mouseDragFromPointToPoint } from '../../event-helpers.test-utils' +import { CanvasControlsContainerID } from '../../controls/new-canvas-controls' +import { mouseDragFromPointToPoint, mouseUpAtPoint } from '../../event-helpers.test-utils' import type { EditorRenderResult } from '../../ui-jsx.test-utils' import { renderTestEditorWithCode } from '../../ui-jsx.test-utils' import type { GridCellCoordinates } from './grid-cell-bounds' @@ -572,6 +573,7 @@ export var storyboard = ( childCenter, offsetPoint(childCenter, windowPoint({ x: 280, y: 120 })), { + staggerMoveEvents: false, moveBeforeMouseDown: true, }, ) @@ -767,24 +769,28 @@ async function runMoveTest( const sourceRect = sourceGridCell.getBoundingClientRect() const targetRect = targetGridCell.getBoundingClientRect() + const endPoint = getRectCenter( + localRectangle({ + x: targetRect.x, + y: targetRect.y, + width: targetRect.width, + height: targetRect.height, + }), + ) + await mouseDragFromPointToPoint( sourceGridCell, { x: sourceRect.x + 10, y: sourceRect.y + 10, }, - getRectCenter( - localRectangle({ - x: targetRect.x, - y: targetRect.y, - width: targetRect.width, - height: targetRect.height, - }), - ), + endPoint, { + staggerMoveEvents: false, moveBeforeMouseDown: true, }, ) + await mouseUpAtPoint(editor.renderedDOM.getByTestId(CanvasControlsContainerID), endPoint) return editor.renderedDOM.getByTestId(props.testId).style } @@ -941,7 +947,7 @@ export var storyboard = ( height: '100%', }} data-uid='pink' - data-testid='pink' + data-testid='pink' data-label='pink' />
diff --git a/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts index 9ff15bde58c9..00429caaba56 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/rearrange-grid-swap-strategy.ts @@ -104,11 +104,11 @@ export const rearrangeGridSwapStrategy: CanvasStrategyFactory = ( if ( pointerOverChild != null && - EP.toUid(pointerOverChild.elementPath) !== interactionSession.activeControl.id + EP.toUid(pointerOverChild.elementPath) !== EP.toUid(interactionSession.activeControl.path) ) { commands.push( ...swapChildrenCommands({ - grabbedElementUid: interactionSession.activeControl.id, + grabbedElementUid: EP.toUid(interactionSession.activeControl.path), swapToElementUid: EP.toUid(pointerOverChild.elementPath), children: children, parentPath: EP.parentPath(selectedElement), diff --git a/editor/src/components/canvas/controls/grid-controls-for-strategies.tsx b/editor/src/components/canvas/controls/grid-controls-for-strategies.tsx index 5d6fd793dfe5..e492acf30673 100644 --- a/editor/src/components/canvas/controls/grid-controls-for-strategies.tsx +++ b/editor/src/components/canvas/controls/grid-controls-for-strategies.tsx @@ -219,9 +219,14 @@ export interface GridControlProps { } export interface GridControlsProps { + type: 'GRID_CONTROLS_PROPS' targets: ElementPath[] } +export function isGridControlsProps(props: unknown): props is GridControlsProps { + return (props as GridControlsProps).type === 'GRID_CONTROLS_PROPS' +} + export const GridControls = controlForStrategyMemoized(GridControlsComponent) interface GridResizeControlProps { @@ -264,13 +269,17 @@ export function edgePositionToGridResizeEdge(position: EdgePosition): GridResize } export function controlsForGridPlaceholders( - gridPath: ElementPath, + gridPath: ElementPath | Array, whenToShow: WhenToShowControl = 'always-visible', + suffix: string | null = null, ): ControlWithProps { return { control: GridControls, - props: { targets: [gridPath] }, - key: GridControlsKey(gridPath), + props: { + type: 'GRID_CONTROLS_PROPS', + targets: Array.isArray(gridPath) ? gridPath : [gridPath], + }, + key: `GridControls${suffix == null ? '' : suffix}`, show: whenToShow, priority: 'bottom', } diff --git a/editor/src/components/canvas/controls/grid-controls.tsx b/editor/src/components/canvas/controls/grid-controls.tsx index 47f20a9bc565..0ca1c10c48db 100644 --- a/editor/src/components/canvas/controls/grid-controls.tsx +++ b/editor/src/components/canvas/controls/grid-controls.tsx @@ -563,9 +563,10 @@ export const GridRowColumnResizingControlsComponent = ({ interface GridControlProps { grid: GridData + controlsVisible: 'visible' | 'not-visible' } -const GridControl = React.memo(({ grid }) => { +const GridControl = React.memo(({ grid, controlsVisible }) => { const dispatch = useDispatch() const controls = useAnimationControls() const colorTheme = useColorTheme() @@ -598,7 +599,7 @@ const GridControl = React.memo(({ grid }) => { store.editor.canvas.interactionSession?.interactionData.type === 'DRAG' && store.editor.canvas.interactionSession?.interactionData.modifiers.cmd !== true && store.editor.canvas.interactionSession?.interactionData.drag != null - ? store.editor.canvas.interactionSession.activeControl.id + ? EP.toUid(store.editor.canvas.interactionSession.activeControl.path) : null, 'GridControl activelyDraggingOrResizingCell', ) @@ -609,6 +610,12 @@ const GridControl = React.memo(({ grid }) => { 'GridControl currentHoveredCell', ) + const currentHoveredGrid = useEditorState( + Substores.canvas, + (store) => store.editor.canvas.controls.gridControlData?.grid ?? null, + 'GridControl currentHoveredGrid', + ) + const targetsAreCellsWithPositioning = useEditorState( Substores.metadata, (store) => @@ -638,7 +645,7 @@ const GridControl = React.memo(({ grid }) => { const canvasOffsetRef = useRefEditorState((store) => store.editor.canvas.roundedCanvasOffset) const startInteractionWithUid = React.useCallback( - (params: { uid: string; row: number; column: number; frame: CanvasRectangle }) => + (params: { path: ElementPath; row: number; column: number; frame: CanvasRectangle }) => (event: React.MouseEvent) => { setInitialShadowFrame(params.frame) @@ -653,7 +660,7 @@ const GridControl = React.memo(({ grid }) => { createInteractionViaMouse( start.canvasPositionRounded, Modifier.modifiersForEvent(event), - gridCellHandle({ id: params.uid }), + gridCellHandle({ path: params.path }), 'zero-drag-not-permitted', ), ), @@ -699,13 +706,17 @@ const GridControl = React.memo(({ grid }) => { (store) => store.editor.canvas.interactionSession != null && store.editor.canvas.interactionSession.activeControl.type === 'GRID_CELL_HANDLE' - ? store.editor.canvas.interactionSession.activeControl.id + ? store.editor.canvas.interactionSession.activeControl.path : null, 'GridControl dragging', ) const shadow = React.useMemo(() => { - return cells.find((cell) => EP.toUid(cell.elementPath) === dragging) ?? null + return ( + cells.find( + (cell) => EP.toUid(cell.elementPath) === (dragging == null ? null : EP.toUid(dragging)), + ) ?? null + ) }, [cells, dragging]) const [initialShadowFrame, setInitialShadowFrame] = React.useState( @@ -795,14 +806,18 @@ const GridControl = React.memo(({ grid }) => { gridPath: gridPath, }) - const placeholders = range(0, grid.cells) + const placeholders = controlsVisible === 'visible' ? range(0, grid.cells) : [] const baseStyle = getGridHelperStyleMatchingTargetGrid(grid) const style = { ...baseStyle, backgroundColor: - activelyDraggingOrResizingCell != null ? colorTheme.primary10.value : 'transparent', + activelyDraggingOrResizingCell == null || controlsVisible === 'not-visible' + ? 'transparent' + : colorTheme.primary10.value, outline: `1px solid ${ - activelyDraggingOrResizingCell != null ? colorTheme.primary.value : 'transparent' + activelyDraggingOrResizingCell == null || controlsVisible === 'not-visible' + ? 'transparent' + : colorTheme.primary.value }`, } @@ -821,8 +836,13 @@ const GridControl = React.memo(({ grid }) => { const id = gridCellTargetId(grid.elementPath, countedRow, countedColumn) const borderID = `${id}-border` + const isActiveGrid = + (dragging != null && EP.isParentOf(grid.elementPath, dragging)) || + (currentHoveredGrid != null && EP.pathsEqual(grid.elementPath, currentHoveredGrid)) const isActiveCell = - countedColumn === currentHoveredCell?.column && countedRow === currentHoveredCell?.row + isActiveGrid && + countedColumn === currentHoveredCell?.column && + countedRow === currentHoveredCell?.row const activePositioningTarget = isActiveCell && targetsAreCellsWithPositioning @@ -834,33 +854,29 @@ const GridControl = React.memo(({ grid }) => { key={id} id={id} data-testid={id} - data-wtf={`data-wtf`} style={{ position: 'relative', - pointerEvents: 'initial', zIndex: activePositioningTarget ? 1 : undefined, }} data-grid-row={countedRow} data-grid-column={countedColumn} > - -
- +
) })} @@ -870,7 +886,7 @@ const GridControl = React.memo(({ grid }) => { return (
(({ grid }) => { alignItems: 'flex-end', backgroundColor: activelyDraggingOrResizingCell != null && - EP.toUid(cell.elementPath) !== activelyDraggingOrResizingCell + EP.toUid(cell.elementPath) !== activelyDraggingOrResizingCell && + controlsVisible === 'visible' ? '#ffffff66' : 'transparent', borderRadius: @@ -905,7 +922,8 @@ const GridControl = React.memo(({ grid }) => { initialShadowFrame != null && interactionData?.dragStart != null && interactionData?.drag != null && - hoveringStart != null ? ( + hoveringStart != null && + controlsVisible === 'visible' ? ( (({ elemen GridMeasurementHelper.displayName = 'GridMeasurementHelper' export const GridControlsComponent = ({ targets }: GridControlsProps) => { + const ancestorPaths = React.useMemo(() => { + return targets.flatMap((target) => EP.getAncestors(target)) + }, [targets]) + const ancestorGrids: Array = useEditorState( + Substores.metadata, + (store) => { + return ancestorPaths.filter((ancestorPath) => { + const ancestorMetadata = MetadataUtils.findElementByElementPath( + store.editor.jsxMetadata, + ancestorPath, + ) + return MetadataUtils.isGridLayoutedContainer(ancestorMetadata) + }) + }, + 'GridControlsComponent ancestorGrids', + ) + const targetRootCell = useEditorState( Substores.canvas, (store) => store.editor.canvas.controls.gridControlData?.rootCell ?? null, - 'GridControls targetRootCell', + 'GridControlsComponent targetRootCell', ) const hoveredGrids = useEditorState( Substores.canvas, (store) => stripNulls([store.editor.canvas.controls.gridControlData?.grid]), - 'GridControls hoveredGrids', + 'GridControlsComponent hoveredGrids', ) - const grids = useGridData(uniqBy([...targets, ...hoveredGrids], (a, b) => EP.pathsEqual(a, b))) + const gridsWithVisibleControls: Array = [...targets, ...hoveredGrids] + + // Uniqify the grid paths, and then sort them by depth. + // With the lowest depth grid at the end so that it renders on top and catches the events + // before those above it in the hierarchy. + const grids = useGridData( + uniqBy([...gridsWithVisibleControls, ...ancestorGrids], (a, b) => EP.pathsEqual(a, b)).sort( + (a, b) => { + return EP.fullDepth(a) - EP.fullDepth(b) + }, + ), + ) if (grids.length === 0) { return null @@ -1016,7 +1062,17 @@ export const GridControlsComponent = ({ targets }: GridControlsProps) => {
{grids.map((grid) => { - return + const shouldHaveVisibleControls = EP.containsPath( + grid.elementPath, + gridsWithVisibleControls, + ) + return ( + + ) })} diff --git a/editor/src/components/editor/store/store-deep-equality-instances.ts b/editor/src/components/editor/store/store-deep-equality-instances.ts index f22431510ddb..62195fe69d70 100644 --- a/editor/src/components/editor/store/store-deep-equality-instances.ts +++ b/editor/src/components/editor/store/store-deep-equality-instances.ts @@ -2995,9 +2995,9 @@ export const BorderRadiusResizeHandleKeepDeepEquality: KeepDeepEqualityCall< export const GridCellHandleKeepDeepEquality: KeepDeepEqualityCall = combine1EqualityCall( - (handle) => handle.id, - createCallWithTripleEquals(), - (id) => gridCellHandle({ id }), + (handle) => handle.path, + ElementPathKeepDeepEquality, + (path) => gridCellHandle({ path }), ) export const GridAxisHandleKeepDeepEquality: KeepDeepEqualityCall = diff --git a/editor/src/core/shared/array-utils.ts b/editor/src/core/shared/array-utils.ts index 8549b2d15c1b..d56455fcdae3 100644 --- a/editor/src/core/shared/array-utils.ts +++ b/editor/src/core/shared/array-utils.ts @@ -292,7 +292,7 @@ export function addAllUniquelyBy( ): Array { let workingArray = [...array] fastForEach(values, (value) => { - if (array.findIndex((a) => eq(a, value)) === -1) { + if (!workingArray.some((a) => eq(a, value))) { workingArray.push(value) } })