From d318e4659c344cbc9461371fd601cf371ded042a Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 8 Jul 2025 12:58:28 -0700 Subject: [PATCH 1/7] fix: paste into correct workspace --- src/actions/clipboard.ts | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/actions/clipboard.ts b/src/actions/clipboard.ts index 7ef4d2fa..9a0679c0 100644 --- a/src/actions/clipboard.ts +++ b/src/actions/clipboard.ts @@ -11,6 +11,7 @@ import { Msg, ShortcutItems, WorkspaceSvg, + clipboard, } from 'blockly'; import * as Constants from '../constants'; import {Navigation} from '../navigation'; @@ -31,9 +32,6 @@ const BASE_WEIGHT = 12; * In the long term, this will likely merge with the clipboard code in core. */ export class Clipboard { - /** The workspace a copy or cut keyboard shortcut happened in. */ - private copyWorkspace: WorkspaceSvg | null = null; - private oldCutShortcut: ShortcutRegistry.KeyboardShortcut | undefined; private oldCopyShortcut: ShortcutRegistry.KeyboardShortcut | undefined; private oldPasteShortcut: ShortcutRegistry.KeyboardShortcut | undefined; @@ -174,11 +172,23 @@ export class Clipboard { * @returns 'enabled' if the node can be pasted, 'disabled' otherwise. */ private pastePrecondition(scope: ContextMenuRegistry.Scope): string { - if (!this.copyWorkspace) return 'disabled'; + // Only show the paste option on the context menu for a workspace. + if (!(scope.focusedNode instanceof WorkspaceSvg)) return 'hidden'; + const workspace = scope.focusedNode; + + // Don't paste into flyouts. + if (workspace.isFlyout) return 'hidden'; + + // Only paste into the same workspace that was copied from + // or the parent workspace of a flyout that was copied from. + let copiedWorkspace = clipboard.getLastCopiedWorkspace(); + if (copiedWorkspace?.isFlyout) + copiedWorkspace = copiedWorkspace.targetWorkspace; + if (copiedWorkspace !== workspace) return 'disabled'; if ( this.oldPasteShortcut?.preconditionFn && - this.oldPasteShortcut.preconditionFn(this.copyWorkspace, scope) + this.oldPasteShortcut.preconditionFn(workspace, scope) ) { return 'enabled'; } @@ -207,7 +217,6 @@ export class Clipboard { !!this.oldCutShortcut?.callback && this.oldCutShortcut.callback(workspace, e, shortcut, scope); if (didCut) { - this.copyWorkspace = workspace; showCutHint(workspace); } return didCut; @@ -285,9 +294,6 @@ export class Clipboard { !!this.oldCopyShortcut?.callback && this.oldCopyShortcut.callback(workspace, e, shortcut, scope); if (didCopy) { - this.copyWorkspace = workspace.isFlyout - ? workspace.targetWorkspace - : workspace; showCopiedHint(workspace); } return didCopy; @@ -307,8 +313,6 @@ export class Clipboard { name: Constants.SHORTCUT_NAMES.PASTE, preconditionFn: this.oldPasteShortcut.preconditionFn, callback: this.pasteCallback.bind(this), - // The registry gives back keycodes as an object instead of an array - // See https://github.com/google/blockly/issues/9008 keyCodes: this.oldPasteShortcut.keyCodes, allowCollision: false, }; @@ -330,8 +334,8 @@ export class Clipboard { getMenuItem(Msg['PASTE_SHORTCUT'], Constants.SHORTCUT_NAMES.PASTE), preconditionFn: (scope) => this.pastePrecondition(scope), callback: (scope: ContextMenuRegistry.Scope, menuOpenEvent: Event) => { - const workspace = this.copyWorkspace; - if (!workspace) return; + const workspace = scope.focusedNode; + if (!(workspace instanceof WorkspaceSvg)) return false; return this.pasteCallback(workspace, menuOpenEvent, undefined, scope); }, id: 'blockPasteFromContextMenu', From 0d692edd24b86b786d57a9c79b6b97251eac13d5 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 8 Jul 2025 13:01:21 -0700 Subject: [PATCH 2/7] chore: remove outdated comment --- src/actions/clipboard.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/actions/clipboard.ts b/src/actions/clipboard.ts index 9a0679c0..f60c74d5 100644 --- a/src/actions/clipboard.ts +++ b/src/actions/clipboard.ts @@ -82,8 +82,6 @@ export class Clipboard { name: Constants.SHORTCUT_NAMES.CUT, preconditionFn: this.oldCutShortcut.preconditionFn, callback: this.cutCallback.bind(this), - // The registry gives back keycodes as an object instead of an array - // See https://github.com/google/blockly/issues/9008 keyCodes: this.oldCutShortcut.keyCodes, allowCollision: false, }; @@ -236,8 +234,6 @@ export class Clipboard { name: Constants.SHORTCUT_NAMES.COPY, preconditionFn: this.oldCopyShortcut.preconditionFn, callback: this.copyCallback.bind(this), - // The registry gives back keycodes as an object instead of an array - // See https://github.com/google/blockly/issues/9008 keyCodes: this.oldCopyShortcut.keyCodes, allowCollision: false, }; From 8af1489b4a95e242d896c031f41565cb19fbc869 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 8 Jul 2025 15:13:31 -0700 Subject: [PATCH 3/7] feat: add option for allowing cross-tab-copy-paste --- src/actions/clipboard.ts | 46 ++++++++++++++++++++++++++---------- src/index.ts | 14 +++++++++-- src/navigation_controller.ts | 10 +++++++- 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/actions/clipboard.ts b/src/actions/clipboard.ts index f60c74d5..dfddf483 100644 --- a/src/actions/clipboard.ts +++ b/src/actions/clipboard.ts @@ -12,6 +12,7 @@ import { ShortcutItems, WorkspaceSvg, clipboard, + isSelectable, } from 'blockly'; import * as Constants from '../constants'; import {Navigation} from '../navigation'; @@ -36,7 +37,12 @@ export class Clipboard { private oldCopyShortcut: ShortcutRegistry.KeyboardShortcut | undefined; private oldPasteShortcut: ShortcutRegistry.KeyboardShortcut | undefined; - constructor(private navigation: Navigation) {} + constructor( + private navigation: Navigation, + private options: {allowCrossWorkspacePaste: boolean} = { + allowCrossWorkspacePaste: false, + }, + ) {} /** * Install these actions as both keyboard shortcuts and context menu items. @@ -162,6 +168,20 @@ export class Clipboard { return 'disabled'; } + private getPasteWorkspace( + scope: ContextMenuRegistry.Scope, + ): WorkspaceSvg | undefined { + let workspace; + if (scope.focusedNode instanceof WorkspaceSvg) { + workspace = scope.focusedNode; + } else if (isSelectable(scope.focusedNode)) { + workspace = scope.focusedNode.workspace; + } + + if (!workspace || !(workspace instanceof WorkspaceSvg)) return undefined; + return workspace; + } + /** * Precondition function for the paste context menu. This wraps the core * paste precondition to support context menus. @@ -170,19 +190,21 @@ export class Clipboard { * @returns 'enabled' if the node can be pasted, 'disabled' otherwise. */ private pastePrecondition(scope: ContextMenuRegistry.Scope): string { - // Only show the paste option on the context menu for a workspace. - if (!(scope.focusedNode instanceof WorkspaceSvg)) return 'hidden'; - const workspace = scope.focusedNode; + const workspace = this.getPasteWorkspace(scope); + // If we can't identify what workspace to paste into, hide. + if (!workspace) return 'hidden'; // Don't paste into flyouts. if (workspace.isFlyout) return 'hidden'; - // Only paste into the same workspace that was copied from - // or the parent workspace of a flyout that was copied from. - let copiedWorkspace = clipboard.getLastCopiedWorkspace(); - if (copiedWorkspace?.isFlyout) - copiedWorkspace = copiedWorkspace.targetWorkspace; - if (copiedWorkspace !== workspace) return 'disabled'; + if (!this.options.allowCrossWorkspacePaste) { + // Only paste into the same workspace that was copied from + // or the parent workspace of a flyout that was copied from. + let copiedWorkspace = clipboard.getLastCopiedWorkspace(); + if (copiedWorkspace?.isFlyout) + copiedWorkspace = copiedWorkspace.targetWorkspace; + if (copiedWorkspace !== workspace) return 'disabled'; + } if ( this.oldPasteShortcut?.preconditionFn && @@ -330,8 +352,8 @@ export class Clipboard { getMenuItem(Msg['PASTE_SHORTCUT'], Constants.SHORTCUT_NAMES.PASTE), preconditionFn: (scope) => this.pastePrecondition(scope), callback: (scope: ContextMenuRegistry.Scope, menuOpenEvent: Event) => { - const workspace = scope.focusedNode; - if (!(workspace instanceof WorkspaceSvg)) return false; + const workspace = this.getPasteWorkspace(scope); + if (!workspace) return false; return this.pasteCallback(workspace, menuOpenEvent, undefined, scope); }, id: 'blockPasteFromContextMenu', diff --git a/src/index.ts b/src/index.ts index 09d6c33e..840f87d8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -41,11 +41,21 @@ export class KeyboardNavigation { * Constructs the keyboard navigation. * * @param workspace The workspace that the plugin will be added to. + * @param options Options for plugin + * @param options.allowCrossWorkspacePaste If true, will allow paste + * option to appear enabled when pasting in a different workspace + * than was copied from. Defaults to false. Set to true if using + * cross-tab-copy-paste plugin or similar. */ - constructor(workspace: Blockly.WorkspaceSvg) { + constructor( + workspace: Blockly.WorkspaceSvg, + options: {allowCrossWorkspacePaste: boolean} = { + allowCrossWorkspacePaste: false, + }, + ) { this.workspace = workspace; - this.navigationController = new NavigationController(); + this.navigationController = new NavigationController(options); this.navigationController.init(); this.navigationController.addWorkspace(workspace); this.navigationController.enable(workspace); diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 2641800a..74ff3bce 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -58,7 +58,7 @@ export class NavigationController { /** Keyboard shortcut for disconnection. */ disconnectAction: DisconnectAction = new DisconnectAction(this.navigation); - clipboard: Clipboard = new Clipboard(this.navigation); + clipboard: Clipboard; duplicateAction = new DuplicateAction(); @@ -75,6 +75,14 @@ export class NavigationController { moveActions = new MoveActions(this.mover); + constructor( + private options: {allowCrossWorkspacePaste: boolean} = { + allowCrossWorkspacePaste: false, + }, + ) { + this.clipboard = new Clipboard(this.navigation, options); + } + /** * Original Toolbox.prototype.onShortcut method, saved by * addShortcutHandlers. From 9c1f7d7442b6c90abca8dd947b66d3c4a2dd83ec Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 8 Jul 2025 15:17:09 -0700 Subject: [PATCH 4/7] chore: put the clipboard methods into a sane order --- src/actions/clipboard.ts | 145 ++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 70 deletions(-) diff --git a/src/actions/clipboard.ts b/src/actions/clipboard.ts index dfddf483..ce40ac34 100644 --- a/src/actions/clipboard.ts +++ b/src/actions/clipboard.ts @@ -145,76 +145,6 @@ export class Clipboard { return 'disabled'; } - /** - * Precondition function for the copy context menu. This wraps the core copy - * precondition to support context menus. - * - * @param scope scope of the shortcut or context menu item - * @returns 'enabled' if the node can be copied, 'disabled' otherwise. - */ - private copyPrecondition(scope: ContextMenuRegistry.Scope): string { - const focused = scope.focusedNode; - if (!focused || !isCopyable(focused)) return 'hidden'; - - const workspace = focused.workspace; - if (!(workspace instanceof WorkspaceSvg)) return 'hidden'; - - if ( - this.oldCopyShortcut?.preconditionFn && - this.oldCopyShortcut.preconditionFn(workspace, scope) - ) { - return 'enabled'; - } - return 'disabled'; - } - - private getPasteWorkspace( - scope: ContextMenuRegistry.Scope, - ): WorkspaceSvg | undefined { - let workspace; - if (scope.focusedNode instanceof WorkspaceSvg) { - workspace = scope.focusedNode; - } else if (isSelectable(scope.focusedNode)) { - workspace = scope.focusedNode.workspace; - } - - if (!workspace || !(workspace instanceof WorkspaceSvg)) return undefined; - return workspace; - } - - /** - * Precondition function for the paste context menu. This wraps the core - * paste precondition to support context menus. - * - * @param scope scope of the shortcut or context menu item - * @returns 'enabled' if the node can be pasted, 'disabled' otherwise. - */ - private pastePrecondition(scope: ContextMenuRegistry.Scope): string { - const workspace = this.getPasteWorkspace(scope); - // If we can't identify what workspace to paste into, hide. - if (!workspace) return 'hidden'; - - // Don't paste into flyouts. - if (workspace.isFlyout) return 'hidden'; - - if (!this.options.allowCrossWorkspacePaste) { - // Only paste into the same workspace that was copied from - // or the parent workspace of a flyout that was copied from. - let copiedWorkspace = clipboard.getLastCopiedWorkspace(); - if (copiedWorkspace?.isFlyout) - copiedWorkspace = copiedWorkspace.targetWorkspace; - if (copiedWorkspace !== workspace) return 'disabled'; - } - - if ( - this.oldPasteShortcut?.preconditionFn && - this.oldPasteShortcut.preconditionFn(workspace, scope) - ) { - return 'enabled'; - } - return 'disabled'; - } - /** * The callback for the cut action. Uses the registered version of the cut callback * to perform the cut logic, then pops a toast if cut happened. @@ -290,6 +220,29 @@ export class Clipboard { ContextMenuRegistry.registry.register(copyAction); } + /** + * Precondition function for the copy context menu. This wraps the core copy + * precondition to support context menus. + * + * @param scope scope of the shortcut or context menu item + * @returns 'enabled' if the node can be copied, 'disabled' otherwise. + */ + private copyPrecondition(scope: ContextMenuRegistry.Scope): string { + const focused = scope.focusedNode; + if (!focused || !isCopyable(focused)) return 'hidden'; + + const workspace = focused.workspace; + if (!(workspace instanceof WorkspaceSvg)) return 'hidden'; + + if ( + this.oldCopyShortcut?.preconditionFn && + this.oldCopyShortcut.preconditionFn(workspace, scope) + ) { + return 'enabled'; + } + return 'disabled'; + } + /** * The callback for the copy action. Uses the registered version of the copy callback * to perform the copy logic, then pops a toast if copy happened. @@ -363,6 +316,58 @@ export class Clipboard { ContextMenuRegistry.registry.register(pasteAction); } + /** + * Get the workspace to paste into based on which type of thing the menu was opened on. + * + * @returns WorkspaceSvg to paste into or undefined + */ + private getPasteWorkspace( + scope: ContextMenuRegistry.Scope, + ): WorkspaceSvg | undefined { + let workspace; + if (scope.focusedNode instanceof WorkspaceSvg) { + workspace = scope.focusedNode; + } else if (isSelectable(scope.focusedNode)) { + workspace = scope.focusedNode.workspace; + } + + if (!workspace || !(workspace instanceof WorkspaceSvg)) return undefined; + return workspace; + } + + /** + * Precondition function for the paste context menu. This wraps the core + * paste precondition to support context menus. + * + * @param scope scope of the shortcut or context menu item + * @returns 'enabled' if the node can be pasted, 'disabled' otherwise. + */ + private pastePrecondition(scope: ContextMenuRegistry.Scope): string { + const workspace = this.getPasteWorkspace(scope); + // If we can't identify what workspace to paste into, hide. + if (!workspace) return 'hidden'; + + // Don't paste into flyouts. + if (workspace.isFlyout) return 'hidden'; + + if (!this.options.allowCrossWorkspacePaste) { + // Only paste into the same workspace that was copied from + // or the parent workspace of a flyout that was copied from. + let copiedWorkspace = clipboard.getLastCopiedWorkspace(); + if (copiedWorkspace?.isFlyout) + copiedWorkspace = copiedWorkspace.targetWorkspace; + if (copiedWorkspace !== workspace) return 'disabled'; + } + + if ( + this.oldPasteShortcut?.preconditionFn && + this.oldPasteShortcut.preconditionFn(workspace, scope) + ) { + return 'enabled'; + } + return 'disabled'; + } + /** * The callback for the paste action. Uses the registered version of the paste callback * to perform the paste logic, then clears any toasts about pasting. From 980797c9357dfc57c367c23c840b0859ba6bda94 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 8 Jul 2025 15:25:01 -0700 Subject: [PATCH 5/7] chore: update readme --- README.md | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4d221616..6e19d248 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ on this repository! Include information about how to reproduce the bug, what the bad behaviour was, and what you expected it to do. The Blockly team will triage the bug and add it to the roadmap. -## Testing in your app +## Using in your app ### Installation @@ -67,7 +67,7 @@ npm install @blockly/keyboard-navigation --save ```js import * as Blockly from 'blockly'; -import {KeyboardNavigation} from '@blockly/keyboard-experiment'; +import {KeyboardNavigation} from '@blockly/keyboard-navigation'; // Inject Blockly. const workspace = Blockly.inject('blocklyDiv', { toolbox: toolboxCategories, @@ -76,6 +76,41 @@ const workspace = Blockly.inject('blocklyDiv', { const keyboardNav = new KeyboardNavigation(workspace); ``` +### Usage with cross-tab-copy-paste plugin + +This plugin adds context menu items for copying & pasting. It also adds feedback to copying & pasting as toasts that are shown to the user upon successful copy or cut. It is compatible with the `@blockly/plugin-cross-tab-copy-paste` by following these steps: + +```js +import * as Blockly from 'blockly'; +import {KeyboardNavigation} from '@blockly/keyboard-navigation'; +import {CrossTabCopyPaste} from '@blockly/plugin-cross-tab-copy-paste'; + +// Inject Blockly. +const workspace = Blockly.inject('blocklyDiv', { + toolbox: toolboxCategories, +}); + +// Initialize cross-tab-copy-paste +// Must be done before keyboard-navigation +const crossTabOptions = { + // Don't use the context menu options from the ctcp plugin, + // because the keyboard-navigation plugin provides its own. + contextMenu: false, + shortcut: true, +}; +const plugin = new CrossTabCopyPaste(); +plugin.init(crossTabOptions, () => { + console.log('Use this error callback to handle TypeError while pasting'); +}); + +// Initialize keyboard-navigation. +// You must pass the `allowCrossWorkspacePaste` option in order for paste +// to appear correctly enabled/disabled in the context menu. +const keyboardNav = new KeyboardNavigation(workspace, { + allowCrossWorkspacePaste: true, +}); +``` + ## Contributing To learn more about contributing to this project, see the [contributing page](https://github.com/google/blockly-keyboard-experimentation/blob/main/CONTRIBUTING.md). From 016cb1a7718aa8729cd5b90fc43f301dcb0cfee7 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 8 Jul 2025 16:13:16 -0700 Subject: [PATCH 6/7] chore: format and lint --- src/actions/clipboard.ts | 1 + src/navigation_controller.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/actions/clipboard.ts b/src/actions/clipboard.ts index ce40ac34..ad67833b 100644 --- a/src/actions/clipboard.ts +++ b/src/actions/clipboard.ts @@ -319,6 +319,7 @@ export class Clipboard { /** * Get the workspace to paste into based on which type of thing the menu was opened on. * + * @param scope scope of shortcut or context menu item * @returns WorkspaceSvg to paste into or undefined */ private getPasteWorkspace( diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index b8427f59..3c29c128 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -75,7 +75,7 @@ export class NavigationController { actionMenu: ActionMenu = new ActionMenu(this.navigation); moveActions = new MoveActions(this.mover); - + stackNavigationAction: StackNavigationAction = new StackNavigationAction(); constructor( From a86a8eea8f619a7d552063a02de4932103fbb7c4 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Tue, 8 Jul 2025 16:20:33 -0700 Subject: [PATCH 7/7] chore: fix test --- test/webdriverio/test/actions_test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/webdriverio/test/actions_test.ts b/test/webdriverio/test/actions_test.ts index cf33e322..7f9b27e2 100644 --- a/test/webdriverio/test/actions_test.ts +++ b/test/webdriverio/test/actions_test.ts @@ -86,14 +86,12 @@ suite('Menus test', function () { {'disabled': true, 'text': 'Move Block M'}, {'disabled': true, 'text': 'Cut ⌘ X'}, {'text': 'Copy ⌘ C'}, - {'disabled': true, 'text': 'Paste ⌘ V'}, ] : [ {'text': 'Help'}, {'disabled': true, 'text': 'Move Block M'}, {'disabled': true, 'text': 'Cut Ctrl + X'}, {'text': 'Copy Ctrl + C'}, - {'disabled': true, 'text': 'Paste Ctrl + V'}, ], await contextMenuItems(this.browser), );