Skip to content

Commit

Permalink
Rename grid movement strategies (#6621)
Browse files Browse the repository at this point in the history
**Problem:**

The grid "move" strategies should be renamed to better reflect what they
do and to be less confusing.

**Fix:**
This PR is a followup to
#6606

1. Rename "move rearrange" to "change grid element location" (and
duplicate)
2. Rename "move reorder" to "reorder"

Fixes #6620

---------

Co-authored-by: Federico Ruggi <federico.ruggi@shopify.com>
  • Loading branch information
2 people authored and liady committed Dec 13, 2024
1 parent 65d219e commit e3e9c8f
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ import type { InsertionSubject, InsertionSubjectWrapper } from '../../editor/edi
import { generateUidWithExistingComponents } from '../../../core/model/element-template-utils'
import { retargetStrategyToChildrenOfFragmentLikeElements } from './strategies/fragment-like-helpers'
import { MetadataUtils } from '../../../core/model/element-metadata-utils'
import { gridMoveRearrangeStrategy } from './strategies/grid-move-rearrange-strategy'
import { gridChangeElementLocationStrategy } from './strategies/grid-change-element-location-strategy'
import { resizeGridStrategy } from './strategies/resize-grid-strategy'
import { gridResizeElementStrategy } from './strategies/grid-resize-element-strategy'
import { gridMoveRearrangeDuplicateStrategy } from './strategies/grid-move-rearrange-duplicate-strategy'
import { gridChangeElementLocationDuplicateStrategy } from './strategies/grid-change-element-location-duplicate-strategy'
import { setGridGapStrategy } from './strategies/set-grid-gap-strategy'
import type { CanvasCommand } from '../commands/commands'
import { foldAndApplyCommandsInner } from '../commands/commands'
Expand All @@ -79,15 +79,15 @@ import { wrapInContainerCommand } from '../commands/wrap-in-container-command'
import type { ElementPath } from 'utopia-shared/src/types'
import { reparentSubjectsForInteractionTarget } from './strategies/reparent-helpers/reparent-strategy-helpers'
import { getReparentTargetUnified } from './strategies/reparent-helpers/reparent-strategy-parent-lookup'
import { gridRearrangeResizeKeyboardStrategy } from './strategies/grid-rearrange-keyboard-strategy'
import { gridChangeElementLocationResizeKeyboardStrategy } from './strategies/grid-change-element-location-keyboard-strategy'
import createCachedSelector from 're-reselect'
import { getActivePlugin } from '../plugins/style-plugins'
import {
controlsForGridPlaceholders,
GridControls,
isGridControlsProps,
} from '../controls/grid-controls-for-strategies'
import { gridMoveReorderStrategy } from './strategies/grid-move-reorder-strategy'
import { gridReorderStrategy } from './strategies/grid-reorder-strategy'
import { gridMoveAbsoluteStrategy } from './strategies/grid-move-absolute'

export type CanvasStrategyFactory = (
Expand Down Expand Up @@ -116,10 +116,10 @@ const moveOrReorderStrategies: MetaCanvasStrategy = (
convertToAbsoluteAndMoveStrategy,
convertToAbsoluteAndMoveAndSetParentFixedStrategy,
reorderSliderStategy,
gridMoveRearrangeStrategy,
gridMoveRearrangeDuplicateStrategy,
gridMoveReorderStrategy,
gridRearrangeResizeKeyboardStrategy,
gridChangeElementLocationStrategy,
gridChangeElementLocationDuplicateStrategy,
gridReorderStrategy,
gridChangeElementLocationResizeKeyboardStrategy,
gridMoveAbsoluteStrategy,
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import {
getParentGridTemplatesFromChildMeasurements,
gridMoveStrategiesExtraCommands,
} from './grid-helpers'
import { runGridMoveRearrange } from './grid-move-rearrange-strategy'
import { runGridChangeElementLocation } from './grid-change-element-location-strategy'

export const gridMoveRearrangeDuplicateStrategy: CanvasStrategyFactory = (
export const gridChangeElementLocationDuplicateStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
interactionSession: InteractionSession | null,
customState: CustomStrategyState,
Expand Down Expand Up @@ -79,9 +79,9 @@ export const gridMoveRearrangeDuplicateStrategy: CanvasStrategyFactory = (
}

return {
id: 'rearrange-grid-move-duplicate-strategy',
name: 'Grid Rearrange (Duplicate)',
descriptiveLabel: 'Grid Rearrange (Duplicate)',
id: 'grid-change-element-location-duplicate-strategy',
name: 'Change Location (Duplicate)',
descriptiveLabel: 'Change Location (Duplicate)',
icon: {
category: 'tools',
type: 'pointer',
Expand Down Expand Up @@ -115,7 +115,7 @@ export const gridMoveRearrangeDuplicateStrategy: CanvasStrategyFactory = (
return emptyStrategyApplicationResult
}

const moveCommands = runGridMoveRearrange(
const moveCommands = runGridChangeElementLocation(
canvasState.startingMetadata,
interactionSession.interactionData,
selectedElementMetadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { findOriginalGrid, setGridPropsCommands } from './grid-helpers'
import { getGridChildCellCoordBoundsFromCanvas } from './grid-cell-bounds'
import { accumulatePresses } from './shared-keyboard-strategy-helpers'

export function gridRearrangeResizeKeyboardStrategy(
export function gridChangeElementLocationResizeKeyboardStrategy(
canvasState: InteractionCanvasState,
interactionSession: InteractionSession | null,
): CanvasStrategy | null {
Expand Down Expand Up @@ -62,10 +62,10 @@ export function gridRearrangeResizeKeyboardStrategy(
interactionSession.interactionData.keyStates.length - 1,
)?.modifiers.shift ?? false

const label = resizing ? 'Grid resize' : 'Grid rearrange'
const label = resizing ? 'Grid resize' : 'Change Location'

return {
id: 'GRID_KEYBOARD_REARRANGE_RESIZE',
id: 'GRID_KEYBOARD_CHANGE_ELEMENT_LOCATION_RESIZE',
name: label,
descriptiveLabel: label,
icon: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
} from './grid-helpers'
import { getTargetGridCellData } from '../../../inspector/grid-helpers'

export const gridMoveRearrangeStrategy: CanvasStrategyFactory = (
export const gridChangeElementLocationStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
interactionSession: InteractionSession | null,
) => {
Expand Down Expand Up @@ -89,9 +89,9 @@ export const gridMoveRearrangeStrategy: CanvasStrategyFactory = (
}

return {
id: 'rearrange-grid-move-strategy',
name: 'Grid rearrange',
descriptiveLabel: 'Grid rearrange',
id: 'grid-change-element-location-strategy',
name: 'Change Location',
descriptiveLabel: 'Change Location',
icon: {
category: 'tools',
type: 'pointer',
Expand All @@ -108,7 +108,7 @@ export const gridMoveRearrangeStrategy: CanvasStrategyFactory = (
return emptyStrategyApplicationResult
}

const { commands, elementsToRerender } = getCommandsAndPatchForGridRearrange(
const { commands, elementsToRerender } = getCommandsAndPatchForGridChangeElementLocation(
canvasState,
interactionSession.interactionData,
selectedElement,
Expand All @@ -122,6 +122,7 @@ export const gridMoveRearrangeStrategy: CanvasStrategyFactory = (
parentGridPath,
initialTemplates,
)

return strategyApplicationResult(
[...midInteractionCommands, ...onCompleteCommands, ...commands],
elementsToRerender,
Expand All @@ -130,7 +131,7 @@ export const gridMoveRearrangeStrategy: CanvasStrategyFactory = (
}
}

function getCommandsAndPatchForGridRearrange(
function getCommandsAndPatchForGridChangeElementLocation(
canvasState: InteractionCanvasState,
interactionData: DragInteractionData,
selectedElement: ElementPath,
Expand All @@ -157,7 +158,7 @@ function getCommandsAndPatchForGridRearrange(
return { commands: [], elementsToRerender: [] }
}

const commands = runGridMoveRearrange(
const commands = runGridChangeElementLocation(
canvasState.startingMetadata,
interactionData,
selectedElementMetadata,
Expand All @@ -173,7 +174,7 @@ function getCommandsAndPatchForGridRearrange(
}
}

export function runGridMoveRearrange(
export function runGridChangeElementLocation(
jsxMetadata: ElementInstanceMetadataMap,
interactionData: DragInteractionData,
selectedElementMetadata: ElementInstanceMetadata,
Expand Down Expand Up @@ -251,7 +252,7 @@ export function runGridMoveRearrange(
})
.sort(sortElementsByGridPosition(templateColumnsCount))

const indexInSortedCellsForRearrange = cellsSortedByPosition.findIndex((s) =>
const indexInSortedCellsForChangeLocation = cellsSortedByPosition.findIndex((s) =>
EP.pathsEqual(selectedElementMetadata.elementPath, s.path),
)

Expand All @@ -267,7 +268,7 @@ export function runGridMoveRearrange(
reorderElement(
'always',
pathForCommands,
absolute(Math.max(indexInSortedCellsForRearrange, 0)),
absolute(Math.max(indexInSortedCellsForChangeLocation, 0)),
),
updateGridControlsCommand,
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import { renderTestEditorWithCode } from '../../ui-jsx.test-utils'
import type { GridCellCoordinates } from './grid-cell-bounds'
import { gridCellTargetId } from './grid-cell-bounds'

describe('grid rearrange move strategy', () => {
it('can rearrange elements on a grid', async () => {
describe('grid element change location strategy', () => {
it('can change the location of elements on a grid', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')

const testId = 'aaa'
Expand All @@ -41,7 +41,7 @@ describe('grid rearrange move strategy', () => {
})
})

it('can rearrange elements in a grid component', async () => {
it('can change the location of elements in a grid component', async () => {
const editor = await renderTestEditorWithCode(
ProjectCodeGridComponent,
'await-first-dom-report',
Expand All @@ -61,7 +61,7 @@ describe('grid rearrange move strategy', () => {
})
})

it('can not rearrange multicell element out of the grid', async () => {
it('can not change location of a multicell element out of the grid', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')

const testId = 'aaa'
Expand All @@ -81,7 +81,7 @@ describe('grid rearrange move strategy', () => {
})
})

it('can rearrange element with no explicit grid props set', async () => {
it('can change the location of element with no explicit grid props set', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')

const testId = 'bbb'
Expand All @@ -100,7 +100,7 @@ describe('grid rearrange move strategy', () => {
})
})

it('can rearrange elements on a grid (zoom out)', async () => {
it('can change the location of elements on a grid (zoom out)', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')

const testId = 'aaa'
Expand All @@ -117,7 +117,7 @@ describe('grid rearrange move strategy', () => {
})
})

it('can rearrange elements on a grid (zoom in)', async () => {
it('can change the location of elements on a grid (zoom in)', async () => {
const editor = await renderTestEditorWithCode(ProjectCode, 'await-first-dom-report')

const testId = 'aaa'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
} from '../../../../core/shared/element-template'
import type { CanvasRectangle } from '../../../../core/shared/math-utils'
import {
canvasPoint,
canvasRectangle,
isInfinityRectangle,
offsetPoint,
Expand Down Expand Up @@ -39,7 +38,7 @@ import {
getParentGridTemplatesFromChildMeasurements,
gridMoveStrategiesExtraCommands,
} from './grid-helpers'
import { runGridMoveRearrange } from './grid-move-rearrange-strategy'
import { runGridChangeElementLocation } from './grid-change-element-location-strategy'
import { getTargetGridCellData } from '../../../inspector/grid-helpers'

export const gridMoveAbsoluteStrategy: CanvasStrategyFactory = (
Expand Down Expand Up @@ -226,9 +225,9 @@ function runGridMoveAbsolute(
]
}

// otherwise, return a rearrange move + absolute adjustment
// otherwise, return a change location + absolute adjustment
return [
...runGridMoveRearrange(
...runGridChangeElementLocation(
jsxMetadata,
interactionData,
selectedElementMetadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
} from './grid-helpers'
import { getTargetGridCellData } from '../../../inspector/grid-helpers'

export const gridMoveReorderStrategy: CanvasStrategyFactory = (
export const gridReorderStrategy: CanvasStrategyFactory = (
canvasState: InteractionCanvasState,
interactionSession: InteractionSession | null,
) => {
Expand Down Expand Up @@ -171,7 +171,7 @@ function getCommandsAndPatchForGridReorder(
return { commands: [], elementsToRerender: [] }
}

const commands = runGridMoveReorder(
const commands = runGridReorder(
canvasState.startingMetadata,
interactionData,
selectedElementMetadata,
Expand All @@ -186,7 +186,7 @@ function getCommandsAndPatchForGridReorder(
}
}

function runGridMoveReorder(
function runGridReorder(
jsxMetadata: ElementInstanceMetadataMap,
interactionData: DragInteractionData,
selectedElementMetadata: ElementInstanceMetadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
import type { DragInteractionData, InteractionSession, UpdatedPathMap } from '../interaction-state'
import { honoursPropsPosition, shouldKeepMovingDraggedGroupChildren } from './absolute-utils'
import { replaceFragmentLikePathsWithTheirChildrenRecursive } from './fragment-like-helpers'
import { runGridMoveRearrange } from './grid-move-rearrange-strategy'
import { runGridChangeElementLocation } from './grid-change-element-location-strategy'
import { ifAllowedToReparent, isAllowedToReparent } from './reparent-helpers/reparent-helpers'
import { removeAbsolutePositioningProps } from './reparent-helpers/reparent-property-changes'
import type { ReparentTarget } from './reparent-helpers/reparent-strategy-helpers'
Expand Down Expand Up @@ -283,7 +283,7 @@ function gridReparentCommands(
return null
}

const gridPropsCommands = runGridMoveRearrange(
const gridPropsCommands = runGridChangeElementLocation(
jsxMetadata,
interactionData,
targetElement,
Expand Down
2 changes: 1 addition & 1 deletion editor/src/components/canvas/controls/grid-controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ const GridControl = React.memo<GridControlProps>(({ grid, controlsVisible }) =>
countedColumn === currentHoveredCell?.column &&
countedRow === currentHoveredCell?.row

const activePositioningTarget = isActiveCell && !dontShowActiveCellHighlight // TODO: move the logic into runGridRearrangeMove and do not set targetCell prop in these cases
const activePositioningTarget = isActiveCell && !dontShowActiveCellHighlight // TODO: move the logic into runGridChangeElementLocation and do not set targetCell prop in these cases

const borderColor = activePositioningTarget
? colorTheme.brandNeonPink.value
Expand Down

0 comments on commit e3e9c8f

Please sign in to comment.