chore: part 1 of addressing reviewer comments.

This commit is contained in:
Ben Henning
2025-04-03 22:25:50 +00:00
parent 3dc4d17b30
commit 902b26b1a1
5 changed files with 997 additions and 955 deletions

View File

@@ -106,7 +106,7 @@ import {FlyoutItem} from './flyout_item.js';
import {FlyoutMetricsManager} from './flyout_metrics_manager.js';
import {FlyoutSeparator} from './flyout_separator.js';
import {VerticalFlyout} from './flyout_vertical.js';
import {FocusManager, getFocusManager} from './focus_manager.js';
import {FocusManager, getFocusManager, ReturnEphemeralFocus} from './focus_manager.js';
import {CodeGenerator} from './generator.js';
import {Gesture} from './gesture.js';
import {Grid} from './grid.js';
@@ -523,6 +523,7 @@ export {
FlyoutMetricsManager,
FlyoutSeparator,
FocusManager,
ReturnEphemeralFocus,
CodeGenerator as Generator,
Gesture,
Grid,

View File

@@ -30,6 +30,9 @@ export type ReturnEphemeralFocus = () => void;
* focusNode().
*/
export class FocusManager {
static readonly ACTIVE_FOCUS_NODE_CSS_CLASS_NAME = 'blocklyActiveFocus';
static readonly PASSIVE_FOCUS_NODE_CSS_CLASS_NAME = 'blocklyPassiveFocus';
focusedNode: IFocusableNode | null = null;
registeredTrees: Array<IFocusableTree> = [];
@@ -45,7 +48,7 @@ export class FocusManager {
// The target that now has focus.
const activeElement = document.activeElement;
let newNode: IFocusableNode | null = null;
let newNode: IFocusableNode | null | undefined = null;
if (
activeElement instanceof HTMLElement ||
activeElement instanceof SVGElement
@@ -53,10 +56,10 @@ export class FocusManager {
// If the target losing focus maps to any tree, then it should be
// updated. Per the contract of findFocusableNodeFor only one tree
// should claim the element.
const matchingNodes = this.registeredTrees.map((tree) =>
tree.findFocusableNodeFor(activeElement),
);
newNode = matchingNodes.find((node) => !!node) ?? null;
for (const tree of this.registeredTrees) {
newNode = tree.findFocusableNodeFor(activeElement);
if (newNode) break;
}
}
if (newNode) {
@@ -91,7 +94,7 @@ export class FocusManager {
* unregisterTree.
*/
isRegistered(tree: IFocusableTree): boolean {
return this.registeredTrees.findIndex((reg) => reg == tree) !== -1;
return this.registeredTrees.findIndex((reg) => reg === tree) !== -1;
}
/**
@@ -107,13 +110,13 @@ export class FocusManager {
if (!this.isRegistered(tree)) {
throw Error(`Attempted to unregister not registered tree: ${tree}.`);
}
const treeIndex = this.registeredTrees.findIndex((tree) => tree == tree);
const treeIndex = this.registeredTrees.findIndex((tree) => tree === tree);
this.registeredTrees.splice(treeIndex, 1);
const focusedNode = tree.getFocusedNode();
const root = tree.getRootFocusableNode();
if (focusedNode != null) this.removeHighlight(focusedNode);
if (this.focusedNode == focusedNode || this.focusedNode == root) {
if (focusedNode) this.removeHighlight(focusedNode);
if (this.focusedNode === focusedNode || this.focusedNode === root) {
this.focusedNode = null;
}
this.removeHighlight(root);
@@ -181,25 +184,25 @@ export class FocusManager {
* focus.
*/
focusNode(focusableNode: IFocusableNode): void {
const curTree = focusableNode.getFocusableTree();
if (!this.isRegistered(curTree)) {
const nextTree = focusableNode.getFocusableTree();
if (!this.isRegistered(nextTree)) {
throw Error(`Attempted to focus unregistered node: ${focusableNode}.`);
}
const prevNode = this.focusedNode;
if (prevNode && prevNode.getFocusableTree() !== curTree) {
if (prevNode && prevNode.getFocusableTree() !== nextTree) {
this.setNodeToPassive(prevNode);
}
// If there's a focused node in the new node's tree, ensure it's reset.
const prevNodeCurTree = curTree.getFocusedNode();
const curTreeRoot = curTree.getRootFocusableNode();
if (prevNodeCurTree) {
this.removeHighlight(prevNodeCurTree);
const prevNodeNextTree = nextTree.getFocusedNode();
const nextTreeRoot = nextTree.getRootFocusableNode();
if (prevNodeNextTree) {
this.removeHighlight(prevNodeNextTree);
}
// For caution, ensure that the root is always reset since getFocusedNode()
// is expected to return null if the root was highlighted, if the root is
// not the node now being set to active.
if (curTreeRoot !== focusableNode) {
this.removeHighlight(curTreeRoot);
if (nextTreeRoot !== focusableNode) {
this.removeHighlight(nextTreeRoot);
}
if (!this.currentlyHoldsEphemeralFocus) {
// Only change the actively focused node if ephemeral state isn't held.
@@ -271,21 +274,21 @@ export class FocusManager {
private setNodeToActive(node: IFocusableNode): void {
const element = node.getFocusableElement();
dom.addClass(element, 'blocklyActiveFocus');
dom.removeClass(element, 'blocklyPassiveFocus');
dom.addClass(element, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
dom.removeClass(element, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
element.focus();
}
private setNodeToPassive(node: IFocusableNode): void {
const element = node.getFocusableElement();
dom.removeClass(element, 'blocklyActiveFocus');
dom.addClass(element, 'blocklyPassiveFocus');
dom.removeClass(element, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
dom.addClass(element, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
}
private removeHighlight(node: IFocusableNode): void {
const element = node.getFocusableElement();
dom.removeClass(element, 'blocklyActiveFocus');
dom.removeClass(element, 'blocklyPassiveFocus');
dom.removeClass(element, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
dom.removeClass(element, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
}
}

View File

@@ -4,6 +4,7 @@
* 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';
@@ -13,6 +14,9 @@ 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}`;
/**
* Returns the current IFocusableNode that either has the CSS class
* 'blocklyActiveFocus' or 'blocklyPassiveFocus', only considering HTML and
@@ -26,28 +30,28 @@ export class FocusableTreeTraverser {
static findFocusedNode(tree: IFocusableTree): IFocusableNode | null {
const root = tree.getRootFocusableNode().getFocusableElement();
if (
dom.hasClass(root, 'blocklyActiveFocus') ||
dom.hasClass(root, 'blocklyPassiveFocus')
dom.hasClass(root, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME) ||
dom.hasClass(root, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME)
) {
// The root has focus.
return tree.getRootFocusableNode();
}
const activeEl = root.querySelector('.blocklyActiveFocus');
let active: IFocusableNode | null = null;
const activeEl = root.querySelector(this.ACTIVE_FOCUS_NODE_CSS_SELECTOR);
if (activeEl instanceof HTMLElement || activeEl instanceof SVGElement) {
active = tree.findFocusableNodeFor(activeEl);
const active = tree.findFocusableNodeFor(activeEl);
if (active) return active;
}
// At most there should be one passive indicator per tree (not considering
// subtrees).
const passiveEl = root.querySelector('.blocklyPassiveFocus');
let passive: IFocusableNode | null = null;
const passiveEl = root.querySelector(this.PASSIVE_FOCUS_NODE_CSS_SELECTOR);
if (passiveEl instanceof HTMLElement || passiveEl instanceof SVGElement) {
passive = tree.findFocusableNodeFor(passiveEl);
const passive = tree.findFocusableNodeFor(passiveEl);
if (passive) return passive;
}
return active ?? passive;
return null;
}
/**

File diff suppressed because it is too large Load Diff

View File

@@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {FocusManager} 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 {
@@ -11,51 +12,59 @@ import {
sharedTestTeardown,
} from './test_helpers/setup_teardown.js';
class FocusableNodeImpl {
constructor(element, tree) {
this.element = element;
this.tree = tree;
}
getFocusableElement() {
return this.element;
}
getFocusableTree() {
return this.tree;
}
}
class FocusableTreeImpl {
constructor(rootElement, nestedTrees) {
this.nestedTrees = nestedTrees;
this.idToNodeMap = {};
this.rootNode = this.addNode(rootElement);
}
addNode(element) {
const node = new FocusableNodeImpl(element, this);
this.idToNodeMap[element.id] = node;
return node;
}
getFocusedNode() {
throw Error('Unused in test suite.');
}
getRootFocusableNode() {
return this.rootNode;
}
getNestedTrees() {
return this.nestedTrees;
}
lookUpFocusableNode(id) {
return this.idToNodeMap[id];
}
findFocusableNodeFor(element) {
return FocusableTreeTraverser.findFocusableNodeFor(element, this);
}
}
suite('FocusableTreeTraverser', function () {
setup(function () {
sharedTestSetup.call(this);
const FocusableNodeImpl = function (element, tree) {
this.getFocusableElement = function () {
return element;
};
this.getFocusableTree = function () {
return tree;
};
};
const FocusableTreeImpl = function (rootElement, nestedTrees) {
this.idToNodeMap = {};
this.addNode = function (element) {
const node = new FocusableNodeImpl(element, this);
this.idToNodeMap[element.id] = node;
return node;
};
this.getFocusedNode = function () {
throw Error('Unused in test suite.');
};
this.getRootFocusableNode = function () {
return this.rootNode;
};
this.getNestedTrees = function () {
return nestedTrees;
};
this.lookUpFocusableNode = function (id) {
return this.idToNodeMap[id];
};
this.findFocusableNodeFor = function (element) {
return FocusableTreeTraverser.findFocusableNodeFor(element, this);
};
this.rootNode = this.addNode(rootElement);
};
const createFocusableTree = function (rootElementId, nestedTrees) {
return new FocusableTreeImpl(
document.getElementById(rootElementId),
@@ -107,7 +116,7 @@ suite('FocusableTreeTraverser', function () {
sharedTestTeardown.call(this);
const removeFocusIndicators = function (element) {
element.classList.remove('blocklyActiveFocus', 'blocklyPassiveFocus');
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.
@@ -141,101 +150,101 @@ suite('FocusableTreeTraverser', function () {
test('for tree with root active highlight returns root node', function () {
const tree = this.testFocusableTree1;
const rootNode = tree.getRootFocusableNode();
rootNode.getFocusableElement().classList.add('blocklyActiveFocus');
rootNode.getFocusableElement().classList.add(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, rootNode);
assert.strictEqual(finding, rootNode);
});
test('for tree with root passive highlight returns root node', function () {
const tree = this.testFocusableTree1;
const rootNode = tree.getRootFocusableNode();
rootNode.getFocusableElement().classList.add('blocklyPassiveFocus');
rootNode.getFocusableElement().classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, rootNode);
assert.strictEqual(finding, rootNode);
});
test('for tree with node active highlight returns node', function () {
const tree = this.testFocusableTree1;
const node = this.testFocusableTree1Node1;
node.getFocusableElement().classList.add('blocklyActiveFocus');
node.getFocusableElement().classList.add(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, node);
assert.strictEqual(finding, node);
});
test('for tree with node passive highlight returns node', function () {
const tree = this.testFocusableTree1;
const node = this.testFocusableTree1Node1;
node.getFocusableElement().classList.add('blocklyPassiveFocus');
node.getFocusableElement().classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, node);
assert.strictEqual(finding, node);
});
test('for tree with nested node active highlight returns node', function () {
const tree = this.testFocusableTree1;
const node = this.testFocusableTree1Node1Child1;
node.getFocusableElement().classList.add('blocklyActiveFocus');
node.getFocusableElement().classList.add(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, node);
assert.strictEqual(finding, node);
});
test('for tree with nested node passive highlight returns node', function () {
const tree = this.testFocusableTree1;
const node = this.testFocusableTree1Node1Child1;
node.getFocusableElement().classList.add('blocklyPassiveFocus');
node.getFocusableElement().classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, node);
assert.strictEqual(finding, node);
});
test('for tree with nested tree root active no parent highlights returns null', function () {
test('for tree with nested tree root active no parent highlights returns root', function () {
const tree = this.testFocusableNestedTree4;
const rootNode = this.testFocusableNestedTree4.getRootFocusableNode();
rootNode.getFocusableElement().classList.add('blocklyActiveFocus');
rootNode.getFocusableElement().classList.add(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, rootNode);
assert.strictEqual(finding, rootNode);
});
test('for tree with nested tree root passive no parent highlights returns null', function () {
test('for tree with nested tree root passive no parent highlights returns root', function () {
const tree = this.testFocusableNestedTree4;
const rootNode = this.testFocusableNestedTree4.getRootFocusableNode();
rootNode.getFocusableElement().classList.add('blocklyPassiveFocus');
rootNode.getFocusableElement().classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, rootNode);
assert.strictEqual(finding, rootNode);
});
test('for tree with nested tree root active no parent highlights returns null', function () {
test('for tree with nested tree node active no parent highlights returns node', function () {
const tree = this.testFocusableNestedTree4;
const node = this.testFocusableNestedTree4Node1;
node.getFocusableElement().classList.add('blocklyActiveFocus');
node.getFocusableElement().classList.add(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, node);
assert.strictEqual(finding, node);
});
test('for tree with nested tree root passive no parent highlights returns null', function () {
const tree = this.testFocusableNestedTree4;
const node = this.testFocusableNestedTree4Node1;
node.getFocusableElement().classList.add('blocklyPassiveFocus');
node.getFocusableElement().classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(tree);
assert.equal(finding, node);
assert.strictEqual(finding, node);
});
test('for tree with nested tree root active parent node passive returns parent node', function () {
@@ -243,14 +252,14 @@ suite('FocusableTreeTraverser', function () {
const rootNode = this.testFocusableNestedTree4.getRootFocusableNode();
this.testFocusableTree2Node1
.getFocusableElement()
.classList.add('blocklyPassiveFocus');
rootNode.getFocusableElement().classList.add('blocklyActiveFocus');
.classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
rootNode.getFocusableElement().classList.add(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(
this.testFocusableTree2,
);
assert.equal(finding, this.testFocusableTree2Node1);
assert.strictEqual(finding, this.testFocusableTree2Node1);
});
test('for tree with nested tree root passive parent node passive returns parent node', function () {
@@ -258,14 +267,14 @@ suite('FocusableTreeTraverser', function () {
const rootNode = this.testFocusableNestedTree4.getRootFocusableNode();
this.testFocusableTree2Node1
.getFocusableElement()
.classList.add('blocklyPassiveFocus');
rootNode.getFocusableElement().classList.add('blocklyPassiveFocus');
.classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
rootNode.getFocusableElement().classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(
this.testFocusableTree2,
);
assert.equal(finding, this.testFocusableTree2Node1);
assert.strictEqual(finding, this.testFocusableTree2Node1);
});
test('for tree with nested tree node active parent node passive returns parent node', function () {
@@ -273,14 +282,14 @@ suite('FocusableTreeTraverser', function () {
const node = this.testFocusableNestedTree4Node1;
this.testFocusableTree2Node1
.getFocusableElement()
.classList.add('blocklyPassiveFocus');
node.getFocusableElement().classList.add('blocklyActiveFocus');
.classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
node.getFocusableElement().classList.add(FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(
this.testFocusableTree2,
);
assert.equal(finding, this.testFocusableTree2Node1);
assert.strictEqual(finding, this.testFocusableTree2Node1);
});
test('for tree with nested tree node passive parent node passive returns parent node', function () {
@@ -288,14 +297,14 @@ suite('FocusableTreeTraverser', function () {
const node = this.testFocusableNestedTree4Node1;
this.testFocusableTree2Node1
.getFocusableElement()
.classList.add('blocklyPassiveFocus');
node.getFocusableElement().classList.add('blocklyPassiveFocus');
.classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
node.getFocusableElement().classList.add(FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
const finding = FocusableTreeTraverser.findFocusedNode(
this.testFocusableTree2,
);
assert.equal(finding, this.testFocusableTree2Node1);
assert.strictEqual(finding, this.testFocusableTree2Node1);
});
});
@@ -310,7 +319,7 @@ suite('FocusableTreeTraverser', function () {
tree,
);
assert.equal(finding, rootNode);
assert.strictEqual(finding, rootNode);
});
test('for element for different tree root returns null', function () {
@@ -347,7 +356,7 @@ suite('FocusableTreeTraverser', function () {
tree,
);
assert.equal(finding, this.testFocusableTree1Node1);
assert.strictEqual(finding, this.testFocusableTree1Node1);
});
test('for non-node element in tree returns root', function () {
@@ -362,7 +371,7 @@ suite('FocusableTreeTraverser', function () {
);
// An unregistered element should map to the closest node.
assert.equal(finding, this.testFocusableTree1Node2);
assert.strictEqual(finding, this.testFocusableTree1Node2);
});
test('for nested node element in tree returns node', function () {
@@ -375,7 +384,7 @@ suite('FocusableTreeTraverser', function () {
);
// The nested node should be returned.
assert.equal(finding, this.testFocusableTree1Node1Child1);
assert.strictEqual(finding, this.testFocusableTree1Node1Child1);
});
test('for nested node element in tree returns node', function () {
@@ -390,7 +399,7 @@ suite('FocusableTreeTraverser', function () {
);
// An unregistered element should map to the closest node.
assert.equal(finding, this.testFocusableTree1Node1Child1);
assert.strictEqual(finding, this.testFocusableTree1Node1Child1);
});
test('for nested node element in tree returns node', function () {
@@ -405,7 +414,7 @@ suite('FocusableTreeTraverser', function () {
);
// An unregistered element should map to the closest node (or root).
assert.equal(finding, tree.getRootFocusableNode());
assert.strictEqual(finding, tree.getRootFocusableNode());
});
test('for nested tree root returns nested tree root', function () {
@@ -418,7 +427,7 @@ suite('FocusableTreeTraverser', function () {
tree,
);
assert.equal(finding, rootNode);
assert.strictEqual(finding, rootNode);
});
test('for nested tree node returns nested tree node', function () {
@@ -431,7 +440,7 @@ suite('FocusableTreeTraverser', function () {
);
// The node of the nested tree should be returned.
assert.equal(finding, this.testFocusableNestedTree4Node1);
assert.strictEqual(finding, this.testFocusableNestedTree4Node1);
});
test('for nested element in nested tree node returns nearest nested node', function () {
@@ -446,7 +455,7 @@ suite('FocusableTreeTraverser', function () {
);
// An unregistered element should map to the closest node.
assert.equal(finding, this.testFocusableNestedTree4Node1);
assert.strictEqual(finding, this.testFocusableNestedTree4Node1);
});
test('for nested tree node under root with different tree base returns null', function () {