From e30c4acd92b9b8512bc5ca0c7772b04397a206e4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 11 Aug 2023 09:38:50 -0700 Subject: [PATCH] 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',