From 001d9ff2c9609d2288a5cd33cc86f957c58c05cc Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 3 Aug 2023 15:33:58 -0700 Subject: [PATCH] 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 ff251b2efbc..d8c9ef6ee11 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 9e8599ed93e..76cf5d136d8 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 627d356bcbe..ae49a93e467 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; - } - // 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!; +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); } - 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 39634234183..1257f5bdbb5 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 b5fcd354021..abb6b4042ee 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 6035c97ce0b..b7781983995 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 cb2d079c230..321ff118f70 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 6e974718c27..7772f9fb298 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 efe876649ab..4b840fce53e 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 4e1a726112d..0194252eba3 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 01888e55f69..16c84c91eb7 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 00000000000..2db315f6e24 --- /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 a38e0851789..de11f6a94f1 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',