From 5b2b15e49d6e6b99efdc5b484808c0db8966eaf4 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Wed, 28 May 2025 10:38:27 -0700 Subject: [PATCH 1/6] feat: use keyboard mode tracker from core --- src/actions/arrow_navigation.ts | 11 +++++++- src/actions/disconnect.ts | 2 ++ src/actions/edit.ts | 8 +++++- src/actions/move.ts | 2 ++ src/actions/ws_movement.ts | 11 ++++++-- src/index.ts | 8 ------ src/input_mode_tracker.ts | 48 --------------------------------- 7 files changed, 30 insertions(+), 60 deletions(-) delete mode 100644 src/input_mode_tracker.ts diff --git a/src/actions/arrow_navigation.ts b/src/actions/arrow_navigation.ts index e0eef938..7a00d07e 100644 --- a/src/actions/arrow_navigation.ts +++ b/src/actions/arrow_navigation.ts @@ -4,7 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {ShortcutRegistry, utils as BlocklyUtils, Field} from 'blockly/core'; +import { + ShortcutRegistry, + utils as BlocklyUtils, + Field, + keyboardNavigationController, +} from 'blockly/core'; import type {Toolbox, WorkspaceSvg} from 'blockly/core'; @@ -122,6 +127,7 @@ export class ArrowNavigation { preconditionFn: (workspace) => this.navigation.canCurrentlyNavigate(workspace), callback: (workspace, e, shortcut) => { + keyboardNavigationController.setIsActive(true); return workspace.RTL ? navigateOut(workspace, e, shortcut) : navigateIn(workspace, e, shortcut); @@ -135,6 +141,7 @@ export class ArrowNavigation { preconditionFn: (workspace) => this.navigation.canCurrentlyNavigate(workspace), callback: (workspace, e, shortcut) => { + keyboardNavigationController.setIsActive(true); return workspace.RTL ? navigateIn(workspace, e, shortcut) : navigateOut(workspace, e, shortcut); @@ -148,6 +155,7 @@ export class ArrowNavigation { preconditionFn: (workspace) => this.navigation.canCurrentlyNavigate(workspace), callback: (workspace, e, shortcut) => { + keyboardNavigationController.setIsActive(true); const toolbox = workspace.getToolbox() as Toolbox; const flyout = workspace.getFlyout(); let isHandled = false; @@ -205,6 +213,7 @@ export class ArrowNavigation { preconditionFn: (workspace) => this.navigation.canCurrentlyNavigate(workspace), callback: (workspace, e, shortcut) => { + keyboardNavigationController.setIsActive(true); const flyout = workspace.getFlyout(); const toolbox = workspace.getToolbox() as Toolbox; let isHandled = false; diff --git a/src/actions/disconnect.ts b/src/actions/disconnect.ts index a38e4c25..3604dec2 100644 --- a/src/actions/disconnect.ts +++ b/src/actions/disconnect.ts @@ -11,6 +11,7 @@ import { utils as BlocklyUtils, Connection, ConnectionType, + keyboardNavigationController, } from 'blockly'; import * as Constants from '../constants'; import type {WorkspaceSvg} from 'blockly'; @@ -56,6 +57,7 @@ export class DisconnectAction { preconditionFn: (workspace) => this.navigation.canCurrentlyEdit(workspace), callback: (workspace) => { + keyboardNavigationController.setIsActive(true); switch (this.navigation.getState(workspace)) { case Constants.STATE.WORKSPACE: this.disconnectBlocks(workspace); diff --git a/src/actions/edit.ts b/src/actions/edit.ts index 60346080..b864f286 100644 --- a/src/actions/edit.ts +++ b/src/actions/edit.ts @@ -4,7 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {ContextMenuRegistry, LineCursor, Msg} from 'blockly'; +import { + ContextMenuRegistry, + LineCursor, + Msg, + keyboardNavigationController, +} from 'blockly'; import {Navigation} from 'src/navigation'; import {getShortActionShortcut} from '../shortcut_formatting'; import * as Constants from '../constants'; @@ -67,6 +72,7 @@ export class EditAction { return cursor.atEndOfLine() ? 'hidden' : 'enabled'; }, callback: (scope: ContextMenuRegistry.Scope) => { + keyboardNavigationController.setIsActive(true); const workspace = scope.block?.workspace; if (!workspace) return false; workspace.getCursor()?.in(); diff --git a/src/actions/move.ts b/src/actions/move.ts index 415fe40c..045a1098 100644 --- a/src/actions/move.ts +++ b/src/actions/move.ts @@ -11,6 +11,7 @@ import { ShortcutRegistry, utils, WorkspaceSvg, + keyboardNavigationController, } from 'blockly'; import {Direction} from '../drag_direction'; import {Mover} from './mover'; @@ -40,6 +41,7 @@ export class MoveActions { return !!startBlock && this.mover.canMove(workspace, startBlock); }, callback: (workspace) => { + keyboardNavigationController.setIsActive(true); const startBlock = this.getCurrentBlock(workspace); return ( !!startBlock && this.mover.startMove(workspace, startBlock, null) diff --git a/src/actions/ws_movement.ts b/src/actions/ws_movement.ts index 987736d9..3d4dfe50 100644 --- a/src/actions/ws_movement.ts +++ b/src/actions/ws_movement.ts @@ -4,7 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {ShortcutRegistry, utils as BlocklyUtils} from 'blockly'; +import { + ShortcutRegistry, + utils as BlocklyUtils, + keyboardNavigationController, +} from 'blockly'; import * as Constants from '../constants'; import type {WorkspaceSvg} from 'blockly'; import {Navigation} from 'src/navigation'; @@ -66,7 +70,10 @@ export class WorkspaceMovement { name: Constants.SHORTCUT_NAMES.CREATE_WS_CURSOR, preconditionFn: (workspace) => this.navigation.canCurrentlyEdit(workspace), - callback: (workspace) => this.createWSCursor(workspace), + callback: (workspace) => { + keyboardNavigationController.setIsActive(true); + return this.createWSCursor(workspace); + }, keyCodes: [KeyCodes.W], }, ]; diff --git a/src/index.ts b/src/index.ts index 99c9f17e..efe96d58 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,7 +7,6 @@ import * as Blockly from 'blockly/core'; import {NavigationController} from './navigation_controller'; import {enableBlocksOnDrag} from './disabled_blocks'; -import {InputModeTracker} from './input_mode_tracker'; /** Plugin for keyboard navigation. */ export class KeyboardNavigation { @@ -20,11 +19,6 @@ export class KeyboardNavigation { /** Cursor for the main workspace. */ private cursor: Blockly.LineCursor; - /** - * Input mode tracking. - */ - private inputModeTracker: InputModeTracker; - /** * Focus ring in the workspace. */ @@ -54,7 +48,6 @@ export class KeyboardNavigation { this.navigationController.init(); this.navigationController.addWorkspace(workspace); this.navigationController.enable(workspace); - this.inputModeTracker = new InputModeTracker(workspace); this.cursor = new Blockly.LineCursor(workspace); @@ -124,7 +117,6 @@ export class KeyboardNavigation { // Remove the event listener that enables blocks on drag this.workspace.removeChangeListener(enableBlocksOnDrag); this.navigationController.dispose(); - this.inputModeTracker.dispose(); } /** diff --git a/src/input_mode_tracker.ts b/src/input_mode_tracker.ts deleted file mode 100644 index c2a4603b..00000000 --- a/src/input_mode_tracker.ts +++ /dev/null @@ -1,48 +0,0 @@ -import {WorkspaceSvg} from 'blockly'; - -/** - * Types of user input. - */ -const enum InputMode { - Keyboard, - Pointer, -} - -/** - * Tracks the most recent input mode and sets a class indicating we're in - * keyboard nav mode. - */ -export class InputModeTracker { - private lastEventMode: InputMode | null = null; - - private pointerEventHandler = () => { - this.lastEventMode = InputMode.Pointer; - }; - private keyboardEventHandler = () => { - this.lastEventMode = InputMode.Keyboard; - }; - private focusChangeHandler = () => { - const isKeyboard = this.lastEventMode === InputMode.Keyboard; - const classList = this.workspace.getInjectionDiv().classList; - const className = 'blocklyKeyboardNavigation'; - if (isKeyboard) { - classList.add(className); - } else { - classList.remove(className); - } - }; - - constructor(private workspace: WorkspaceSvg) { - document.addEventListener('pointerdown', this.pointerEventHandler, true); - document.addEventListener('keydown', this.keyboardEventHandler, true); - document.addEventListener('focusout', this.focusChangeHandler, true); - document.addEventListener('focusin', this.focusChangeHandler, true); - } - - dispose() { - document.removeEventListener('pointerdown', this.pointerEventHandler, true); - document.removeEventListener('keydown', this.keyboardEventHandler, true); - document.removeEventListener('focusout', this.focusChangeHandler, true); - document.removeEventListener('focusin', this.focusChangeHandler, true); - } -} From c4be1401728e8432f21c86895d41e99bb90cfefc Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Thu, 29 May 2025 11:54:27 -0700 Subject: [PATCH 2/6] feat: enable keyboard mode for toolbox shortcut --- src/navigation_controller.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index f3831077..b27f4ccf 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -18,6 +18,7 @@ import { Toolbox, utils as BlocklyUtils, WorkspaceSvg, + keyboardNavigationController, } from 'blockly/core'; import * as Constants from './constants'; @@ -198,6 +199,7 @@ export class NavigationController { preconditionFn: (workspace) => !workspace.isDragging() && this.navigation.canCurrentlyEdit(workspace), callback: (workspace) => { + keyboardNavigationController.setIsActive(true); switch (this.navigation.getState(workspace)) { case Constants.STATE.WORKSPACE: Blockly.getFocusManager().focusTree( From 713d51ecfb1691b92508ef35aff2e7e1c55bf4b0 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Thu, 29 May 2025 11:54:43 -0700 Subject: [PATCH 3/6] chore: add tests --- test/webdriverio/test/basic_test.ts | 1 + test/webdriverio/test/keyboard_mode_test.ts | 167 ++++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 test/webdriverio/test/keyboard_mode_test.ts diff --git a/test/webdriverio/test/basic_test.ts b/test/webdriverio/test/basic_test.ts index c8701140..2902e28d 100644 --- a/test/webdriverio/test/basic_test.ts +++ b/test/webdriverio/test/basic_test.ts @@ -43,6 +43,7 @@ suite('Keyboard navigation on Blocks', function () { test('Selected block', async function () { await tabNavigateToWorkspace(this.browser); + await this.browser.pause(PAUSE_TIME); await keyDown(this.browser, 14); diff --git a/test/webdriverio/test/keyboard_mode_test.ts b/test/webdriverio/test/keyboard_mode_test.ts new file mode 100644 index 00000000..ebe0aefd --- /dev/null +++ b/test/webdriverio/test/keyboard_mode_test.ts @@ -0,0 +1,167 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as chai from 'chai'; +import * as Blockly from 'blockly'; +import { + setCurrentCursorNodeById, + testSetup, + testFileLocations, + PAUSE_TIME, + focusWorkspace, + getBlockElementById, +} from './test_setup.js'; +import {Key} from 'webdriverio'; + +const isKeyboardNavigating = function (browser: WebdriverIO.Browser) { + return browser.execute(() => { + return document.body.classList.contains('blocklyKeyboardNavigation'); + }); +}; + +suite( + 'Keyboard navigation mode set on mouse or keyboard interaction', + function () { + // Setting timeout to unlimited as these tests take a longer time to run than most mocha tests + this.timeout(0); + + // Setup Selenium for all of the tests + suiteSetup(async function () { + this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS); + }); + + setup(async function () { + // Reset the keyboard navigation state between tests. + await this.browser.execute(() => { + Blockly.keyboardNavigationController.setIsActive(false); + }); + }); + + test('T to open toolbox enables keyboard mode', async function () { + await focusWorkspace(this.browser); + await this.browser.pause(PAUSE_TIME); + await this.browser.keys('t'); + await this.browser.pause(PAUSE_TIME); + + chai.assert.isTrue(await isKeyboardNavigating(this.browser)); + }); + + test('M for move mode enables keyboard mode', async function () { + await focusWorkspace(this.browser); + await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await this.browser.pause(PAUSE_TIME); + await this.browser.keys('m'); + + chai.assert.isTrue(await isKeyboardNavigating(this.browser)); + }); + + test('W for workspace cursor enables keyboard mode', async function () { + await focusWorkspace(this.browser); + await this.browser.pause(PAUSE_TIME); + await this.browser.keys('w'); + await this.browser.pause(PAUSE_TIME); + + chai.assert.isTrue(await isKeyboardNavigating(this.browser)); + }); + + test('X to disconnect enables keyboard mode', async function () { + await focusWorkspace(this.browser); + await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await this.browser.pause(PAUSE_TIME); + await this.browser.keys('x'); + await this.browser.pause(PAUSE_TIME); + + chai.assert.isTrue(await isKeyboardNavigating(this.browser)); + }); + + test('Copy does not change keyboard mode state', async function () { + await focusWorkspace(this.browser); + + // Make sure we're on a copyable block so that copy occurs + await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await this.browser.pause(PAUSE_TIME); + await this.browser.keys(Key.Ctrl); + await this.browser.keys('c'); + await this.browser.keys(Key.Ctrl); // release ctrl key + await this.browser.pause(PAUSE_TIME); + + chai.assert.isFalse(await isKeyboardNavigating(this.browser)); + + this.browser.execute(() => { + Blockly.keyboardNavigationController.setIsActive(true); + }); + + await this.browser.pause(PAUSE_TIME); + await this.browser.keys(Key.Ctrl); + await this.browser.keys('c'); + await this.browser.keys(Key.Ctrl); // release ctrl key + await this.browser.pause(PAUSE_TIME); + + chai.assert.isTrue(await isKeyboardNavigating(this.browser)); + }); + + test('Delete does not change keyboard mode state', async function () { + await focusWorkspace(this.browser); + + // Make sure we're on a deletable block so that delete occurs + await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await this.browser.pause(PAUSE_TIME); + await this.browser.keys(Key.Backspace); + await this.browser.pause(PAUSE_TIME); + + chai.assert.isFalse(await isKeyboardNavigating(this.browser)); + + this.browser.execute(() => { + Blockly.keyboardNavigationController.setIsActive(true); + }); + + // Focus a different deletable block + await setCurrentCursorNodeById(this.browser, 'controls_if_1'); + await this.browser.pause(PAUSE_TIME); + await this.browser.keys(Key.Backspace); + await this.browser.pause(PAUSE_TIME); + + chai.assert.isTrue(await isKeyboardNavigating(this.browser)); + }); + + test('Right clicking a block disables keyboard mode', async function () { + await focusWorkspace(this.browser); + await this.browser.execute(() => { + Blockly.keyboardNavigationController.setIsActive(true); + }); + + await this.browser.pause(PAUSE_TIME); + // Right click a block + const element = await getBlockElementById( + this.browser, + 'simple_circle_1', + ); + await element.click({button: 'right'}); + await this.browser.pause(PAUSE_TIME); + + chai.assert.isFalse(await isKeyboardNavigating(this.browser)); + }); + + test('Dragging a block with mouse disables keyboard mode', async function () { + await focusWorkspace(this.browser); + + await this.browser.execute(() => { + Blockly.keyboardNavigationController.setIsActive(true); + }); + + await this.browser.pause(PAUSE_TIME); + // Drag a block + const element = await getBlockElementById( + this.browser, + 'simple_circle_1', + ); + await element.dragAndDrop({x: 100, y: 100}); + await this.browser.pause(PAUSE_TIME); + + chai.assert.isFalse(await isKeyboardNavigating(this.browser)); + }); + }, +); From eaf28e19681e7244a357792d56dab816b5bd3f49 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Thu, 29 May 2025 12:33:02 -0700 Subject: [PATCH 4/6] fix: rename test method after rebase --- test/webdriverio/test/keyboard_mode_test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/webdriverio/test/keyboard_mode_test.ts b/test/webdriverio/test/keyboard_mode_test.ts index ebe0aefd..b8f6cfe0 100644 --- a/test/webdriverio/test/keyboard_mode_test.ts +++ b/test/webdriverio/test/keyboard_mode_test.ts @@ -7,7 +7,7 @@ import * as chai from 'chai'; import * as Blockly from 'blockly'; import { - setCurrentCursorNodeById, + focusOnBlock, testSetup, testFileLocations, PAUSE_TIME, @@ -51,7 +51,7 @@ suite( test('M for move mode enables keyboard mode', async function () { await focusWorkspace(this.browser); - await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); await this.browser.keys('m'); @@ -69,7 +69,7 @@ suite( test('X to disconnect enables keyboard mode', async function () { await focusWorkspace(this.browser); - await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); await this.browser.keys('x'); await this.browser.pause(PAUSE_TIME); @@ -81,7 +81,7 @@ suite( await focusWorkspace(this.browser); // Make sure we're on a copyable block so that copy occurs - await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); await this.browser.keys(Key.Ctrl); await this.browser.keys('c'); @@ -107,7 +107,7 @@ suite( await focusWorkspace(this.browser); // Make sure we're on a deletable block so that delete occurs - await setCurrentCursorNodeById(this.browser, 'controls_if_2'); + await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); await this.browser.keys(Key.Backspace); await this.browser.pause(PAUSE_TIME); @@ -119,7 +119,7 @@ suite( }); // Focus a different deletable block - await setCurrentCursorNodeById(this.browser, 'controls_if_1'); + await focusOnBlock(this.browser, 'controls_if_1'); await this.browser.pause(PAUSE_TIME); await this.browser.keys(Key.Backspace); await this.browser.pause(PAUSE_TIME); From 2698964ab916d8e15aa21a60a8cf75155e9e58bc Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Thu, 29 May 2025 14:13:37 -0700 Subject: [PATCH 5/6] chore: fix tests --- test/webdriverio/test/keyboard_mode_test.ts | 24 +++++++-------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/test/webdriverio/test/keyboard_mode_test.ts b/test/webdriverio/test/keyboard_mode_test.ts index b8f6cfe0..f9ec9a36 100644 --- a/test/webdriverio/test/keyboard_mode_test.ts +++ b/test/webdriverio/test/keyboard_mode_test.ts @@ -11,8 +11,8 @@ import { testSetup, testFileLocations, PAUSE_TIME, - focusWorkspace, getBlockElementById, + tabNavigateToWorkspace, } from './test_setup.js'; import {Key} from 'webdriverio'; @@ -28,20 +28,22 @@ suite( // Setting timeout to unlimited as these tests take a longer time to run than most mocha tests this.timeout(0); - // Setup Selenium for all of the tests - suiteSetup(async function () { + setup(async function () { + // Reload the page between tests this.browser = await testSetup(testFileLocations.NAVIGATION_TEST_BLOCKS); - }); - setup(async function () { + await this.browser.pause(PAUSE_TIME); + // Reset the keyboard navigation state between tests. await this.browser.execute(() => { Blockly.keyboardNavigationController.setIsActive(false); }); + + // Start with the workspace focused. + await tabNavigateToWorkspace(this.browser); }); test('T to open toolbox enables keyboard mode', async function () { - await focusWorkspace(this.browser); await this.browser.pause(PAUSE_TIME); await this.browser.keys('t'); await this.browser.pause(PAUSE_TIME); @@ -50,7 +52,6 @@ suite( }); test('M for move mode enables keyboard mode', async function () { - await focusWorkspace(this.browser); await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); await this.browser.keys('m'); @@ -59,7 +60,6 @@ suite( }); test('W for workspace cursor enables keyboard mode', async function () { - await focusWorkspace(this.browser); await this.browser.pause(PAUSE_TIME); await this.browser.keys('w'); await this.browser.pause(PAUSE_TIME); @@ -68,7 +68,6 @@ suite( }); test('X to disconnect enables keyboard mode', async function () { - await focusWorkspace(this.browser); await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); await this.browser.keys('x'); @@ -78,8 +77,6 @@ suite( }); test('Copy does not change keyboard mode state', async function () { - await focusWorkspace(this.browser); - // Make sure we're on a copyable block so that copy occurs await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); @@ -104,8 +101,6 @@ suite( }); test('Delete does not change keyboard mode state', async function () { - await focusWorkspace(this.browser); - // Make sure we're on a deletable block so that delete occurs await focusOnBlock(this.browser, 'controls_if_2'); await this.browser.pause(PAUSE_TIME); @@ -128,7 +123,6 @@ suite( }); test('Right clicking a block disables keyboard mode', async function () { - await focusWorkspace(this.browser); await this.browser.execute(() => { Blockly.keyboardNavigationController.setIsActive(true); }); @@ -146,8 +140,6 @@ suite( }); test('Dragging a block with mouse disables keyboard mode', async function () { - await focusWorkspace(this.browser); - await this.browser.execute(() => { Blockly.keyboardNavigationController.setIsActive(true); }); From 361c9f270bbf8f7cd03fc0a174b8429c564c2189 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Thu, 29 May 2025 14:34:40 -0700 Subject: [PATCH 6/6] chore: try to fix tests --- test/webdriverio/test/keyboard_mode_test.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/test/webdriverio/test/keyboard_mode_test.ts b/test/webdriverio/test/keyboard_mode_test.ts index f9ec9a36..98ed5fbe 100644 --- a/test/webdriverio/test/keyboard_mode_test.ts +++ b/test/webdriverio/test/keyboard_mode_test.ts @@ -129,10 +129,7 @@ suite( await this.browser.pause(PAUSE_TIME); // Right click a block - const element = await getBlockElementById( - this.browser, - 'simple_circle_1', - ); + const element = await getBlockElementById(this.browser, 'controls_if_1'); await element.click({button: 'right'}); await this.browser.pause(PAUSE_TIME); @@ -146,11 +143,8 @@ suite( await this.browser.pause(PAUSE_TIME); // Drag a block - const element = await getBlockElementById( - this.browser, - 'simple_circle_1', - ); - await element.dragAndDrop({x: 100, y: 100}); + const element = await getBlockElementById(this.browser, 'controls_if_1'); + await element.dragAndDrop({x: 10, y: 10}); await this.browser.pause(PAUSE_TIME); chai.assert.isFalse(await isKeyboardNavigating(this.browser));