diff --git a/core/clipboard.ts b/core/clipboard.ts index 5fa654d63..c7b22dfc7 100644 --- a/core/clipboard.ts +++ b/core/clipboard.ts @@ -9,6 +9,7 @@ import {BlockCopyData, BlockPaster} from './clipboard/block_paster.js'; import * as registry from './clipboard/registry.js'; import type {ICopyData, ICopyable} from './interfaces/i_copyable.js'; +import {isSelectable} from './interfaces/i_selectable.js'; import * as globalRegistry from './registry.js'; import {Coordinate} from './utils/coordinate.js'; import {WorkspaceSvg} from './workspace_svg.js'; @@ -18,18 +19,119 @@ let stashedCopyData: ICopyData | null = null; let stashedWorkspace: WorkspaceSvg | null = null; +let stashedCoordinates: Coordinate | undefined = undefined; + /** - * Private version of copy for stubbing in tests. + * Copy a copyable item, and record its data and the workspace it was + * copied from. + * + * This function does not perform any checks to ensure the copy + * should be allowed, e.g. to ensure the block is deletable. Such + * checks should be done before calling this function. + * + * Note that if the copyable item is not an `ISelectable` or its + * `workspace` property is not a `WorkspaceSvg`, the copy will be + * successful, but there will be no saved workspace data. This will + * impact the ability to paste the data unless you explictily pass + * a workspace into the paste method. + * + * @param toCopy item to copy. + * @param location location to save as a potential paste location. + * @returns the copied data if copy was successful, otherwise null. */ -function copyInternal(toCopy: ICopyable): T | null { +export function copy( + toCopy: ICopyable, + location?: Coordinate, +): T | null { const data = toCopy.toCopyData(); stashedCopyData = data; - stashedWorkspace = (toCopy as any).workspace ?? null; + if (isSelectable(toCopy) && toCopy.workspace instanceof WorkspaceSvg) { + stashedWorkspace = toCopy.workspace; + } else { + stashedWorkspace = null; + } + + stashedCoordinates = location; return data; } /** - * Paste a pasteable element into the workspace. + * Gets the copy data for the last item copied. This is useful if you + * are implementing custom copy/paste behavior. If you want the default + * behavior, just use the copy and paste methods directly. + * + * @returns copy data for the last item copied, or null if none set. + */ +export function getLastCopiedData() { + return stashedCopyData; +} + +/** + * Sets the last copied item. You should call this method if you implement + * custom copy behavior, so that other callers are working with the correct + * data. This method is called automatically if you use the built-in copy + * method. + * + * @param copyData copy data for the last item copied. + */ +export function setLastCopiedData(copyData: ICopyData) { + stashedCopyData = copyData; +} + +/** + * Gets the workspace that was last copied from. This is useful if you + * are implementing custom copy/paste behavior and want to paste on the + * same workspace that was copied from. If you want the default behavior, + * just use the copy and paste methods directly. + * + * @returns workspace that was last copied from, or null if none set. + */ +export function getLastCopiedWorkspace() { + return stashedWorkspace; +} + +/** + * Sets the workspace that was last copied from. You should call this method + * if you implement custom copy behavior, so that other callers are working + * with the correct data. This method is called automatically if you use the + * built-in copy method. + * + * @param workspace workspace that was last copied from. + */ +export function setLastCopiedWorkspace(workspace: WorkspaceSvg) { + stashedWorkspace = workspace; +} + +/** + * Gets the location that was last copied from. This is useful if you + * are implementing custom copy/paste behavior. If you want the + * default behavior, just use the copy and paste methods directly. + * + * @returns last saved location, or null if none set. + */ +export function getLastCopiedLocation() { + return stashedCoordinates; +} + +/** + * Sets the location that was last copied from. You should call this method + * if you implement custom copy behavior, so that other callers are working + * with the correct data. This method is called automatically if you use the + * built-in copy method. + * + * @param location last saved location, which can be used to paste at. + */ +export function setLastCopiedLocation(location: Coordinate) { + stashedCoordinates = location; +} + +/** + * Paste a pasteable element into the given workspace. + * + * This function does not perform any checks to ensure the paste + * is allowed, e.g. that the workspace is rendered or the block + * is pasteable. Such checks should be done before calling this + * function. * * @param copyData The data to paste into the workspace. * @param workspace The workspace to paste the data into. @@ -43,7 +145,7 @@ export function paste( ): ICopyable | null; /** - * Pastes the last copied ICopyable into the workspace. + * Pastes the last copied ICopyable into the last copied-from workspace. * * @returns the pasted thing if the paste was successful, null otherwise. */ @@ -65,7 +167,7 @@ export function paste( ): ICopyable | null { if (!copyData || !workspace) { if (!stashedCopyData || !stashedWorkspace) return null; - return pasteFromData(stashedCopyData, stashedWorkspace); + return pasteFromData(stashedCopyData, stashedWorkspace, stashedCoordinates); } return pasteFromData(copyData, workspace, coordinate); } @@ -85,31 +187,11 @@ function pasteFromData( ): ICopyable | null { workspace = workspace.isMutator ? workspace - : (workspace.getRootWorkspace() ?? workspace); + : // Use the parent workspace if it exists (e.g. for pasting into flyouts) + (workspace.options.parentWorkspace ?? workspace); return (globalRegistry .getObject(globalRegistry.Type.PASTER, copyData.paster, false) ?.paste(copyData, workspace, coordinate) ?? null) as ICopyable | null; } -/** - * Private version of duplicate for stubbing in tests. - */ -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 = { - duplicateInternal, - copyInternal, -}; - export {BlockCopyData, BlockPaster, registry}; diff --git a/core/shortcut_items.ts b/core/shortcut_items.ts index f621f93d3..f8c955007 100644 --- a/core/shortcut_items.ts +++ b/core/shortcut_items.ts @@ -11,7 +11,7 @@ import * as clipboard from './clipboard.js'; import {RenderedWorkspaceComment} from './comments.js'; import * as eventUtils from './events/utils.js'; import {getFocusManager} from './focus_manager.js'; -import {ICopyData, isCopyable as isICopyable} from './interfaces/i_copyable.js'; +import {isCopyable as isICopyable} from './interfaces/i_copyable.js'; import {isDeletable as isIDeletable} from './interfaces/i_deletable.js'; import {isDraggable} from './interfaces/i_draggable.js'; import {IFocusableNode} from './interfaces/i_focusable_node.js'; @@ -92,9 +92,6 @@ export function registerDelete() { ShortcutRegistry.registry.register(deleteShortcut); } -let copyData: ICopyData | null = null; -let copyCoords: Coordinate | null = null; - /** * Determine if a focusable node can be copied. * @@ -175,12 +172,12 @@ export function registerCopy() { if (!focused.workspace.isFlyout) { targetWorkspace.hideChaff(); } - copyData = focused.toCopyData(); - copyCoords = + + const copyCoords = isDraggable(focused) && focused.workspace == targetWorkspace ? focused.getRelativeToSurfaceXY() - : null; - return !!copyData; + : undefined; + return !!clipboard.copy(focused, copyCoords); }, keyCodes: [ctrlC, metaC], }; @@ -215,10 +212,10 @@ export function registerCut() { if (!focused || !isCuttable(focused) || !isICopyable(focused)) { return false; } - copyData = focused.toCopyData(); - copyCoords = isDraggable(focused) + const copyCoords = isDraggable(focused) ? focused.getRelativeToSurfaceXY() - : null; + : undefined; + const copyData = clipboard.copy(focused, copyCoords); if (focused instanceof BlockSvg) { focused.checkAndDelete(); @@ -246,12 +243,19 @@ export function registerPaste() { const pasteShortcut: KeyboardShortcut = { name: names.PASTE, - preconditionFn(workspace) { + preconditionFn() { + // Regardless of the currently focused workspace, we will only + // paste into the last-copied-from workspace. + const workspace = clipboard.getLastCopiedWorkspace(); + // If we don't know where we copied from, we don't know where to paste. + // If the workspace isn't rendered (e.g. closed mutator workspace), + // we can't paste into it. + if (!workspace || !workspace.rendered) return false; const targetWorkspace = workspace.isFlyout ? workspace.targetWorkspace : workspace; return ( - !!copyData && + !!clipboard.getLastCopiedData() && !!targetWorkspace && !targetWorkspace.isReadOnly() && !targetWorkspace.isDragging() && @@ -259,10 +263,15 @@ export function registerPaste() { ); }, callback(workspace: WorkspaceSvg, e: Event) { + const copyData = clipboard.getLastCopiedData(); if (!copyData) return false; - const targetWorkspace = workspace.isFlyout - ? workspace.targetWorkspace - : workspace; + + const copyWorkspace = clipboard.getLastCopiedWorkspace(); + if (!copyWorkspace) return false; + + const targetWorkspace = copyWorkspace.isFlyout + ? copyWorkspace.targetWorkspace + : copyWorkspace; if (!targetWorkspace || targetWorkspace.isReadOnly()) return false; if (e instanceof PointerEvent) { @@ -278,6 +287,7 @@ export function registerPaste() { return !!clipboard.paste(copyData, targetWorkspace, mouseCoords); } + const copyCoords = clipboard.getLastCopiedLocation(); if (!copyCoords) { // If we don't have location data about the original copyable, let the // paster determine position. diff --git a/tests/mocha/clipboard_test.js b/tests/mocha/clipboard_test.js index 0f2d06770..85cdd2297 100644 --- a/tests/mocha/clipboard_test.js +++ b/tests/mocha/clipboard_test.js @@ -76,7 +76,7 @@ suite('Clipboard', function () { await mutatorIcon.setBubbleVisible(true); const mutatorWorkspace = mutatorIcon.getWorkspace(); const elseIf = mutatorWorkspace.getBlocksByType('controls_if_elseif')[0]; - assert.notEqual(elseIf, undefined); + assert.isDefined(elseIf); assert.lengthOf(mutatorWorkspace.getAllBlocks(), 2); assert.lengthOf(this.workspace.getAllBlocks(), 1); const data = elseIf.toCopyData(); @@ -85,6 +85,34 @@ suite('Clipboard', function () { assert.lengthOf(this.workspace.getAllBlocks(), 1); }); + test('pasting into a mutator flyout pastes into the mutator workspace', async function () { + const block = Blockly.serialization.blocks.append( + { + 'type': 'controls_if', + 'id': 'blockId', + 'extraState': { + 'elseIfCount': 1, + }, + }, + this.workspace, + ); + const mutatorIcon = block.getIcon(Blockly.icons.IconType.MUTATOR); + await mutatorIcon.setBubbleVisible(true); + const mutatorWorkspace = mutatorIcon.getWorkspace(); + const mutatorFlyoutWorkspace = mutatorWorkspace + .getFlyout() + .getWorkspace(); + const elseIf = + mutatorFlyoutWorkspace.getBlocksByType('controls_if_elseif')[0]; + assert.isDefined(elseIf); + assert.lengthOf(mutatorWorkspace.getAllBlocks(), 2); + assert.lengthOf(this.workspace.getAllBlocks(), 1); + const data = elseIf.toCopyData(); + Blockly.clipboard.paste(data, mutatorFlyoutWorkspace); + assert.lengthOf(mutatorWorkspace.getAllBlocks(), 3); + assert.lengthOf(this.workspace.getAllBlocks(), 1); + }); + suite('pasted blocks are placed in unambiguous locations', function () { test('pasted blocks are bumped to not overlap', function () { const block = Blockly.serialization.blocks.append( @@ -139,8 +167,7 @@ suite('Clipboard', function () { }); suite('pasting comments', function () { - // TODO: Reenable test when we readd copy-paste. - test.skip('pasted comments are bumped to not overlap', function () { + test('pasted comments are bumped to not overlap', function () { Blockly.Xml.domToWorkspace( Blockly.utils.xml.textToDom( '', @@ -153,7 +180,7 @@ suite('Clipboard', function () { const newComment = Blockly.clipboard.paste(data, this.workspace); assert.deepEqual( newComment.getRelativeToSurfaceXY(), - new Blockly.utils.Coordinate(60, 60), + new Blockly.utils.Coordinate(40, 40), ); }); }); diff --git a/tests/mocha/shortcut_items_test.js b/tests/mocha/shortcut_items_test.js index eaadef01e..d96ddbfea 100644 --- a/tests/mocha/shortcut_items_test.js +++ b/tests/mocha/shortcut_items_test.js @@ -5,6 +5,7 @@ */ import * as Blockly from '../../build/src/core/blockly.js'; +import {assert} from '../../node_modules/chai/chai.js'; import {defineStackBlock} from './test_helpers/block_definitions.js'; import { sharedTestSetup, @@ -399,6 +400,19 @@ suite('Keyboard Shortcut Items', function () { }); }); + suite('Paste', function () { + test('Disabled when nothing has been copied', function () { + const pasteShortcut = + Blockly.ShortcutRegistry.registry.getRegistry()[ + Blockly.ShortcutItems.names.PASTE + ]; + Blockly.clipboard.setLastCopiedData(undefined); + + const isPasteEnabled = pasteShortcut.preconditionFn(); + assert.isFalse(isPasteEnabled); + }); + }); + suite('Undo', function () { setup(function () { this.undoSpy = sinon.spy(this.workspace, 'undo');