CodingPirates/blockly
1
0
Fork 0
mirror of https://github.com/google/blockly.git synced 2026-01-05 08:00:09 +01:00
Code Issues Packages Projects Releases Wiki Activity

chore: part 2 of addressing reviewer comments.

Browse Source
This commit is contained in:
Ben Henning
2025-04-03 22:55:35 +00:00
parent 902b26b1a1
commit 720e8dab2b
5 changed files with 89 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
core/focus_manager.ts
View File

@@ -6,6 +6,7 @@
import type {IFocusableNode} from './interfaces/i_focusable_node.js';
import type {IFocusableTree} from './interfaces/i_focusable_tree.js';
import {FocusableTreeTraverser} from './utils/focusable_tree_traverser.js';
import * as dom from './utils/dom.js';
/**
@@ -30,7 +31,29 @@ export type ReturnEphemeralFocus = () => void;
* focusNode().
*/
export class FocusManager {
/**
* The CSS class assigned to IFocusableNode elements that presently have
* active DOM and Blockly focus.
*
* This should never be used directly. Instead, rely on FocusManager to ensure
* nodes have active focus (either automatically through DOM focus or manually
* through the various focus* methods provided by this class).
*
* It's recommended to not query using this class name, either. Instead, use
* FocusableTreeTraverser or IFocusableTree's methods to find a specific node.
*/
static readonly ACTIVE_FOCUS_NODE_CSS_CLASS_NAME = 'blocklyActiveFocus';
/**
* The CSS class assigned to IFocusableNode elements that presently have
* passive focus (that is, they were the most recent node in their relative
* tree to have active focus--see ACTIVE_FOCUS_NODE_CSS_CLASS_NAME--and will
* receive active focus again if their surrounding tree is requested to become
* focused, i.e. using focusTree below).
*
* See ACTIVE_FOCUS_NODE_CSS_CLASS_NAME for caveats and limitations around
* using this constant directly (generally it never should need to be used).
*/
static readonly PASSIVE_FOCUS_NODE_CSS_CLASS_NAME = 'blocklyPassiveFocus';
focusedNode: IFocusableNode | null = null;
@@ -57,7 +80,8 @@ export class FocusManager {
// updated. Per the contract of findFocusableNodeFor only one tree
// should claim the element.
for (const tree of this.registeredTrees) {
newNode = tree.findFocusableNodeFor(activeElement);
newNode = FocusableTreeTraverser.findFocusableNodeFor(
activeElement, tree);
if (newNode) break;
}
}
@@ -113,7 +137,7 @@ export class FocusManager {
const treeIndex = this.registeredTrees.findIndex((tree) => tree === tree);
this.registeredTrees.splice(treeIndex, 1);
const focusedNode = tree.getFocusedNode();
const focusedNode = FocusableTreeTraverser.findFocusedNode(tree);
const root = tree.getRootFocusableNode();
if (focusedNode) this.removeHighlight(focusedNode);
if (this.focusedNode === focusedNode || this.focusedNode === root) {
@@ -169,9 +193,8 @@ export class FocusManager {
if (!this.isRegistered(focusableTree)) {
throw Error(`Attempted to focus unregistered tree: ${focusableTree}.`);
}
this.focusNode(
focusableTree.getFocusedNode() ?? focusableTree.getRootFocusableNode(),
);
const currNode = FocusableTreeTraverser.findFocusedNode(focusableTree);
this.focusNode(currNode ?? focusableTree.getRootFocusableNode());
}
/**
@@ -193,7 +216,7 @@ export class FocusManager {
this.setNodeToPassive(prevNode);
}
// If there's a focused node in the new node's tree, ensure it's reset.
const prevNodeNextTree = nextTree.getFocusedNode();
const prevNodeNextTree = FocusableTreeTraverser.findFocusedNode(nextTree);
const nextTreeRoot = nextTree.getRootFocusableNode();
if (prevNodeNextTree) {
this.removeHighlight(prevNodeNextTree);

32
core/interfaces/i_focusable_tree.ts
View File

@@ -20,17 +20,14 @@ import type {IFocusableNode} from './i_focusable_node.js';
* page at any given time). The idea of passive focus is to provide context to
* users on where their focus will be restored upon navigating back to a
* previously focused tree.
*
* Note that if the tree's current focused node (passive or active) is needed,
* FocusableTreeTraverser.findFocusedNode can be used.
*
* Note that if specific nodes are needed to be retrieved for this tree, either
* use lookUpFocusableNode or FocusableTreeTraverser.findFocusableNodeFor.
*/
export interface IFocusableTree {
/**
* Returns the current node with focus in this tree, or null if none (or if
* the root has focus).
*
* Note that this will never return a node from a nested sub-tree as that tree
* should specifically be called in order to retrieve its focused node.
*/
getFocusedNode(): IFocusableNode | null;
/**
* Returns the top-level focusable node of the tree.
*
@@ -61,21 +58,4 @@ export interface IFocusableTree {
* @param id The ID of the node's focusable HTMLElement or SVGElement.
*/
lookUpFocusableNode(id: string): IFocusableNode | null;
/**
* Returns the IFocusableNode corresponding to the select element, or null if
* the element does not have such a node.
*
* The provided element must have a non-null ID that conforms to the contract
* mentioned in IFocusableNode.
*
* This function may match against the root node of the tree. It will also map
* against the nearest node to the provided element if the element does not
* have an exact matching corresponding node. This function filters out
* matches against nested trees, so long as they are represented in the return
* value of getNestedTrees.
*/
findFocusableNodeFor(
element: HTMLElement | SVGElement,
): IFocusableNode | null;
}

39
core/utils/focusable_tree_traverser.ts
View File

@@ -4,7 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {FocusManager} from '../focus_manager.js';
import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
import type {IFocusableTree} from '../interfaces/i_focusable_tree.js';
import * as dom from '../utils/dom.js';
@@ -14,24 +13,31 @@ import * as dom from '../utils/dom.js';
* tree traversals.
*/
export class FocusableTreeTraverser {
static readonly ACTIVE_FOCUS_NODE_CSS_SELECTOR = `.${FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME}`;
static readonly PASSIVE_FOCUS_NODE_CSS_SELECTOR = `.${FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME}`;
private static readonly ACTIVE_CLASS_NAME = 'blocklyActiveFocus';
private static readonly PASSIVE_CSS_CLASS_NAME = 'blocklyPassiveFocus';
private static readonly ACTIVE_FOCUS_NODE_CSS_SELECTOR = (
`.${FocusableTreeTraverser.ACTIVE_CLASS_NAME}`);
private static readonly PASSIVE_FOCUS_NODE_CSS_SELECTOR = (
`.${FocusableTreeTraverser.PASSIVE_CSS_CLASS_NAME}`);
/**
* Returns the current IFocusableNode that either has the CSS class
* 'blocklyActiveFocus' or 'blocklyPassiveFocus', only considering HTML and
* SVG elements.
* Returns the current IFocusableNode that is styled (and thus represented) as
* having either passive or active focus, only considering HTML and SVG
* elements.
*
* This can match against the tree's root.
*
* Note that this will never return a node from a nested sub-tree as that tree
* should specifically be used to retrieve its focused node.
*
* @param tree The IFocusableTree in which to search for a focused node.
* @returns The IFocusableNode currently with focus, or null if none.
*/
static findFocusedNode(tree: IFocusableTree): IFocusableNode | null {
const root = tree.getRootFocusableNode().getFocusableElement();
if (
dom.hasClass(root, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME) ||
dom.hasClass(root, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME)
dom.hasClass(root, FocusableTreeTraverser.ACTIVE_CLASS_NAME) ||
dom.hasClass(root, FocusableTreeTraverser.PASSIVE_CSS_CLASS_NAME)
) {
// The root has focus.
return tree.getRootFocusableNode();
@@ -39,7 +45,8 @@ export class FocusableTreeTraverser {
const activeEl = root.querySelector(this.ACTIVE_FOCUS_NODE_CSS_SELECTOR);
if (activeEl instanceof HTMLElement || activeEl instanceof SVGElement) {
const active = tree.findFocusableNodeFor(activeEl);
const active = FocusableTreeTraverser.findFocusableNodeFor(
activeEl, tree);
if (active) return active;
}
@@ -47,7 +54,8 @@ export class FocusableTreeTraverser {
// subtrees).
const passiveEl = root.querySelector(this.PASSIVE_FOCUS_NODE_CSS_SELECTOR);
if (passiveEl instanceof HTMLElement || passiveEl instanceof SVGElement) {
const passive = tree.findFocusableNodeFor(passiveEl);
const passive = FocusableTreeTraverser.findFocusableNodeFor(
passiveEl, tree);
if (passive) return passive;
}
@@ -59,10 +67,17 @@ export class FocusableTreeTraverser {
* element iff it's the root element or a descendent of the root element of
* the specified IFocusableTree.
*
* If the element exists within the specified tree's DOM structure but does
* not directly correspond to a node, the nearest parent node (or the tree's
* root) will be returned to represent the provided element.
*
* If the tree contains another nested IFocusableTree, the nested tree may be
* traversed but its nodes will never be returned here per the contract of
* IFocusableTree.lookUpFocusableNode.
*
* The provided element must have a non-null ID that conforms to the contract
* mentioned in IFocusableNode.
*
* @param element The HTML or SVG element being sought.
* @param tree The tree under which the provided element may be a descendant.
* @returns The matching IFocusableNode, or null if there is no match.
@@ -74,7 +89,9 @@ export class FocusableTreeTraverser {
// First, match against subtrees.
const subTreeMatches = tree
.getNestedTrees()
.map((tree) => tree.findFocusableNodeFor(element));
.map((tree) => {
return FocusableTreeTraverser.findFocusableNodeFor(element, tree);
});
if (subTreeMatches.findIndex((match) => !!match) !== -1) {
// At least one subtree has a match for the element so it cannot be part
// of the outer tree.

89
tests/mocha/focus_manager_test.js
View File

@@ -8,7 +8,6 @@ import {
FocusManager,
getFocusManager,
} from '../../build/src/core/focus_manager.js';
import {FocusableTreeTraverser} from '../../build/src/core/utils/focusable_tree_traverser.js';
import {assert} from '../../node_modules/chai/chai.js';
import {
sharedTestSetup,
@@ -43,10 +42,6 @@ class FocusableTreeImpl {
return node;
}
getFocusedNode() {
return FocusableTreeTraverser.findFocusedNode(this);
}
getRootFocusableNode() {
return this.rootNode;
}
@@ -58,13 +53,14 @@ class FocusableTreeImpl {
lookUpFocusableNode(id) {
return this.idToNodeMap[id];
}
findFocusableNodeFor(element) {
return FocusableTreeTraverser.findFocusableNodeFor(element, this);
}
}
suite('FocusManager', function () {
const ACTIVE_FOCUS_NODE_CSS_SELECTOR = (
`.${FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME}`);
const PASSIVE_FOCUS_NODE_CSS_SELECTOR = (
`.${FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME}`);
setup(function () {
sharedTestSetup.call(this);
@@ -160,35 +156,15 @@ suite('FocusManager', function () {
const eventListener = this.globalDocumentEventListener;
document.removeEventListener(eventType, eventListener);
const removeFocusIndicators = function (element) {
element.classList.remove(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
};
// Ensure all node CSS styles are reset so that state isn't leaked between tests.
removeFocusIndicators(document.getElementById('testFocusableTree1'));
removeFocusIndicators(document.getElementById('testFocusableTree1.node1'));
removeFocusIndicators(
document.getElementById('testFocusableTree1.node1.child1'),
);
removeFocusIndicators(document.getElementById('testFocusableTree1.node2'));
removeFocusIndicators(document.getElementById('testFocusableTree2'));
removeFocusIndicators(document.getElementById('testFocusableTree2.node1'));
removeFocusIndicators(document.getElementById('testFocusableNestedTree4'));
removeFocusIndicators(
document.getElementById('testFocusableNestedTree4.node1'),
);
removeFocusIndicators(document.getElementById('testFocusableGroup1'));
removeFocusIndicators(document.getElementById('testFocusableGroup1.node1'));
removeFocusIndicators(
document.getElementById('testFocusableGroup1.node1.child1'),
);
removeFocusIndicators(document.getElementById('testFocusableGroup1.node2'));
removeFocusIndicators(document.getElementById('testFocusableGroup2'));
removeFocusIndicators(document.getElementById('testFocusableGroup2.node1'));
removeFocusIndicators(document.getElementById('testFocusableNestedGroup4'));
removeFocusIndicators(
document.getElementById('testFocusableNestedGroup4.node1'),
);
const activeElems = document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR);
const passiveElems = document.querySelectorAll(PASSIVE_FOCUS_NODE_CSS_SELECTOR);
for (const elem of activeElems) {
elem.classList.remove(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
}
for (const elem of passiveElems) {
elem.classList.remove(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
}
// Reset the current active element.
document.body.focus();
@@ -4353,12 +4329,10 @@ suite('FocusManager', function () {
// Taking focus without an existing node having focus should change no focus indicators.
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
const passiveElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.PASSIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(PASSIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.isEmpty(activeElems);
assert.isEmpty(passiveElems);
@@ -4376,12 +4350,10 @@ suite('FocusManager', function () {
// Taking focus without an existing node having focus should change no focus indicators.
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
const passiveElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.PASSIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(PASSIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.isEmpty(activeElems);
assert.strictEqual(passiveElems.length, 1);
@@ -4450,8 +4422,7 @@ suite('FocusManager', function () {
this.testFocusableGroup2,
);
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.isEmpty(activeElems);
});
@@ -4472,8 +4443,7 @@ suite('FocusManager', function () {
this.testFocusableGroup2Node1,
);
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.isEmpty(activeElems);
});
@@ -4497,8 +4467,7 @@ suite('FocusManager', function () {
this.testFocusableGroup2Node1,
);
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.isEmpty(activeElems);
});
@@ -4516,12 +4485,10 @@ suite('FocusManager', function () {
// Finishing ephemeral focus without a previously focused node should not change indicators.
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
const passiveElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.PASSIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(PASSIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.isEmpty(activeElems);
assert.isEmpty(passiveElems);
@@ -4577,8 +4544,7 @@ suite('FocusManager', function () {
// The original focused node should be restored.
const nodeElem = this.testFocusableTree2Node1.getFocusableElement();
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.strictEqual(
this.focusManager.getFocusedNode(),
@@ -4608,8 +4574,7 @@ suite('FocusManager', function () {
.getRootFocusableNode()
.getFocusableElement();
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.strictEqual(
this.focusManager.getFocusedTree(),
@@ -4637,8 +4602,7 @@ suite('FocusManager', function () {
// end of the ephemeral flow.
const nodeElem = this.testFocusableGroup2Node1.getFocusableElement();
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.strictEqual(
this.focusManager.getFocusedNode(),
@@ -4666,8 +4630,7 @@ suite('FocusManager', function () {
// end of the ephemeral flow.
const nodeElem = this.testFocusableGroup2Node1.getFocusableElement();
const activeElems = Array.from(
document.querySelectorAll(
FocusableTreeTraverser.ACTIVE_FOCUS_NODE_CSS_SELECTOR),
document.querySelectorAll(ACTIVE_FOCUS_NODE_CSS_SELECTOR),
);
assert.strictEqual(
this.focusManager.getFocusedNode(),

8
tests/mocha/focusable_tree_traverser_test.js
View File

@@ -40,10 +40,6 @@ class FocusableTreeImpl {
return node;
}
getFocusedNode() {
throw Error('Unused in test suite.');
}
getRootFocusableNode() {
return this.rootNode;
}
@@ -55,10 +51,6 @@ class FocusableTreeImpl {
lookUpFocusableNode(id) {
return this.idToNodeMap[id];
}
findFocusableNodeFor(element) {
return FocusableTreeTraverser.findFocusableNodeFor(element, this);
}
}
suite('FocusableTreeTraverser', function () {