Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/blockly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ import {IVariableModel, IVariableState} from './interfaces/i_variable_model.js';
import * as internalConstants from './internal_constants.js';
import {LineCursor} from './keyboard_nav/line_cursor.js';
import {Marker} from './keyboard_nav/marker.js';
import {
KeyboardNavigationController,
keyboardNavigationController,
} from './keyboard_navigation_controller.js';
import type {LayerManager} from './layer_manager.js';
import * as layers from './layers.js';
import {MarkerManager} from './marker_manager.js';
Expand Down Expand Up @@ -580,6 +584,7 @@ export {
ImageProperties,
Input,
InsertionMarkerPreviewer,
KeyboardNavigationController,
LabelFlyoutInflater,
LayerManager,
Marker,
Expand Down Expand Up @@ -631,6 +636,7 @@ export {
isSelectable,
isSerializable,
isVariableBackedParameterModel,
keyboardNavigationController,
layers,
renderManagement,
serialization,
Expand Down
5 changes: 5 additions & 0 deletions core/gesture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {IDraggable, isDraggable} from './interfaces/i_draggable.js';
import {IDragger} from './interfaces/i_dragger.js';
import type {IFlyout} from './interfaces/i_flyout.js';
import type {IIcon} from './interfaces/i_icon.js';
import {keyboardNavigationController} from './keyboard_navigation_controller.js';
import * as registry from './registry.js';
import * as Tooltip from './tooltip.js';
import * as Touch from './touch.js';
Expand Down Expand Up @@ -541,8 +542,10 @@ export class Gesture {
// have higher priority than workspaces. The ordering within drags does
// not matter, because the three types of dragging are exclusive.
if (this.dragger) {
keyboardNavigationController.setIsActive(false);
this.dragger.onDragEnd(e, this.currentDragDeltaXY);
} else if (this.workspaceDragger) {
keyboardNavigationController.setIsActive(false);
this.workspaceDragger.endDrag(this.currentDragDeltaXY);
} else if (this.isBubbleClick()) {
// Do nothing, bubbles don't currently respond to clicks.
Expand Down Expand Up @@ -743,6 +746,8 @@ export class Gesture {
e.preventDefault();
e.stopPropagation();

keyboardNavigationController.setIsActive(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, what happens if someone tries using the menu key? Does that simulate a right click or require a key event handler? It doesn't currently work in Blockly, and I'm unsure of the expectations for that key for accessibility (or if those expectations even matter since we have so much custom key binding, anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you say menu key, do you mean the keyboard shortcut to open the context menu? That just opens the menu, it doesn't go through the gesture system, so it isn't affected by this.

Copy link
Collaborator

@BenHenning BenHenning May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think even your question to try and disambiguate my question is still ambiguous. :) "Menu key" is a bit confusing.

I actually mean this key: https://en.wikipedia.org/wiki/Menu_key. From testing it seems like it does nothing, so this is probably a no-op, but it occurred to me because it could theoretically be implemented as a right click on systems where right clicks are intended for opening the context menu.


this.dispose();
}

Expand Down
63 changes: 63 additions & 0 deletions core/keyboard_navigation_controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

/**
* The KeyboardNavigationController handles coordinating Blockly-wide
* keyboard navigation behavior, such as enabling/disabling full
* cursor visualization.
*/
export class KeyboardNavigationController {
/** Whether the user is actively using keyboard navigation. */
private isActive = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an important difference here with the version implemented in RaspberryPiFoundation/blockly-keyboard-experimentation#511: this doesn't default on when focus is received.

This brings up a broader question: how do we decide a proper default? For non-plugin users it completely makes sense to default to off since keyboard nav will be confusing and most users are likely to use mouse. However, as we move more of keyboard nav into core there's an eventual question of properly initializing this state based on the user. I'm not sure arrow keys and the like are the correct approach since we're teaching users to tab-navigate before using arrow keys.

Tab navigation right now just means focusing, and focusing happens on click. We could perhaps listen for tabs and forward them to the browser (and use the presence of pressing tab to mean 'enable keyboard nav'), but this gets tricky with the likes of #9049.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think tabbing should activate keyboard navigation mode. I would put it in the same category as delete or ctrl+c.

If a user is using keyboard navigation, they won't see the visualizations until they start using the arrow keys or other shortcuts. In practice, that means they are missing one visualization: the workspace focus ring when any part of the workspace (including blocks on it) has active focus. Any other interaction that would show a visualization would entail first using one of the actions that turns on keyboard mode. And even in this case, they do still get the block focus ring.

This approach also lets the application set their own heuristics. They could enable it if the user has enabled some preference, or if the user was using keyboard navigation last time they loaded the workspace for example. I think it's reasonable for blockly to default to off and let the application to decide a different default if they want.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes sense, though I do wonder if we need to document it somewhere more clearly so that application authors know exactly what to expect with how, and when, keyboard nav mode properly enables.

The two flows I'm unsure of are:

  • A low vision user relying on a combination of tab/keyboard nav and visuals for interaction (with no mouse movement). This might be an extremely specific case though, I'm not sure.
  • A user with motor impairments that has to use limited keyboard actions, but is not visually impaired and is still largely relying on visual indicators.

Both can certainly be made better with application side customizing, as you mentioned.

For low vision users not relying on visuals then the on/off highlights shouldn't matter, and users using a combination of keyboard and mouse I suspect will use mouse to select a block before arrow keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that microbit really wants to avoid having a user-setting for this, but I think for the reasons you mentioned and others, it probably makes sense to have one. But that will be the application's responsibility to figure out :)

/** Css class name added to body if keyboard nav is active. */
private activeClassName = 'blocklyKeyboardNavigation';

/**
* Sets whether a user is actively using keyboard navigation.
*
* If they are, apply a css class to the entire page so that
* focused items can apply additional styling for keyboard users.
*
* Note that since enabling keyboard navigation presents significant UX changes
* (such as cursor visualization and move mode), callers should take care to
* only set active keyboard navigation when they have a high confidence in that
* being the correct state. In general, in any given mouse or key input situation
* callers can choose one of three paths:
* 1. Do nothing. This should be the choice for neutral actions that don't
* predominantly imply keyboard or mouse usage (such as clicking to select a block).
* 2. Disable keyboard navigation. This is the best choice when a user is definitely
* predominantly using the mouse (such as using a right click to open the context menu).
* 3. Enable keyboard navigation. This is the best choice when there's high confidence
* a user actually intends to use it (such as attempting to use the arrow keys to move
* around).
*
* @param isUsing
*/
setIsActive(isUsing: boolean = true) {
this.isActive = isUsing;
this.updateActiveVisualization();
}

/**
* @returns true if the user is actively using keyboard navigation
* (e.g., has recently taken some action that is only relevant to keyboard users)
*/
getIsActive(): boolean {
return this.isActive;
}

/** Adds or removes the css class that indicates keyboard navigation is active. */
private updateActiveVisualization() {
if (this.isActive) {
document.body.classList.add(this.activeClassName);
} else {
document.body.classList.remove(this.activeClassName);
}
}
}

/** Singleton instance of the keyboard navigation controller. */
export const keyboardNavigationController = new KeyboardNavigationController();
1 change: 1 addition & 0 deletions tests/mocha/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@
import './jso_deserialization_test.js';
import './jso_serialization_test.js';
import './json_test.js';
import './keyboard_navigation_controller_test.js';
import './layering_test.js';
import './blocks/lists_test.js';
import './blocks/logic_ternary_test.js';
Expand Down
37 changes: 37 additions & 0 deletions tests/mocha/keyboard_navigation_controller_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import {assert} from '../../node_modules/chai/chai.js';
import {
sharedTestSetup,
sharedTestTeardown,
} from './test_helpers/setup_teardown.js';

suite('Keyboard Navigation Controller', function () {
setup(function () {
sharedTestSetup.call(this);
Blockly.keyboardNavigationController.setIsActive(false);
});

teardown(function () {
sharedTestTeardown.call(this);
Blockly.keyboardNavigationController.setIsActive(false);
});

test('Setting active keyboard navigation adds css class', function () {
Blockly.keyboardNavigationController.setIsActive(true);
assert.isTrue(
document.body.classList.contains('blocklyKeyboardNavigation'),
);
});

test('Disabling active keyboard navigation removes css class', function () {
Blockly.keyboardNavigationController.setIsActive(false);
assert.isFalse(
document.body.classList.contains('blocklyKeyboardNavigation'),
);
});
});