From 25534ba7a39df9c8bf0c35b341895e04932efdb5 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Mon, 10 Mar 2025 12:38:22 -0700 Subject: [PATCH 1/3] refactor: Move arrow key shortcuts into their own class. --- src/actions/arrow_navigation.ts | 198 ++++++++++++++++++++++++++++++++ src/navigation_controller.ts | 165 ++------------------------ 2 files changed, 209 insertions(+), 154 deletions(-) create mode 100644 src/actions/arrow_navigation.ts diff --git a/src/actions/arrow_navigation.ts b/src/actions/arrow_navigation.ts new file mode 100644 index 00000000..d18bd8c1 --- /dev/null +++ b/src/actions/arrow_navigation.ts @@ -0,0 +1,198 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {ASTNode, ShortcutRegistry, utils as BlocklyUtils} from 'blockly/core'; + +import type {Field, Toolbox, WorkspaceSvg} from 'blockly/core'; + +import * as Constants from '../constants'; +import type {Navigation} from '../navigation'; + +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, + ) {} + + /** + * Gives the cursor to the field to handle if the cursor is on a field. + * + * @param workspace The workspace to check. + * @param shortcut The shortcut + * to give to the field. + * @returns True if the shortcut was handled by the field, false + * otherwise. + */ + fieldShortcutHandler( + workspace: WorkspaceSvg, + shortcut: ShortcutRegistry.KeyboardShortcut, + ): boolean { + const cursor = workspace.getCursor(); + if (!cursor || !cursor.getCurNode()) { + return false; + } + const curNode = cursor.getCurNode(); + if (curNode.getType() === ASTNode.types.FIELD) { + return (curNode.getLocation() as Field).onShortcut(shortcut); + } + return false; + } + + /** + * Adds all arrow key navigation shortcuts to the registry. + */ + install() { + const shortcuts: { + [name: string]: ShortcutRegistry.KeyboardShortcut; + } = { + /** Go to the in location. */ + in: { + name: Constants.SHORTCUT_NAMES.IN, + preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), + callback: (workspace, _, shortcut) => { + const toolbox = workspace.getToolbox() as Toolbox; + let isHandled = false; + switch (this.navigation.getState(workspace)) { + case Constants.STATE.WORKSPACE: + isHandled = this.fieldShortcutHandler(workspace, shortcut); + if (!isHandled && workspace) { + workspace.getCursor()?.in(); + isHandled = true; + } + return isHandled; + case Constants.STATE.TOOLBOX: + isHandled = + toolbox && typeof toolbox.onShortcut === 'function' + ? toolbox.onShortcut(shortcut) + : false; + if (!isHandled) { + this.navigation.focusFlyout(workspace); + } + return true; + default: + return false; + } + }, + keyCodes: [KeyCodes.RIGHT], + }, + + /** Go to the out location. */ + out: { + name: Constants.SHORTCUT_NAMES.OUT, + preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), + callback: (workspace, _, shortcut) => { + const toolbox = workspace.getToolbox() as Toolbox; + let isHandled = false; + switch (this.navigation.getState(workspace)) { + case Constants.STATE.WORKSPACE: + isHandled = this.fieldShortcutHandler(workspace, shortcut); + if (!isHandled && workspace) { + workspace.getCursor()?.out(); + isHandled = true; + } + return isHandled; + case Constants.STATE.FLYOUT: + this.navigation.focusToolbox(workspace); + return true; + case Constants.STATE.TOOLBOX: + return toolbox && typeof toolbox.onShortcut === 'function' + ? toolbox.onShortcut(shortcut) + : false; + default: + return false; + } + }, + keyCodes: [KeyCodes.LEFT], + }, + + /** Go to the next location. */ + next: { + name: Constants.SHORTCUT_NAMES.NEXT, + preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), + callback: (workspace, _, shortcut) => { + const toolbox = workspace.getToolbox() as Toolbox; + const flyout = workspace.getFlyout(); + let isHandled = false; + switch (this.navigation.getState(workspace)) { + case Constants.STATE.WORKSPACE: + isHandled = this.fieldShortcutHandler(workspace, shortcut); + if (!isHandled && workspace) { + workspace.getCursor()?.next(); + isHandled = true; + } + return isHandled; + case Constants.STATE.FLYOUT: + isHandled = this.fieldShortcutHandler(workspace, shortcut); + if (!isHandled && flyout) { + flyout.getWorkspace()?.getCursor()?.next(); + isHandled = true; + } + return isHandled; + case Constants.STATE.TOOLBOX: + return toolbox && typeof toolbox.onShortcut === 'function' + ? toolbox.onShortcut(shortcut) + : false; + default: + return false; + } + }, + keyCodes: [KeyCodes.DOWN], + }, + /** Go to the previous location. */ + previous: { + name: Constants.SHORTCUT_NAMES.PREVIOUS, + preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), + callback: (workspace, _, shortcut) => { + const flyout = workspace.getFlyout(); + const toolbox = workspace.getToolbox() as Toolbox; + let isHandled = false; + switch (this.navigation.getState(workspace)) { + case Constants.STATE.WORKSPACE: + isHandled = this.fieldShortcutHandler(workspace, shortcut); + if (!isHandled) { + workspace.getCursor()?.prev(); + isHandled = true; + } + return isHandled; + case Constants.STATE.FLYOUT: + isHandled = this.fieldShortcutHandler(workspace, shortcut); + if (!isHandled && flyout) { + flyout.getWorkspace()?.getCursor()?.prev(); + isHandled = true; + } + return isHandled; + case Constants.STATE.TOOLBOX: + return toolbox && typeof toolbox.onShortcut === 'function' + ? toolbox.onShortcut(shortcut) + : false; + default: + return false; + } + }, + keyCodes: [KeyCodes.UP], + }, + }; + + for (const shortcut of Object.values(shortcuts)) { + ShortcutRegistry.registry.register(shortcut); + } + } + + /** + * Removes all the arrow navigation shortcuts. + */ + uninstall() { + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.IN); + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.OUT); + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.NEXT); + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.PREVIOUS); + } +} diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 724799c3..cc67dd31 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -30,6 +30,7 @@ import {DeleteAction} from './actions/delete'; import {InsertAction} from './actions/insert'; import {Clipboard} from './actions/clipboard'; import {WorkspaceMovement} from './actions/ws_movement'; +import {ArrowNavigation} from './actions/arrow_navigation'; const KeyCodes = BlocklyUtils.KeyCodes; const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind( @@ -65,6 +66,12 @@ export class NavigationController { this.canCurrentlyEdit.bind(this), ); + /** Keyboard navigation actions for the arrow keys. */ + arrowNavigation: ArrowNavigation = new ArrowNavigation( + this.navigation, + this.canCurrentlyNavigate.bind(this), + ); + hasNavigationFocus: boolean = false; /** @@ -218,30 +225,6 @@ export class NavigationController { this.navigation.disableKeyboardAccessibility(workspace); } - /** - * Gives the cursor to the field to handle if the cursor is on a field. - * - * @param workspace The workspace to check. - * @param shortcut The shortcut - * to give to the field. - * @returns True if the shortcut was handled by the field, false - * otherwise. - */ - protected fieldShortcutHandler( - workspace: WorkspaceSvg, - shortcut: ShortcutRegistry.KeyboardShortcut, - ): boolean { - const cursor = workspace.getCursor(); - if (!cursor || !cursor.getCurNode()) { - return false; - } - const curNode = cursor.getCurNode(); - if (curNode.getType() === ASTNode.types.FIELD) { - return (curNode.getLocation() as Blockly.Field).onShortcut(shortcut); - } - return false; - } - /** * List all the currently registered shortcuts. */ @@ -255,40 +238,6 @@ export class NavigationController { protected shortcuts: { [name: string]: ShortcutRegistry.KeyboardShortcut; } = { - /** Go to the previous location. */ - previous: { - name: Constants.SHORTCUT_NAMES.PREVIOUS, - preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), - callback: (workspace, _, shortcut) => { - const flyout = workspace.getFlyout(); - const toolbox = workspace.getToolbox() as Blockly.Toolbox; - let isHandled = false; - switch (this.navigation.getState(workspace)) { - case Constants.STATE.WORKSPACE: - isHandled = this.fieldShortcutHandler(workspace, shortcut); - if (!isHandled) { - workspace.getCursor()?.prev(); - isHandled = true; - } - return isHandled; - case Constants.STATE.FLYOUT: - isHandled = this.fieldShortcutHandler(workspace, shortcut); - if (!isHandled && flyout) { - flyout.getWorkspace()?.getCursor()?.prev(); - isHandled = true; - } - return isHandled; - case Constants.STATE.TOOLBOX: - return toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; - default: - return false; - } - }, - keyCodes: [KeyCodes.UP], - }, - /** Turn keyboard navigation on or off. */ toggleKeyboardNav: { name: Constants.SHORTCUT_NAMES.TOGGLE_KEYBOARD_NAV, @@ -305,100 +254,6 @@ export class NavigationController { ], }, - /** Go to the out location. */ - out: { - name: Constants.SHORTCUT_NAMES.OUT, - preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), - callback: (workspace, _, shortcut) => { - const toolbox = workspace.getToolbox() as Blockly.Toolbox; - let isHandled = false; - switch (this.navigation.getState(workspace)) { - case Constants.STATE.WORKSPACE: - isHandled = this.fieldShortcutHandler(workspace, shortcut); - if (!isHandled && workspace) { - workspace.getCursor()?.out(); - isHandled = true; - } - return isHandled; - case Constants.STATE.FLYOUT: - this.navigation.focusToolbox(workspace); - return true; - case Constants.STATE.TOOLBOX: - return toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; - default: - return false; - } - }, - keyCodes: [KeyCodes.LEFT], - }, - - /** Go to the next location. */ - next: { - name: Constants.SHORTCUT_NAMES.NEXT, - preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), - callback: (workspace, _, shortcut) => { - const toolbox = workspace.getToolbox() as Blockly.Toolbox; - const flyout = workspace.getFlyout(); - let isHandled = false; - switch (this.navigation.getState(workspace)) { - case Constants.STATE.WORKSPACE: - isHandled = this.fieldShortcutHandler(workspace, shortcut); - if (!isHandled && workspace) { - workspace.getCursor()?.next(); - isHandled = true; - } - return isHandled; - case Constants.STATE.FLYOUT: - isHandled = this.fieldShortcutHandler(workspace, shortcut); - if (!isHandled && flyout) { - flyout.getWorkspace()?.getCursor()?.next(); - isHandled = true; - } - return isHandled; - case Constants.STATE.TOOLBOX: - return toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; - default: - return false; - } - }, - keyCodes: [KeyCodes.DOWN], - }, - - /** Go to the in location. */ - in: { - name: Constants.SHORTCUT_NAMES.IN, - preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), - callback: (workspace, _, shortcut) => { - const toolbox = workspace.getToolbox() as Blockly.Toolbox; - let isHandled = false; - switch (this.navigation.getState(workspace)) { - case Constants.STATE.WORKSPACE: - isHandled = this.fieldShortcutHandler(workspace, shortcut); - if (!isHandled && workspace) { - workspace.getCursor()?.in(); - isHandled = true; - } - return isHandled; - case Constants.STATE.TOOLBOX: - isHandled = - toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; - if (!isHandled) { - this.navigation.focusFlyout(workspace); - } - return true; - default: - return false; - } - }, - keyCodes: [KeyCodes.RIGHT], - }, - /** * Enter key: * @@ -556,7 +411,7 @@ export class NavigationController { const cursor = workspace.getCursor() as LineCursor; if (this.navigation.getState(workspace) === Constants.STATE.WORKSPACE) { - if (this.fieldShortcutHandler(workspace, shortcut)) { + if (this.arrowNavigation.fieldShortcutHandler(workspace, shortcut)) { this.announcer.setText('next sibling (handled by field)'); return true; } @@ -580,7 +435,7 @@ export class NavigationController { const cursor = workspace.getCursor() as LineCursor; if (this.navigation.getState(workspace) === Constants.STATE.WORKSPACE) { - if (this.fieldShortcutHandler(workspace, shortcut)) { + if (this.arrowNavigation.fieldShortcutHandler(workspace, shortcut)) { this.announcer.setText('previous sibling (handled by field)'); return true; } @@ -680,6 +535,7 @@ export class NavigationController { this.deleteAction.install(); this.insertAction.install(); this.workspaceMovement.install(); + this.arrowNavigation.install(); this.clipboard.install(); @@ -701,6 +557,7 @@ export class NavigationController { this.insertAction.uninstall(); this.clipboard.uninstall(); this.workspaceMovement.uninstall(); + this.arrowNavigation.uninstall(); this.removeShortcutHandlers(); this.navigation.dispose(); From 8ceb8d60c7ea6851c257ef18d5b15b0fd22ea8f6 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Wed, 12 Mar 2025 15:31:28 -0700 Subject: [PATCH 2/3] chore: Remove inadvertently reintroduced code. --- src/navigation_controller.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 6c614911..7078ecb3 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -303,13 +303,6 @@ export class NavigationController { this.navigation.disableKeyboardAccessibility(workspace); } - /** - * List all the currently registered shortcuts. - */ - listShortcuts() { - this.announcer.listShortcuts(); - } - /** * Dictionary of KeyboardShortcuts. */ From 18a32d88439a9e9f5d652a0dc0416155d453424b Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Thu, 13 Mar 2025 10:50:19 -0700 Subject: [PATCH 3/3] chore: Update names of arrow key actions and remove unused actions. --- src/actions/arrow_navigation.ts | 32 ++++++++++++++++---------------- src/constants.ts | 26 ++++++++------------------ src/navigation_controller.ts | 8 ++++---- 3 files changed, 28 insertions(+), 38 deletions(-) diff --git a/src/actions/arrow_navigation.ts b/src/actions/arrow_navigation.ts index d18bd8c1..8df4cfc5 100644 --- a/src/actions/arrow_navigation.ts +++ b/src/actions/arrow_navigation.ts @@ -53,9 +53,9 @@ export class ArrowNavigation { const shortcuts: { [name: string]: ShortcutRegistry.KeyboardShortcut; } = { - /** Go to the in location. */ - in: { - name: Constants.SHORTCUT_NAMES.IN, + /** Go to the next location to the right. */ + right: { + name: Constants.SHORTCUT_NAMES.RIGHT, preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), callback: (workspace, _, shortcut) => { const toolbox = workspace.getToolbox() as Toolbox; @@ -84,9 +84,9 @@ export class ArrowNavigation { keyCodes: [KeyCodes.RIGHT], }, - /** Go to the out location. */ - out: { - name: Constants.SHORTCUT_NAMES.OUT, + /** Go to the next location to the left. */ + left: { + name: Constants.SHORTCUT_NAMES.LEFT, preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), callback: (workspace, _, shortcut) => { const toolbox = workspace.getToolbox() as Toolbox; @@ -113,9 +113,9 @@ export class ArrowNavigation { keyCodes: [KeyCodes.LEFT], }, - /** Go to the next location. */ - next: { - name: Constants.SHORTCUT_NAMES.NEXT, + /** Go down to the next location. */ + down: { + name: Constants.SHORTCUT_NAMES.DOWN, preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), callback: (workspace, _, shortcut) => { const toolbox = workspace.getToolbox() as Toolbox; @@ -146,9 +146,9 @@ export class ArrowNavigation { }, keyCodes: [KeyCodes.DOWN], }, - /** Go to the previous location. */ - previous: { - name: Constants.SHORTCUT_NAMES.PREVIOUS, + /** Go up to the previous location. */ + up: { + name: Constants.SHORTCUT_NAMES.UP, preconditionFn: (workspace) => this.canCurrentlyNavigate(workspace), callback: (workspace, _, shortcut) => { const flyout = workspace.getFlyout(); @@ -190,9 +190,9 @@ export class ArrowNavigation { * Removes all the arrow navigation shortcuts. */ uninstall() { - ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.IN); - ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.OUT); - ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.NEXT); - ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.PREVIOUS); + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.LEFT); + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.RIGHT); + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.DOWN); + ShortcutRegistry.registry.unregister(Constants.SHORTCUT_NAMES.UP); } } diff --git a/src/constants.ts b/src/constants.ts index e077eff2..98f6aa17 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -24,10 +24,10 @@ export enum STATE { * Default keyboard navigation shortcut names. */ export enum SHORTCUT_NAMES { - PREVIOUS = 'previous', - NEXT = 'next', - IN = 'in', - OUT = 'out', + UP = 'up', + DOWN = 'down', + RIGHT = 'right', + LEFT = 'left', INSERT = 'insert', EDIT_OR_CONFIRM = 'edit_or_confirm', DISCONNECT = 'disconnect', @@ -43,15 +43,8 @@ export enum SHORTCUT_NAMES { MOVE_WS_CURSOR_DOWN = 'workspace_down', MOVE_WS_CURSOR_LEFT = 'workspace_left', MOVE_WS_CURSOR_RIGHT = 'workspace_right', - TOGGLE_KEYBOARD_NAV = 'toggle_keyboard_nav', /* eslint-enable @typescript-eslint/naming-convention */ LIST_SHORTCUTS = 'list_shortcuts', - ANNOUNCE = 'announce', - GO_TO_NEXT_SIBLING = 'go_to_next_sibling', - GO_TO_PREVIOUS_SIBLING = 'go_to_previous_sibling', - JUMP_TO_ROOT = 'jump_to_root_of_current_stack', - CONTEXT_OUT = 'context_out', - CONTEXT_IN = 'context_in', CLEAN_UP = 'clean_up_workspace', } @@ -95,12 +88,9 @@ export const SHORTCUT_CATEGORIES: Record< 'redo', ], 'Code navigation': [ - SHORTCUT_NAMES.PREVIOUS, - SHORTCUT_NAMES.NEXT, - SHORTCUT_NAMES.IN, - SHORTCUT_NAMES.OUT, - SHORTCUT_NAMES.GO_TO_NEXT_SIBLING, - SHORTCUT_NAMES.GO_TO_PREVIOUS_SIBLING, - SHORTCUT_NAMES.JUMP_TO_ROOT, + SHORTCUT_NAMES.UP, + SHORTCUT_NAMES.DOWN, + SHORTCUT_NAMES.RIGHT, + SHORTCUT_NAMES.LEFT, ], }; diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 7078ecb3..cd5464a8 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -160,13 +160,13 @@ export class NavigationController { return false; } switch (shortcut.name) { - case Constants.SHORTCUT_NAMES.PREVIOUS: + case Constants.SHORTCUT_NAMES.UP: return (this as any).selectPrevious(); - case Constants.SHORTCUT_NAMES.OUT: + case Constants.SHORTCUT_NAMES.LEFT: return (this as any).selectParent(); - case Constants.SHORTCUT_NAMES.NEXT: + case Constants.SHORTCUT_NAMES.DOWN: return (this as any).selectNext(); - case Constants.SHORTCUT_NAMES.IN: + case Constants.SHORTCUT_NAMES.RIGHT: return (this as any).selectChild(); default: return false;