-
Notifications
You must be signed in to change notification settings - Fork 13
feat: duplicate shortcut for blocks and comments #642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
maribethb
merged 3 commits into
RaspberryPiFoundation:main
from
microbit-matt-hillsdon:duplicate
Jul 7, 2025
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { | ||
| BlockSvg, | ||
| clipboard, | ||
| ContextMenuRegistry, | ||
| ICopyable, | ||
| ShortcutRegistry, | ||
| utils, | ||
| comments, | ||
| ICopyData, | ||
| } from 'blockly'; | ||
| import * as Constants from '../constants'; | ||
| import {getMenuItem} from '../shortcut_formatting'; | ||
|
|
||
| /** | ||
| * Duplicate action that adds a keyboard shortcut for duplicate and overrides | ||
| * the context menu item to show it if the context menu item is registered. | ||
| */ | ||
| export class DuplicateAction { | ||
| private duplicateShortcut: ShortcutRegistry.KeyboardShortcut | null = null; | ||
| private uninstallHandlers: Array<() => void> = []; | ||
|
|
||
| /** | ||
| * Install the shortcuts and override context menu entries. | ||
| * | ||
| * No change is made if there's already a 'duplicate' shortcut. | ||
| */ | ||
| install() { | ||
| this.duplicateShortcut = this.registerDuplicateShortcut(); | ||
| if (this.duplicateShortcut) { | ||
| this.uninstallHandlers.push( | ||
| overrideContextMenuItemForShortcutText( | ||
| 'blockDuplicate', | ||
| Constants.SHORTCUT_NAMES.DUPLICATE, | ||
| ), | ||
| ); | ||
| this.uninstallHandlers.push( | ||
| overrideContextMenuItemForShortcutText( | ||
| 'commentDuplicate', | ||
| Constants.SHORTCUT_NAMES.DUPLICATE, | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Unregister the shortcut and reinstate the original context menu entries. | ||
| */ | ||
| uninstall() { | ||
| this.uninstallHandlers.forEach((handler) => handler()); | ||
| this.uninstallHandlers.length = 0; | ||
| if (this.duplicateShortcut) { | ||
| ShortcutRegistry.registry.unregister(this.duplicateShortcut.name); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Create and register the keyboard shortcut for the duplicate action. | ||
| * Same behaviour as for the core context menu. | ||
| * Skipped if there is a shortcut with a matching name already. | ||
| */ | ||
| private registerDuplicateShortcut(): ShortcutRegistry.KeyboardShortcut | null { | ||
| if ( | ||
| ShortcutRegistry.registry.getRegistry()[ | ||
| Constants.SHORTCUT_NAMES.DUPLICATE | ||
| ] | ||
| ) { | ||
| return null; | ||
| } | ||
|
|
||
| const shortcut: ShortcutRegistry.KeyboardShortcut = { | ||
| name: Constants.SHORTCUT_NAMES.DUPLICATE, | ||
| // Equivalent to the core context menu entry. | ||
| preconditionFn(workspace, scope) { | ||
| const {focusedNode} = scope; | ||
| if (focusedNode instanceof BlockSvg) { | ||
| return ( | ||
| !focusedNode.isInFlyout && | ||
| focusedNode.isDeletable() && | ||
| focusedNode.isMovable() && | ||
| focusedNode.isDuplicatable() | ||
| ); | ||
| } else if (focusedNode instanceof comments.RenderedWorkspaceComment) { | ||
| return focusedNode.isMovable(); | ||
| } | ||
| return false; | ||
| }, | ||
| callback(workspace, e, shortcut, scope) { | ||
| const copiable = scope.focusedNode as ICopyable<ICopyData>; | ||
| const data = copiable.toCopyData(); | ||
| if (!data) return false; | ||
| return !!clipboard.paste(data, workspace); | ||
| }, | ||
| keyCodes: [utils.KeyCodes.D], | ||
| }; | ||
| ShortcutRegistry.registry.register(shortcut); | ||
| return shortcut; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Replace a context menu item to add shortcut text to its displayText. | ||
| * | ||
| * Nothing happens if there is not a matching context menu item registered. | ||
| * | ||
| * @param registryId Context menu registry id to replace if present. | ||
| * @param shortcutName The corresponding shortcut name. | ||
| * @returns A function to reinstate the original context menu entry. | ||
| */ | ||
| function overrideContextMenuItemForShortcutText( | ||
| registryId: string, | ||
| shortcutName: string, | ||
| ): () => void { | ||
| const original = ContextMenuRegistry.registry.getItem(registryId); | ||
| if (!original || 'separator' in original) { | ||
| return () => {}; | ||
| } | ||
|
|
||
| const override: ContextMenuRegistry.RegistryItem = { | ||
| ...original, | ||
| displayText: (scope: ContextMenuRegistry.Scope) => { | ||
| const displayText = | ||
| typeof original.displayText === 'function' | ||
| ? original.displayText(scope) | ||
| : original.displayText; | ||
| if (displayText instanceof HTMLElement) { | ||
| // We can't cope in this scenario. | ||
| return displayText; | ||
| } | ||
| return getMenuItem(displayText, shortcutName); | ||
| }, | ||
| }; | ||
| ContextMenuRegistry.registry.unregister(registryId); | ||
| ContextMenuRegistry.registry.register(override); | ||
|
|
||
| return () => { | ||
| ContextMenuRegistry.registry.unregister(registryId); | ||
| ContextMenuRegistry.registry.register(original); | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ suite('Menus test', function () { | |
| await this.browser.keys([Key.Ctrl, Key.Return]); | ||
| await this.browser.pause(PAUSE_TIME); | ||
| chai.assert.isTrue( | ||
| await contextMenuExists(this.browser, 'Duplicate'), | ||
| await contextMenuExists(this.browser, 'Collapse Block'), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding a shortcut to duplicate meant that these didn't match anymore. Simplest to switch to something else without a shortcut. |
||
| 'The menu should be openable on a block', | ||
| ); | ||
| }); | ||
|
|
@@ -66,7 +66,7 @@ suite('Menus test', function () { | |
| await this.browser.keys([Key.Ctrl, Key.Return]); | ||
| await this.browser.pause(PAUSE_TIME); | ||
| chai.assert.isTrue( | ||
| await contextMenuExists(this.browser, 'Duplicate', true), | ||
| await contextMenuExists(this.browser, 'Collapse Block', true), | ||
| 'The menu should not be openable during a move', | ||
| ); | ||
| }); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import * as Blockly from 'blockly'; | ||
| import * as chai from 'chai'; | ||
| import { | ||
| focusOnBlock, | ||
| getCurrentFocusedBlockId, | ||
| getFocusedBlockType, | ||
| PAUSE_TIME, | ||
| tabNavigateToWorkspace, | ||
| testFileLocations, | ||
| testSetup, | ||
| } from './test_setup.js'; | ||
|
|
||
| suite('Duplicate test', function () { | ||
| // Setting timeout to unlimited as these tests take longer time to run | ||
| this.timeout(0); | ||
|
|
||
| // Clear the workspace and load start blocks | ||
| setup(async function () { | ||
| this.browser = await testSetup(testFileLocations.BASE); | ||
| await this.browser.pause(PAUSE_TIME); | ||
| }); | ||
|
|
||
| test('Duplicate block', async function () { | ||
| // Navigate to draw_circle_1. | ||
| await tabNavigateToWorkspace(this.browser); | ||
| await focusOnBlock(this.browser, 'draw_circle_1'); | ||
|
|
||
| // Duplicate | ||
| await this.browser.keys('d'); | ||
| await this.browser.pause(PAUSE_TIME); | ||
|
|
||
| // Check a different block of the same type has focus. | ||
| chai.assert.notEqual( | ||
| 'draw_circle_1', | ||
| await getCurrentFocusedBlockId(this.browser), | ||
| ); | ||
| chai.assert.equal('simple_circle', await getFocusedBlockType(this.browser)); | ||
| }); | ||
|
|
||
| test('Duplicate workspace comment', async function () { | ||
| await tabNavigateToWorkspace(this.browser); | ||
| const text = 'A comment with text'; | ||
|
|
||
| // Create a single comment. | ||
| await this.browser.execute((text: string) => { | ||
| const workspace = Blockly.getMainWorkspace(); | ||
| Blockly.serialization.workspaceComments.append( | ||
| { | ||
| text, | ||
| x: 200, | ||
| y: 200, | ||
| }, | ||
| workspace, | ||
| ); | ||
| Blockly.getFocusManager().focusNode( | ||
| (workspace as Blockly.WorkspaceSvg).getTopComments()[0], | ||
| ); | ||
| }, text); | ||
| await this.browser.pause(PAUSE_TIME); | ||
|
|
||
| // Duplicate. | ||
| await this.browser.keys('d'); | ||
|
|
||
| // Assert we have two comments with the same text. | ||
| const commentTexts = await this.browser.execute(() => | ||
| Blockly.getMainWorkspace() | ||
| .getTopComments(true) | ||
| .map((comment) => comment.getText()), | ||
| ); | ||
| chai.assert.deepEqual(commentTexts, [text, text]); | ||
| // Assert it's the duplicate that is focused (positioned below). | ||
| const [comment1, comment2] = await this.browser.$$('.blocklyComment'); | ||
| chai.assert.isTrue(await comment2.isFocused()); | ||
| chai.assert.isTrue( | ||
| (await comment2.getLocation()).y > (await comment1.getLocation()).y, | ||
| ); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you spell this
copyablefor consistency?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks. ec3964c