From 64aa3e7df40c35fd2a92c4a9e2edf4172196189d Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Wed, 26 Apr 2023 00:22:10 +0200 Subject: [PATCH] feat: Add 'reason' field to move event (#6996) * feat: Add 'reason' field to move event There are many types of move. This addition allows one to detect what the reason for each move is. * Clang formatting. And unsaved editor tabs. * Change reason string to array. --- core/block.ts | 6 +++-- core/block_dragger.ts | 1 + core/block_svg.ts | 13 ++++++---- core/bump_objects.ts | 2 +- core/connection.ts | 2 ++ core/events/events_block_move.ts | 37 ++++++++++++++++++++++++++-- core/events/utils.ts | 10 ++++++++ core/flyout_base.ts | 3 ++- core/interfaces/i_bounded_element.ts | 3 ++- core/rendered_connection.ts | 2 +- core/workspace_svg.ts | 3 ++- core/xml.ts | 5 ++-- 12 files changed, 71 insertions(+), 16 deletions(-) diff --git a/core/block.ts b/core/block.ts index c3da45136..ada8ef9ea 100644 --- a/core/block.ts +++ b/core/block.ts @@ -2130,13 +2130,15 @@ export class Block implements IASTNodeLocation, IDeletable { * * @param dx Horizontal offset, in workspace units. * @param dy Vertical offset, in workspace units. + * @param reason Why is this move happening? 'drag', 'bump', 'snap', ... */ - moveBy(dx: number, dy: number) { + moveBy(dx: number, dy: number, reason?: string[]) { if (this.parentBlock_) { - throw Error('Block has parent.'); + throw Error('Block has parent'); } const event = new (eventUtils.get(eventUtils.BLOCK_MOVE))(this) as BlockMove; + reason && event.setReason(reason); this.xy_.translate(dx, dy); event.recordNew(); eventUtils.fire(event); diff --git a/core/block_dragger.ts b/core/block_dragger.ts index 0a2cb0f33..f0200870d 100644 --- a/core/block_dragger.ts +++ b/core/block_dragger.ts @@ -349,6 +349,7 @@ export class BlockDragger implements IBlockDragger { if (this.draggingBlock_.isDeadOrDying()) return; const event = new (eventUtils.get(eventUtils.BLOCK_MOVE))( this.draggingBlock_) as BlockMove; + event.setReason(['drag']); event.oldCoordinate = this.startXY_; event.recordNew(); eventUtils.fire(event); diff --git a/core/block_svg.ts b/core/block_svg.ts index 504bff63f..5939ce44e 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -392,15 +392,17 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, * * @param dx Horizontal offset in workspace units. * @param dy Vertical offset in workspace units. + * @param reason Why is this move happening? 'drag', 'bump', 'snap', ... */ - override moveBy(dx: number, dy: number) { + override moveBy(dx: number, dy: number, reason?: string[]) { if (this.parentBlock_) { - throw Error('Block has parent.'); + throw Error('Block has parent'); } const eventsEnabled = eventUtils.isEnabled(); let event: BlockMove|null = null; if (eventsEnabled) { event = new (eventUtils.get(eventUtils.BLOCK_MOVE))!(this) as BlockMove; + reason && event.setReason(reason); } const xy = this.getRelativeToSurfaceXY(); this.translate(xy.x + dx, xy.y + dy); @@ -463,10 +465,11 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, * Move a block to a position. * * @param xy The position to move to in workspace units. + * @param reason Why is this move happening? 'drag', 'bump', 'snap', ... */ - moveTo(xy: Coordinate) { + moveTo(xy: Coordinate, reason?: string[]) { const curXY = this.getRelativeToSurfaceXY(); - this.moveBy(xy.x - curXY.x, xy.y - curXY.y); + this.moveBy(xy.x - curXY.x, xy.y - curXY.y, reason); } /** @@ -541,7 +544,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, const dy = Math.round(Math.round((xy.y - half) / spacing) * spacing + half - xy.y); if (dx || dy) { - this.moveBy(dx, dy); + this.moveBy(dx, dy, ['snap']); } } diff --git a/core/bump_objects.ts b/core/bump_objects.ts index 00b5e3f11..cf17e2777 100644 --- a/core/bump_objects.ts +++ b/core/bump_objects.ts @@ -71,7 +71,7 @@ function bumpObjectIntoBounds( const deltaX = newXPosition - objectMetrics.left; if (deltaX || deltaY) { - object.moveBy(deltaX, deltaY); + object.moveBy(deltaX, deltaY, ['inbounds']); return true; } return false; diff --git a/core/connection.ts b/core/connection.ts index c34c0abbc..bc7a5931e 100644 --- a/core/connection.ts +++ b/core/connection.ts @@ -115,6 +115,7 @@ export class Connection implements IASTNodeLocationWithBlock { if (eventUtils.isEnabled()) { event = new (eventUtils.get(eventUtils.BLOCK_MOVE))(childBlock) as BlockMove; + event.setReason(['connect']); } connectReciprocally(this, childConnection); childBlock.setParent(parentBlock); @@ -272,6 +273,7 @@ export class Connection implements IASTNodeLocationWithBlock { if (eventUtils.isEnabled()) { event = new (eventUtils.get(eventUtils.BLOCK_MOVE))( childConnection.getSourceBlock()) as BlockMove; + event.setReason(['disconnect']); } const otherConnection = this.targetConnection; if (otherConnection) { diff --git a/core/events/events_block_move.ts b/core/events/events_block_move.ts index e886a06ac..27f71570a 100644 --- a/core/events/events_block_move.ts +++ b/core/events/events_block_move.ts @@ -61,11 +61,25 @@ export class BlockMove extends BlockBase { newInputName?: string; /** - * The new X and Y workspace coordinates of the block if it is a top level + * The new X and Y workspace coordinates of the block if it is a top-level * block. Undefined if it is not a top level block. */ newCoordinate?: Coordinate; + /** + * An explanation of what this move is for. Known values include: + * 'drag' -- A drag operation completed. + * 'bump' -- Block got bumped away from an invalid connection. + * 'snap' -- Block got shifted to line up with the grid. + * 'inbounds' -- Block got pushed back into a non-scrolling workspace. + * 'connect' -- Block got connected to another block. + * 'disconnect' -- Block got disconnected from another block. + * 'create' -- Block created via XML. + * 'cleanup' -- Workspace aligned top-level blocks. + * Event merging may create multiple reasons: ['drag', 'bump', 'snap']. + */ + reason?: string[]; + /** @param opt_block The moved block. Undefined for a blank event. */ constructor(opt_block?: Block) { super(opt_block); @@ -104,6 +118,9 @@ export class BlockMove extends BlockBase { json['newCoordinate'] = `${Math.round(this.newCoordinate.x)}, ` + `${Math.round(this.newCoordinate.y)}`; } + if (this.reason) { + json['reason'] = this.reason; + } if (!this.recordUndo) { json['recordUndo'] = this.recordUndo; } @@ -132,6 +149,9 @@ export class BlockMove extends BlockBase { const xy = json['newCoordinate'].split(','); this.newCoordinate = new Coordinate(Number(xy[0]), Number(xy[1])); } + if (json['reason'] !== undefined) { + this.reason = json['reason']; + } if (json['recordUndo'] !== undefined) { this.recordUndo = json['recordUndo']; } @@ -162,6 +182,9 @@ export class BlockMove extends BlockBase { const xy = json['newCoordinate'].split(','); newEvent.newCoordinate = new Coordinate(Number(xy[0]), Number(xy[1])); } + if (json['reason'] !== undefined) { + newEvent.reason = json['reason']; + } if (json['recordUndo'] !== undefined) { newEvent.recordUndo = json['recordUndo']; } @@ -176,6 +199,15 @@ export class BlockMove extends BlockBase { this.newCoordinate = location.coordinate; } + /** + * Set the reason for a move event. + * + * @param reason Why is this move happening? 'drag', 'bump', 'snap', ... + */ + setReason(reason: string[]) { + this.reason = reason; + } + /** * Returns the parentId and input if the block is connected, * or the XY location if disconnected. @@ -253,7 +285,7 @@ export class BlockMove extends BlockBase { } if (coordinate) { const xy = block.getRelativeToSurfaceXY(); - block.moveBy(coordinate.x - xy.x, coordinate.y - xy.y); + block.moveBy(coordinate.x - xy.x, coordinate.y - xy.y, this.reason); } else { let blockConnection = block.outputConnection; if (!blockConnection || @@ -286,6 +318,7 @@ export interface BlockMoveJson extends BlockBaseJson { newParentId?: string; newInputName?: string; newCoordinate?: string; + reason?: string[]; recordUndo?: boolean; } diff --git a/core/events/utils.ts b/core/events/utils.ts index e8505f38d..573f241e2 100644 --- a/core/events/utils.ts +++ b/core/events/utils.ts @@ -290,6 +290,16 @@ export function filter(queueIn: Abstract[], forward: boolean): Abstract[] { lastEvent.newParentId = moveEvent.newParentId; lastEvent.newInputName = moveEvent.newInputName; lastEvent.newCoordinate = moveEvent.newCoordinate; + if (moveEvent.reason) { + if (lastEvent.reason) { + // Concatenate reasons without duplicates. + const reasonSet = + new Set(moveEvent.reason.concat(lastEvent.reason)); + lastEvent.reason = Array.from(reasonSet); + } else { + lastEvent.reason = moveEvent.reason; + } + } lastEntry.index = i; } else if ( event.type === CHANGE && diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 1c976a3f3..a4ae4d707 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -1121,7 +1121,7 @@ export abstract class Flyout extends DeleteArea implements IFlyout { const targetWorkspace = this.targetWorkspace; const svgRootOld = oldBlock.getSvgRoot(); if (!svgRootOld) { - throw Error('oldBlock is not rendered.'); + throw Error('oldBlock is not rendered'); } // Clone the block. @@ -1170,6 +1170,7 @@ export abstract class Flyout extends DeleteArea implements IFlyout { // The position of the old block in main workspace coordinates. finalOffset.scale(1 / targetWorkspace.scale); + // No 'reason' provided since events are disabled. block.moveTo(new Coordinate(finalOffset.x, finalOffset.y)); } } diff --git a/core/interfaces/i_bounded_element.ts b/core/interfaces/i_bounded_element.ts index 870d7b1e8..f6535b5bf 100644 --- a/core/interfaces/i_bounded_element.ts +++ b/core/interfaces/i_bounded_element.ts @@ -25,6 +25,7 @@ export interface IBoundedElement { * * @param dx Horizontal offset in workspace units. * @param dy Vertical offset in workspace units. + * @param reason Why is this move happening? 'user', 'bump', 'snap'... */ - moveBy(dx: number, dy: number): void; + moveBy(dx: number, dy: number, reason?: string[]): void; } diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 0e46963a9..7ac0e8562 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -174,7 +174,7 @@ export class RenderedConnection extends Connection { dx = staticConnection.x - config.snapRadius - Math.floor(Math.random() * BUMP_RANDOMNESS) - this.x; } - rootBlock.moveBy(dx, dy); + rootBlock.moveBy(dx, dy, ['bump']); selected || rootBlock.removeSelect(); } diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 735573e31..b2d05437b 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -1454,6 +1454,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { blockY += config.snapRadius * 2; } } while (collide); + // No 'reason' provided since events are disabled. block!.moveTo(new Coordinate(blockX, blockY)); } } finally { @@ -1818,7 +1819,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { continue; } const xy = block.getRelativeToSurfaceXY(); - block.moveBy(-xy.x, cursorY - xy.y); + block.moveBy(-xy.x, cursorY - xy.y, ['cleanup']); block.snapToGrid(); cursorY = block.getRelativeToSurfaceXY().y + block.getHeightWidth().height + diff --git a/core/xml.ts b/core/xml.ts index ffc2f2bfc..c947896b7 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -433,7 +433,8 @@ export function domToWorkspace(xml: Element, workspace: Workspace): string[] { const blockX = parseInt(xmlChildElement.getAttribute('x') ?? '10', 10); const blockY = parseInt(xmlChildElement.getAttribute('y') ?? '10', 10); if (!isNaN(blockX) && !isNaN(blockY)) { - block.moveBy(workspace.RTL ? width - blockX : blockX, blockY); + block.moveBy( + workspace.RTL ? width - blockX : blockX, blockY, ['create']); } variablesFirst = false; } else if (name === 'shadow') { @@ -516,7 +517,7 @@ export function appendDomToWorkspace( offsetX = workspace.RTL ? topX - newRightX : topX - newLeftX; for (let i = 0; i < newBlockIds.length; i++) { const block = workspace.getBlockById(newBlockIds[i]); - block!.moveBy(offsetX, offsetY); + block!.moveBy(offsetX, offsetY, ['create']); } } return newBlockIds;