Merge branch 'main' into toast

Author: microbit-matt-hillsdon

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@blockly/keyboard-experiment",
-  "version": "0.0.4",
+  "version": "0.0.5",
   "description": "A plugin for keyboard navigation.",
   "scripts": {
     "audit:fix": "blockly-scripts auditFix",
@@ -48,7 +48,7 @@
     "@types/p5": "^1.7.6",
     "@typescript-eslint/eslint-plugin": "^6.7.2",
     "@typescript-eslint/parser": "^6.7.2",
-    "blockly": "^11.2.1",
+    "blockly": "12.0.0-beta.2",
     "eslint": "^8.49.0",
     "eslint-config-google": "^0.14.0",
     "eslint-config-prettier": "^9.0.0",
@@ -61,7 +61,12 @@
     "typescript": "^5.4.5"
   },
   "peerDependencies": {
-    "blockly": "^11.1.0"
+    "blockly": "^12.0.0-beta.2"
+  },
+  "overrides": {
+    "@blockly/field-colour": {
+      "blockly": "^12.0.0-beta.2"
+    }
   },
   "publishConfig": {
     "access": "public",

package.json

@@ -15,7 +15,7 @@ import {
   WidgetDiv,
 } from 'blockly';
 import * as Constants from '../constants';
-import type {BlockSvg, WorkspaceSvg} from 'blockly';
+import type {BlockSvg, RenderedConnection, WorkspaceSvg} from 'blockly';
 import {Navigation} from '../navigation';
 
 const KeyCodes = BlocklyUtils.KeyCodes;
@@ -111,6 +111,7 @@ export class ActionMenu {
     const cursor = workspace.getCursor();
     if (!cursor) throw new Error('workspace has no cursor');
     const node = cursor.getCurNode();
+    if (!node) throw new Error('No node is currently selected');
     const nodeType = node.getType();
     switch (nodeType) {
       case ASTNode.types.BLOCK:
@@ -140,34 +141,7 @@ export class ActionMenu {
         // Slightly hacky: get insert action from registry.  Hacky
         // because registry typings don't include {connection: ...} as
         // a possible kind of scope.
-        const insertAction = ContextMenuRegistry.registry.getItem('insert');
-        if (!insertAction) throw new Error("can't find insert action");
-
-        const pasteAction = ContextMenuRegistry.registry.getItem(
-          'blockPasteFromContextMenu',
-        );
-        if (!pasteAction) throw new Error("can't find paste action");
-        const possibleOptions = [insertAction, pasteAction /* etc.*/];
-
-        // Check preconditions and get menu texts.
-        const scope = {
-          connection,
-        } as unknown as ContextMenuRegistry.Scope;
-        for (const option of possibleOptions) {
-          const precondition = option.preconditionFn(scope);
-          if (precondition === 'hidden') continue;
-          const displayText =
-            typeof option.displayText === 'function'
-              ? option.displayText(scope)
-              : option.displayText;
-          menuOptions.push({
-            text: displayText,
-            enabled: precondition === 'enabled',
-            callback: option.callback,
-            scope,
-            weight: option.weight,
-          });
-        }
+        this.addConnectionItems(connection, menuOptions);
         break;
 
       default:
@@ -195,6 +169,50 @@ export class ActionMenu {
     return true;
   }
 
+  /**
+   * Add menu items for a context menu on a connection scope.
+   *
+   * @param connection The connection on which the menu is shown.
+   * @param menuOptions The list of options, which may be modified by this method.
+   */
+  private addConnectionItems(
+    connection: Connection,
+    menuOptions: (
+      | ContextMenuRegistry.ContextMenuOption
+      | ContextMenuRegistry.LegacyContextMenuOption
+    )[],
+  ) {
+    const insertAction = ContextMenuRegistry.registry.getItem('insert');
+    if (!insertAction) throw new Error("can't find insert action");
+
+    const pasteAction = ContextMenuRegistry.registry.getItem(
+      'blockPasteFromContextMenu',
+    );
+    if (!pasteAction) throw new Error("can't find paste action");
+    const possibleOptions = [insertAction, pasteAction /* etc.*/];
+
+    // Check preconditions and get menu texts.
+    const scope = {
+      connection,
+    } as unknown as ContextMenuRegistry.Scope;
+    for (const option of possibleOptions) {
+      const precondition = option.preconditionFn?.(scope);
+      if (precondition === 'hidden') continue;
+      const displayText =
+        (typeof option.displayText === 'function'
+          ? option.displayText(scope)
+          : option.displayText) ?? '';
+      menuOptions.push({
+        text: displayText,
+        enabled: precondition === 'enabled',
+        callback: option.callback!,
+        scope,
+        weight: option.weight,
+      });
+    }
+    return menuOptions;
+  }
+
   /**
    * Create a fake PointerEvent for opening the action menu for the
    * given ASTNode.
@@ -205,31 +223,28 @@ export class ActionMenu {
   private fakeEventForNode(node: ASTNode): PointerEvent {
     switch (node.getType()) {
       case ASTNode.types.BLOCK:
-        return this.fakeEventForBlockNode(node);
+        return this.fakeEventForBlock(node.getLocation() as BlockSvg);
       case ASTNode.types.NEXT:
       case ASTNode.types.PREVIOUS:
       case ASTNode.types.INPUT:
-        return this.fakeEventForConnectionNode(node);
+        return this.fakeEventForConnectionNode(
+          node.getLocation() as RenderedConnection,
+        );
       default:
         throw new TypeError('unhandled node type');
     }
   }
 
   /**
-   * Create a fake PointerEvent for opening the action menu for the
-   * given ASTNode of type BLOCK.
+   * Create a fake PointerEvent for opening the action menu on the specified
+   * block.
    *
-   * @param node The node to open the action menu for.
+   * @param block The block to open the action menu for.
    * @returns A synthetic pointerdown PointerEvent.
    */
-  private fakeEventForBlockNode(node: ASTNode): PointerEvent {
-    if (node.getType() !== ASTNode.types.BLOCK) {
-      throw new TypeError('can only create PointerEvents for BLOCK nodes');
-    }
-
+  private fakeEventForBlock(block: BlockSvg) {
     // Get the location of the top-left corner of the block in
     // screen coordinates.
-    const block = node.getLocation() as BlockSvg;
     const blockCoords = BlocklyUtils.svgMath.wsToScreenCoordinates(
       block.workspace,
       block.getRelativeToSurfaceXY(),
@@ -257,31 +272,23 @@ export class ActionMenu {
 
   /**
    * Create a fake PointerEvent for opening the action menu for the
-   * given ASTNode of type NEXT, PREVIOUS or INPUT.
+   * given connection.
    *
    * For now this just puts the action menu in the same place as the
    * context menu for the source block.
    *
-   * @param node The node to open the action menu for.
+   * @param connection The node to open the action menu for.
    * @returns A synthetic pointerdown PointerEvent.
    */
-  private fakeEventForConnectionNode(node: ASTNode): PointerEvent {
-    if (
-      node.getType() !== ASTNode.types.NEXT &&
-      node.getType() !== ASTNode.types.PREVIOUS &&
-      node.getType() !== ASTNode.types.INPUT
-    ) {
-      throw new TypeError('can only create PointerEvents for connection nodes');
-    }
-
-    const connection = node.getLocation() as Connection;
-    const block = connection.getSourceBlock();
+  private fakeEventForConnectionNode(
+    connection: RenderedConnection,
+  ): PointerEvent {
+    const block = connection.getSourceBlock() as BlockSvg;
     const workspace = block.workspace as WorkspaceSvg;
 
     if (typeof connection.x !== 'number') {
       // No coordinates for connection?  Fall back to the parent block.
-      const blockNode = new ASTNode(ASTNode.types.BLOCK, block);
-      return this.fakeEventForBlockNode(blockNode);
+      return this.fakeEventForBlock(block);
     }
     const connectionWSCoords = new BlocklyUtils.Coordinate(
       connection.x,

src/actions/action_menu.ts

@@ -40,7 +40,7 @@ export class ArrowNavigation {
       return false;
     }
     const curNode = cursor.getCurNode();
-    if (curNode.getType() === ASTNode.types.FIELD) {
+    if (curNode?.getType() === ASTNode.types.FIELD) {
       return (curNode.getLocation() as Field).onShortcut(shortcut);
     }
     return false;
@@ -64,7 +64,9 @@ export class ArrowNavigation {
             case Constants.STATE.WORKSPACE:
               isHandled = this.fieldShortcutHandler(workspace, shortcut);
               if (!isHandled && workspace) {
-                workspace.getCursor()?.in();
+                if (!this.navigation.defaultCursorPositionIfNeeded(workspace)) {
+                  workspace.getCursor()?.in();
+                }
                 isHandled = true;
               }
               return isHandled;
@@ -95,7 +97,9 @@ export class ArrowNavigation {
             case Constants.STATE.WORKSPACE:
               isHandled = this.fieldShortcutHandler(workspace, shortcut);
               if (!isHandled && workspace) {
-                workspace.getCursor()?.out();
+                if (!this.navigation.defaultCursorPositionIfNeeded(workspace)) {
+                  workspace.getCursor()?.out();
+                }
                 isHandled = true;
               }
               return isHandled;
@@ -125,7 +129,9 @@ export class ArrowNavigation {
             case Constants.STATE.WORKSPACE:
               isHandled = this.fieldShortcutHandler(workspace, shortcut);
               if (!isHandled && workspace) {
-                workspace.getCursor()?.next();
+                if (!this.navigation.defaultCursorPositionIfNeeded(workspace)) {
+                  workspace.getCursor()?.next();
+                }
                 isHandled = true;
               }
               return isHandled;
@@ -158,7 +164,14 @@ export class ArrowNavigation {
             case Constants.STATE.WORKSPACE:
               isHandled = this.fieldShortcutHandler(workspace, shortcut);
               if (!isHandled) {
-                workspace.getCursor()?.prev();
+                if (
+                  !this.navigation.defaultCursorPositionIfNeeded(
+                    workspace,
+                    'last',
+                  )
+                ) {
+                  workspace.getCursor()?.prev();
+                }
                 isHandled = true;
               }
               return isHandled;

src/actions/arrow_navigation.ts

@@ -32,7 +32,7 @@ const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind(
  * menu; changing individual weights relative to base weight can change
  * the order within the clipboard group.
  */
-const BASE_WEIGHT = 11;
+const BASE_WEIGHT = 12;
 
 /**
  * Logic and state for cut/copy/paste actions as both keyboard shortcuts
@@ -171,7 +171,9 @@ export class Clipboard {
   private cutCallback(workspace: WorkspaceSvg) {
     const cursor = workspace.getCursor();
     if (!cursor) throw new TypeError('no cursor');
-    const sourceBlock = cursor.getCurNode().getSourceBlock() as BlockSvg | null;
+    const sourceBlock = cursor
+      .getCurNode()
+      ?.getSourceBlock() as BlockSvg | null;
     if (!sourceBlock) throw new TypeError('no source block');
     this.copyData = sourceBlock.toCopyData();
     this.copyWorkspace = sourceBlock.workspace;
@@ -276,17 +278,22 @@ export class Clipboard {
     const sourceBlock = activeWorkspace
       ?.getCursor()
       ?.getCurNode()
-      .getSourceBlock() as BlockSvg;
-    workspace.hideChaff();
+      ?.getSourceBlock() as BlockSvg;
+    if (!sourceBlock) return false;
+
     this.copyData = sourceBlock.toCopyData();
     this.copyWorkspace = sourceBlock.workspace;
-    if (this.copyData) {
+    const copied = !!this.copyData;
+    if (copied) {
+      if (navigationState === Constants.STATE.FLYOUT) {
+        this.navigation.focusWorkspace(workspace);
+      }
       toast(workspace, {
-        message: `Copied. Press ${formatMetaShortcut("V")} to paste.`,
+        message: `Copied. Press ${formatMetaShortcut('V')} to paste.`,
         duration: 4500,
       });
     }
-    return !!this.copyData;
+    return copied;
   }
 
   /**
@@ -368,8 +375,10 @@ export class Clipboard {
       ? workspace
       : this.copyWorkspace;
 
-    // Do this before clipoard.paste due to cursor/focus workaround in getCurNode.
-    const targetNode = pasteWorkspace.getCursor()?.getCurNode();
+    const targetNode = this.navigation.getStationaryNode(pasteWorkspace);
+    // If we're pasting in the flyout it still targets the workspace. Focus first
+    // so ensure correct selection handling.
+    this.navigation.focusWorkspace(workspace);
 
     Events.setGroup(true);
     const block = clipboard.paste(this.copyData, pasteWorkspace) as BlockSvg;
@@ -381,7 +390,6 @@ export class Clipboard {
           ASTNode.createBlockNode(block)!,
         );
       }
-      this.navigation.removeMark(pasteWorkspace);
       Events.setGroup(false);
       return true;
     }