diff --git a/core/shortcut_items.ts b/core/shortcut_items.ts index 0793e6213..afbea3ab2 100644 --- a/core/shortcut_items.ts +++ b/core/shortcut_items.ts @@ -8,12 +8,12 @@ import {BlockSvg} from './block_svg.js'; import * as clipboard from './clipboard.js'; -import * as common from './common.js'; import * as eventUtils from './events/utils.js'; import {Gesture} from './gesture.js'; import {ICopyData, isCopyable} from './interfaces/i_copyable.js'; import {isDeletable} from './interfaces/i_deletable.js'; import {isDraggable} from './interfaces/i_draggable.js'; +import {isSelectable} from './interfaces/i_selectable.js'; import {KeyboardShortcut, ShortcutRegistry} from './shortcut_registry.js'; import {Coordinate} from './utils/coordinate.js'; import {KeyCodes} from './utils/keycodes.js'; @@ -43,9 +43,7 @@ export function registerEscape() { return !workspace.isReadOnly(); }, callback(workspace) { - // AnyDuringMigration because: Property 'hideChaff' does not exist on - // type 'Workspace'. - (workspace as AnyDuringMigration).hideChaff(); + workspace.hideChaff(); return true; }, keyCodes: [KeyCodes.ESC], @@ -59,28 +57,28 @@ export function registerEscape() { export function registerDelete() { const deleteShortcut: KeyboardShortcut = { name: names.DELETE, - preconditionFn(workspace) { - const selected = common.getSelected(); + preconditionFn(workspace, scope) { + const focused = scope.focusedNode; return ( !workspace.isReadOnly() && - selected != null && - isDeletable(selected) && - selected.isDeletable() && + focused != null && + isDeletable(focused) && + focused.isDeletable() && !Gesture.inProgress() ); }, - callback(workspace, e) { + callback(workspace, e, shortcut, scope) { // Delete or backspace. // Stop the browser from going back to the previous page. // Do this first to prevent an error in the delete code from resulting in // data loss. e.preventDefault(); - const selected = common.getSelected(); - if (selected instanceof BlockSvg) { - selected.checkAndDelete(); - } else if (isDeletable(selected) && selected.isDeletable()) { + const focused = scope.focusedNode; + if (focused instanceof BlockSvg) { + focused.checkAndDelete(); + } else if (isDeletable(focused) && focused.isDeletable()) { eventUtils.setGroup(true); - selected.dispose(); + focused.dispose(); eventUtils.setGroup(false); } return true; @@ -110,33 +108,33 @@ export function registerCopy() { const copyShortcut: KeyboardShortcut = { name: names.COPY, - preconditionFn(workspace) { - const selected = common.getSelected(); + preconditionFn(workspace, scope) { + const focused = scope.focusedNode; return ( !workspace.isReadOnly() && !Gesture.inProgress() && - selected != null && - isDeletable(selected) && - selected.isDeletable() && - isDraggable(selected) && - selected.isMovable() && - isCopyable(selected) + focused != null && + isDeletable(focused) && + focused.isDeletable() && + isDraggable(focused) && + focused.isMovable() && + isCopyable(focused) ); }, - callback(workspace, e) { + callback(workspace, e, shortcut, scope) { // Prevent the default copy behavior, which may beep or otherwise indicate // an error due to the lack of a selection. e.preventDefault(); workspace.hideChaff(); - const selected = common.getSelected(); - if (!selected || !isCopyable(selected)) return false; - copyData = selected.toCopyData(); + const focused = scope.focusedNode; + if (!focused || !isCopyable(focused)) return false; + copyData = focused.toCopyData(); copyWorkspace = - selected.workspace instanceof WorkspaceSvg - ? selected.workspace + focused.workspace instanceof WorkspaceSvg + ? focused.workspace : workspace; - copyCoords = isDraggable(selected) - ? selected.getRelativeToSurfaceXY() + copyCoords = isDraggable(focused) + ? focused.getRelativeToSurfaceXY() : null; return !!copyData; }, @@ -161,39 +159,40 @@ export function registerCut() { const cutShortcut: KeyboardShortcut = { name: names.CUT, - preconditionFn(workspace) { - const selected = common.getSelected(); + preconditionFn(workspace, scope) { + const focused = scope.focusedNode; return ( !workspace.isReadOnly() && !Gesture.inProgress() && - selected != null && - isDeletable(selected) && - selected.isDeletable() && - isDraggable(selected) && - selected.isMovable() && - !selected.workspace!.isFlyout + focused != null && + isDeletable(focused) && + focused.isDeletable() && + isDraggable(focused) && + focused.isMovable() && + isSelectable(focused) && + !focused.workspace.isFlyout ); }, - callback(workspace) { - const selected = common.getSelected(); + callback(workspace, e, shortcut, scope) { + const focused = scope.focusedNode; - if (selected instanceof BlockSvg) { - copyData = selected.toCopyData(); + if (focused instanceof BlockSvg) { + copyData = focused.toCopyData(); copyWorkspace = workspace; - copyCoords = selected.getRelativeToSurfaceXY(); - selected.checkAndDelete(); + copyCoords = focused.getRelativeToSurfaceXY(); + focused.checkAndDelete(); return true; } else if ( - isDeletable(selected) && - selected.isDeletable() && - isCopyable(selected) + isDeletable(focused) && + focused.isDeletable() && + isCopyable(focused) ) { - copyData = selected.toCopyData(); + copyData = focused.toCopyData(); copyWorkspace = workspace; - copyCoords = isDraggable(selected) - ? selected.getRelativeToSurfaceXY() + copyCoords = isDraggable(focused) + ? focused.getRelativeToSurfaceXY() : null; - selected.dispose(); + focused.dispose(); return true; } return false; diff --git a/core/shortcut_registry.ts b/core/shortcut_registry.ts index 09bd867e7..b819cbbf7 100644 --- a/core/shortcut_registry.ts +++ b/core/shortcut_registry.ts @@ -12,6 +12,8 @@ */ // Former goog.module ID: Blockly.ShortcutRegistry +import {Scope} from './contextmenu_registry.js'; +import {getFocusManager} from './focus_manager.js'; import {KeyCodes} from './utils/keycodes.js'; import * as object from './utils/object.js'; import {WorkspaceSvg} from './workspace_svg.js'; @@ -249,12 +251,20 @@ export class ShortcutRegistry { const shortcut = this.shortcuts.get(shortcutName); if ( !shortcut || - (shortcut.preconditionFn && !shortcut.preconditionFn(workspace)) + (shortcut.preconditionFn && + !shortcut.preconditionFn(workspace, { + focusedNode: getFocusManager().getFocusedNode(), + })) ) { continue; } // If the key has been handled, stop processing shortcuts. - if (shortcut.callback?.(workspace, e, shortcut)) return true; + if ( + shortcut.callback?.(workspace, e, shortcut, { + focusedNode: getFocusManager().getFocusedNode(), + }) + ) + return true; } return false; } @@ -372,6 +382,8 @@ export namespace ShortcutRegistry { * @param e The event that caused the shortcut to be activated. * @param shortcut The `KeyboardShortcut` that was activated * (i.e., the one this callback is attached to). + * @param scope Information about the focused item when the + * shortcut was invoked. * @returns Returning true ends processing of the invoked keycode. * Returning false causes processing to continue with the * next-most-recently registered shortcut for the invoked @@ -381,6 +393,7 @@ export namespace ShortcutRegistry { workspace: WorkspaceSvg, e: Event, shortcut: KeyboardShortcut, + scope: Scope, ) => boolean; /** The name of the shortcut. Should be unique. */ @@ -393,9 +406,11 @@ export namespace ShortcutRegistry { * * @param workspace The `WorkspaceSvg` where the shortcut was * invoked. + * @param scope Information about the focused item when the + * shortcut would be invoked. * @returns True iff `callback` function should be called. */ - preconditionFn?: (workspace: WorkspaceSvg) => boolean; + preconditionFn?: (workspace: WorkspaceSvg, scope: Scope) => boolean; /** Optional arbitray extra data attached to the shortcut. */ metadata?: object; diff --git a/tests/mocha/keydown_test.js b/tests/mocha/keydown_test.js index 82293f224..4ad0da2d4 100644 --- a/tests/mocha/keydown_test.js +++ b/tests/mocha/keydown_test.js @@ -31,6 +31,7 @@ suite('Key Down', function () { defineStackBlock(); const block = workspace.newBlock('stack_block'); Blockly.common.setSelected(block); + sinon.stub(Blockly.getFocusManager(), 'getFocusedNode').returns(block); return block; } diff --git a/tests/mocha/shortcut_registry_test.js b/tests/mocha/shortcut_registry_test.js index 37c1b9c20..9f412be33 100644 --- a/tests/mocha/shortcut_registry_test.js +++ b/tests/mocha/shortcut_registry_test.js @@ -5,6 +5,7 @@ */ import {assert} from '../../node_modules/chai/chai.js'; +import {createTestBlock} from './test_helpers/block_definitions.js'; import { sharedTestSetup, sharedTestTeardown, @@ -299,7 +300,7 @@ suite('Keyboard Shortcut Registry Test', function () { 'callback': function () { return true; }, - 'precondition': function () { + 'preconditionFn': function () { return true; }, }; @@ -319,6 +320,27 @@ suite('Keyboard Shortcut Registry Test', function () { const event = createKeyDownEvent(Blockly.utils.KeyCodes.D); assert.isFalse(this.registry.onKeyDown(this.workspace, event)); }); + test('No callback if precondition fails', function () { + const shortcut = { + 'name': 'test_shortcut', + 'callback': function () { + return true; + }, + 'preconditionFn': function () { + return false; + }, + }; + const callBackStub = addShortcut( + this.registry, + shortcut, + Blockly.utils.KeyCodes.C, + true, + ); + const event = createKeyDownEvent(Blockly.utils.KeyCodes.C); + assert.isFalse(this.registry.onKeyDown(this.workspace, event)); + sinon.assert.notCalled(callBackStub); + }); + test('No precondition available - execute callback', function () { delete this.testShortcut['precondition']; const event = createKeyDownEvent(Blockly.utils.KeyCodes.C); @@ -332,8 +354,8 @@ suite('Keyboard Shortcut Registry Test', function () { 'callback': function () { return false; }, - 'precondition': function () { - return false; + 'preconditionFn': function () { + return true; }, }; const testShortcut2Stub = addShortcut( @@ -353,8 +375,8 @@ suite('Keyboard Shortcut Registry Test', function () { 'callback': function () { return false; }, - 'precondition': function () { - return false; + 'preconditionFn': function () { + return true; }, }; const testShortcut2Stub = addShortcut( @@ -367,6 +389,63 @@ suite('Keyboard Shortcut Registry Test', function () { sinon.assert.calledOnce(testShortcut2Stub); sinon.assert.notCalled(this.callBackStub); }); + suite('interaction with FocusManager', function () { + setup(function () { + this.testShortcutWithScope = { + 'name': 'test_shortcut', + 'callback': function (workspace, e, shortcut, scope) { + return true; + }, + 'preconditionFn': function (workspace, scope) { + return true; + }, + }; + + // Stub the focus manager + this.focusedBlock = createTestBlock(); + sinon + .stub(Blockly.getFocusManager(), 'getFocusedNode') + .returns(this.focusedBlock); + }); + test('Callback receives the focused node', function () { + const event = createKeyDownEvent(Blockly.utils.KeyCodes.C); + const callbackStub = addShortcut( + this.registry, + this.testShortcutWithScope, + Blockly.utils.KeyCodes.C, + true, + ); + this.registry.onKeyDown(this.workspace, event); + + const expectedScope = {focusedNode: this.focusedBlock}; + sinon.assert.calledWithExactly( + callbackStub, + this.workspace, + event, + this.testShortcutWithScope, + expectedScope, + ); + }); + test('Precondition receives the focused node', function () { + const event = createKeyDownEvent(Blockly.utils.KeyCodes.C); + const callbackStub = addShortcut( + this.registry, + this.testShortcutWithScope, + Blockly.utils.KeyCodes.C, + true, + ); + const preconditionStub = sinon + .stub(this.testShortcutWithScope, 'preconditionFn') + .returns(true); + this.registry.onKeyDown(this.workspace, event); + const expectedScope = {focusedNode: this.focusedBlock}; + sinon.assert.calledWithExactly( + preconditionStub, + this.workspace, + expectedScope, + ); + }); + }); }); suite('createSerializedKey', function () {