refactor: move editable/navigable checks to navigation

Less indirection and more freedom for actions to pick finer grained
preconditions.

Author: microbit-matt-hillsdon

src/actions/action_menu.ts

@@ -30,23 +30,12 @@ export interface ScopeWithConnection extends ContextMenuRegistry.Scope {
  * Keyboard shortcut to show the action menu on Cmr/Ctrl/Alt+Enter key.
  */
 export class ActionMenu {
-  /**
-   * Function provided by the navigation controller to say whether navigation
-   * is allowed.
-   */
-  private canCurrentlyNavigate: (ws: WorkspaceSvg) => boolean;
-
   /**
    * Registration name for the keyboard shortcut.
    */
   private shortcutName = Constants.SHORTCUT_NAMES.MENU;
 
-  constructor(
-    private navigation: Navigation,
-    canNavigate: (ws: WorkspaceSvg) => boolean,
-  ) {
-    this.canCurrentlyNavigate = canNavigate;
-  }
+  constructor(private navigation: Navigation) {}
 
   /**
    * Install this action.
@@ -68,7 +57,8 @@ export class ActionMenu {
   private registerShortcut() {
     const menuShortcut: ShortcutRegistry.KeyboardShortcut = {
       name: Constants.SHORTCUT_NAMES.MENU,
-      preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
+      preconditionFn: (workspace) =>
+        this.navigation.canCurrentlyNavigate(workspace),
       callback: (workspace) => {
         switch (this.navigation.getState(workspace)) {
           case Constants.STATE.WORKSPACE:

src/actions/arrow_navigation.ts

@@ -17,10 +17,7 @@ const KeyCodes = BlocklyUtils.KeyCodes;
  * Class for registering shortcuts for navigating the workspace with arrow keys.
  */
 export class ArrowNavigation {
-  constructor(
-    private navigation: Navigation,
-    private canCurrentlyNavigate: (ws: WorkspaceSvg) => boolean,
-  ) {}
+  constructor(private navigation: Navigation) {}
 
   /**
    * Gives the cursor to the field to handle if the cursor is on a field.
@@ -56,7 +53,8 @@ export class ArrowNavigation {
       /** Go to the next location to the right. */
       right: {
         name: Constants.SHORTCUT_NAMES.RIGHT,
-        preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
+        preconditionFn: (workspace) =>
+          this.navigation.canCurrentlyNavigate(workspace),
         callback: (workspace, e, shortcut) => {
           const toolbox = workspace.getToolbox() as Toolbox;
           let isHandled = false;
@@ -93,7 +91,8 @@ export class ArrowNavigation {
       /** Go to the next location to the left. */
       left: {
         name: Constants.SHORTCUT_NAMES.LEFT,
-        preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
+        preconditionFn: (workspace) =>
+          this.navigation.canCurrentlyNavigate(workspace),
         callback: (workspace, e, shortcut) => {
           const toolbox = workspace.getToolbox() as Toolbox;
           let isHandled = false;
@@ -128,7 +127,8 @@ export class ArrowNavigation {
       /** Go down to the next location. */
       down: {
         name: Constants.SHORTCUT_NAMES.DOWN,
-        preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
+        preconditionFn: (workspace) =>
+          this.navigation.canCurrentlyNavigate(workspace),
         callback: (workspace, e, shortcut) => {
           const toolbox = workspace.getToolbox() as Toolbox;
           const flyout = workspace.getFlyout();
@@ -169,7 +169,8 @@ export class ArrowNavigation {
       /** Go up to the previous location. */
       up: {
         name: Constants.SHORTCUT_NAMES.UP,
-        preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
+        preconditionFn: (workspace) =>
+          this.navigation.canCurrentlyNavigate(workspace),
         callback: (workspace, e, shortcut) => {
           const flyout = workspace.getFlyout();
           const toolbox = workspace.getToolbox() as Toolbox;

src/actions/clipboard.ts

@@ -44,18 +44,7 @@ export class Clipboard {
   /** The workspace a copy or cut keyboard shortcut happened in. */
   private copyWorkspace: WorkspaceSvg | null = null;
 
-  /**
-   * Function provided by the navigation controller to say whether editing
-   * is allowed.
-   */
-  private canCurrentlyEdit: (ws: WorkspaceSvg) => boolean;
-
-  constructor(
-    private navigation: Navigation,
-    canEdit: (ws: WorkspaceSvg) => boolean,
-  ) {
-    this.canCurrentlyEdit = canEdit;
-  }
+  constructor(private navigation: Navigation) {}
 
   /**
    * Install these actions as both keyboard shortcuts and context menu items.
@@ -141,7 +130,7 @@ export class Clipboard {
    * @returns True iff `cutCallback` function should be called.
    */
   private cutPrecondition(workspace: WorkspaceSvg) {
-    if (this.canCurrentlyEdit(workspace)) {
+    if (this.navigation.canCurrentlyEdit(workspace)) {
       const curNode = workspace.getCursor()?.getCurNode();
       if (curNode && curNode.getSourceBlock()) {
         const sourceBlock = curNode.getSourceBlock();
@@ -236,7 +225,7 @@ export class Clipboard {
    * @returns True iff `copyCallback` function should be called.
    */
   private copyPrecondition(workspace: WorkspaceSvg) {
-    if (!this.canCurrentlyEdit(workspace)) return false;
+    if (!this.navigation.canCurrentlyEdit(workspace)) return false;
     switch (this.navigation.getState(workspace)) {
       case Constants.STATE.WORKSPACE: {
         const curNode = workspace?.getCursor()?.getCurNode();
@@ -348,7 +337,7 @@ export class Clipboard {
   private pastePrecondition(workspace: WorkspaceSvg) {
     if (!this.copyData || !this.copyWorkspace) return false;
 
-    return this.canCurrentlyEdit(workspace) && !Gesture.inProgress();
+    return this.navigation.canCurrentlyEdit(workspace) && !Gesture.inProgress();
   }
 
   /**

src/actions/delete.ts

@@ -28,23 +28,12 @@ export class DeleteAction {
    */
   private oldContextMenuItem: ContextMenuRegistry.RegistryItem | null = null;
 
-  /**
-   * Function provided by the navigation controller to say whether editing
-   * is allowed.
-   */
-  private canCurrentlyEdit: (ws: WorkspaceSvg) => boolean;
-
   /**
    * Registration name for the keyboard shortcut.
    */
   private deleteShortcutName = Constants.SHORTCUT_NAMES.DELETE;
 
-  constructor(
-    private navigation: Navigation,
-    canEdit: (ws: WorkspaceSvg) => boolean,
-  ) {
-    this.canCurrentlyEdit = canEdit;
-  }
+  constructor(private navigation: Navigation) {}
 
   /**
    * Install this action as both a keyboard shortcut and a context menu item.
@@ -148,10 +137,11 @@ export class DeleteAction {
    * @returns True iff `deleteCallback` function should be called.
    */
   private deletePrecondition(workspace: WorkspaceSvg) {
-    if (!this.canCurrentlyEdit(workspace)) return false;
-
     const sourceBlock = workspace.getCursor()?.getCurNode()?.getSourceBlock();
-    return !!sourceBlock?.isDeletable();
+    return (
+      this.navigation.canCurrentlyEdit(workspace) &&
+      !!sourceBlock?.isDeletable()
+    );
   }
 
   /**

src/actions/disconnect.ts

@@ -24,23 +24,12 @@ const KeyCodes = BlocklyUtils.KeyCodes;
  * item.
  */
 export class DisconnectAction {
-  /**
-   * Function provided by the navigation controller to say whether editing
-   * is allowed.
-   */
-  private canCurrentlyEdit: (ws: WorkspaceSvg) => boolean;
-
   /**
    * Registration name for the keyboard shortcut.
    */
   private shortcutName = Constants.SHORTCUT_NAMES.DISCONNECT;
 
-  constructor(
-    private navigation: Navigation,
-    canEdit: (ws: WorkspaceSvg) => boolean,
-  ) {
-    this.canCurrentlyEdit = canEdit;
-  }
+  constructor(private navigation: Navigation) {}
 
   /**
    * Install this action as both a keyboard shortcut and a context menu item.
@@ -63,7 +52,8 @@ export class DisconnectAction {
   private registerShortcut() {
     const disconnectShortcut: ShortcutRegistry.KeyboardShortcut = {
       name: this.shortcutName,
-      preconditionFn: (workspace) => this.canCurrentlyEdit(workspace),
+      preconditionFn: (workspace) =>
+        this.navigation.canCurrentlyEdit(workspace),
       callback: (workspace) => {
         switch (this.navigation.getState(workspace)) {
           case Constants.STATE.WORKSPACE:

src/actions/edit.ts

@@ -5,8 +5,8 @@
  */
 
 import {ContextMenuRegistry} from 'blockly';
-import type {WorkspaceSvg} from 'blockly';
 import {LineCursor} from '../line_cursor';
+import {Navigation} from 'src/navigation';
 
 /**
  * Action to edit a block.  This just moves the cursor to the first
@@ -29,7 +29,7 @@ import {LineCursor} from '../line_cursor';
  * is already a corresponding "right" shortcut item.
  */
 export class EditAction {
-  constructor(private canCurrentlyNavigate: (ws: WorkspaceSvg) => boolean) {}
+  constructor(private navigation: Navigation) {}
 
   /**
    * Install this action as a context menu item.
@@ -54,7 +54,7 @@ export class EditAction {
       displayText: 'Edit Block contents (→︎)',
       preconditionFn: (scope: ContextMenuRegistry.Scope) => {
         const workspace = scope.block?.workspace;
-        if (!workspace || !this.canCurrentlyNavigate(workspace)) {
+        if (!workspace || !this.navigation.canCurrentlyNavigate(workspace)) {
           return 'disabled';
         }
         const cursor = workspace.getCursor() as LineCursor | null;

src/actions/enter.ts

@@ -29,10 +29,7 @@ const KeyCodes = BlocklyUtils.KeyCodes;
  * Class for registering a shortcut for the enter action.
  */
 export class EnterAction {
-  constructor(
-    private navigation: Navigation,
-    private canCurrentlyEdit: (ws: WorkspaceSvg) => boolean,
-  ) {}
+  constructor(private navigation: Navigation) {}
 
   /**
    * Adds the enter action shortcut to the registry.
@@ -47,7 +44,8 @@ export class EnterAction {
      */
     ShortcutRegistry.registry.register({
       name: Constants.SHORTCUT_NAMES.EDIT_OR_CONFIRM,
-      preconditionFn: (workspace) => this.canCurrentlyEdit(workspace),
+      preconditionFn: (workspace) =>
+        this.navigation.canCurrentlyEdit(workspace),
       callback: (workspace, event) => {
         event.preventDefault();
 

src/actions/exit.ts

@@ -6,8 +6,6 @@
 
 import {ShortcutRegistry, utils as BlocklyUtils} from 'blockly/core';
 
-import type {WorkspaceSvg} from 'blockly/core';
-
 import * as Constants from '../constants';
 import type {Navigation} from '../navigation';
 
@@ -17,18 +15,16 @@ const KeyCodes = BlocklyUtils.KeyCodes;
  * Class for registering a shortcut for the exit action.
  */
 export class ExitAction {
-  constructor(
-    private navigation: Navigation,
-    private canCurrentlyNavigate: (ws: WorkspaceSvg) => boolean,
-  ) {}
+  constructor(private navigation: Navigation) {}
 
   /**
    * Adds the exit action shortcut to the registry.
    */
   install() {
     ShortcutRegistry.registry.register({
       name: Constants.SHORTCUT_NAMES.EXIT,
-      preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace),
+      preconditionFn: (workspace) =>
+        this.navigation.canCurrentlyNavigate(workspace),
       callback: (workspace) => {
         switch (this.navigation.getState(workspace)) {
           case Constants.STATE.FLYOUT:

src/actions/insert.ts

@@ -23,23 +23,12 @@ const KeyCodes = BlocklyUtils.KeyCodes;
  * item.
  */
 export class InsertAction {
-  /**
-   * Function provided by the navigation controller to say whether editing
-   * is allowed.
-   */
-  private canCurrentlyEdit: (ws: WorkspaceSvg) => boolean;
-
   /**
    * Registration name for the keyboard shortcut.
    */
   private insertShortcutName = Constants.SHORTCUT_NAMES.INSERT;
 
-  constructor(
-    private navigation: Navigation,
-    canEdit: (ws: WorkspaceSvg) => boolean,
-  ) {
-    this.canCurrentlyEdit = canEdit;
-  }
+  constructor(private navigation: Navigation) {}
 
   /**
    * Install this action as both a keyboard shortcut and a context menu item.
@@ -112,7 +101,7 @@ export class InsertAction {
    * @returns True iff `insertCallback` function should be called.
    */
   private insertPrecondition(workspace: WorkspaceSvg): boolean {
-    return this.canCurrentlyEdit(workspace);
+    return this.navigation.canCurrentlyEdit(workspace);
   }
 
   /**

src/actions/mover.ts

@@ -56,10 +56,7 @@ export class Mover {
    */
   private oldDragStrategy: IDragStrategy | null = null;
 
-  constructor(
-    protected navigation: Navigation,
-    protected canEdit: (ws: WorkspaceSvg) => boolean,
-  ) {}
+  constructor(protected navigation: Navigation) {}
 
   private shortcuts: ShortcutRegistry.KeyboardShortcut[] = [
     // Begin and end move.
@@ -212,7 +209,7 @@ export class Mover {
 
     return !!(
       this.navigation.getState(workspace) === Constants.STATE.WORKSPACE &&
-      this.canEdit(workspace) &&
+      this.navigation.canCurrentlyEdit(workspace) &&
       !this.moves.has(workspace) && // No move in progress.
       block?.isMovable()
     );
@@ -226,7 +223,9 @@ export class Mover {
    * @returns True iff we are moving.
    */
   isMoving(workspace: WorkspaceSvg) {
-    return this.canEdit(workspace) && this.moves.has(workspace);
+    return (
+      this.navigation.canCurrentlyEdit(workspace) && this.moves.has(workspace)
+    );
   }
 
   /**