From 68a8a5ff194ae2cc23ceadb06c74a76d76fbdec4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 31 Jul 2023 09:22:24 -0700 Subject: [PATCH 1/6] feat: add basic pasters (#7331) * feat: implement basic IPaster interface * feat: add pasters for blocks and workspace comments --- core/clipboard/block_paster.ts | 31 ++++++++++++++++++++++ core/clipboard/workspace_comment_paster.ts | 30 +++++++++++++++++++++ core/interfaces/i_paster.ts | 23 ++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 core/clipboard/block_paster.ts create mode 100644 core/clipboard/workspace_comment_paster.ts create mode 100644 core/interfaces/i_paster.ts diff --git a/core/clipboard/block_paster.ts b/core/clipboard/block_paster.ts new file mode 100644 index 000000000..363f70cad --- /dev/null +++ b/core/clipboard/block_paster.ts @@ -0,0 +1,31 @@ +/** + * @license + * Copyright 2023 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {BlockSvg} from '../block_svg.js'; +import {CopyData} from '../interfaces/i_copyable'; +import {IPaster} from '../interfaces/i_paster.js'; +import {State, append} from '../serialization/blocks'; +import {Coordinate} from '../utils/coordinate.js'; +import {WorkspaceSvg} from '../workspace_svg.js'; + +export class BlockPaster implements IPaster { + paste( + copyData: BlockCopyData, + workspace: WorkspaceSvg, + coordinate?: Coordinate, + ): BlockSvg | null { + if (!workspace.isCapacityAvailable(copyData.typeCounts!)) return null; + + const state = copyData.saveInfo as State; + if (coordinate) { + state['x'] = coordinate.x; + state['y'] = coordinate.y; + } + return append(state, workspace, {recordUndo: true}) as BlockSvg; + } +} + +export interface BlockCopyData extends CopyData {} diff --git a/core/clipboard/workspace_comment_paster.ts b/core/clipboard/workspace_comment_paster.ts new file mode 100644 index 000000000..11211564c --- /dev/null +++ b/core/clipboard/workspace_comment_paster.ts @@ -0,0 +1,30 @@ +/** + * @license + * Copyright 2023 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {IPaster} from '../interfaces/i_paster.js'; +import {CopyData} from '../interfaces/i_copyable.js'; +import {Coordinate} from '../utils/coordinate.js'; +import {WorkspaceSvg} from '../workspace_svg.js'; +import {WorkspaceCommentSvg} from '../workspace_comment_svg.js'; + +export class WorkspaceCommentPaster + implements IPaster +{ + paste( + copyData: WorkspaceCommentCopyData, + workspace: WorkspaceSvg, + coordinate?: Coordinate, + ): WorkspaceCommentSvg { + const state = copyData.saveInfo as Element; + if (coordinate) { + state.setAttribute('x', `${coordinate.x}`); + state.setAttribute('y', `${coordinate.y}`); + } + return WorkspaceCommentSvg.fromXmlRendered(state, workspace); + } +} + +export interface WorkspaceCommentCopyData extends CopyData {} diff --git a/core/interfaces/i_paster.ts b/core/interfaces/i_paster.ts new file mode 100644 index 000000000..9653d25df --- /dev/null +++ b/core/interfaces/i_paster.ts @@ -0,0 +1,23 @@ +/** + * @license + * Copyright 2023 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {Coordinate} from '../utils/coordinate.js'; +import {WorkspaceSvg} from '../workspace_svg.js'; +import {ICopyable, CopyData} from './i_copyable.js'; + +/** An object that can paste data into a workspace. */ +export interface IPaster { + paste( + copyData: U, + workspace: WorkspaceSvg, + coordinate?: Coordinate, + ): T | null; +} + +/** @returns True if the given object is a paster. */ +export function isPaster(obj: any): obj is IPaster { + return obj.paste !== undefined; +} From 74c01d27941a0a56ba4b733beb77d142fb0db943 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 31 Jul 2023 10:46:49 -0700 Subject: [PATCH 2/6] chore: fix paster exports and registration (#7343) --- core/blockly.ts | 2 ++ core/clipboard.ts | 4 +++ core/clipboard/block_paster.ts | 9 +++++-- core/clipboard/registry.ts | 31 ++++++++++++++++++++++ core/clipboard/workspace_comment_paster.ts | 5 ++++ core/registry.ts | 5 ++++ 6 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 core/clipboard/registry.ts diff --git a/core/blockly.ts b/core/blockly.ts index c14f3452b..9e8599ed9 100644 --- a/core/blockly.ts +++ b/core/blockly.ts @@ -152,6 +152,7 @@ import {IKeyboardAccessible} from './interfaces/i_keyboard_accessible.js'; import {IMetricsManager} from './interfaces/i_metrics_manager.js'; import {IMovable} from './interfaces/i_movable.js'; import {IObservable, isObservable} from './interfaces/i_observable.js'; +import {IPaster, isPaster} from './interfaces/i_paster.js'; import {IPositionable} from './interfaces/i_positionable.js'; import {IRegistrable} from './interfaces/i_registrable.js'; import {ISelectable} from './interfaces/i_selectable.js'; @@ -606,6 +607,7 @@ export {Input}; export {inputs}; export {InsertionMarkerManager}; export {IObservable, isObservable}; +export {IPaster, isPaster}; export {IPositionable}; export {IRegistrable}; export {ISelectable}; diff --git a/core/clipboard.ts b/core/clipboard.ts index 662a1c973..9cad96751 100644 --- a/core/clipboard.ts +++ b/core/clipboard.ts @@ -8,6 +8,8 @@ import * as goog from '../closure/goog/goog.js'; goog.declareModuleId('Blockly.clipboard'); import type {CopyData, ICopyable} from './interfaces/i_copyable.js'; +import {BlockPaster} from './clipboard/block_paster.js'; +import * as registry from './clipboard/registry.js'; /** Metadata about the object that is currently on the clipboard. */ let copyData: CopyData | null = null; @@ -82,3 +84,5 @@ export const TEST_ONLY = { duplicateInternal, copyInternal, }; + +export {BlockPaster, registry}; diff --git a/core/clipboard/block_paster.ts b/core/clipboard/block_paster.ts index 363f70cad..60ca336a5 100644 --- a/core/clipboard/block_paster.ts +++ b/core/clipboard/block_paster.ts @@ -5,13 +5,16 @@ */ import {BlockSvg} from '../block_svg.js'; -import {CopyData} from '../interfaces/i_copyable'; +import {registry} from '../clipboard.js'; +import {CopyData} from '../interfaces/i_copyable.js'; import {IPaster} from '../interfaces/i_paster.js'; -import {State, append} from '../serialization/blocks'; +import {State, append} from '../serialization/blocks.js'; import {Coordinate} from '../utils/coordinate.js'; import {WorkspaceSvg} from '../workspace_svg.js'; export class BlockPaster implements IPaster { + static TYPE = 'block'; + paste( copyData: BlockCopyData, workspace: WorkspaceSvg, @@ -29,3 +32,5 @@ export class BlockPaster implements IPaster { } export interface BlockCopyData extends CopyData {} + +registry.register(BlockPaster.TYPE, new BlockPaster()); diff --git a/core/clipboard/registry.ts b/core/clipboard/registry.ts new file mode 100644 index 000000000..5a8c8342e --- /dev/null +++ b/core/clipboard/registry.ts @@ -0,0 +1,31 @@ +/** + * @license + * Copyright 2023 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {ICopyable, CopyData} from '../interfaces/i_copyable.js'; +import type {IPaster} from '../interfaces/i_paster.js'; +import * as registry from '../registry.js'; + +/** + * Registers the given paster so that it cna be used for pasting. + * + * @param type The type of the paster to register, e.g. 'block', 'comment', etc. + * @param paster The paster to register. + */ +export function register( + type: string, + paster: IPaster, +) { + registry.register(registry.Type.PASTER, type, paster); +} + +/** + * Unregisters the paster associated with the given type. + * + * @param type The type of the paster to unregister. + */ +export function unregister(type: string) { + registry.unregister(registry.Type.PASTER, type); +} diff --git a/core/clipboard/workspace_comment_paster.ts b/core/clipboard/workspace_comment_paster.ts index 11211564c..7959802d6 100644 --- a/core/clipboard/workspace_comment_paster.ts +++ b/core/clipboard/workspace_comment_paster.ts @@ -9,10 +9,13 @@ import {CopyData} from '../interfaces/i_copyable.js'; import {Coordinate} from '../utils/coordinate.js'; import {WorkspaceSvg} from '../workspace_svg.js'; import {WorkspaceCommentSvg} from '../workspace_comment_svg.js'; +import {registry} from '../clipboard.js'; export class WorkspaceCommentPaster implements IPaster { + static TYPE = 'workspace-comment'; + paste( copyData: WorkspaceCommentCopyData, workspace: WorkspaceSvg, @@ -28,3 +31,5 @@ export class WorkspaceCommentPaster } export interface WorkspaceCommentCopyData extends CopyData {} + +registry.register(WorkspaceCommentPaster.TYPE, new WorkspaceCommentPaster()); diff --git a/core/registry.ts b/core/registry.ts index a20227000..2893f3d0f 100644 --- a/core/registry.ts +++ b/core/registry.ts @@ -22,6 +22,8 @@ import type {Options} from './options.js'; import type {Renderer} from './renderers/common/renderer.js'; import type {Theme} from './theme.js'; import type {ToolboxItem} from './toolbox/toolbox_item.js'; +import {IPaster} from './interfaces/i_paster.js'; +import {CopyData, ICopyable} from './interfaces/i_copyable.js'; /** * A map of maps. With the keys being the type and name of the class we are @@ -96,6 +98,9 @@ export class Type<_T> { /** @internal */ static ICON = new Type('icon'); + + /** @internal */ + static PASTER = new Type>('paster'); } /** From ce1678e8a757b02299a305c3729b5c406dd1a498 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Tue, 1 Aug 2023 11:51:21 -0700 Subject: [PATCH 3/6] fix: refactor CopyData interface to have the correct structure (#7344) * chore: rename CopyData to ICopyData * fix: ICopyable data structures * fix: switch clipboard over to use new copy data interfaces * chore: rename saveInfo to somthing more descriptive --- core/block_svg.ts | 10 +++---- core/clipboard.ts | 31 +++++++++++++--------- core/clipboard/block_paster.ts | 18 ++++++++----- core/clipboard/registry.ts | 4 +-- core/clipboard/workspace_comment_paster.ts | 18 ++++++++----- core/interfaces/i_copyable.ts | 12 +++------ core/interfaces/i_paster.ts | 6 ++--- core/registry.ts | 4 +-- core/workspace_comment_svg.ts | 14 +++++----- 9 files changed, 65 insertions(+), 52 deletions(-) diff --git a/core/block_svg.ts b/core/block_svg.ts index e4053de0a..ff251b2ef 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -37,7 +37,7 @@ import {FieldLabel} from './field_label.js'; import type {Input} from './inputs/input.js'; import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js'; import type {IBoundedElement} from './interfaces/i_bounded_element.js'; -import type {CopyData, ICopyable} from './interfaces/i_copyable.js'; +import type {ICopyable} from './interfaces/i_copyable.js'; import type {IDraggable} from './interfaces/i_draggable.js'; import {IIcon} from './interfaces/i_icon.js'; import * as internalConstants from './internal_constants.js'; @@ -62,6 +62,7 @@ import type {WorkspaceSvg} from './workspace_svg.js'; import * as renderManagement from './render_management.js'; import * as deprecation from './utils/deprecation.js'; import {IconType} from './icons/icon_types.js'; +import {BlockCopyData, BlockPaster} from './clipboard/block_paster.js'; /** * Class for a block's SVG representation. @@ -823,18 +824,17 @@ export class BlockSvg * Encode a block for copying. * * @returns Copy metadata, or null if the block is an insertion marker. - * @internal */ - toCopyData(): CopyData | null { + toCopyData(): BlockCopyData | null { if (this.isInsertionMarker_) { return null; } return { - saveInfo: blocks.save(this, { + paster: BlockPaster.TYPE, + blockState: blocks.save(this, { addCoordinates: true, addNextBlocks: false, }) as blocks.State, - source: this.workspace, typeCounts: common.getBlockTypeCounts(this, true), }; } diff --git a/core/clipboard.ts b/core/clipboard.ts index 9cad96751..627d356bc 100644 --- a/core/clipboard.ts +++ b/core/clipboard.ts @@ -7,12 +7,16 @@ import * as goog from '../closure/goog/goog.js'; goog.declareModuleId('Blockly.clipboard'); -import type {CopyData, ICopyable} from './interfaces/i_copyable.js'; +import type {ICopyData, ICopyable} from './interfaces/i_copyable.js'; import {BlockPaster} from './clipboard/block_paster.js'; +import * as globalRegistry from './registry.js'; +import {WorkspaceSvg} from './workspace_svg.js'; import * as registry from './clipboard/registry.js'; /** Metadata about the object that is currently on the clipboard. */ -let copyData: CopyData | null = null; +let copyData: ICopyData | null = null; + +let source: WorkspaceSvg | null = null; /** * Copy a block or workspace comment onto the local clipboard. @@ -29,6 +33,7 @@ export function copy(toCopy: ICopyable) { */ function copyInternal(toCopy: ICopyable) { copyData = toCopy.toCopyData(); + source = (toCopy as any).workspace ?? null; } /** @@ -43,17 +48,16 @@ export function paste(): ICopyable | null { } // Pasting always pastes to the main workspace, even if the copy // started in a flyout workspace. - let workspace = copyData.source; - if (workspace.isFlyout) { + let workspace = source; + if (workspace?.isFlyout) { workspace = workspace.targetWorkspace!; } - if ( - copyData.typeCounts && - workspace.isCapacityAvailable(copyData.typeCounts) - ) { - return workspace.paste(copyData.saveInfo); - } - return null; + if (!workspace) return null; + return ( + globalRegistry + .getObject(globalRegistry.Type.PASTER, copyData.paster, false) + ?.paste(copyData, workspace) ?? null + ); } /** @@ -74,8 +78,11 @@ export function duplicate(toDuplicate: ICopyable): ICopyable | null { function duplicateInternal(toDuplicate: ICopyable): ICopyable | null { const oldCopyData = copyData; copy(toDuplicate); + if (!copyData || !source) return null; const pastedThing = - toDuplicate.toCopyData()?.source?.paste(copyData!.saveInfo) ?? null; + globalRegistry + .getObject(globalRegistry.Type.PASTER, copyData.paster, false) + ?.paste(copyData, source) ?? null; copyData = oldCopyData; return pastedThing; } diff --git a/core/clipboard/block_paster.ts b/core/clipboard/block_paster.ts index 60ca336a5..3ed8383db 100644 --- a/core/clipboard/block_paster.ts +++ b/core/clipboard/block_paster.ts @@ -5,8 +5,8 @@ */ import {BlockSvg} from '../block_svg.js'; -import {registry} from '../clipboard.js'; -import {CopyData} from '../interfaces/i_copyable.js'; +import * as registry from './registry.js'; +import {ICopyData} from '../interfaces/i_copyable.js'; import {IPaster} from '../interfaces/i_paster.js'; import {State, append} from '../serialization/blocks.js'; import {Coordinate} from '../utils/coordinate.js'; @@ -22,15 +22,19 @@ export class BlockPaster implements IPaster { ): BlockSvg | null { if (!workspace.isCapacityAvailable(copyData.typeCounts!)) return null; - const state = copyData.saveInfo as State; if (coordinate) { - state['x'] = coordinate.x; - state['y'] = coordinate.y; + copyData.blockState['x'] = coordinate.x; + copyData.blockState['y'] = coordinate.y; } - return append(state, workspace, {recordUndo: true}) as BlockSvg; + return append(copyData.blockState, workspace, { + recordUndo: true, + }) as BlockSvg; } } -export interface BlockCopyData extends CopyData {} +export interface BlockCopyData extends ICopyData { + blockState: State; + typeCounts: {[key: string]: number}; +} registry.register(BlockPaster.TYPE, new BlockPaster()); diff --git a/core/clipboard/registry.ts b/core/clipboard/registry.ts index 5a8c8342e..396342341 100644 --- a/core/clipboard/registry.ts +++ b/core/clipboard/registry.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {ICopyable, CopyData} from '../interfaces/i_copyable.js'; +import {ICopyable, ICopyData} from '../interfaces/i_copyable.js'; import type {IPaster} from '../interfaces/i_paster.js'; import * as registry from '../registry.js'; @@ -14,7 +14,7 @@ import * as registry from '../registry.js'; * @param type The type of the paster to register, e.g. 'block', 'comment', etc. * @param paster The paster to register. */ -export function register( +export function register( type: string, paster: IPaster, ) { diff --git a/core/clipboard/workspace_comment_paster.ts b/core/clipboard/workspace_comment_paster.ts index 7959802d6..b9933f5a1 100644 --- a/core/clipboard/workspace_comment_paster.ts +++ b/core/clipboard/workspace_comment_paster.ts @@ -5,11 +5,11 @@ */ import {IPaster} from '../interfaces/i_paster.js'; -import {CopyData} from '../interfaces/i_copyable.js'; +import {ICopyData} from '../interfaces/i_copyable.js'; import {Coordinate} from '../utils/coordinate.js'; import {WorkspaceSvg} from '../workspace_svg.js'; import {WorkspaceCommentSvg} from '../workspace_comment_svg.js'; -import {registry} from '../clipboard.js'; +import * as registry from './registry.js'; export class WorkspaceCommentPaster implements IPaster @@ -21,15 +21,19 @@ export class WorkspaceCommentPaster workspace: WorkspaceSvg, coordinate?: Coordinate, ): WorkspaceCommentSvg { - const state = copyData.saveInfo as Element; if (coordinate) { - state.setAttribute('x', `${coordinate.x}`); - state.setAttribute('y', `${coordinate.y}`); + copyData.commentState.setAttribute('x', `${coordinate.x}`); + copyData.commentState.setAttribute('y', `${coordinate.y}`); } - return WorkspaceCommentSvg.fromXmlRendered(state, workspace); + return WorkspaceCommentSvg.fromXmlRendered( + copyData.commentState, + workspace, + ); } } -export interface WorkspaceCommentCopyData extends CopyData {} +export interface WorkspaceCommentCopyData extends ICopyData { + commentState: Element; +} registry.register(WorkspaceCommentPaster.TYPE, new WorkspaceCommentPaster()); diff --git a/core/interfaces/i_copyable.ts b/core/interfaces/i_copyable.ts index d62d9ef83..6035c97ce 100644 --- a/core/interfaces/i_copyable.ts +++ b/core/interfaces/i_copyable.ts @@ -7,7 +7,6 @@ import * as goog from '../../closure/goog/goog.js'; goog.declareModuleId('Blockly.ICopyable'); -import type {WorkspaceSvg} from '../workspace_svg.js'; import type {ISelectable} from './i_selectable.js'; export interface ICopyable extends ISelectable { @@ -15,17 +14,14 @@ export interface ICopyable extends ISelectable { * Encode for copying. * * @returns Copy metadata. - * @internal */ - toCopyData(): CopyData | null; + toCopyData(): ICopyData | null; } export namespace ICopyable { - export interface CopyData { - saveInfo: Object | Element; - source: WorkspaceSvg; - typeCounts: {[key: string]: number} | null; + export interface ICopyData { + paster: string; } } -export type CopyData = ICopyable.CopyData; +export type ICopyData = ICopyable.ICopyData; diff --git a/core/interfaces/i_paster.ts b/core/interfaces/i_paster.ts index 9653d25df..cb2d079c2 100644 --- a/core/interfaces/i_paster.ts +++ b/core/interfaces/i_paster.ts @@ -6,10 +6,10 @@ import {Coordinate} from '../utils/coordinate.js'; import {WorkspaceSvg} from '../workspace_svg.js'; -import {ICopyable, CopyData} from './i_copyable.js'; +import {ICopyable, ICopyData} from './i_copyable.js'; /** An object that can paste data into a workspace. */ -export interface IPaster { +export interface IPaster { paste( copyData: U, workspace: WorkspaceSvg, @@ -18,6 +18,6 @@ export interface IPaster { } /** @returns True if the given object is a paster. */ -export function isPaster(obj: any): obj is IPaster { +export function isPaster(obj: any): obj is IPaster { return obj.paste !== undefined; } diff --git a/core/registry.ts b/core/registry.ts index 2893f3d0f..6e974718c 100644 --- a/core/registry.ts +++ b/core/registry.ts @@ -23,7 +23,7 @@ import type {Renderer} from './renderers/common/renderer.js'; import type {Theme} from './theme.js'; import type {ToolboxItem} from './toolbox/toolbox_item.js'; import {IPaster} from './interfaces/i_paster.js'; -import {CopyData, ICopyable} from './interfaces/i_copyable.js'; +import {ICopyData, ICopyable} from './interfaces/i_copyable.js'; /** * A map of maps. With the keys being the type and name of the class we are @@ -100,7 +100,7 @@ export class Type<_T> { static ICON = new Type('icon'); /** @internal */ - static PASTER = new Type>('paster'); + static PASTER = new Type>('paster'); } /** diff --git a/core/workspace_comment_svg.ts b/core/workspace_comment_svg.ts index ab57fd5ef..4e1a72611 100644 --- a/core/workspace_comment_svg.ts +++ b/core/workspace_comment_svg.ts @@ -23,7 +23,7 @@ import type {CommentMove} from './events/events_comment_move.js'; import * as eventUtils from './events/utils.js'; import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import type {IBubble} from './interfaces/i_bubble.js'; -import type {CopyData, ICopyable} from './interfaces/i_copyable.js'; +import type {ICopyable} from './interfaces/i_copyable.js'; import * as Touch from './touch.js'; import {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; @@ -32,6 +32,10 @@ import {Svg} from './utils/svg.js'; import * as svgMath from './utils/svg_math.js'; import {WorkspaceComment} from './workspace_comment.js'; import type {WorkspaceSvg} from './workspace_svg.js'; +import { + WorkspaceCommentCopyData, + WorkspaceCommentPaster, +} from './clipboard/workspace_comment_paster.js'; /** Size of the resize icon. */ const RESIZE_SIZE = 8; @@ -566,13 +570,11 @@ export class WorkspaceCommentSvg * Encode a comment for copying. * * @returns Copy metadata. - * @internal */ - toCopyData(): CopyData { + toCopyData(): WorkspaceCommentCopyData { return { - saveInfo: this.toXmlWithXY(), - source: this.workspace, - typeCounts: null, + paster: WorkspaceCommentPaster.TYPE, + commentState: this.toXmlWithXY(), }; } From 001d9ff2c9609d2288a5cd33cc86f957c58c05cc Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 3 Aug 2023 15:33:58 -0700 Subject: [PATCH 4/6] feat: make ICopyable generic and update clipboard APIs (#7348) * chore: rename module-local variables to not conflict * feat: make ICopyable generic and update clipboard APIs * chore: switch over more things to use generic ICopyables * chore: fix shortcut items using copy paste * chore: add test for interface between clipboard and pasters * chore: export isCopyable * chore: format * chore: fixup PR comments * chore: add deprecation tags --- core/block_svg.ts | 6 +- core/blockly.ts | 4 +- core/clipboard.ts | 142 ++++++++++++++++++++++++---------- core/clipboard/registry.ts | 2 +- core/common.ts | 8 +- core/interfaces/i_copyable.ts | 9 ++- core/interfaces/i_paster.ts | 6 +- core/registry.ts | 2 +- core/shortcut_items.ts | 11 ++- core/workspace_comment_svg.ts | 2 +- core/workspace_svg.ts | 4 +- tests/mocha/clipboard_test.js | 35 +++++++++ tests/mocha/index.html | 1 + 13 files changed, 169 insertions(+), 63 deletions(-) create mode 100644 tests/mocha/clipboard_test.js diff --git a/core/block_svg.ts b/core/block_svg.ts index ff251b2ef..d8c9ef6ee 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -70,7 +70,11 @@ import {BlockCopyData, BlockPaster} from './clipboard/block_paster.js'; */ export class BlockSvg extends Block - implements IASTNodeLocationSvg, IBoundedElement, ICopyable, IDraggable + implements + IASTNodeLocationSvg, + IBoundedElement, + ICopyable, + IDraggable { /** * Constant for identifying rows that are to be rendered inline. diff --git a/core/blockly.ts b/core/blockly.ts index 9e8599ed9..76cf5d136 100644 --- a/core/blockly.ts +++ b/core/blockly.ts @@ -140,7 +140,7 @@ import {ICollapsibleToolboxItem} from './interfaces/i_collapsible_toolbox_item.j import {IComponent} from './interfaces/i_component.js'; import {IConnectionChecker} from './interfaces/i_connection_checker.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; -import {ICopyable} from './interfaces/i_copyable.js'; +import {ICopyable, isCopyable} from './interfaces/i_copyable.js'; import {IDeletable} from './interfaces/i_deletable.js'; import {IDeleteArea} from './interfaces/i_delete_area.js'; import {IDragTarget} from './interfaces/i_drag_target.js'; @@ -592,7 +592,7 @@ export {IComponent}; export {IConnectionChecker}; export {IContextMenu}; export {icons}; -export {ICopyable}; +export {ICopyable, isCopyable}; export {IDeletable}; export {IDeleteArea}; export {IDragTarget}; diff --git a/core/clipboard.ts b/core/clipboard.ts index 627d356bc..ae49a93e4 100644 --- a/core/clipboard.ts +++ b/core/clipboard.ts @@ -12,79 +12,139 @@ import {BlockPaster} from './clipboard/block_paster.js'; import * as globalRegistry from './registry.js'; import {WorkspaceSvg} from './workspace_svg.js'; import * as registry from './clipboard/registry.js'; +import {Coordinate} from './utils/coordinate.js'; +import * as deprecation from './utils/deprecation.js'; /** Metadata about the object that is currently on the clipboard. */ -let copyData: ICopyData | null = null; +let stashedCopyData: ICopyData | null = null; -let source: WorkspaceSvg | null = null; +let stashedWorkspace: WorkspaceSvg | null = null; /** - * Copy a block or workspace comment onto the local clipboard. + * Copy a copyable element onto the local clipboard. * - * @param toCopy Block or Workspace Comment to be copied. + * @param toCopy The copyable element to be copied. + * @deprecated v11. Use `myCopyable.toCopyData()` instead. To be removed v12. * @internal */ -export function copy(toCopy: ICopyable) { - TEST_ONLY.copyInternal(toCopy); +export function copy(toCopy: ICopyable): T | null { + deprecation.warn( + 'Blockly.clipboard.copy', + 'v11', + 'v12', + 'myCopyable.toCopyData()', + ); + return TEST_ONLY.copyInternal(toCopy); } /** * Private version of copy for stubbing in tests. */ -function copyInternal(toCopy: ICopyable) { - copyData = toCopy.toCopyData(); - source = (toCopy as any).workspace ?? null; +function copyInternal(toCopy: ICopyable): T | null { + const data = toCopy.toCopyData(); + stashedCopyData = data; + stashedWorkspace = (toCopy as any).workspace ?? null; + return data; } /** - * Paste a block or workspace comment on to the main workspace. + * Paste a pasteable element into the workspace. * + * @param copyData The data to paste into the workspace. + * @param workspace The workspace to paste the data into. + * @param coordinate The location to paste the thing at. * @returns The pasted thing if the paste was successful, null otherwise. - * @internal */ -export function paste(): ICopyable | null { - if (!copyData) { - return null; +export function paste( + copyData: T, + workspace: WorkspaceSvg, + coordinate?: Coordinate, +): ICopyable | null; + +/** + * Pastes the last copied ICopyable into the workspace. + * + * @returns the pasted thing if the paste was successful, null otherwise. + */ +export function paste(): ICopyable | null; + +/** + * Pastes the given data into the workspace, or the last copied ICopyable if + * no data is passed. + * + * @param copyData The data to paste into the workspace. + * @param workspace The workspace to paste the data into. + * @param coordinate The location to paste the thing at. + * @returns The pasted thing if the paste was successful, null otherwise. + */ +export function paste( + copyData?: T, + workspace?: WorkspaceSvg, + coordinate?: Coordinate, +): ICopyable | null { + if (!copyData || !workspace) { + if (!stashedCopyData || !stashedWorkspace) return null; + return pasteFromData(stashedCopyData, stashedWorkspace); } - // Pasting always pastes to the main workspace, even if the copy - // started in a flyout workspace. - let workspace = source; - if (workspace?.isFlyout) { - workspace = workspace.targetWorkspace!; - } - if (!workspace) return null; - return ( - globalRegistry - .getObject(globalRegistry.Type.PASTER, copyData.paster, false) - ?.paste(copyData, workspace) ?? null - ); + return pasteFromData(copyData, workspace, coordinate); } /** - * Duplicate this block and its children, or a workspace comment. + * Paste a pasteable element into the workspace. * - * @param toDuplicate Block or Workspace Comment to be duplicated. - * @returns The block or workspace comment that was duplicated, or null if the - * duplication failed. + * @param copyData The data to paste into the workspace. + * @param workspace The workspace to paste the data into. + * @param coordinate The location to paste the thing at. + * @returns The pasted thing if the paste was successful, null otherwise. + */ +function pasteFromData( + copyData: T, + workspace: WorkspaceSvg, + coordinate?: Coordinate, +): ICopyable | null { + workspace = workspace.getRootWorkspace() ?? workspace; + return (globalRegistry + .getObject(globalRegistry.Type.PASTER, copyData.paster, false) + ?.paste(copyData, workspace, coordinate) ?? null) as ICopyable | null; +} + +/** + * Duplicate this copy-paste-able element. + * + * @param toDuplicate The element to be duplicated. + * @returns The element that was duplicated, or null if the duplication failed. + * @deprecated v11. Use + * `Blockly.clipboard.paste(myCopyable.toCopyData(), myWorkspace)` instead. + * To be removed v12. * @internal */ -export function duplicate(toDuplicate: ICopyable): ICopyable | null { +export function duplicate< + U extends ICopyData, + T extends ICopyable & IHasWorkspace, +>(toDuplicate: T): T | null { + deprecation.warn( + 'Blockly.clipboard.duplicate', + 'v11', + 'v12', + 'Blockly.clipboard.paste(myCopyable.toCopyData(), myWorkspace)', + ); return TEST_ONLY.duplicateInternal(toDuplicate); } /** * Private version of duplicate for stubbing in tests. */ -function duplicateInternal(toDuplicate: ICopyable): ICopyable | null { - const oldCopyData = copyData; - copy(toDuplicate); - if (!copyData || !source) return null; - const pastedThing = - globalRegistry - .getObject(globalRegistry.Type.PASTER, copyData.paster, false) - ?.paste(copyData, source) ?? null; - copyData = oldCopyData; - return pastedThing; +function duplicateInternal< + U extends ICopyData, + T extends ICopyable & IHasWorkspace, +>(toDuplicate: T): T | null { + const data = toDuplicate.toCopyData(); + if (!data) return null; + return paste(data, toDuplicate.workspace) as T; +} + +interface IHasWorkspace { + workspace: WorkspaceSvg; } export const TEST_ONLY = { diff --git a/core/clipboard/registry.ts b/core/clipboard/registry.ts index 396342341..1257f5bdb 100644 --- a/core/clipboard/registry.ts +++ b/core/clipboard/registry.ts @@ -14,7 +14,7 @@ import * as registry from '../registry.js'; * @param type The type of the paster to register, e.g. 'block', 'comment', etc. * @param paster The paster to register. */ -export function register( +export function register>( type: string, paster: IPaster, ) { diff --git a/core/common.ts b/core/common.ts index b5fcd3540..abb6b4042 100644 --- a/core/common.ts +++ b/core/common.ts @@ -9,9 +9,9 @@ goog.declareModuleId('Blockly.common'); /* eslint-disable-next-line no-unused-vars */ import type {Block} from './block.js'; +import {ISelectable} from './blockly.js'; import {BlockDefinition, Blocks} from './blocks.js'; import type {Connection} from './connection.js'; -import type {ICopyable} from './interfaces/i_copyable.js'; import type {Workspace} from './workspace.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -88,12 +88,12 @@ export function setMainWorkspace(workspace: Workspace) { /** * Currently selected copyable object. */ -let selected: ICopyable | null = null; +let selected: ISelectable | null = null; /** * Returns the currently selected copyable object. */ -export function getSelected(): ICopyable | null { +export function getSelected(): ISelectable | null { return selected; } @@ -105,7 +105,7 @@ export function getSelected(): ICopyable | null { * @param newSelection The newly selected block. * @internal */ -export function setSelected(newSelection: ICopyable | null) { +export function setSelected(newSelection: ISelectable | null) { selected = newSelection; } diff --git a/core/interfaces/i_copyable.ts b/core/interfaces/i_copyable.ts index 6035c97ce..b77819839 100644 --- a/core/interfaces/i_copyable.ts +++ b/core/interfaces/i_copyable.ts @@ -9,13 +9,13 @@ goog.declareModuleId('Blockly.ICopyable'); import type {ISelectable} from './i_selectable.js'; -export interface ICopyable extends ISelectable { +export interface ICopyable extends ISelectable { /** * Encode for copying. * * @returns Copy metadata. */ - toCopyData(): ICopyData | null; + toCopyData(): T | null; } export namespace ICopyable { @@ -25,3 +25,8 @@ export namespace ICopyable { } export type ICopyData = ICopyable.ICopyData; + +/** @returns true if the given object is copyable. */ +export function isCopyable(obj: any): obj is ICopyable { + return obj.toCopyData !== undefined; +} diff --git a/core/interfaces/i_paster.ts b/core/interfaces/i_paster.ts index cb2d079c2..321ff118f 100644 --- a/core/interfaces/i_paster.ts +++ b/core/interfaces/i_paster.ts @@ -9,7 +9,7 @@ import {WorkspaceSvg} from '../workspace_svg.js'; import {ICopyable, ICopyData} from './i_copyable.js'; /** An object that can paste data into a workspace. */ -export interface IPaster { +export interface IPaster> { paste( copyData: U, workspace: WorkspaceSvg, @@ -18,6 +18,8 @@ export interface IPaster { } /** @returns True if the given object is a paster. */ -export function isPaster(obj: any): obj is IPaster { +export function isPaster( + obj: any, +): obj is IPaster> { return obj.paste !== undefined; } diff --git a/core/registry.ts b/core/registry.ts index 6e974718c..7772f9fb2 100644 --- a/core/registry.ts +++ b/core/registry.ts @@ -100,7 +100,7 @@ export class Type<_T> { static ICON = new Type('icon'); /** @internal */ - static PASTER = new Type>('paster'); + static PASTER = new Type>>('paster'); } /** diff --git a/core/shortcut_items.ts b/core/shortcut_items.ts index efe876649..4b840fce5 100644 --- a/core/shortcut_items.ts +++ b/core/shortcut_items.ts @@ -11,7 +11,7 @@ import {BlockSvg} from './block_svg.js'; import * as clipboard from './clipboard.js'; import * as common from './common.js'; import {Gesture} from './gesture.js'; -import type {ICopyable} from './interfaces/i_copyable.js'; +import {isCopyable} from './interfaces/i_copyable.js'; import {KeyboardShortcut, ShortcutRegistry} from './shortcut_registry.js'; import {KeyCodes} from './utils/keycodes.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -114,7 +114,9 @@ export function registerCopy() { // AnyDuringMigration because: Property 'hideChaff' does not exist on // type 'Workspace'. (workspace as AnyDuringMigration).hideChaff(); - clipboard.copy(common.getSelected() as ICopyable); + const selected = common.getSelected(); + if (!selected || !isCopyable(selected)) return false; + clipboard.copy(selected); return true; }, keyCodes: [ctrlC, altC, metaC], @@ -152,10 +154,7 @@ export function registerCut() { }, callback() { const selected = common.getSelected(); - if (!selected) { - // Shouldn't happen but appeases the type system - return false; - } + if (!selected || !isCopyable(selected)) return false; clipboard.copy(selected); (selected as BlockSvg).checkAndDelete(); return true; diff --git a/core/workspace_comment_svg.ts b/core/workspace_comment_svg.ts index 4e1a72611..0194252eb 100644 --- a/core/workspace_comment_svg.ts +++ b/core/workspace_comment_svg.ts @@ -51,7 +51,7 @@ const TEXTAREA_OFFSET = 2; */ export class WorkspaceCommentSvg extends WorkspaceComment - implements IBoundedElement, IBubble, ICopyable + implements IBoundedElement, IBubble, ICopyable { /** * The width and height to use to size a workspace comment when it is first diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 01888e55f..16c84c91e 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -36,7 +36,7 @@ import {Gesture} from './gesture.js'; import {Grid} from './grid.js'; import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js'; import type {IBoundedElement} from './interfaces/i_bounded_element.js'; -import type {ICopyable} from './interfaces/i_copyable.js'; +import type {ICopyData, ICopyable} from './interfaces/i_copyable.js'; import type {IDragTarget} from './interfaces/i_drag_target.js'; import type {IFlyout} from './interfaces/i_flyout.js'; import type {IMetricsManager} from './interfaces/i_metrics_manager.js'; @@ -1300,7 +1300,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { */ paste( state: AnyDuringMigration | Element | DocumentFragment, - ): ICopyable | null { + ): ICopyable | null { if (!this.rendered || (!state['type'] && !state['tagName'])) { return null; } diff --git a/tests/mocha/clipboard_test.js b/tests/mocha/clipboard_test.js new file mode 100644 index 000000000..2db315f6e --- /dev/null +++ b/tests/mocha/clipboard_test.js @@ -0,0 +1,35 @@ +/** + * @license + * Copyright 2019 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +goog.declareModuleId('Blockly.test.clipboard'); + +import { + sharedTestSetup, + sharedTestTeardown, +} from './test_helpers/setup_teardown.js'; + +suite('Clipboard', function () { + setup(function () { + this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock; + this.workspace = new Blockly.WorkspaceSvg(new Blockly.Options({})); + }); + + teardown(function () { + sharedTestTeardown.call(this); + }); + + test('a paster registered with a given type is called when pasting that type', function () { + const paster = { + paste: sinon.stub().returns(null), + }; + Blockly.clipboard.registry.register('test-paster', paster); + + Blockly.clipboard.paste({paster: 'test-paster'}, this.workspace); + chai.assert.isTrue(paster.paste.calledOnce); + + Blockly.clipboard.registry.unregister('test-paster'); + }); +}); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index a38e08517..de11f6a94 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -38,6 +38,7 @@ 'Blockly.test.astNode', 'Blockly.test.blockJson', 'Blockly.test.blocks', + 'Blockly.test.clipboard', 'Blockly.test.comments', 'Blockly.test.commentDeserialization', 'Blockly.test.connectionChecker', From a901c62d0c0d1755668b459980cedb86f1233a91 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Wed, 9 Aug 2023 10:31:29 -0700 Subject: [PATCH 5/6] fix: bumping copied objects (#7349) * fix: add logic for bumping pasted blocks * chore: add tests for bumping pasted blocks to the correct location * fix: add logic for bumping pasted comments * chore: add tests for bumping pasted comments --- core/clipboard/block_paster.ts | 86 ++++++++++++++++- core/clipboard/workspace_comment_paster.ts | 10 +- core/serialization/blocks.ts | 4 +- core/workspace_svg.ts | 7 ++ tests/mocha/clipboard_test.js | 105 ++++++++++++++++++++- tests/mocha/test_helpers/events.js | 11 +-- 6 files changed, 208 insertions(+), 15 deletions(-) diff --git a/core/clipboard/block_paster.ts b/core/clipboard/block_paster.ts index 3ed8383db..0bb707c36 100644 --- a/core/clipboard/block_paster.ts +++ b/core/clipboard/block_paster.ts @@ -11,6 +11,8 @@ import {IPaster} from '../interfaces/i_paster.js'; import {State, append} from '../serialization/blocks.js'; import {Coordinate} from '../utils/coordinate.js'; import {WorkspaceSvg} from '../workspace_svg.js'; +import * as eventUtils from '../events/utils.js'; +import {config} from '../config.js'; export class BlockPaster implements IPaster { static TYPE = 'block'; @@ -26,12 +28,90 @@ export class BlockPaster implements IPaster { copyData.blockState['x'] = coordinate.x; copyData.blockState['y'] = coordinate.y; } - return append(copyData.blockState, workspace, { - recordUndo: true, - }) as BlockSvg; + + eventUtils.disable(); + let block; + try { + block = append(copyData.blockState, workspace) as BlockSvg; + moveBlockToNotConflict(block); + } finally { + eventUtils.enable(); + } + + if (!block) return block; + + if (eventUtils.isEnabled() && !block.isShadow()) { + eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_CREATE))(block)); + } + block.select(); + return block; } } +/** + * Moves the given block to a location where it does not: (1) overlap exactly + * with any other blocks, or (2) look like it is connected to any other blocks. + * + * Exported for testing. + * + * @param block The block to move to an unambiguous location. + * @internal + */ +export function moveBlockToNotConflict(block: BlockSvg) { + const workspace = block.workspace; + const snapRadius = config.snapRadius; + const coord = block.getRelativeToSurfaceXY(); + const offset = new Coordinate(0, 0); + // getRelativeToSurfaceXY is really expensive, so we want to cache this. + const otherCoords = workspace + .getAllBlocks(false) + .filter((otherBlock) => otherBlock.id != block.id) + .map((b) => b.getRelativeToSurfaceXY()); + + while ( + blockOverlapsOtherExactly(Coordinate.sum(coord, offset), otherCoords) || + blockIsInSnapRadius(block, offset, snapRadius) + ) { + if (workspace.RTL) { + offset.translate(-snapRadius, snapRadius * 2); + } else { + offset.translate(snapRadius, snapRadius * 2); + } + } + + block!.moveTo(Coordinate.sum(coord, offset)); +} + +/** + * @returns true if the given block coordinates are less than a delta of 1 from + * any of the other coordinates. + */ +function blockOverlapsOtherExactly( + coord: Coordinate, + otherCoords: Coordinate[], +): boolean { + return otherCoords.some( + (otherCoord) => + Math.abs(otherCoord.x - coord.x) <= 1 && + Math.abs(otherCoord.y - coord.y) <= 1, + ); +} + +/** + * @returns true if the given block (when offset by the given amount) is close + * enough to any other connections (within the snap radius) that it looks + * like they could connect. + */ +function blockIsInSnapRadius( + block: BlockSvg, + offset: Coordinate, + snapRadius: number, +): boolean { + return block + .getConnections_(false) + .some((connection) => !!connection.closest(snapRadius, offset).connection); +} + export interface BlockCopyData extends ICopyData { blockState: State; typeCounts: {[key: string]: number}; diff --git a/core/clipboard/workspace_comment_paster.ts b/core/clipboard/workspace_comment_paster.ts index b9933f5a1..aeedbfb2b 100644 --- a/core/clipboard/workspace_comment_paster.ts +++ b/core/clipboard/workspace_comment_paster.ts @@ -21,9 +21,15 @@ export class WorkspaceCommentPaster workspace: WorkspaceSvg, coordinate?: Coordinate, ): WorkspaceCommentSvg { + const state = copyData.commentState; if (coordinate) { - copyData.commentState.setAttribute('x', `${coordinate.x}`); - copyData.commentState.setAttribute('y', `${coordinate.y}`); + state.setAttribute('x', `${coordinate.x}`); + state.setAttribute('y', `${coordinate.y}`); + } else { + const x = parseInt(state.getAttribute('x') ?? '0') + 50; + const y = parseInt(state.getAttribute('y') ?? '0') + 50; + state.setAttribute('x', `${x}`); + state.setAttribute('y', `${y}`); } return WorkspaceCommentSvg.fromXmlRendered( copyData.commentState, diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index c9c6395ac..f11a7d8cd 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -396,7 +396,9 @@ export function appendInternal( const block = appendPrivate(state, workspace, {parentConnection, isShadow}); eventUtils.enable(); - eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_CREATE))(block)); + if (eventUtils.isEnabled()) { + eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_CREATE))(block)); + } eventUtils.setGroup(existingGroup); eventUtils.setRecordUndo(prevRecordUndo); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 16c84c91e..5978c3197 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -1388,6 +1388,10 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { for (let i = 0, connection; (connection = connections[i]); i++) { const neighbour = connection.closest( config.snapRadius, + // TODO: This code doesn't work because it's passing an absolute + // coordinate instead of a relative coordinate. Need to + // figure out if I'm deprecating this function or if I + // need to fix this. new Coordinate(blockX, blockY), ); if (neighbour.connection) { @@ -1441,6 +1445,9 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { // with any blocks. commentX += 50; commentY += 50; + // TODO: This code doesn't work because it's using absolute coords + // where relative coords are expected. Need to figure out what I'm + // doing with this function and if I need to fix it. comment.moveBy(commentX, commentY); } } finally { diff --git a/tests/mocha/clipboard_test.js b/tests/mocha/clipboard_test.js index 2db315f6e..f6cf10a25 100644 --- a/tests/mocha/clipboard_test.js +++ b/tests/mocha/clipboard_test.js @@ -1,6 +1,6 @@ /** * @license - * Copyright 2019 Google LLC + * Copyright 2023 Google LLC * SPDX-License-Identifier: Apache-2.0 */ @@ -10,11 +10,15 @@ import { sharedTestSetup, sharedTestTeardown, } from './test_helpers/setup_teardown.js'; +import { + assertEventFired, + createChangeListenerSpy, +} from './test_helpers/events.js'; suite('Clipboard', function () { setup(function () { this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock; - this.workspace = new Blockly.WorkspaceSvg(new Blockly.Options({})); + this.workspace = Blockly.inject('blocklyDiv'); }); teardown(function () { @@ -32,4 +36,101 @@ suite('Clipboard', function () { Blockly.clipboard.registry.unregister('test-paster'); }); + + suite('pasting blocks', function () { + test('pasting blocks fires a create event', function () { + const eventSpy = createChangeListenerSpy(this.workspace); + const block = Blockly.serialization.blocks.append( + { + 'type': 'controls_if', + 'id': 'blockId', + }, + this.workspace, + ); + const data = block.toCopyData(); + this.clock.runAll(); + eventSpy.resetHistory(); + + Blockly.clipboard.paste(data, this.workspace); + this.clock.runAll(); + + assertEventFired( + eventSpy, + Blockly.Events.BlockCreate, + {'recordUndo': true, 'type': Blockly.Events.BLOCK_CREATE}, + this.workspace.id, + ); + }); + + suite('pasted blocks are placed in unambiguous locations', function () { + test('pasted blocks are bumped to not overlap', function () { + const block = Blockly.serialization.blocks.append( + { + 'type': 'controls_if', + 'x': 38, + 'y': 13, + }, + this.workspace, + ); + const data = block.toCopyData(); + + const newBlock = Blockly.clipboard.paste(data, this.workspace); + chai.assert.deepEqual( + newBlock.getRelativeToSurfaceXY(), + new Blockly.utils.Coordinate(66, 69), + ); + }); + + test('pasted blocks are bumped to be outside the connection snap radius', function () { + Blockly.serialization.workspaces.load( + { + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'controls_if', + 'id': 'sourceBlockId', + 'x': 38, + 'y': 13, + }, + { + 'type': 'logic_compare', + 'x': 113, + 'y': 63, + }, + ], + }, + }, + this.workspace, + ); + this.clock.runAll(); // Update the connection DB. + const data = this.workspace.getBlockById('sourceBlockId').toCopyData(); + + const newBlock = Blockly.clipboard.paste(data, this.workspace); + chai.assert.deepEqual( + newBlock.getRelativeToSurfaceXY(), + new Blockly.utils.Coordinate(94, 125), + ); + }); + }); + }); + + suite('pasting comments', function () { + test('pasted comments are bumped to not overlap', function () { + Blockly.Xml.domToWorkspace( + Blockly.utils.xml.textToDom( + '', + ), + this.workspace, + ); + const comment = this.workspace.getTopComments(false)[0]; + const data = comment.toCopyData(); + + const newComment = Blockly.clipboard.paste(data, this.workspace); + chai.assert.deepEqual( + newComment.getRelativeToSurfaceXY(), + new Blockly.utils.Coordinate(60, 60), + ); + }); + }); }); diff --git a/tests/mocha/test_helpers/events.js b/tests/mocha/test_helpers/events.js index 46ce32830..ff0b5fb3c 100644 --- a/tests/mocha/test_helpers/events.js +++ b/tests/mocha/test_helpers/events.js @@ -149,13 +149,10 @@ export function assertEventFired( expectedWorkspaceId, expectedBlockId, ) { - expectedProperties = Object.assign( - { - workspaceId: expectedWorkspaceId, - blockId: expectedBlockId, - }, - expectedProperties, - ); + const baseProps = {}; + if (expectedWorkspaceId) baseProps.workspaceId = expectedWorkspaceId; + if (expectedBlockId) baseProps.blockId = expectedBlockId; + expectedProperties = Object.assign(baseProps, expectedProperties); const expectedEvent = sinon.match .instanceOf(instanceType) .and(sinon.match(expectedProperties)); From e30c4acd92b9b8512bc5ca0c7772b04397a206e4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 11 Aug 2023 09:38:50 -0700 Subject: [PATCH 6/6] chore: upgrade keyboard shortcuts and context menus to use non-deprecated APIs (#7352) * chore: remove references to clipboard.copy in shortcuts * chore: remove references to clipboard.copy in context menus * chore: fix tests * chore: format * fix: PR comments --- core/contextmenu.ts | 4 +++- core/contextmenu_items.ts | 7 ++++--- core/shortcut_items.ts | 25 +++++++++++++++---------- core/shortcut_registry.ts | 8 ++++---- tests/mocha/contextmenu_items_test.js | 12 ++++++------ tests/mocha/keydown_test.js | 9 ++++++--- 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/core/contextmenu.ts b/core/contextmenu.ts index b7e107e01..abc729470 100644 --- a/core/contextmenu.ts +++ b/core/contextmenu.ts @@ -297,7 +297,9 @@ export function commentDuplicateOption( text: Msg['DUPLICATE_COMMENT'], enabled: true, callback: function () { - clipboard.duplicate(comment); + const data = comment.toCopyData(); + if (!data) return; + clipboard.paste(data, comment.workspace); }, }; return duplicateOption; diff --git a/core/contextmenu_items.ts b/core/contextmenu_items.ts index aedd72614..c784e9513 100644 --- a/core/contextmenu_items.ts +++ b/core/contextmenu_items.ts @@ -331,9 +331,10 @@ export function registerDuplicate() { return 'hidden'; }, callback(scope: Scope) { - if (scope.block) { - clipboard.duplicate(scope.block); - } + if (!scope.block) return; + const data = scope.block.toCopyData(); + if (!data) return; + clipboard.paste(data, scope.block.workspace); }, scopeType: ContextMenuRegistry.ScopeType.BLOCK, id: 'blockDuplicate', diff --git a/core/shortcut_items.ts b/core/shortcut_items.ts index 4b840fce5..940dea3f9 100644 --- a/core/shortcut_items.ts +++ b/core/shortcut_items.ts @@ -11,7 +11,7 @@ import {BlockSvg} from './block_svg.js'; import * as clipboard from './clipboard.js'; import * as common from './common.js'; import {Gesture} from './gesture.js'; -import {isCopyable} from './interfaces/i_copyable.js'; +import {ICopyData, isCopyable} from './interfaces/i_copyable.js'; import {KeyboardShortcut, ShortcutRegistry} from './shortcut_registry.js'; import {KeyCodes} from './utils/keycodes.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -81,6 +81,9 @@ export function registerDelete() { ShortcutRegistry.registry.register(deleteShortcut); } +let copyData: ICopyData | null = null; +let copyWorkspace: WorkspaceSvg | null = null; + /** * Keyboard shortcut to copy a block on ctrl+c, cmd+c, or alt+c. */ @@ -104,20 +107,20 @@ export function registerCopy() { !Gesture.inProgress() && selected != null && selected.isDeletable() && - selected.isMovable() + selected.isMovable() && + isCopyable(selected) ); }, callback(workspace, e) { // Prevent the default copy behavior, which may beep or otherwise indicate // an error due to the lack of a selection. e.preventDefault(); - // AnyDuringMigration because: Property 'hideChaff' does not exist on - // type 'Workspace'. - (workspace as AnyDuringMigration).hideChaff(); + workspace.hideChaff(); const selected = common.getSelected(); if (!selected || !isCopyable(selected)) return false; - clipboard.copy(selected); - return true; + copyData = selected.toCopyData(); + copyWorkspace = workspace; + return !!copyData; }, keyCodes: [ctrlC, altC, metaC], }; @@ -152,10 +155,11 @@ export function registerCut() { !selected.workspace!.isFlyout ); }, - callback() { + callback(workspace) { const selected = common.getSelected(); if (!selected || !isCopyable(selected)) return false; - clipboard.copy(selected); + copyData = selected.toCopyData(); + copyWorkspace = workspace; (selected as BlockSvg).checkAndDelete(); return true; }, @@ -185,7 +189,8 @@ export function registerPaste() { return !workspace.options.readOnly && !Gesture.inProgress(); }, callback() { - return !!clipboard.paste(); + if (!copyData || !copyWorkspace) return false; + return !!clipboard.paste(copyData, copyWorkspace); }, keyCodes: [ctrlV, altV, metaV], }; diff --git a/core/shortcut_registry.ts b/core/shortcut_registry.ts index e4d157495..d6442f51d 100644 --- a/core/shortcut_registry.ts +++ b/core/shortcut_registry.ts @@ -15,7 +15,7 @@ goog.declareModuleId('Blockly.ShortcutRegistry'); import {KeyCodes} from './utils/keycodes.js'; import * as object from './utils/object.js'; -import type {Workspace} from './workspace.js'; +import {WorkspaceSvg} from './workspace_svg.js'; /** * Class for the registry of keyboard shortcuts. This is intended to be a @@ -224,7 +224,7 @@ export class ShortcutRegistry { * @param e The key down event. * @returns True if the event was handled, false otherwise. */ - onKeyDown(workspace: Workspace, e: KeyboardEvent): boolean { + onKeyDown(workspace: WorkspaceSvg, e: KeyboardEvent): boolean { const key = this.serializeKeyEvent_(e); const shortcutNames = this.getShortcutNamesByKeyCode(key); if (!shortcutNames) { @@ -346,9 +346,9 @@ export class ShortcutRegistry { export namespace ShortcutRegistry { export interface KeyboardShortcut { - callback?: (p1: Workspace, p2: Event, p3: KeyboardShortcut) => boolean; + callback?: (p1: WorkspaceSvg, p2: Event, p3: KeyboardShortcut) => boolean; name: string; - preconditionFn?: (p1: Workspace) => boolean; + preconditionFn?: (p1: WorkspaceSvg) => boolean; metadata?: object; keyCodes?: (number | string)[]; allowCollision?: boolean; diff --git a/tests/mocha/contextmenu_items_test.js b/tests/mocha/contextmenu_items_test.js index 4e18a6801..a9d1a5009 100644 --- a/tests/mocha/contextmenu_items_test.js +++ b/tests/mocha/contextmenu_items_test.js @@ -419,13 +419,13 @@ suite('Context Menu Items', function () { ); }); - test('Calls duplicate', function () { - const spy = sinon.spy(Blockly.clipboard.TEST_ONLY, 'duplicateInternal'); - + test('the block is duplicated', function () { this.duplicateOption.callback(this.scope); - - sinon.assert.calledOnce(spy); - sinon.assert.calledWith(spy, this.block); + chai.assert.equal( + this.workspace.getTopBlocks(false).length, + 2, + 'Expected a second block', + ); }); test('Has correct label', function () { diff --git a/tests/mocha/keydown_test.js b/tests/mocha/keydown_test.js index e33f3c39d..f0f94310d 100644 --- a/tests/mocha/keydown_test.js +++ b/tests/mocha/keydown_test.js @@ -26,10 +26,13 @@ suite('Key Down', function () { /** * Creates a block and sets it as Blockly.selected. * @param {Blockly.Workspace} workspace The workspace to create a new block on. + * @return {Blockly.Block} The block being selected. */ function setSelectedBlock(workspace) { defineStackBlock(); - Blockly.common.setSelected(workspace.newBlock('stack_block')); + const block = workspace.newBlock('stack_block'); + Blockly.common.setSelected(block); + return block; } /** @@ -109,8 +112,8 @@ suite('Key Down', function () { suite('Copy', function () { setup(function () { - setSelectedBlock(this.workspace); - this.copySpy = sinon.spy(Blockly.clipboard.TEST_ONLY, 'copyInternal'); + this.block = setSelectedBlock(this.workspace); + this.copySpy = sinon.spy(this.block, 'toCopyData'); this.hideChaffSpy = sinon.spy( Blockly.WorkspaceSvg.prototype, 'hideChaff',