-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: add keyboard navigation controller #8924
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 { | ||
maribethb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** Whether the user is actively using keyboard navigation. */ | ||
| private isActive = false; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
BenHenning marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| */ | ||
| 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(); | ||
| 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'), | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.