From 23e90c0fdbe098fba104359e840f9e8ab1bf5e66 Mon Sep 17 00:00:00 2001 From: Parman Mohammadalizadeh Date: Fri, 31 Jul 2026 09:27:44 +0200 Subject: [PATCH 1/2] feat: add toolbox and flyout navigation to the optional jump shortcuts Page Up, Page Down, Ctrl/Cmd + Home and Ctrl/Cmd + End only acted on workspace blocks. In the toolbox they moved focus out of the toolbox and into the workspace, and in the flyout the page shortcuts did nothing. Ctrl/Cmd + Home and End now move to the first and last item of whichever of the workspace, toolbox or flyout holds focus. Page Up and Page Down move focus to the first or last currently visible item of the toolbox or flyout and scroll it to the edge of the viewport it moved towards, so that nothing between the old and new position is passed over unseen. No new key bindings are introduced, and everything stays inside the opt-in registerNavigationShortcuts bundle. Fixes #10187 --- .../core/keyboard_nav/navigators/navigator.ts | 19 + packages/blockly/core/shortcut_items.ts | 310 +++++++++++++++- packages/blockly/msg/json/en.json | 2 + packages/blockly/msg/json/qqq.json | 2 + packages/blockly/msg/messages.js | 6 + .../tests/mocha/keyboard_navigation_test.js | 342 ++++++++++++++++++ .../tests/mocha/shortcut_items_test.js | 9 +- .../docs/guides/configure/keyboard-nav.mdx | 15 + 8 files changed, 702 insertions(+), 3 deletions(-) diff --git a/packages/blockly/core/keyboard_nav/navigators/navigator.ts b/packages/blockly/core/keyboard_nav/navigators/navigator.ts index 785f5e46075..112505b75bd 100644 --- a/packages/blockly/core/keyboard_nav/navigators/navigator.ts +++ b/packages/blockly/core/keyboard_nav/navigators/navigator.ts @@ -336,6 +336,25 @@ export class Navigator { return this.navigationLoops; } + /** + * Returns the navigable top-level items of a tree, in navigation order. + * + * For a workspace these are its stacks and comments; for a toolbox its + * categories; for a flyout its blocks, buttons and labels. + * + * @param root The root node of the tree to list, defaulting to the root of + * the currently focused tree. + * @returns The navigable top-level items, or an empty list if there is no + * tree to list. + */ + getNavigableItems( + root = getFocusManager().getFocusedTree()?.getRootFocusableNode(), + ): IFocusableNode[] { + if (!root) return []; + + return this.getTopLevelItems(root).filter((item) => this.isNavigable(item)); + } + /** * Get the first navigable node on the workspace, or null if none exist. * diff --git a/packages/blockly/core/shortcut_items.ts b/packages/blockly/core/shortcut_items.ts index 8c7609317ff..4adb8f778b7 100644 --- a/packages/blockly/core/shortcut_items.ts +++ b/packages/blockly/core/shortcut_items.ts @@ -22,6 +22,10 @@ import { showCutHint, showScreenreaderModeHint, } from './hints.js'; +import { + type IBoundedElement, + isBoundedElement, +} from './interfaces/i_bounded_element.js'; import {hasContextMenu} from './interfaces/i_contextmenu.js'; import {isCopyable as isICopyable} from './interfaces/i_copyable.js'; import {isDeletable as isIDeletable} from './interfaces/i_deletable.js'; @@ -29,6 +33,7 @@ import {type IDraggable, isDraggable} from './interfaces/i_draggable.js'; import {type IFlyout} from './interfaces/i_flyout.js'; import {type IFocusableNode} from './interfaces/i_focusable_node.js'; import {isSelectable} from './interfaces/i_selectable.js'; +import type {IToolbox} from './interfaces/i_toolbox.js'; import {Direction, KeyboardMover} from './keyboard_nav/keyboard_mover.js'; import {keyboardNavigationController} from './keyboard_navigation_controller.js'; import {Msg} from './msg.js'; @@ -87,6 +92,8 @@ export enum names { JUMP_BLOCK_END = 'jump_to_block_end', JUMP_FIRST_BLOCK = 'jump_to_first_block', JUMP_LAST_BLOCK = 'jump_to_last_block', + JUMP_PREVIOUS_PAGE = 'jump_to_previous_page', + JUMP_NEXT_PAGE = 'jump_to_next_page', } /** @@ -1472,6 +1479,46 @@ export function registerJumpBottomStack() { ShortcutRegistry.registry.register(jumpBottomStackShortcut); } +/** + * Returns the toolbox of the given workspace if the toolbox currently holds + * focus, otherwise null. + * + * @param workspace The workspace the shortcut is being handled on. + * @returns The focused toolbox, or null if focus is elsewhere. + */ +const getFocusedToolbox = (workspace: WorkspaceSvg) => { + const toolbox = workspace.getToolbox(); + if (!toolbox || getFocusManager().getFocusedTree() !== toolbox) return null; + return toolbox; +}; + +/** + * Returns whether the given workspace is a flyout workspace that currently + * holds focus. + * + * @param workspace The workspace the shortcut is being handled on. + * @returns True if focus is inside this flyout, otherwise false. + */ +const isFocusedFlyout = (workspace: WorkspaceSvg) => { + return workspace.isFlyout && getFocusManager().getFocusedTree() === workspace; +}; + +/** + * Returns the navigable items of the toolbox or flyout that currently holds + * focus. + * + * @param workspace The workspace the shortcut is being handled on. + * @returns The items to move between, or null if neither the toolbox nor a + * flyout has focus. + */ +const getFocusedItems = (workspace: WorkspaceSvg) => { + const tree = + getFocusedToolbox(workspace) ?? + (isFocusedFlyout(workspace) ? workspace : null); + if (!tree) return null; + return tree.getNavigator().getNavigableItems(tree.getRootFocusableNode()); +}; + /** * Registers a keyboard shortcut that sets the focus to the first * block in the workspace. @@ -1489,6 +1536,13 @@ export function registerJumpFirstBlock() { ); }, callback(workspace) { + const items = getFocusedItems(workspace); + if (items) { + if (!items.length) return false; + getFocusManager().focusNode(items[0]); + return true; + } + const topBlocks = workspace.getTopBlocks(true); if (!topBlocks.length) return false; getFocusManager().focusNode(topBlocks[0]); @@ -1517,6 +1571,13 @@ export function registerJumpLastBlock() { ); }, callback(workspace) { + const items = getFocusedItems(workspace); + if (items) { + if (!items.length) return false; + getFocusManager().focusNode(items[items.length - 1]); + return true; + } + const allBlocks = workspace.getAllBlocks(true); if (!allBlocks.length) return false; getFocusManager().focusNode(allBlocks[allBlocks.length - 1]); @@ -1528,6 +1589,250 @@ export function registerJumpLastBlock() { ShortcutRegistry.registry.register(jumpLastBlockShortcut); } +/** + * A list of items laid out along one axis, with coordinates oriented so that + * they increase in navigation order. Expressing paging in this space keeps it + * free of RTL and horizontal-layout special cases. + */ +interface PagedItemList { + /** The navigable items, in navigation order. */ + items: IFocusableNode[]; + /** The edge of the item at the given index that is reached first. */ + leadingEdge(index: number): number; + /** The edge of the item at the given index that is reached last. */ + trailingEdge(index: number): number; + /** The edge of the visible area that is reached first. */ + viewLeadingEdge: number; + /** The edge of the visible area that is reached last. */ + viewTrailingEdge: number; + /** Moves the visible area forwards in navigation order by the given amount. */ + scrollBy(delta: number): void; +} + +/** The extent of an item along the axis it is laid out on. */ +interface Span { + start: number; + end: number; +} + +/** + * Orients raw measurements taken in a container's own coordinate system so + * that they increase in navigation order. + * + * @param items The navigable items, in navigation order. + * @param spans The extent of each item, in the same order as `items`. These + * are a snapshot: paging scrolls the container, so they must not be + * re-measured partway through. + * @param view The extent of the visible area. + * @param scrollRawBy Moves the visible area by the given amount, in the + * container's own coordinate system. + * @returns The oriented list. + */ +function createPagedItemList( + items: IFocusableNode[], + spans: Span[], + view: Span, + scrollRawBy: (delta: number) => void, +): PagedItemList { + // Items may run either way along the axis; an RTL horizontal flyout, for + // example, lays its first item out on the right. + const forwards = spans[spans.length - 1].start >= spans[0].start; + + return { + items, + leadingEdge: (index) => (forwards ? spans[index].start : -spans[index].end), + trailingEdge: (index) => + forwards ? spans[index].end : -spans[index].start, + viewLeadingEdge: forwards ? view.start : -view.end, + viewTrailingEdge: forwards ? view.end : -view.start, + scrollBy: (delta) => scrollRawBy(forwards ? delta : -delta), + }; +} + +/** + * Measures the categories of a toolbox for paging. + * + * @param toolbox The toolbox to measure. + * @returns The measured list, or null if it cannot be paged through. + */ +function createToolboxPagedList(toolbox: IToolbox): PagedItemList | null { + const items = toolbox + .getNavigator() + .getNavigableItems(toolbox.getRootFocusableNode()); + if (!items.length) return null; + + const horizontal = toolbox.isHorizontal(); + const measure = (element: Element): Span => { + const rect = element.getBoundingClientRect(); + return horizontal + ? {start: rect.left, end: rect.right} + : {start: rect.top, end: rect.bottom}; + }; + const container = toolbox.getRootFocusableNode().getFocusableElement(); + const spans = items.map((item) => measure(item.getFocusableElement())); + + return createPagedItemList(items, spans, measure(container), (delta) => { + if (horizontal) { + container.scrollLeft += delta; + } else { + container.scrollTop += delta; + } + }); +} + +/** + * Measures the contents of a flyout for paging. + * + * @param workspace The flyout's workspace. + * @returns The measured list, or null if it cannot be paged through. + */ +function createFlyoutPagedList(workspace: WorkspaceSvg): PagedItemList | null { + const items = workspace + .getNavigator() + .getNavigableItems(workspace.getRootFocusableNode()) + .filter((item): item is IFocusableNode & IBoundedElement => + isBoundedElement(item), + ); + if (!items.length) return null; + + const horizontal = !!workspace.targetWorkspace?.getFlyout()?.horizontalLayout; + const metrics = workspace.getMetricsManager().getViewMetrics(true); + const view = horizontal + ? {start: metrics.left, end: metrics.left + metrics.width} + : {start: metrics.top, end: metrics.top + metrics.height}; + const spans = items.map((item): Span => { + const rect = item.getBoundingRectangle(); + return horizontal + ? {start: rect.left, end: rect.right} + : {start: rect.top, end: rect.bottom}; + }); + const scale = workspace.getScale(); + + return createPagedItemList(items, spans, view, (delta) => { + // Moving the visible area forwards moves the contents backwards. + const contentDelta = -delta * scale; + workspace.scroll( + workspace.scrollX + (horizontal ? contentDelta : 0), + workspace.scrollY + (horizontal ? 0 : contentDelta), + ); + }); +} + +/** + * Moves focus by one viewport's worth of items through the focused toolbox or + * flyout, then scrolls the newly focused item to the edge of the viewport it + * was moved towards. Landing on the edge rather than merely in view is what + * lets someone paging through a long list see that nothing was skipped. + * + * @param workspace The workspace the shortcut is being handled on. + * @param forward True to page towards the end of the list, false towards the + * start. + * @returns True if focus moved, otherwise false. + */ +function jumpPage(workspace: WorkspaceSvg, forward: boolean): boolean { + const toolbox = getFocusedToolbox(workspace); + const list = toolbox + ? createToolboxPagedList(toolbox) + : createFlyoutPagedList(workspace); + if (!list) return false; + + const items = list.items; + const current = getFocusManager().getFocusedNode(); + const currentIndex = current ? items.indexOf(current) : -1; + const pageSize = list.viewTrailingEdge - list.viewLeadingEdge; + let targetIndex = -1; + + if (forward) { + // Page from whichever is further along: the leading edge of the viewport, + // or the focused item, which may have been scrolled past it. + const from = + currentIndex >= 0 + ? Math.max(list.viewLeadingEdge, list.leadingEdge(currentIndex)) + : list.viewLeadingEdge; + for (let i = 0; i < items.length; i++) { + if (list.leadingEdge(i) >= from + pageSize) break; + targetIndex = i; + } + } else { + const from = + currentIndex >= 0 + ? Math.min(list.viewTrailingEdge, list.trailingEdge(currentIndex)) + : list.viewTrailingEdge; + for (let i = items.length - 1; i >= 0; i--) { + if (list.trailingEdge(i) <= from - pageSize) break; + targetIndex = i; + } + } + + if (targetIndex < 0) return false; + if (targetIndex === currentIndex) { + // An item longer than the viewport fills the page on its own; step over it + // so that the shortcut always moves. + targetIndex += forward ? 1 : -1; + if (targetIndex < 0 || targetIndex >= items.length) return false; + } + + getFocusManager().focusNode(items[targetIndex]); + list.scrollBy( + forward + ? list.leadingEdge(targetIndex) - list.viewLeadingEdge + : list.trailingEdge(targetIndex) - list.viewTrailingEdge, + ); + return true; +} + +/** + * @param workspace + * @returns true if the paging shortcuts should be allowed, false otherwise. + */ +const shouldDoItemPaging = (workspace: WorkspaceSvg) => { + return ( + !workspace.isDragging() && + !getFocusManager().ephemeralFocusTaken() && + (!!getFocusedToolbox(workspace) || isFocusedFlyout(workspace)) + ); +}; + +/** + * Registers a keyboard shortcut that pages backwards through the items of the + * focused toolbox or flyout. + */ +export function registerJumpPreviousPage() { + const jumpPreviousPageShortcut: KeyboardShortcut = { + name: names.JUMP_PREVIOUS_PAGE, + preconditionFn: shouldDoItemPaging, + callback(workspace, e) { + if (!jumpPage(workspace, false)) return false; + e.preventDefault(); + return true; + }, + keyCodes: [KeyCodes.PAGE_UP], + allowCollision: true, + displayText: () => Msg['SHORTCUTS_JUMP_PREVIOUS_PAGE'], + }; + ShortcutRegistry.registry.register(jumpPreviousPageShortcut); +} + +/** + * Registers a keyboard shortcut that pages forwards through the items of the + * focused toolbox or flyout. + */ +export function registerJumpNextPage() { + const jumpNextPageShortcut: KeyboardShortcut = { + name: names.JUMP_NEXT_PAGE, + preconditionFn: shouldDoItemPaging, + callback(workspace, e) { + if (!jumpPage(workspace, true)) return false; + e.preventDefault(); + return true; + }, + keyCodes: [KeyCodes.PAGE_DOWN], + allowCollision: true, + displayText: () => Msg['SHORTCUTS_JUMP_NEXT_PAGE'], + }; + ShortcutRegistry.registry.register(jumpNextPageShortcut); +} + /** * Registers all default keyboard shortcut item. This should be called once per * instance of KeyboardShortcutRegistry. @@ -1573,7 +1878,8 @@ export function registerScreenReaderShortcuts() { } /** - * Registers keyboard shortcuts used to jump between blocks and stacks in the workspace. + * Registers keyboard shortcuts used to jump between blocks and stacks in the workspace, + * and between items in the toolbox and flyout. * Note these are not registered by default, so call this function to enable them if desired. */ export function registerNavigationShortcuts() { @@ -1583,6 +1889,8 @@ export function registerNavigationShortcuts() { registerJumpBottomStack(); registerJumpFirstBlock(); registerJumpLastBlock(); + registerJumpPreviousPage(); + registerJumpNextPage(); } registerDefaultShortcuts(); diff --git a/packages/blockly/msg/json/en.json b/packages/blockly/msg/json/en.json index bcb9daf2311..10432ea20d0 100644 --- a/packages/blockly/msg/json/en.json +++ b/packages/blockly/msg/json/en.json @@ -493,6 +493,8 @@ "SHORTCUTS_JUMP_BOTTOM_STACK": "Jump to bottom of stack", "SHORTCUTS_JUMP_FIRST_BLOCK": "Jump to first block", "SHORTCUTS_JUMP_LAST_BLOCK": "Jump to last block", + "SHORTCUTS_JUMP_PREVIOUS_PAGE": "Jump to previous page", + "SHORTCUTS_JUMP_NEXT_PAGE": "Jump to next page", "KEYBOARD_NAV_UNCONSTRAINED_MOVE_HINT": "Hold %1 and use arrow keys to move freely, then %2 to accept the position.", "KEYBOARD_NAV_CONSTRAINED_MOVE_HINT": "Use the arrow keys to move, then %1 to accept the position.", "KEYBOARD_NAV_COPIED_HINT": "Copied. Press %1 to paste.", diff --git a/packages/blockly/msg/json/qqq.json b/packages/blockly/msg/json/qqq.json index c4b04f7240b..4fee22b3d60 100644 --- a/packages/blockly/msg/json/qqq.json +++ b/packages/blockly/msg/json/qqq.json @@ -498,6 +498,8 @@ "SHORTCUTS_JUMP_BOTTOM_STACK": "shortcut display text for a shortcut that jumps focus to the bottom block of the current stack.", "SHORTCUTS_JUMP_FIRST_BLOCK": "shortcut display text for a shortcut that jumps focus to the first block in the workspace.", "SHORTCUTS_JUMP_LAST_BLOCK": "shortcut display text for a shortcut that jumps focus to the last block in the workspace.", + "SHORTCUTS_JUMP_PREVIOUS_PAGE": "shortcut display text for a shortcut that jumps focus back one page of items in the toolbox or flyout.", + "SHORTCUTS_JUMP_NEXT_PAGE": "shortcut display text for a shortcut that jumps focus forward one page of items in the toolbox or flyout.", "KEYBOARD_NAV_UNCONSTRAINED_MOVE_HINT": "Message shown to inform users how to move blocks to arbitrary locations with the keyboard.", "KEYBOARD_NAV_CONSTRAINED_MOVE_HINT": "Message shown to inform users how to move blocks with the keyboard.", "KEYBOARD_NAV_COPIED_HINT": "Message shown when an item is copied in keyboard navigation mode.", diff --git a/packages/blockly/msg/messages.js b/packages/blockly/msg/messages.js index d88df44ca61..60d6461c37d 100644 --- a/packages/blockly/msg/messages.js +++ b/packages/blockly/msg/messages.js @@ -1917,6 +1917,12 @@ Blockly.Msg.SHORTCUTS_JUMP_FIRST_BLOCK = 'Jump to first block'; /// shortcut display text for a shortcut that jumps focus to the last block in the workspace. Blockly.Msg.SHORTCUTS_JUMP_LAST_BLOCK = 'Jump to last block'; /** @type {string} */ +/// shortcut display text for a shortcut that jumps focus back one page of items in the toolbox or flyout. +Blockly.Msg.SHORTCUTS_JUMP_PREVIOUS_PAGE = 'Jump to previous page'; +/** @type {string} */ +/// shortcut display text for a shortcut that jumps focus forward one page of items in the toolbox or flyout. +Blockly.Msg.SHORTCUTS_JUMP_NEXT_PAGE = 'Jump to next page'; +/** @type {string} */ /// Message shown to inform users how to move blocks to arbitrary locations /// with the keyboard. Blockly.Msg.KEYBOARD_NAV_UNCONSTRAINED_MOVE_HINT = 'Hold %1 and use arrow keys to move freely, then %2 to accept the position.'; diff --git a/packages/blockly/tests/mocha/keyboard_navigation_test.js b/packages/blockly/tests/mocha/keyboard_navigation_test.js index e5cffbc7f22..087d4c70328 100644 --- a/packages/blockly/tests/mocha/keyboard_navigation_test.js +++ b/packages/blockly/tests/mocha/keyboard_navigation_test.js @@ -6,6 +6,7 @@ import * as Blockly from '#core/blockly.js'; import {assert} from 'chai'; +import {defineStackBlock} from './test_helpers/block_definitions.js'; import {navigationTestBlocks} from './test_helpers/navigation_test_blocks.js'; import {p5blocks} from './test_helpers/p5_blocks.js'; import { @@ -1073,3 +1074,344 @@ suite('Flyout heading navigation with no headings', function () { assert.equal(Blockly.getFocusManager().getFocusedNode(), firstBlock); }); }); + +suite('Toolbox and flyout jump shortcuts (Ctrl/Cmd + Home / End)', function () { + suiteSetup(function () { + Blockly.ShortcutItems.registerNavigationShortcuts(); + }); + + suiteTeardown(function () { + for (const shortcut of [ + 'jump_to_top_of_stack', + 'jump_to_bottom_of_stack', + 'jump_to_block_start', + 'jump_to_block_end', + 'jump_to_first_block', + 'jump_to_last_block', + 'jump_to_previous_page', + 'jump_to_next_page', + ]) { + Blockly.ShortcutRegistry.registry.unregister(shortcut); + } + }); + + setup(function () { + sharedTestSetup.call(this); + Blockly.defineBlocksWithJsonArray([ + { + type: 'basic_block', + message0: '%1', + args0: [{type: 'field_input', name: 'TEXT', text: 'default'}], + }, + ]); + defineStackBlock(); + }); + + teardown(function () { + sharedTestTeardown.call(this); + }); + + /** + * Presses Home or End with the platform's control key held down. + * + * @param {!Blockly.WorkspaceSvg} workspace The workspace to dispatch on. + * @param {number} keyCode Either KeyCodes.HOME or KeyCodes.END. + */ + function pressCtrlKey(workspace, keyCode) { + pressKey(workspace, keyCode, [Blockly.utils.KeyCodes.CTRL_CMD]); + } + + suite('in the toolbox', function () { + setup(function () { + this.workspace = Blockly.inject('blocklyDiv', { + toolbox: document.getElementById('toolbox-test'), + }); + this.toolbox = this.workspace.getToolbox(); + // toolbox-test starts and ends with a category, with a non-focusable + // separator in between. + const allItems = this.toolbox.getToolboxItems(); + this.firstItem = allItems[0]; + this.lastItem = allItems[allItems.length - 1]; + }); + + test('Navigable items exclude the separator', function () { + const navigable = this.toolbox + .getNavigator() + .getNavigableItems(this.toolbox.getRootFocusableNode()); + assert.isAbove(this.toolbox.getToolboxItems().length, navigable.length); + assert.isFalse( + navigable.some((item) => item instanceof Blockly.ToolboxSeparator), + ); + }); + + test('CtrlHome focuses the first toolbox item', function () { + Blockly.getFocusManager().focusNode(this.lastItem); + pressCtrlKey(this.workspace, Blockly.utils.KeyCodes.HOME); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.firstItem); + }); + + test('CtrlEnd focuses the last toolbox item', function () { + Blockly.getFocusManager().focusNode(this.firstItem); + pressCtrlKey(this.workspace, Blockly.utils.KeyCodes.END); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.lastItem); + }); + + test('CtrlHome does not move focus out of the toolbox', function () { + const block = this.workspace.newBlock('basic_block'); + block.initSvg(); + block.render(); + Blockly.getFocusManager().focusNode(this.lastItem); + pressCtrlKey(this.workspace, Blockly.utils.KeyCodes.HOME); + assert.notEqual(Blockly.getFocusManager().getFocusedNode(), block); + }); + + test('CtrlHome still focuses the first block when the workspace has focus', function () { + const block = this.workspace.newBlock('basic_block'); + block.initSvg(); + block.render(); + Blockly.getFocusManager().focusNode(block); + pressCtrlKey(this.workspace, Blockly.utils.KeyCodes.HOME); + assert.equal(Blockly.getFocusManager().getFocusedNode(), block); + }); + }); + + suite('in the flyout', function () { + setup(function () { + this.workspace = Blockly.inject('blocklyDiv', { + toolbox: { + kind: 'flyoutToolbox', + contents: [ + {kind: 'label', text: 'First heading'}, + {kind: 'block', type: 'basic_block'}, + {kind: 'block', type: 'basic_block'}, + {kind: 'label', text: 'Last heading'}, + ], + }, + }); + this.flyoutWorkspace = this.workspace.getFlyout().getWorkspace(); + // The flyout opens and closes with a label, so the first and last + // navigable items are not blocks. A trailing separator is appended to + // the contents but cannot be focused. + this.labels = this.workspace + .getFlyout() + .getContents() + .map((item) => item.getElement()) + .filter( + (element) => + element instanceof Blockly.FlyoutButton && element.isLabel(), + ); + }); + + test('CtrlHome focuses the first item, which is a label rather than a block', function () { + Blockly.getFocusManager().focusNode( + this.flyoutWorkspace.getTopBlocks()[1], + ); + pressCtrlKey(this.workspace, Blockly.utils.KeyCodes.HOME); + const focused = Blockly.getFocusManager().getFocusedNode(); + assert.equal(focused, this.labels[0]); + assert.notEqual(focused, this.flyoutWorkspace.getTopBlocks()[0]); + }); + + test('CtrlEnd focuses the last item, which is a label rather than a block', function () { + Blockly.getFocusManager().focusNode( + this.flyoutWorkspace.getTopBlocks()[0], + ); + pressCtrlKey(this.workspace, Blockly.utils.KeyCodes.END); + const focused = Blockly.getFocusManager().getFocusedNode(); + assert.equal(focused, this.labels[this.labels.length - 1]); + assert.notEqual( + focused, + this.flyoutWorkspace.getTopBlocks().slice(-1)[0], + ); + }); + }); +}); + +suite('Toolbox and flyout paging shortcuts (Page Up / Page Down)', function () { + suiteSetup(function () { + Blockly.ShortcutItems.registerNavigationShortcuts(); + }); + + suiteTeardown(function () { + for (const shortcut of [ + 'jump_to_top_of_stack', + 'jump_to_bottom_of_stack', + 'jump_to_block_start', + 'jump_to_block_end', + 'jump_to_first_block', + 'jump_to_last_block', + 'jump_to_previous_page', + 'jump_to_next_page', + ]) { + Blockly.ShortcutRegistry.registry.unregister(shortcut); + } + }); + + setup(function () { + sharedTestSetup.call(this); + Blockly.defineBlocksWithJsonArray([ + { + type: 'basic_block', + message0: '%1', + args0: [{type: 'field_input', name: 'TEXT', text: 'default'}], + }, + ]); + defineStackBlock(); + }); + + teardown(function () { + sharedTestTeardown.call(this); + }); + + // Items are laid out 30 long with a 10 gap, so item i spans [i * 40, i * 40 + // + 30]. A 100-long viewport therefore holds three of them. + const ITEM_PITCH = 40; + const ITEM_LENGTH = 30; + const VIEWPORT_LENGTH = 100; + + suite('in the flyout', function () { + setup(function () { + this.workspace = Blockly.inject('blocklyDiv', { + toolbox: { + kind: 'flyoutToolbox', + contents: new Array(6).fill({kind: 'block', type: 'basic_block'}), + }, + }); + this.flyoutWorkspace = this.workspace.getFlyout().getWorkspace(); + this.blocks = this.flyoutWorkspace.getTopBlocks(true); + this.scrollSpy = sinon.stub(this.flyoutWorkspace, 'scroll'); + sinon.stub(this.flyoutWorkspace, 'getScale').returns(1); + + /** + * Lays the flyout's blocks out at a known pitch and puts the viewport at + * the given offset, so that paging can be asserted exactly. + * + * @param {number} viewportTop Offset of the top of the viewport. + */ + this.layOutFlyout = (viewportTop) => { + this.blocks.forEach((block, i) => { + sinon + .stub(block, 'getBoundingRectangle') + .returns( + new Blockly.utils.Rect( + i * ITEM_PITCH, + i * ITEM_PITCH + ITEM_LENGTH, + 0, + 50, + ), + ); + }); + sinon + .stub(this.flyoutWorkspace.getMetricsManager(), 'getViewMetrics') + .returns({ + top: viewportTop, + left: 0, + width: 50, + height: VIEWPORT_LENGTH, + }); + }; + }); + + test('PageDown focuses the last visible block', function () { + this.layOutFlyout(0); + Blockly.getFocusManager().focusNode(this.blocks[0]); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.blocks[2]); + }); + + test('PageDown scrolls the newly focused block to the top of the viewport', function () { + this.layOutFlyout(0); + Blockly.getFocusManager().focusNode(this.blocks[0]); + this.scrollSpy.resetHistory(); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); + // Moving the viewport down by 80 moves the contents up by the same. + sinon.assert.calledWith( + this.scrollSpy, + this.flyoutWorkspace.scrollX, + this.flyoutWorkspace.scrollY - 2 * ITEM_PITCH, + ); + }); + + test('A second PageDown advances by another page', function () { + this.layOutFlyout(0); + Blockly.getFocusManager().focusNode(this.blocks[0]); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.blocks[4]); + }); + + test('PageUp focuses the first visible block', function () { + // Viewport spans [150, 250], holding blocks 4 and 5 plus the tail of 3. + this.layOutFlyout(150); + Blockly.getFocusManager().focusNode(this.blocks[5]); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_UP); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.blocks[3]); + }); + + test('PageDown from the last block does nothing', function () { + this.layOutFlyout(200); + const last = this.blocks[this.blocks.length - 1]; + Blockly.getFocusManager().focusNode(last); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); + assert.equal(Blockly.getFocusManager().getFocusedNode(), last); + }); + + test('PageUp from the first block does nothing', function () { + this.layOutFlyout(0); + Blockly.getFocusManager().focusNode(this.blocks[0]); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_UP); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.blocks[0]); + }); + }); + + suite('in the toolbox', function () { + setup(function () { + this.workspace = Blockly.inject('blocklyDiv', { + toolbox: document.getElementById('toolbox-test'), + }); + this.toolbox = this.workspace.getToolbox(); + this.items = this.toolbox + .getNavigator() + .getNavigableItems(this.toolbox.getRootFocusableNode()); + + this.container = this.toolbox + .getRootFocusableNode() + .getFocusableElement(); + this.container.scrollTop = 0; + sinon.stub(this.container, 'getBoundingClientRect').returns({ + top: 0, + bottom: VIEWPORT_LENGTH, + left: 0, + right: 200, + }); + this.items.forEach((item, i) => { + sinon + .stub(item.getFocusableElement(), 'getBoundingClientRect') + .returns({ + top: i * ITEM_PITCH, + bottom: i * ITEM_PITCH + ITEM_LENGTH, + left: 0, + right: 200, + }); + }); + }); + + test('PageDown focuses the last visible category', function () { + Blockly.getFocusManager().focusNode(this.items[0]); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.items[2]); + }); + + test('PageDown scrolls the newly focused category to the top', function () { + Blockly.getFocusManager().focusNode(this.items[0]); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); + assert.equal(this.container.scrollTop, 2 * ITEM_PITCH); + }); + + test('PageUp focuses the first visible category', function () { + Blockly.getFocusManager().focusNode(this.items[2]); + pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_UP); + assert.equal(Blockly.getFocusManager().getFocusedNode(), this.items[0]); + }); + }); +}); diff --git a/packages/blockly/tests/mocha/shortcut_items_test.js b/packages/blockly/tests/mocha/shortcut_items_test.js index 0077766d247..e64e3b639d3 100644 --- a/packages/blockly/tests/mocha/shortcut_items_test.js +++ b/packages/blockly/tests/mocha/shortcut_items_test.js @@ -2211,6 +2211,8 @@ suite('Keyboard Shortcut Items', function () { 'jump_to_block_end', 'jump_to_first_block', 'jump_to_last_block', + 'jump_to_previous_page', + 'jump_to_next_page', ]) { Blockly.ShortcutRegistry.registry.unregister(shortcut); } @@ -2402,7 +2404,10 @@ suite('Keyboard Shortcut Items', function () { ); sinon.assert.calledWith(this.focusNodeSpy, lastBlock); }); - test('PageUp has no effect', function () { + // Page Up and Page Down page through flyout items instead of jumping + // between stacks, which flyouts do not have. The paging behaviour itself + // is covered by keyboard_navigation_test.js, against a real flyout. + test('PageUp does not jump to the top of a stack', function () { this.workspace.internalIsFlyout = true; const inListBlock = this.workspace.getBlockById('lists_getIndex_1'); this.getFocusedNodeStub.returns(inListBlock); @@ -2411,7 +2416,7 @@ suite('Keyboard Shortcut Items', function () { ); sinon.assert.notCalled(this.focusNodeSpy); }); - test('PageDown has no effect', function () { + test('PageDown does not jump to the bottom of a stack', function () { this.workspace.internalIsFlyout = true; const inListBlock = this.workspace.getBlockById('lists_getIndex_1'); this.getFocusedNodeStub.returns(inListBlock); diff --git a/packages/docs/docs/guides/configure/keyboard-nav.mdx b/packages/docs/docs/guides/configure/keyboard-nav.mdx index 69ccd112c1d..f8b0085477d 100644 --- a/packages/docs/docs/guides/configure/keyboard-nav.mdx +++ b/packages/docs/docs/guides/configure/keyboard-nav.mdx @@ -95,6 +95,21 @@ We also provide optional navigation shortcuts to make navigating the workspace e | Ctrl/Cmd + Home | Jump to first block | | Ctrl/Cmd + End | Jump to last block | +When focus is in the toolbox or the flyout, the same keys move between the +items there instead: + +| Key | Action | +| --- | --- | +| Page Up | Jump to previous page | +| Page Down | Jump to next page | +| Ctrl/Cmd + Home | Jump to first item | +| Ctrl/Cmd + End | Jump to last item | + +Paging moves focus to the last item that is currently visible and scrolls it to +the edge of the viewport it moved towards, so that nothing between the old and +new position is passed over unseen. Home and End keep their workspace meaning +and do nothing in the toolbox or flyout. + We recommend that you enable these for your application by calling: ```js From 48b6ad851e71e4fb9a6ea5ef24029c16b00f56fd Mon Sep 17 00:00:00 2001 From: Parman Mohammadalizadeh Date: Sat, 1 Aug 2026 11:47:50 +0200 Subject: [PATCH 2/2] fix: scroll a focused toolbox item into view, and only as far as needed Toolbox items take focus with preventScroll, so Ctrl/Cmd + Home and End moved focus to a category that stayed off screen. ToolboxItem.onNodeFocus now scrolls the item into view with block and inline set to nearest. Paging no longer scrolls the newly focused item to the far edge of the viewport, which moved it a long way when it was already partly visible. The item's own scroll-into-view handles it instead, so it moves only as far as it takes to show it and stays near the edge it came from. --- packages/blockly/core/shortcut_items.ts | 41 ++++--------------- packages/blockly/core/toolbox/category.ts | 1 + packages/blockly/core/toolbox/toolbox_item.ts | 9 +++- .../tests/mocha/keyboard_navigation_test.js | 29 ++++++------- .../mocha/test_helpers/svg_layout_stubs.js | 6 +++ packages/blockly/tests/mocha/toolbox_test.js | 17 ++++++++ .../docs/guides/configure/keyboard-nav.mdx | 8 ++-- 7 files changed, 56 insertions(+), 55 deletions(-) diff --git a/packages/blockly/core/shortcut_items.ts b/packages/blockly/core/shortcut_items.ts index 4adb8f778b7..3521de412da 100644 --- a/packages/blockly/core/shortcut_items.ts +++ b/packages/blockly/core/shortcut_items.ts @@ -1605,8 +1605,6 @@ interface PagedItemList { viewLeadingEdge: number; /** The edge of the visible area that is reached last. */ viewTrailingEdge: number; - /** Moves the visible area forwards in navigation order by the given amount. */ - scrollBy(delta: number): void; } /** The extent of an item along the axis it is laid out on. */ @@ -1621,18 +1619,15 @@ interface Span { * * @param items The navigable items, in navigation order. * @param spans The extent of each item, in the same order as `items`. These - * are a snapshot: paging scrolls the container, so they must not be - * re-measured partway through. + * are a snapshot: focusing an item scrolls the container, so they must not + * be re-measured partway through. * @param view The extent of the visible area. - * @param scrollRawBy Moves the visible area by the given amount, in the - * container's own coordinate system. * @returns The oriented list. */ function createPagedItemList( items: IFocusableNode[], spans: Span[], view: Span, - scrollRawBy: (delta: number) => void, ): PagedItemList { // Items may run either way along the axis; an RTL horizontal flyout, for // example, lays its first item out on the right. @@ -1645,7 +1640,6 @@ function createPagedItemList( forwards ? spans[index].end : -spans[index].start, viewLeadingEdge: forwards ? view.start : -view.end, viewTrailingEdge: forwards ? view.end : -view.start, - scrollBy: (delta) => scrollRawBy(forwards ? delta : -delta), }; } @@ -1671,13 +1665,7 @@ function createToolboxPagedList(toolbox: IToolbox): PagedItemList | null { const container = toolbox.getRootFocusableNode().getFocusableElement(); const spans = items.map((item) => measure(item.getFocusableElement())); - return createPagedItemList(items, spans, measure(container), (delta) => { - if (horizontal) { - container.scrollLeft += delta; - } else { - container.scrollTop += delta; - } - }); + return createPagedItemList(items, spans, measure(container)); } /** @@ -1706,23 +1694,15 @@ function createFlyoutPagedList(workspace: WorkspaceSvg): PagedItemList | null { ? {start: rect.left, end: rect.right} : {start: rect.top, end: rect.bottom}; }); - const scale = workspace.getScale(); - - return createPagedItemList(items, spans, view, (delta) => { - // Moving the visible area forwards moves the contents backwards. - const contentDelta = -delta * scale; - workspace.scroll( - workspace.scrollX + (horizontal ? contentDelta : 0), - workspace.scrollY + (horizontal ? 0 : contentDelta), - ); - }); + + return createPagedItemList(items, spans, view); } /** * Moves focus by one viewport's worth of items through the focused toolbox or - * flyout, then scrolls the newly focused item to the edge of the viewport it - * was moved towards. Landing on the edge rather than merely in view is what - * lets someone paging through a long list see that nothing was skipped. + * flyout. The target is the furthest item that is at least partly visible after + * the move, so paging never skips one; scrolling it into view is left to the + * item itself, which moves only as far as it takes to show it. * * @param workspace The workspace the shortcut is being handled on. * @param forward True to page towards the end of the list, false towards the @@ -1773,11 +1753,6 @@ function jumpPage(workspace: WorkspaceSvg, forward: boolean): boolean { } getFocusManager().focusNode(items[targetIndex]); - list.scrollBy( - forward - ? list.leadingEdge(targetIndex) - list.viewLeadingEdge - : list.trailingEdge(targetIndex) - list.viewTrailingEdge, - ); return true; } diff --git a/packages/blockly/core/toolbox/category.ts b/packages/blockly/core/toolbox/category.ts index eccbc42f547..bc2f95ba440 100644 --- a/packages/blockly/core/toolbox/category.ts +++ b/packages/blockly/core/toolbox/category.ts @@ -607,6 +607,7 @@ export class ToolboxCategory * toolbox that it has been selected. */ override onNodeFocus(): void { + super.onNodeFocus(); if (this.getParentToolbox().getSelectedItem() !== this) { this.getParentToolbox().setSelectedItem(this); } diff --git a/packages/blockly/core/toolbox/toolbox_item.ts b/packages/blockly/core/toolbox/toolbox_item.ts index 92c3721363a..8655dcab83b 100644 --- a/packages/blockly/core/toolbox/toolbox_item.ts +++ b/packages/blockly/core/toolbox/toolbox_item.ts @@ -168,7 +168,14 @@ export class ToolboxItem implements IToolboxItem { } /** See IFocusableNode.onNodeFocus. */ - onNodeFocus(): void {} + onNodeFocus(): void { + // Focus is taken with preventScroll, so bring the item into view here, + // moving it no further than it takes to make it fully visible. + this.getFocusableElement().scrollIntoView({ + block: 'nearest', + inline: 'nearest', + }); + } /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} diff --git a/packages/blockly/tests/mocha/keyboard_navigation_test.js b/packages/blockly/tests/mocha/keyboard_navigation_test.js index 087d4c70328..b7ec3ec9896 100644 --- a/packages/blockly/tests/mocha/keyboard_navigation_test.js +++ b/packages/blockly/tests/mocha/keyboard_navigation_test.js @@ -1279,7 +1279,9 @@ suite('Toolbox and flyout paging shortcuts (Page Up / Page Down)', function () { }); this.flyoutWorkspace = this.workspace.getFlyout().getWorkspace(); this.blocks = this.flyoutWorkspace.getTopBlocks(true); - this.scrollSpy = sinon.stub(this.flyoutWorkspace, 'scroll'); + // Focusing a block scrolls it into view; stub that out so the layout set + // up below stays valid for the whole test. + sinon.stub(this.flyoutWorkspace, 'scroll'); sinon.stub(this.flyoutWorkspace, 'getScale').returns(1); /** @@ -1319,19 +1321,6 @@ suite('Toolbox and flyout paging shortcuts (Page Up / Page Down)', function () { assert.equal(Blockly.getFocusManager().getFocusedNode(), this.blocks[2]); }); - test('PageDown scrolls the newly focused block to the top of the viewport', function () { - this.layOutFlyout(0); - Blockly.getFocusManager().focusNode(this.blocks[0]); - this.scrollSpy.resetHistory(); - pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); - // Moving the viewport down by 80 moves the contents up by the same. - sinon.assert.calledWith( - this.scrollSpy, - this.flyoutWorkspace.scrollX, - this.flyoutWorkspace.scrollY - 2 * ITEM_PITCH, - ); - }); - test('A second PageDown advances by another page', function () { this.layOutFlyout(0); Blockly.getFocusManager().focusNode(this.blocks[0]); @@ -1377,7 +1366,6 @@ suite('Toolbox and flyout paging shortcuts (Page Up / Page Down)', function () { this.container = this.toolbox .getRootFocusableNode() .getFocusableElement(); - this.container.scrollTop = 0; sinon.stub(this.container, 'getBoundingClientRect').returns({ top: 0, bottom: VIEWPORT_LENGTH, @@ -1402,10 +1390,17 @@ suite('Toolbox and flyout paging shortcuts (Page Up / Page Down)', function () { assert.equal(Blockly.getFocusManager().getFocusedNode(), this.items[2]); }); - test('PageDown scrolls the newly focused category to the top', function () { + test('PageDown scrolls the newly focused category into view', function () { Blockly.getFocusManager().focusNode(this.items[0]); + const scrollIntoView = sinon.spy( + this.items[2].getFocusableElement(), + 'scrollIntoView', + ); pressKey(this.workspace, Blockly.utils.KeyCodes.PAGE_DOWN); - assert.equal(this.container.scrollTop, 2 * ITEM_PITCH); + sinon.assert.calledWith(scrollIntoView, { + block: 'nearest', + inline: 'nearest', + }); }); test('PageUp focuses the first visible category', function () { diff --git a/packages/blockly/tests/mocha/test_helpers/svg_layout_stubs.js b/packages/blockly/tests/mocha/test_helpers/svg_layout_stubs.js index 79da167ef1d..2aecd5de4a2 100644 --- a/packages/blockly/tests/mocha/test_helpers/svg_layout_stubs.js +++ b/packages/blockly/tests/mocha/test_helpers/svg_layout_stubs.js @@ -209,6 +209,12 @@ export function installSvgLayoutStubs(window) { }; } + // JSDom performs no layout and so provides no scrollIntoView. Blockly calls + // it to keep a newly focused toolbox item within the toolbox's visible area. + if (element && !element.scrollIntoView) { + element.scrollIntoView = function () {}; + } + // getElementById is only defined on Document/ShadowRoot. Blockly calls it on // the result of getRootNode(), which can be a detached SVG element during // disposal. Provide a descendant search so those code paths work headless. diff --git a/packages/blockly/tests/mocha/toolbox_test.js b/packages/blockly/tests/mocha/toolbox_test.js index f120e82ea04..2fc101cdcb0 100644 --- a/packages/blockly/tests/mocha/toolbox_test.js +++ b/packages/blockly/tests/mocha/toolbox_test.js @@ -227,6 +227,23 @@ suite('Toolbox', function () { assert.isTrue(this.toolbox.getFlyout().isVisible()); }); + test('Focusing an item scrolls it into view', function () { + const item = getNonCollapsibleItem(this.toolbox); + const scrollIntoView = sinon.spy( + item.getFocusableElement(), + 'scrollIntoView', + ); + + Blockly.getFocusManager().focusNode(item); + + // Focus is taken with preventScroll, so an item outside the visible area + // of a scrolling toolbox would otherwise stay out of sight. + sinon.assert.calledWith(scrollIntoView, { + block: 'nearest', + inline: 'nearest', + }); + }); + test('Tab order follows toolbox, flyout, workspace DOM order', function () { const injectionDiv = this.toolbox.getWorkspace().getInjectionDiv(); const children = Array.from(injectionDiv.children); diff --git a/packages/docs/docs/guides/configure/keyboard-nav.mdx b/packages/docs/docs/guides/configure/keyboard-nav.mdx index f8b0085477d..6b859c556ac 100644 --- a/packages/docs/docs/guides/configure/keyboard-nav.mdx +++ b/packages/docs/docs/guides/configure/keyboard-nav.mdx @@ -105,10 +105,10 @@ items there instead: | Ctrl/Cmd + Home | Jump to first item | | Ctrl/Cmd + End | Jump to last item | -Paging moves focus to the last item that is currently visible and scrolls it to -the edge of the viewport it moved towards, so that nothing between the old and -new position is passed over unseen. Home and End keep their workspace meaning -and do nothing in the toolbox or flyout. +Paging moves focus to the last item that is currently visible, so that nothing +between the old and new position is passed over unseen, and scrolls just far +enough to bring that item fully into view. Plain Home and End keep their +workspace meaning and do nothing in the toolbox or flyout. We recommend that you enable these for your application by calling: