From 0a6ad3f2471836e98f35cffea09d6a7c7bdbb2c3 Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:24:34 +0100 Subject: [PATCH] Split controls for grid rulers and placeholders, don't show placeholders during item resize (#6720) **Problem:** Grid control placeholders should not show while resizing a grid item. **Fix:** 1. Split the grid controls into two components - one for the placeholders and one for the ruler/markers. 2. Do not show the placeholders while resizing a grid item (via its edges) 3. Do some prep work to only show row/col placeholder lines during marker resize (an incremental PR will take care of that) | Before | After | |-----------|--------------| | ![Kapture 2024-12-11 at 12 22 44](https://github.com/user-attachments/assets/bca7def9-91fc-4095-9cff-9058b77ee609) | ![Kapture 2024-12-11 at 12 22 02](https://github.com/user-attachments/assets/255ceb5f-6a5e-4085-82b3-5e057ffb4c3e) | Fixes #[6719](https://github.com/concrete-utopia/utopia/issues/6719) --- .../strategies/basic-resize-strategy.tsx | 12 ++- ...nge-element-location-duplicate-strategy.ts | 10 +- ...ange-element-location-keyboard-strategy.ts | 10 +- .../grid-change-element-location-strategy.ts | 6 +- .../strategies/grid-move-absolute.ts | 6 +- .../strategies/grid-reorder-strategy.ts | 6 +- .../grid-resize-element-ruler-strategy.ts | 2 + .../grid-resize-element-strategy.ts | 4 +- .../controls/grid-controls-for-strategies.tsx | 26 +++++ .../canvas/controls/grid-controls.tsx | 100 +++++++++++------- 10 files changed, 134 insertions(+), 48 deletions(-) diff --git a/editor/src/components/canvas/canvas-strategies/strategies/basic-resize-strategy.tsx b/editor/src/components/canvas/canvas-strategies/strategies/basic-resize-strategy.tsx index 8c3a65d3baf0..976888db8e84 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/basic-resize-strategy.tsx +++ b/editor/src/components/canvas/canvas-strategies/strategies/basic-resize-strategy.tsx @@ -36,7 +36,10 @@ 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-for-strategies' +import { + controlsForGridPlaceholders, + controlsForGridRulers, +} 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' @@ -130,7 +133,12 @@ export function basicResizeStrategy( key: 'parent-bounds-control', show: 'visible-only-while-active', }), - ...(isGridCell ? [controlsForGridPlaceholders(gridItemIdentifier(selectedElement))] : []), + ...(isGridCell + ? [ + controlsForGridPlaceholders(gridItemIdentifier(selectedElement)), + controlsForGridRulers(gridItemIdentifier(selectedElement)), + ] + : []), ], fitness: interactionSession != null && diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts index ffa2066191ae..b10120351cef 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-duplicate-strategy.ts @@ -7,7 +7,10 @@ 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-for-strategies' +import { + controlsForGridPlaceholders, + controlsForGridRulers, +} 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' @@ -79,7 +82,10 @@ export const gridChangeElementLocationDuplicateStrategy: CanvasStrategyFactory = category: 'tools', type: 'pointer', }, - controlsToRender: [controlsForGridPlaceholders(gridItemIdentifier(selectedElement))], + controlsToRender: [ + controlsForGridPlaceholders(gridItemIdentifier(selectedElement)), + controlsForGridRulers(gridItemIdentifier(selectedElement)), + ], fitness: onlyFitWhenDraggingThisControl(interactionSession, 'GRID_CELL_HANDLE', 3), apply: () => { if ( diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-keyboard-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-keyboard-strategy.ts index 0bee8900873a..be83591b9b74 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-keyboard-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-keyboard-strategy.ts @@ -2,7 +2,10 @@ 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-for-strategies' +import { + controlsForGridPlaceholders, + controlsForGridRulers, +} from '../../controls/grid-controls-for-strategies' import type { CanvasStrategy, InteractionCanvasState } from '../canvas-strategy-types' import { emptyStrategyApplicationResult, @@ -68,7 +71,10 @@ export function gridChangeElementLocationResizeKeyboardStrategy( category: 'modalities', type: 'reorder-large', }, - controlsToRender: [controlsForGridPlaceholders(gridItemIdentifier(target))], + controlsToRender: [ + controlsForGridPlaceholders(gridItemIdentifier(target)), + controlsForGridRulers(gridItemIdentifier(target)), + ], fitness: fitness(interactionSession), apply: () => { if (interactionSession == null || interactionSession.interactionData.type !== 'KEYBOARD') { diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts index 7ef0e98a20e6..03d6a8fcb7a7 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-change-element-location-strategy.ts @@ -16,7 +16,10 @@ import { getTargetGridCellData } from '../../../inspector/grid-helpers' import type { CanvasCommand } from '../../commands/commands' import { reorderElement } from '../../commands/reorder-element-command' import { showGridControls } from '../../commands/show-grid-controls-command' -import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies' +import { + controlsForGridPlaceholders, + controlsForGridRulers, +} from '../../controls/grid-controls-for-strategies' import type { CanvasStrategyFactory } from '../canvas-strategies' import { onlyFitWhenDraggingThisControl } from '../canvas-strategies' import type { InteractionCanvasState } from '../canvas-strategy-types' @@ -97,6 +100,7 @@ export const gridChangeElementLocationStrategy: CanvasStrategyFactory = ( }, controlsToRender: [ controlsForGridPlaceholders(gridItemIdentifier(selectedElement), 'visible-only-while-active'), + controlsForGridRulers(gridItemIdentifier(selectedElement), 'visible-only-while-active'), ], fitness: onlyFitWhenDraggingThisControl(interactionSession, 'GRID_CELL_HANDLE', 2), apply: () => { diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-move-absolute.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-move-absolute.ts index 2ecadc5fa9b0..aa206cd67988 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-move-absolute.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-move-absolute.ts @@ -23,7 +23,10 @@ import { } from '../../../../core/shared/math-utils' import type { CanvasCommand } from '../../commands/commands' import { showGridControls } from '../../commands/show-grid-controls-command' -import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies' +import { + controlsForGridPlaceholders, + controlsForGridRulers, +} from '../../controls/grid-controls-for-strategies' import type { CanvasStrategyFactory } from '../canvas-strategies' import { onlyFitWhenDraggingThisControl } from '../canvas-strategies' import type { InteractionCanvasState } from '../canvas-strategy-types' @@ -102,6 +105,7 @@ export const gridMoveAbsoluteStrategy: CanvasStrategyFactory = ( }, controlsToRender: [ controlsForGridPlaceholders(gridItemIdentifier(selectedElement), 'visible-only-while-active'), + controlsForGridRulers(gridItemIdentifier(selectedElement), 'visible-only-while-active'), ], fitness: onlyFitWhenDraggingThisControl(interactionSession, 'GRID_CELL_HANDLE', 2), apply: () => { diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-reorder-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-reorder-strategy.ts index abec7c1437cd..52e38352c41f 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-reorder-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-reorder-strategy.ts @@ -11,7 +11,10 @@ import { absolute } from '../../../../utils/utils' import type { CanvasCommand } from '../../commands/commands' import { reorderElement } from '../../commands/reorder-element-command' import { showGridControls } from '../../commands/show-grid-controls-command' -import { controlsForGridPlaceholders } from '../../controls/grid-controls-for-strategies' +import { + controlsForGridPlaceholders, + controlsForGridRulers, +} from '../../controls/grid-controls-for-strategies' import type { CanvasStrategyFactory } from '../canvas-strategies' import { onlyFitWhenDraggingThisControl } from '../canvas-strategies' import type { InteractionCanvasState } from '../canvas-strategy-types' @@ -95,6 +98,7 @@ export const gridReorderStrategy: CanvasStrategyFactory = ( }, controlsToRender: [ controlsForGridPlaceholders(gridItemIdentifier(selectedElement), 'visible-only-while-active'), + controlsForGridRulers(gridItemIdentifier(selectedElement), 'visible-only-while-active'), ], fitness: onlyFitWhenDraggingThisControl( interactionSession, diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-ruler-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-ruler-strategy.ts index 30c1fb75487a..d8eeee81929c 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-ruler-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-ruler-strategy.ts @@ -17,6 +17,7 @@ import { gridContainerIdentifier, gridItemIdentifier } from '../../../editor/sto import { isCSSKeyword } from '../../../inspector/common/css-utils' import { controlsForGridPlaceholders, + controlsForGridRulers, GridResizeControls, } from '../../controls/grid-controls-for-strategies' import type { CanvasStrategyFactory } from '../canvas-strategies' @@ -82,6 +83,7 @@ export const gridResizeElementRulerStrategy: CanvasStrategyFactory = ( show: 'always-visible', }, controlsForGridPlaceholders(gridItemIdentifier(selectedElement)), + controlsForGridRulers(gridItemIdentifier(selectedElement)), ], fitness: onlyFitWhenDraggingThisControl(interactionSession, 'GRID_RESIZE_RULER_HANDLE', 1), apply: () => { diff --git a/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts b/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts index 2141f6add524..1c9129be903a 100644 --- a/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts +++ b/editor/src/components/canvas/canvas-strategies/strategies/grid-resize-element-strategy.ts @@ -20,7 +20,7 @@ import { isFillOrStretchModeAppliedOnAnySide } from '../../../inspector/inspecto import { CSSCursor } from '../../canvas-types' import { setCursorCommand } from '../../commands/set-cursor-command' import { - controlsForGridPlaceholders, + controlsForGridRulers, gridEdgeToEdgePosition, GridResizeControls, } from '../../controls/grid-controls-for-strategies' @@ -88,7 +88,7 @@ export const gridResizeElementStrategy: CanvasStrategyFactory = ( key: `grid-resize-controls-${EP.toString(selectedElement)}`, show: 'always-visible', }, - controlsForGridPlaceholders(gridItemIdentifier(selectedElement)), + controlsForGridRulers(gridItemIdentifier(selectedElement)), ], fitness: onlyFitWhenDraggingThisControl(interactionSession, 'GRID_RESIZE_HANDLE', 1), apply: () => { 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 9a828d007812..b91113d0677d 100644 --- a/editor/src/components/canvas/controls/grid-controls-for-strategies.tsx +++ b/editor/src/components/canvas/controls/grid-controls-for-strategies.tsx @@ -25,6 +25,7 @@ import { GridControlsComponent, GridResizeControlsComponent, GridRowColumnResizingControlsComponent, + GridRulersControlsComponent, } from './grid-controls' import { isEdgePositionOnSide } from '../canvas-utils' import { useMonitorChangesToElements } from '../../../components/editor/store/store-monitor' @@ -167,6 +168,15 @@ export function isGridControlsProps(props: unknown): props is GridControlsProps export const GridControls = controlForStrategyMemoized(GridControlsComponent) +export interface GridRulersControlsProps { + type: 'GRID_RULERS_CONTROLS_PROPS' + targets: GridIdentifier[] +} + +export const GridRulersControls = controlForStrategyMemoized( + GridRulersControlsComponent, +) + interface GridResizeControlProps { target: GridIdentifier } @@ -221,3 +231,19 @@ export function controlsForGridPlaceholders( show: whenToShow, } } + +export function controlsForGridRulers( + gridPath: GridIdentifier | Array, + whenToShow: WhenToShowControl = 'always-visible', + suffix: string | null = null, +): ControlWithProps { + return { + control: GridRulersControls, + props: { + type: 'GRID_RULERS_CONTROLS_PROPS', + targets: Array.isArray(gridPath) ? gridPath : [gridPath], + }, + key: `GridRulersControls${suffix == null ? '' : suffix}`, + show: whenToShow, + } +} diff --git a/editor/src/components/canvas/controls/grid-controls.tsx b/editor/src/components/canvas/controls/grid-controls.tsx index b0ccf8058658..4a2475f88264 100644 --- a/editor/src/components/canvas/controls/grid-controls.tsx +++ b/editor/src/components/canvas/controls/grid-controls.tsx @@ -135,6 +135,7 @@ import { import type { Property } from 'csstype' import { isFeatureEnabled } from '../../../utils/feature-switches' import type { ThemeObject } from '../../../uuiui/styles/theme/theme-helpers' +import { atom, useAtom } from 'jotai' const CELL_ANIMATION_DURATION = 0.15 // seconds @@ -160,6 +161,8 @@ function getLabelForAxis( const GRID_RESIZE_HANDLE_SIZE = 15 // px +const forceShowGridPlaceholdersAtom = atom('not-visible') + interface GridResizingControlProps { dimension: GridDimension dimensionIndex: number @@ -646,9 +649,11 @@ export const GridRowColumnResizingControlsComponent = ({ ) } +type GridControlVisibility = 'all' | 'not-visible' + interface GridControlProps { grid: GridData - controlsVisible: 'visible' | 'not-visible' + controlsVisible: GridControlVisibility } const GridControl = React.memo(({ grid, controlsVisible }) => { @@ -917,7 +922,7 @@ const GridControl = React.memo(({ grid, controlsVisible }) => gridPath: gridPath, }) - const placeholders = controlsVisible === 'visible' ? range(0, grid.cells) : [] + const placeholders = controlsVisible !== 'not-visible' ? range(0, grid.cells) : [] const baseStyle = getGridHelperStyleMatchingTargetGrid(grid) const style = { ...baseStyle, @@ -1029,7 +1034,7 @@ const GridControl = React.memo(({ grid, controlsVisible }) => backgroundColor: activelyDraggingOrResizingCell != null && EP.toUid(cell.elementPath) !== activelyDraggingOrResizingCell && - controlsVisible === 'visible' + controlsVisible === 'all' ? '#ffffff66' : 'transparent', borderRadius: cell.borderRadius ?? 0, @@ -1044,7 +1049,7 @@ const GridControl = React.memo(({ grid, controlsVisible }) => interactionData?.dragStart != null && interactionData?.drag != null && hoveringStart != null && - controlsVisible === 'visible' ? ( + controlsVisible === 'all' ? ( { }), ) - const [showGridCellOutlines, setShowGridCellOutlines] = React.useState(false) - const isGridItemSelectedWithoutInteraction = selectedGridItems.length > 0 && !isGridItemInteractionActive + const [forceShowGridPlaceholders] = useAtom(forceShowGridPlaceholdersAtom) + if (grids.length === 0) { return null } @@ -1222,31 +1227,34 @@ export const GridControlsComponent = ({ targets }: GridControlsProps) => { ) })} - {/* Ruler markers */} - {when( - isFeatureEnabled('Grid Ruler Markers'), - selectedGridItems.map((path) => { - return ( - - ) - }), - )} ) } +export const GridRulersControlsComponent = React.memo(() => { + const selectedGridItems = useSelectedGridItems() + + if (!isFeatureEnabled('Grid Ruler Markers')) { + return null + } + return ( +
+ + {selectedGridItems.map((path) => { + return + })} + +
+ ) +}) +GridRulersControlsComponent.displayName = 'GridRulersControlsComponent' + const MIN_INDICATORS_DISTANCE = 32 // px const AbsoluteDistanceIndicators = React.memo( @@ -2195,7 +2203,6 @@ type RulerMarkerPositionData = { } interface RulerMarkersProps { - setShowGridCellOutlines: (show: boolean) => void path: ElementPath } @@ -2411,17 +2418,19 @@ const RulerMarkers = React.memo((props: RulerMarkersProps) => { [canvasOffsetRef, dispatch, canvasScale], ) + const [, setForceShowGridPlaceholders] = useAtom(forceShowGridPlaceholdersAtom) + const markerMouseUp = React.useCallback( (event: MouseEvent) => { event.preventDefault() event.stopPropagation() setShowExtraMarkers(null) setFrozenMarkers(null) - props.setShowGridCellOutlines(false) + setForceShowGridPlaceholders('not-visible') window.removeEventListener('mouseup', markerMouseUp) }, - [props], + [setForceShowGridPlaceholders], ) const rowMarkerMouseDown = React.useCallback( @@ -2430,13 +2439,13 @@ const RulerMarkers = React.memo((props: RulerMarkersProps) => { event.stopPropagation() setShowExtraMarkers('row') - props.setShowGridCellOutlines(true) + setForceShowGridPlaceholders('all') setFrozenMarkers(rulerMarkerData) startResizeInteraction(EP.toUid(props.path), edge)(event) window.addEventListener('mouseup', markerMouseUp) }, - [markerMouseUp, props, startResizeInteraction, rulerMarkerData], + [markerMouseUp, props, startResizeInteraction, rulerMarkerData, setForceShowGridPlaceholders], ) const columnMarkerMouseDown = React.useCallback( @@ -2445,13 +2454,13 @@ const RulerMarkers = React.memo((props: RulerMarkersProps) => { event.stopPropagation() setShowExtraMarkers('column') - props.setShowGridCellOutlines(true) + setForceShowGridPlaceholders('all') setFrozenMarkers(rulerMarkerData) startResizeInteraction(EP.toUid(props.path), edge)(event) window.addEventListener('mouseup', markerMouseUp) }, - [markerMouseUp, props, startResizeInteraction, rulerMarkerData], + [markerMouseUp, props, startResizeInteraction, rulerMarkerData, setForceShowGridPlaceholders], ) if (rulerMarkerData == null || gridRect == null) { @@ -2647,23 +2656,41 @@ const SnapLine = React.memo( }) => { const colorTheme = useColorTheme() - const [targetMarker, targetFrozenMarker] = React.useMemo(() => { + const targetMarker = React.useMemo(() => { + if (props.edge == null) { + return null + } + switch (props.edge) { + case 'column-end': + return props.markers.columnEnd + case 'column-start': + return props.markers.columnStart + case 'row-end': + return props.markers.rowEnd + case 'row-start': + return props.markers.rowStart + default: + assertNever(props.edge) + } + }, [props.edge, props.markers]) + + const targetFrozenMarker = React.useMemo(() => { if (props.edge == null || props.frozenMarkers == null) { - return [] + return null } switch (props.edge) { case 'column-end': - return [props.markers.columnEnd, props.frozenMarkers.columnEnd] + return props.frozenMarkers.columnEnd case 'column-start': - return [props.markers.columnStart, props.frozenMarkers.columnStart] + return props.frozenMarkers.columnStart case 'row-end': - return [props.markers.rowEnd, props.frozenMarkers.rowEnd] + return props.frozenMarkers.rowEnd case 'row-start': - return [props.markers.rowStart, props.frozenMarkers.rowStart] + return props.frozenMarkers.rowStart default: assertNever(props.edge) } - }, [props.edge, props.markers, props.frozenMarkers]) + }, [props.edge, props.frozenMarkers]) const axis = props.edge === 'column-end' || props.edge === 'column-start' ? 'column' : 'row' @@ -2672,7 +2699,6 @@ const SnapLine = React.memo( (store) => store.editor.canvas.scale, 'SnapLine canvasScale', ) - if ( props.edge == null || targetMarker == null ||