feat: keyboard navigation delete option in context menu (#159)

* feat: add keyboard nav delete option to block context menu

* chore: add FIXME comments

* chore: lint

* chore: format navigation_controller

* chore: code cleanup as suggested by reviewer

Co-authored-by: Christopher Allen <[email protected]>

* chore: code cleanup as suggested by reviewer 2

Co-authored-by: Christopher Allen <[email protected]>

* chore: code cleanup as suggested by reviewer 3

Co-authored-by: Christopher Allen <[email protected]>

---------

Co-authored-by: Christopher Allen <[email protected]>

Author: rachel-fenichel

src/navigation_controller.ts

@@ -16,6 +16,7 @@ import * as Blockly from 'blockly/core';
 import {
   ASTNode,
   BlockSvg,
+  ContextMenuRegistry,
   ICopyData,
   ShortcutRegistry,
   Toolbox,
@@ -224,6 +225,56 @@ export class NavigationController {
     return false;
   }
 
+  /**
+   * Precondition function for deleting a block from keyboard
+   * navigation. This precondition is shared between keyboard shortcuts
+   * and context menu items.
+   *
+   * FIXME: This should be better encapsulated.
+   *
+   * @param workspace The `WorkspaceSvg` where the shortcut was
+   *     invoked.
+   * @returns True iff `deleteCallbackFn` function should be called.
+   */
+  protected deletePreconditionFn(workspace: WorkspaceSvg) {
+    if (!this.canCurrentlyEdit(workspace)) return false;
+    const sourceBlock = workspace.getCursor()?.getCurNode().getSourceBlock();
+    return !!(sourceBlock?.isDeletable());
+  }
+
+  /**
+   * Callback function for deleting a block from keyboard
+   * navigation. This callback is shared between keyboard shortcuts
+   * and context menu items.
+   *
+   * FIXME: This should be better encapsulated.
+   *
+   * @param workspace The `WorkspaceSvg` where the shortcut was
+   *     invoked.
+   * @param e The originating event for a keyboard shortcut, or null
+   *     if called from a context menu.
+   * @returns True if this function successfully handled deletion.
+   */
+  protected deleteCallbackFn(workspace: WorkspaceSvg, e: Event | null) {
+    const cursor = workspace.getCursor();
+    if (!cursor) return false;
+    const sourceBlock = cursor.getCurNode().getSourceBlock() as BlockSvg;
+    // Delete or backspace.
+    // There is an event if this is triggered from a keyboard shortcut,
+    // but not if it's triggered from a context menu.
+    if (e) {
+      // 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();
+    }
+    // Don't delete while dragging.  Jeez.
+    if (Blockly.Gesture.inProgress()) false;
+    this.navigation.moveCursorOnBlockDelete(workspace, sourceBlock);
+    sourceBlock.checkAndDelete();
+    return true;
+  }
+
   /**
    * List all the currently registered shortcuts.
    */
@@ -404,7 +455,7 @@ export class NavigationController {
      * - On the workspace: open the context menu.
      */
     enter: {
-      name: Constants.SHORTCUT_NAMES.MARK,  // FIXME
+      name: Constants.SHORTCUT_NAMES.MARK, // FIXME
       preconditionFn: (workspace) => this.canCurrentlyEdit(workspace),
       callback: (workspace) => {
         let flyoutCursor;
@@ -545,11 +596,11 @@ export class NavigationController {
             case Constants.STATE.WORKSPACE:
               const curNode = workspace?.getCursor()?.getCurNode();
               const source = curNode?.getSourceBlock();
-	              return !!(
-	                source?.isDeletable() &&
-	                source?.isMovable() &&
-	                !Blockly.Gesture.inProgress()
-	              );
+              return !!(
+                source?.isDeletable() &&
+                source?.isMovable() &&
+                !Blockly.Gesture.inProgress()
+              );
             case Constants.STATE.FLYOUT:
               const flyoutWorkspace = workspace.getFlyout()?.getWorkspace();
               const sourceBlock = flyoutWorkspace
@@ -650,35 +701,8 @@ export class NavigationController {
     /** Keyboard shortcut to delete the block the cursor is currently on. */
     delete: {
       name: Constants.SHORTCUT_NAMES.DELETE,
-      preconditionFn: (workspace) => {
-        if (this.canCurrentlyEdit(workspace)) {
-          const curNode = workspace.getCursor()?.getCurNode();
-          if (curNode && curNode.getSourceBlock()) {
-            const sourceBlock = curNode.getSourceBlock();
-            return !!(sourceBlock && sourceBlock.isDeletable());
-          }
-        }
-        return false;
-      },
-      callback: (workspace, e) => {
-        const cursor = workspace.getCursor();
-        if (!cursor) {
-          return false;
-        }
-        const sourceBlock = cursor.getCurNode().getSourceBlock() as BlockSvg;
-        // 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();
-        // Don't delete while dragging.  Jeez.
-        if (Blockly.Gesture.inProgress()) {
-          return false;
-        }
-        this.navigation.moveCursorOnBlockDelete(workspace, sourceBlock);
-        sourceBlock.checkAndDelete();
-        return true;
-      },
+      preconditionFn: this.deletePreconditionFn,
+      callback: this.deleteCallbackFn,
       keyCodes: [KeyCodes.DELETE, KeyCodes.BACKSPACE],
       allowCollision: true,
     },
@@ -827,6 +851,56 @@ export class NavigationController {
     },
   };
 
+  /**
+   * Register the delete block action as a context menu item on blocks.
+   * This function mixes together the keyboard and context menu preconditions
+   * but only calls the keyboard callback.
+   */
+  protected registerDeleteAction() {
+    const originalDeleteItem =
+      ContextMenuRegistry.registry.getItem('blockDelete');
+    if (!originalDeleteItem) return;
+
+    const deleteItem: ContextMenuRegistry.RegistryItem = {
+      displayText: (scope) => {
+        // FIXME: Consider using the original delete item's display text,
+        // which is dynamic based on the nubmer of blocks to delete.
+        return 'Keyboard Navigation: delete';
+      },
+      preconditionFn: (scope) => {
+        // FIXME: Find a better way to get the workspace, or use `as WorkspaceSvg`.
+        const ws = scope.block?.workspace;
+
+        // Run the original precondition code, from the context menu option.
+        // If the item would be hidden or disabled, respect it.
+        const originalPreconditionResult =
+          originalDeleteItem.preconditionFn(scope);
+        if (!ws || originalPreconditionResult != 'enabled') {
+          return originalPreconditionResult;
+        }
+
+        // Return enabled if the keyboard shortcut precondition is allowed,
+        // and disabled if the context menu precondition is met but the keyboard
+        // shortcut precondition is not met.
+        return this.deletePreconditionFn(ws) ? 'enabled' : 'disabled';
+      },
+      callback: (scope) => {
+        // FIXME: Find a better way to get the workspace, or use `as WorkspaceSvg`.
+        const ws = scope.block?.workspace;
+        if (!ws) return;
+
+        // Delete the block(s), and put the cursor back in a sane location.
+        return this.deleteCallbackFn(ws, null);
+      },
+      scopeType: ContextMenuRegistry.ScopeType.BLOCK,
+      id: 'blockDeleteFromContextMenu',
+      weight: 10,
+    };
+
+    // FIXME: Decide whether to unregister the original item.
+    ContextMenuRegistry.registry.register(deleteItem);
+  }
+
   /**
    * Registers all default keyboard shortcut items for keyboard
    * navigation. This should be called once per instance of
@@ -837,6 +911,8 @@ export class NavigationController {
       ShortcutRegistry.registry.register(shortcut);
     }
 
+    this.registerDeleteAction();
+
     // Initalise the shortcut modal with available shortcuts.  Needs
     // to be done separately rather at construction, as many shortcuts
     // are not registered at that point.