From 4c5bd550081542fcdc28816c21a8128fc3413da1 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 11 Mar 2025 14:58:49 -0700 Subject: [PATCH 1/7] chore: Upgrade Blockly dependency to v12. --- package-lock.json | 14 +++++++------- package.json | 4 ++-- src/navigation.ts | 10 ++++++---- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2caed871..33c16420 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@blockly/keyboard-experiment", - "version": "0.0.3", + "version": "0.0.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@blockly/keyboard-experiment", - "version": "0.0.3", + "version": "0.0.4", "license": "Apache-2.0", "devDependencies": { "@blockly/dev-scripts": "^4.0.1", @@ -16,7 +16,7 @@ "@types/p5": "^1.7.6", "@typescript-eslint/eslint-plugin": "^6.7.2", "@typescript-eslint/parser": "^6.7.2", - "blockly": "^11.2.1", + "blockly": "^12.0.0-beta.1", "eslint": "^8.49.0", "eslint-config-google": "^0.14.0", "eslint-config-prettier": "^9.0.0", @@ -29,7 +29,7 @@ "typescript": "^5.4.5" }, "peerDependencies": { - "blockly": "^11.1.0" + "blockly": "^12.0.0-beta.1" } }, "node_modules/@asamuzakjp/css-color": { @@ -2187,9 +2187,9 @@ } }, "node_modules/blockly": { - "version": "11.2.1", - "resolved": "https://registry.npmjs.org/blockly/-/blockly-11.2.1.tgz", - "integrity": "sha512-20sCwSwX2Z6UxR/er0B5y6wRFukuIdvOjc7jMuIwyCO/yT35+UbAqYueMga3JFA9NoWPwQc+3s6/XnLkyceAww==", + "version": "12.0.0-beta.1", + "resolved": "https://registry.npmjs.org/blockly/-/blockly-12.0.0-beta.1.tgz", + "integrity": "sha512-lECwZ4K+YuLXMM0yxWTz1lwkmDl424sst7h/dhtSefuCki8afjI/F87byYK/ZIZsMKBEz2+8wEJ1Wlx5cYWIAg==", "dev": true, "license": "Apache-2.0", "dependencies": { diff --git a/package.json b/package.json index 4fa421e0..7c2a924b 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "@types/p5": "^1.7.6", "@typescript-eslint/eslint-plugin": "^6.7.2", "@typescript-eslint/parser": "^6.7.2", - "blockly": "^11.2.1", + "blockly": "^12.0.0-beta.1", "eslint": "^8.49.0", "eslint-config-google": "^0.14.0", "eslint-config-prettier": "^9.0.0", @@ -61,7 +61,7 @@ "typescript": "^5.4.5" }, "peerDependencies": { - "blockly": "^11.1.0" + "blockly": "^12.0.0-beta.1" }, "publishConfig": { "access": "public", diff --git a/src/navigation.ts b/src/navigation.ts index 1d44c9c6..416fcd1f 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -448,13 +448,15 @@ export class Navigation { const flyoutContents = flyout.getContents(); const firstFlyoutItem = flyoutContents[0]; if (!firstFlyoutItem) return; - if (firstFlyoutItem.button) { + if (firstFlyoutItem.getType() === 'button') { const astNode = Blockly.ASTNode.createButtonNode( - firstFlyoutItem.button, + firstFlyoutItem.getElement() as Blockly.FlyoutButton, ); this.getFlyoutCursor(workspace)!.setCurNode(astNode!); - } else if (firstFlyoutItem.block) { - const astNode = Blockly.ASTNode.createStackNode(firstFlyoutItem.block); + } else if (firstFlyoutItem.getType() === 'block') { + const astNode = Blockly.ASTNode.createStackNode( + firstFlyoutItem.getElement() as Blockly.BlockSvg, + ); this.getFlyoutCursor(workspace)!.setCurNode(astNode!); } } From c8a4c4081950c513bb3e669951d77b20540aa02d Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 11 Mar 2025 23:42:33 +0000 Subject: [PATCH 2/7] Merge test a11y changes to test v12. --- src/index.ts | 4 ++-- src/line_cursor.ts | 2 ++ src/navigation_controller.ts | 1 + test/index.ts | 1 + 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 6f8c0e49..1308fb2a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -88,8 +88,8 @@ export class KeyboardNavigation { this.navigationController.setHasFocus(workspace, false); }; - workspace.getSvgGroup().addEventListener('focus', this.focusListener); - workspace.getSvgGroup().addEventListener('blur', this.blurListener); + workspace.getSvgGroup().addEventListener('focusin', this.focusListener); + workspace.getSvgGroup().addEventListener('focusout', this.blurListener); // Temporary workaround for #136. // TODO(#136): fix in core. workspace.getParentSvg().addEventListener('focus', this.focusListener); diff --git a/src/line_cursor.ts b/src/line_cursor.ts index 0f1bd406..d2bf4f7e 100644 --- a/src/line_cursor.ts +++ b/src/line_cursor.ts @@ -712,6 +712,8 @@ export class LineCursor extends Marker { // select the block or make it look selected. super.hide(); // Calls this.drawer?.hide(). const block = curNode.getLocation() as Blockly.BlockSvg; + block.getSvgRoot().setAttribute('tabindex', '0'); + block.getSvgRoot().focus(); if (!block.isShadow()) { Blockly.common.setSelected(block); } else { diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index daf1d050..3424bb7f 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -330,6 +330,7 @@ export class NavigationController { case Constants.STATE.WORKSPACE: isHandled = this.fieldShortcutHandler(workspace, shortcut); if (!isHandled && workspace) { + console.log('Pre cursor next, cursor: ', workspace.getCursor()); workspace.getCursor()?.next(); isHandled = true; } diff --git a/test/index.ts b/test/index.ts index fdb71d61..b6e67091 100644 --- a/test/index.ts +++ b/test/index.ts @@ -80,6 +80,7 @@ function createWorkspace(): Blockly.WorkspaceSvg { const injectOptions = { toolbox: toolboxCategories, + comments: true, renderer, }; const blocklyDiv = document.getElementById('blocklyDiv')!; From 2c06524ef286d59f50ed9c837cf2f69d3540f53a Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 8 Apr 2025 16:41:51 +0000 Subject: [PATCH 3/7] More basic a11y testing work. This hooks up to FocusManager. --- src/index.ts | 1 + src/line_cursor.ts | 53 +++++++++++++++++++++++++++++----------------- test/index.html | 4 ++-- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/index.ts b/src/index.ts index 1308fb2a..92a4a2da 100644 --- a/src/index.ts +++ b/src/index.ts @@ -79,6 +79,7 @@ export class KeyboardNavigation { .getParentSvg() .getAttribute('tabindex'); // We add a focus listener below so use -1 so it doesn't become focusable. + workspace.getSvgGroup().setAttribute('tabindex', '-1'); workspace.getParentSvg().setAttribute('tabindex', '-1'); this.focusListener = () => { diff --git a/src/line_cursor.ts b/src/line_cursor.ts index d2bf4f7e..2d800f52 100644 --- a/src/line_cursor.ts +++ b/src/line_cursor.ts @@ -14,7 +14,7 @@ */ import * as Blockly from 'blockly/core'; -import {ASTNode, Marker} from 'blockly/core'; +import {ASTNode, Marker, IFocusableNode, isFocusableNode} from 'blockly/core'; import {scrollBoundsIntoView} from './workspace_utilities'; /** Options object for LineCursor instances. */ @@ -74,7 +74,9 @@ export class LineCursor extends Marker { const markerManager = this.workspace.getMarkerManager(); this.oldCursor = markerManager.getCursor(); markerManager.setCursor(this); - if (this.oldCursor) this.setCurNode(this.oldCursor.getCurNode()); + if (this.oldCursor && this.oldCursor.getCurNode()) { + this.setCurNode(this.oldCursor.getCurNode()); + } this.workspace.addChangeListener(this.selectListener); this.installed = true; } @@ -641,7 +643,33 @@ export class LineCursor extends Marker { super.setCurNode(newNode); this.setDrawer(drawer); // Draw this marker the way we want to. + + // If old node was a block, unselect it or remove fake selection. + if (oldNode?.getType() === ASTNode.types.BLOCK) { + const block = oldNode.getLocation() as Blockly.BlockSvg; + if (!block.isShadow()) { + Blockly.common.setSelected(null); + } else { + block.removeSelect(); + } + } + this.drawMarker(oldNode, newNode); + + const newNodeLocation = newNode?.getLocation(); + console.trace('@@@@@@', 'setCurNode', newNode?.getLocation()); + if (isFocusableNode(newNodeLocation)) { + Blockly.getFocusManager().focusNode(newNodeLocation); + } else console.log('Error: node is not focusable:', newNodeLocation); + if (newNode?.getType() === ASTNode.types.BLOCK) { + const block = newNode.getLocation() as Blockly.BlockSvg; + if (!block.isShadow()) { + Blockly.common.setSelected(block); + } else { + block.addSelect(); + } + } + // Try to scroll cursor into view. if (newNode?.getType() === ASTNode.types.BLOCK) { const block = newNode.getLocation() as Blockly.BlockSvg; @@ -692,16 +720,6 @@ export class LineCursor extends Marker { * @param curNode The current node. */ private drawMarker(oldNode: ASTNode, curNode: ASTNode) { - // If old node was a block, unselect it or remove fake selection. - if (oldNode?.getType() === ASTNode.types.BLOCK) { - const block = oldNode.getLocation() as Blockly.BlockSvg; - if (!block.isShadow()) { - Blockly.common.setSelected(null); - } else { - block.removeSelect(); - } - } - // If curNode node is not block, just use the drawer. if (curNode?.getType() !== ASTNode.types.BLOCK) { this.getDrawer()?.draw(oldNode, curNode); @@ -711,14 +729,9 @@ export class LineCursor extends Marker { // curNode is a block. Hide any visible marker SVG and instead // select the block or make it look selected. super.hide(); // Calls this.drawer?.hide(). - const block = curNode.getLocation() as Blockly.BlockSvg; - block.getSvgRoot().setAttribute('tabindex', '0'); - block.getSvgRoot().focus(); - if (!block.isShadow()) { - Blockly.common.setSelected(block); - } else { - block.addSelect(); - } + // const block = curNode.getLocation() as Blockly.BlockSvg; + // block.getSvgRoot().setAttribute('tabindex', '0'); + // block.getSvgRoot().focus(); // Call MarkerSvg.prototype.fireMarkerEvent like // MarkerSvg.prototype.draw would (even though it's private). diff --git a/test/index.html b/test/index.html index 2ac168dd..e4c9fa91 100644 --- a/test/index.html +++ b/test/index.html @@ -34,10 +34,10 @@ --outline-width: 5px; } - .blocklyWorkspace:focus .blocklyMainBackground { + /* .blocklyWorkspace:focus .blocklyMainBackground { outline: Highlight solid var(--outline-width); outline-offset: -5px; - } + } */ .blocklyFlyout { top: var(--outline-width); From bc6d4f41f34c819b1a24d3841aa83e2011356d8e Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 9 Apr 2025 23:01:19 +0000 Subject: [PATCH 4/7] feat: start using FocusManager (partly). This contains some initial code changes to incorporate FocusManager. More is happening in Core, and there's still a lot left to figure out. --- src/actions/arrow_navigation.ts | 51 +++++++++++++++++++++++---------- src/actions/exit.ts | 7 ++++- src/navigation.ts | 23 ++++++++++----- src/navigation_controller.ts | 45 ++++++++++++++++++----------- 4 files changed, 86 insertions(+), 40 deletions(-) diff --git a/src/actions/arrow_navigation.ts b/src/actions/arrow_navigation.ts index 0bae1c83..12444997 100644 --- a/src/actions/arrow_navigation.ts +++ b/src/actions/arrow_navigation.ts @@ -8,6 +8,7 @@ import {ASTNode, ShortcutRegistry, utils as BlocklyUtils} from 'blockly/core'; import type {Field, Toolbox, WorkspaceSvg} from 'blockly/core'; +import * as Blockly from 'blockly/core'; import * as Constants from '../constants'; import type {Navigation} from '../navigation'; @@ -47,6 +48,7 @@ export class ArrowNavigation { * Adds all arrow key navigation shortcuts to the registry. */ install() { + console.log('@@@@@ install arrow keys'); const shortcuts: { [name: string]: ShortcutRegistry.KeyboardShortcut; } = { @@ -73,12 +75,15 @@ export class ArrowNavigation { } return isHandled; case Constants.STATE.TOOLBOX: - isHandled = - toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; - if (!isHandled) { - this.navigation.focusFlyout(workspace); + isHandled = toolbox.selectChild(); + // isHandled = + // toolbox && typeof toolbox.onShortcut === 'function' + // ? toolbox.onShortcut(shortcut) + // : false; + const flyout = toolbox.getFlyout(); + if (!isHandled && flyout) { + Blockly.getFocusManager().focusTree(flyout); + // this.navigation.focusFlyout(workspace); } return true; default: @@ -114,9 +119,11 @@ export class ArrowNavigation { this.navigation.focusToolbox(workspace); return true; case Constants.STATE.TOOLBOX: - return toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; + isHandled = toolbox.selectParent(); + return isHandled; + // return toolbox && typeof toolbox.onShortcut === 'function' + // ? toolbox.onShortcut(shortcut) + // : false; default: return false; } @@ -157,9 +164,16 @@ export class ArrowNavigation { } return isHandled; case Constants.STATE.TOOLBOX: - return toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; + // TODO: Move this into cursor? + isHandled = toolbox.selectNext(); + const selectedItem = toolbox.getSelectedItem(); + if (selectedItem) { + Blockly.getFocusManager().focusNode(selectedItem); + } + return isHandled; + // return toolbox && typeof toolbox.onShortcut === 'function' + // ? toolbox.onShortcut(shortcut) + // : false; default: return false; } @@ -205,9 +219,16 @@ export class ArrowNavigation { } return isHandled; case Constants.STATE.TOOLBOX: - return toolbox && typeof toolbox.onShortcut === 'function' - ? toolbox.onShortcut(shortcut) - : false; + // TODO: Move this into cursor? + isHandled = toolbox.selectPrevious(); + const selectedItem = toolbox.getSelectedItem(); + if (selectedItem) { + Blockly.getFocusManager().focusNode(selectedItem); + } + return isHandled; + // return toolbox && typeof toolbox.onShortcut === 'function' + // ? toolbox.onShortcut(shortcut) + // : false; default: return false; } diff --git a/src/actions/exit.ts b/src/actions/exit.ts index 3b9666be..90840a02 100644 --- a/src/actions/exit.ts +++ b/src/actions/exit.ts @@ -6,6 +6,7 @@ import {ShortcutRegistry, utils as BlocklyUtils} from 'blockly/core'; +import * as Blockly from 'blockly/core'; import * as Constants from '../constants'; import type {Navigation} from '../navigation'; @@ -29,7 +30,11 @@ export class ExitAction { switch (this.navigation.getState(workspace)) { case Constants.STATE.FLYOUT: case Constants.STATE.TOOLBOX: - this.navigation.focusWorkspace(workspace); + Blockly.getFocusManager().focusTree(workspace); + if (!Blockly.Gesture.inProgress()) { + workspace.hideChaff(); + } + // this.navigation.focusWorkspace(workspace); return true; default: return false; diff --git a/src/navigation.ts b/src/navigation.ts index dcf5e79c..5fb63065 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -139,7 +139,15 @@ export class Navigation { * @returns The state of the given workspace. */ getState(workspace: Blockly.WorkspaceSvg): Constants.STATE { - return this.workspaceStates[workspace.id]; + const focusedTree = Blockly.getFocusManager().getFocusedTree(); + if (focusedTree instanceof Blockly.WorkspaceSvg) { + return Constants.STATE.WORKSPACE; + } else if (focusedTree instanceof Blockly.Toolbox) { + return Constants.STATE.TOOLBOX; + } else if (focusedTree instanceof Blockly.Flyout) { + return Constants.STATE.FLYOUT; + } else return Constants.STATE.NOWHERE; + // return this.workspaceStates[workspace.id]; } /** @@ -381,7 +389,7 @@ export class Navigation { * @param workspace The workspace to focus. */ focusWorkspace(workspace: Blockly.WorkspaceSvg) { - getWorkspaceElement(workspace).focus(); + // getWorkspaceElement(workspace).focus(); } /** @@ -458,9 +466,9 @@ export class Navigation { // https://github.com/google/blockly-samples/issues/2498 return; } - if (relatedTarget !== getWorkspaceElement(workspace)) { - this.handleBlurWorkspace(workspace, true); - } + // if (relatedTarget !== getWorkspaceElement(workspace)) { + // this.handleBlurWorkspace(workspace, true); + // } } /** @@ -469,7 +477,7 @@ export class Navigation { * @param workspace The workspace with the toolbox. */ focusToolbox(workspace: Blockly.WorkspaceSvg) { - getToolboxElement(workspace)?.focus(); + // getToolboxElement(workspace)?.focus(); } /** @@ -517,7 +525,7 @@ export class Navigation { * @param workspace The workspace with the flyout. */ focusFlyout(workspace: Blockly.WorkspaceSvg) { - getFlyoutElement(workspace)?.focus(); + // getFlyoutElement(workspace)?.focus(); } /** @@ -1105,6 +1113,7 @@ export class Navigation { * @returns whether keyboard navigation is currently allowed. */ canCurrentlyNavigate(workspace: Blockly.WorkspaceSvg) { + console.log('@@@@@@@ current state:', this.getState(workspace)); return ( workspace.keyboardAccessibilityMode && this.getState(workspace) !== Constants.STATE.NOWHERE diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index f64b247b..8584a293 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -129,16 +129,12 @@ export class NavigationController { } switch (shortcut.name) { case Constants.SHORTCUT_NAMES.UP: - // @ts-expect-error private method return this.selectPrevious(); case Constants.SHORTCUT_NAMES.LEFT: - // @ts-expect-error private method return this.selectParent(); case Constants.SHORTCUT_NAMES.DOWN: - // @ts-expect-error private method return this.selectNext(); case Constants.SHORTCUT_NAMES.RIGHT: - // @ts-expect-error private method return this.selectChild(); default: return false; @@ -169,46 +165,46 @@ export class NavigationController { } focusWorkspace(workspace: WorkspaceSvg) { - this.navigation.focusWorkspace(workspace); + // this.navigation.focusWorkspace(workspace); } handleFocusWorkspace(workspace: Blockly.WorkspaceSvg) { - this.navigation.handleFocusWorkspace(workspace); + // this.navigation.handleFocusWorkspace(workspace); } handleBlurWorkspace(workspace: Blockly.WorkspaceSvg) { - this.navigation.handleBlurWorkspace(workspace); + // this.navigation.handleBlurWorkspace(workspace); } handleFocusOutWidgetDropdownDiv( workspace: Blockly.WorkspaceSvg, relatedTarget: EventTarget | null, ) { - this.navigation.handleFocusOutWidgetDropdownDiv(workspace, relatedTarget); + // this.navigation.handleFocusOutWidgetDropdownDiv(workspace, relatedTarget); } focusToolbox(workspace: Blockly.WorkspaceSvg) { - this.navigation.focusToolbox(workspace); + // this.navigation.focusToolbox(workspace); } handleFocusToolbox(workspace: Blockly.WorkspaceSvg) { - this.navigation.handleFocusToolbox(workspace); + // this.navigation.handleFocusToolbox(workspace); } handleBlurToolbox(workspace: Blockly.WorkspaceSvg, closeFlyout: boolean) { - this.navigation.handleBlurToolbox(workspace, closeFlyout); + // this.navigation.handleBlurToolbox(workspace, closeFlyout); } focusFlyout(workspace: Blockly.WorkspaceSvg) { - this.navigation.focusFlyout(workspace); + // this.navigation.focusFlyout(workspace); } handleFocusFlyout(workspace: Blockly.WorkspaceSvg) { - this.navigation.handleFocusFlyout(workspace); + // this.navigation.handleFocusFlyout(workspace); } handleBlurFlyout(workspace: Blockly.WorkspaceSvg, closeFlyout: boolean) { - this.navigation.handleBlurFlyout(workspace, closeFlyout); + // this.navigation.handleBlurFlyout(workspace, closeFlyout); } /** @@ -245,10 +241,25 @@ export class NavigationController { callback: (workspace) => { switch (this.navigation.getState(workspace)) { case Constants.STATE.WORKSPACE: - if (!workspace.getToolbox()) { - this.navigation.focusFlyout(workspace); + const toolbox = workspace.getToolbox(); + if (!toolbox) { + Blockly.getFocusManager().focusTree(workspace); + // this.navigation.focusFlyout(workspace); } else { - this.navigation.focusToolbox(workspace); + Blockly.getFocusManager().focusTree(toolbox); + // Ensure that the first item is selected. + // TODO: Retain this across contexts? + if (!toolbox.getSelectedItem() && toolbox instanceof Blockly.Toolbox) { + // Find the first item that is selectable. + const toolboxItems = toolbox.getToolboxItems(); + for (let i = 0, toolboxItem; (toolboxItem = toolboxItems[i]); i++) { + if (toolboxItem.isSelectable()) { + toolbox.selectItemByPosition(i); + break; + } + } + } + // this.navigation.focusToolbox(workspace); } return true; default: From 48882435aebf7e7f08533b072f8f7af9ee90c237 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Fri, 11 Apr 2025 01:43:38 +0000 Subject: [PATCH 5/7] feat: more feature reduction. This further reduces the amount of responsibility that the plugin has in favor of core + FocusManager changes. --- src/actions/arrow_navigation.ts | 5 +-- src/index.ts | 56 ++++++++++++++++----------------- src/navigation.ts | 5 ++- src/navigation_controller.ts | 22 +++++++------ 4 files changed, 47 insertions(+), 41 deletions(-) diff --git a/src/actions/arrow_navigation.ts b/src/actions/arrow_navigation.ts index 12444997..7070473c 100644 --- a/src/actions/arrow_navigation.ts +++ b/src/actions/arrow_navigation.ts @@ -82,7 +82,7 @@ export class ArrowNavigation { // : false; const flyout = toolbox.getFlyout(); if (!isHandled && flyout) { - Blockly.getFocusManager().focusTree(flyout); + Blockly.getFocusManager().focusTree(flyout.getWorkspace()); // this.navigation.focusFlyout(workspace); } return true; @@ -116,7 +116,8 @@ export class ArrowNavigation { } return isHandled; case Constants.STATE.FLYOUT: - this.navigation.focusToolbox(workspace); + Blockly.getFocusManager().focusTree(toolbox); + // this.navigation.focusToolbox(workspace); return true; case Constants.STATE.TOOLBOX: isHandled = toolbox.selectParent(); diff --git a/src/index.ts b/src/index.ts index 7bbfebe0..d5bcb755 100644 --- a/src/index.ts +++ b/src/index.ts @@ -54,8 +54,8 @@ export class KeyboardNavigation { * These fields are used to preserve the workspace's initial state to restore * it when/if keyboard navigation is disabled. */ - private injectionDivTabIndex: string | null; - private workspaceParentTabIndex: string | null; + // private injectionDivTabIndex: string | null; + // private workspaceParentTabIndex: string | null; private originalTheme: Blockly.Theme; /** @@ -93,17 +93,17 @@ export class KeyboardNavigation { this.cursor = new Blockly.LineCursor(workspace, options.cursor); - // Ensure that only the root SVG G (group) has a tab index. - this.injectionDivTabIndex = workspace - .getInjectionDiv() - .getAttribute('tabindex'); - workspace.getInjectionDiv().removeAttribute('tabindex'); - this.workspaceParentTabIndex = workspace - .getParentSvg() - .getAttribute('tabindex'); - // We add a focus listener below so use -1 so it doesn't become focusable. - workspace.getSvgGroup().setAttribute('tabindex', '-1'); - workspace.getParentSvg().setAttribute('tabindex', '-1'); + // // Ensure that only the root SVG G (group) has a tab index. + // this.injectionDivTabIndex = workspace + // .getInjectionDiv() + // .getAttribute('tabindex'); + // workspace.getInjectionDiv().removeAttribute('tabindex'); + // this.workspaceParentTabIndex = workspace + // .getParentSvg() + // .getAttribute('tabindex'); + // // We add a focus listener below so use -1 so it doesn't become focusable. + // workspace.getSvgGroup().setAttribute('tabindex', '-1'); + // workspace.getParentSvg().setAttribute('tabindex', '-1'); // Move the flyout for logical tab order. const flyoutElement = getFlyoutElement(workspace); @@ -259,21 +259,21 @@ export class KeyboardNavigation { flyoutElement?.removeEventListener('focus', this.flyoutFocusListener); flyoutElement?.removeEventListener('blur', this.flyoutBlurListener); - if (this.workspaceParentTabIndex) { - this.workspace - .getParentSvg() - .setAttribute('tabindex', this.workspaceParentTabIndex); - } else { - this.workspace.getParentSvg().removeAttribute('tabindex'); - } - - if (this.injectionDivTabIndex) { - this.workspace - .getInjectionDiv() - .setAttribute('tabindex', this.injectionDivTabIndex); - } else { - this.workspace.getInjectionDiv().removeAttribute('tabindex'); - } + // if (this.workspaceParentTabIndex) { + // this.workspace + // .getParentSvg() + // .setAttribute('tabindex', this.workspaceParentTabIndex); + // } else { + // this.workspace.getParentSvg().removeAttribute('tabindex'); + // } + + // if (this.injectionDivTabIndex) { + // this.workspace + // .getInjectionDiv() + // .setAttribute('tabindex', this.injectionDivTabIndex); + // } else { + // this.workspace.getInjectionDiv().removeAttribute('tabindex'); + // } this.workspace.setTheme(this.originalTheme); diff --git a/src/navigation.ts b/src/navigation.ts index 5fb63065..0150dcbb 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -141,7 +141,10 @@ export class Navigation { getState(workspace: Blockly.WorkspaceSvg): Constants.STATE { const focusedTree = Blockly.getFocusManager().getFocusedTree(); if (focusedTree instanceof Blockly.WorkspaceSvg) { - return Constants.STATE.WORKSPACE; + // TODO: Is the instanceof Flyout below ever needed now? Probably not since Flyout shouldn't actually be a real IFocusableTree... + if (focusedTree.isFlyout) { + return Constants.STATE.FLYOUT; + } else return Constants.STATE.WORKSPACE; } else if (focusedTree instanceof Blockly.Toolbox) { return Constants.STATE.TOOLBOX; } else if (focusedTree instanceof Blockly.Flyout) { diff --git a/src/navigation_controller.ts b/src/navigation_controller.ts index 8584a293..5975f942 100644 --- a/src/navigation_controller.ts +++ b/src/navigation_controller.ts @@ -246,19 +246,21 @@ export class NavigationController { Blockly.getFocusManager().focusTree(workspace); // this.navigation.focusFlyout(workspace); } else { + // The toolbox receiving focus should ensure it has at least its first item selected + // if there was no previous focus yet. Blockly.getFocusManager().focusTree(toolbox); // Ensure that the first item is selected. // TODO: Retain this across contexts? - if (!toolbox.getSelectedItem() && toolbox instanceof Blockly.Toolbox) { - // Find the first item that is selectable. - const toolboxItems = toolbox.getToolboxItems(); - for (let i = 0, toolboxItem; (toolboxItem = toolboxItems[i]); i++) { - if (toolboxItem.isSelectable()) { - toolbox.selectItemByPosition(i); - break; - } - } - } + // if (!toolbox.getSelectedItem() && toolbox instanceof Blockly.Toolbox) { + // // Find the first item that is selectable. + // const toolboxItems = toolbox.getToolboxItems(); + // for (let i = 0, toolboxItem; (toolboxItem = toolboxItems[i]); i++) { + // if (toolboxItem.isSelectable()) { + // toolbox.selectItemByPosition(i); + // break; + // } + // } + // } // this.navigation.focusToolbox(workspace); } return true; From bc588203eb6a612b4a1379ae40c773fe1f9e82ca Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Fri, 18 Apr 2025 00:35:47 +0000 Subject: [PATCH 6/7] chore: couple of fixes. This removes the passive indicator rendering since FocusManager handles it. It also updates part of toolbox navigation to select a first item if there isn't currently a selection (which state is now possible due to tab navigation). --- src/actions/arrow_navigation.ts | 6 +++++- src/navigation.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/actions/arrow_navigation.ts b/src/actions/arrow_navigation.ts index 7070473c..dd91bcbb 100644 --- a/src/actions/arrow_navigation.ts +++ b/src/actions/arrow_navigation.ts @@ -166,7 +166,11 @@ export class ArrowNavigation { return isHandled; case Constants.STATE.TOOLBOX: // TODO: Move this into cursor? - isHandled = toolbox.selectNext(); + if (!toolbox.getSelectedItem()) { + const firstItem = toolbox.getToolboxItems().find((item) => item.isSelectable()) ?? null; + toolbox.setSelectedItem(firstItem); + isHandled = true; + } else isHandled = toolbox.selectNext(); const selectedItem = toolbox.getSelectedItem(); if (selectedItem) { Blockly.getFocusManager().focusNode(selectedItem); diff --git a/src/navigation.ts b/src/navigation.ts index 0150dcbb..7b41c087 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -441,7 +441,7 @@ export class Navigation { if (cursor && (ignorePopUpDivs || !popUpDivsShowing)) { const curNode = cursor.getCurNode(); if (curNode) { - this.passiveFocusIndicator.show(curNode); + // this.passiveFocusIndicator.show(curNode); } // It's initially null so this is a valid state despite the types. cursor.setCurNode(null); From b86dfd7bb20c9e674d8b00467d0a56d936bbb1aa Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Mon, 28 Apr 2025 20:16:55 +0000 Subject: [PATCH 7/7] fix: add support for comments (for testing). --- test/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/index.ts b/test/index.ts index 34c0794a..a878522e 100644 --- a/test/index.ts +++ b/test/index.ts @@ -104,6 +104,8 @@ function createWorkspace(): Blockly.WorkspaceSvg { new KeyboardNavigation(workspace, navigationOptions); registerRunCodeShortcut(); + Blockly.ContextMenuItems.registerCommentOptions(); + // Disable blocks that aren't inside the setup or draw loops. workspace.addChangeListener(Blockly.Events.disableOrphans);