diff --git a/core/focus_manager.ts b/core/focus_manager.ts index c1fc295b991..88eef46b530 100644 --- a/core/focus_manager.ts +++ b/core/focus_manager.ts @@ -60,6 +60,7 @@ export class FocusManager { registeredTrees: Array = []; private currentlyHoldsEphemeralFocus: boolean = false; + private lockFocusStateChanges: boolean = false; constructor( addGlobalEventListener: (type: string, listener: EventListener) => void, @@ -89,7 +90,16 @@ export class FocusManager { } if (newNode) { - this.focusNode(newNode); + const newTree = newNode.getFocusableTree(); + const oldTree = this.focusedNode?.getFocusableTree(); + if (newNode === newTree.getRootFocusableNode() && newTree !== oldTree) { + // If the root of the tree is the one taking focus (such as due to + // being tabbed), try to focus the whole tree explicitly to ensure the + // correct node re-receives focus. + this.focusTree(newTree); + } else { + this.focusNode(newNode); + } } else { this.defocusCurrentFocusedNode(); } @@ -108,6 +118,7 @@ export class FocusManager { * certain whether the tree has been registered. */ registerTree(tree: IFocusableTree): void { + this.ensureManagerIsUnlocked(); if (this.isRegistered(tree)) { throw Error(`Attempted to re-register already registered tree: ${tree}.`); } @@ -133,10 +144,11 @@ export class FocusManager { * this manager. */ unregisterTree(tree: IFocusableTree): void { + this.ensureManagerIsUnlocked(); 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((reg) => reg === tree); this.registeredTrees.splice(treeIndex, 1); const focusedNode = FocusableTreeTraverser.findFocusedNode(tree); @@ -192,11 +204,14 @@ export class FocusManager { * focus. */ focusTree(focusableTree: IFocusableTree): void { + this.ensureManagerIsUnlocked(); if (!this.isRegistered(focusableTree)) { throw Error(`Attempted to focus unregistered tree: ${focusableTree}.`); } const currNode = FocusableTreeTraverser.findFocusedNode(focusableTree); - this.focusNode(currNode ?? focusableTree.getRootFocusableNode()); + const nodeToRestore = focusableTree.getRestoredFocusableNode(currNode); + const rootFallback = focusableTree.getRootFocusableNode(); + this.focusNode(nodeToRestore ?? currNode ?? rootFallback); } /** @@ -205,18 +220,37 @@ export class FocusManager { * Any previously focused node will be updated to be passively highlighted (if * it's in a different focusable tree) or blurred (if it's in the same one). * - * @param focusableNode The node that should receive active - * focus. + * @param focusableNode The node that should receive active focus. */ focusNode(focusableNode: IFocusableNode): void { + this.ensureManagerIsUnlocked(); + if (this.focusedNode === focusableNode) return; // State is unchanged. + const nextTree = focusableNode.getFocusableTree(); if (!this.isRegistered(nextTree)) { throw Error(`Attempted to focus unregistered node: ${focusableNode}.`); } + + // Safety check for ensuring focusNode() doesn't get called for a node that + // isn't actually hooked up to its parent tree correctly (since this can + // cause weird inconsistencies). + const matchedNode = FocusableTreeTraverser.findFocusableNodeFor( + focusableNode.getFocusableElement(), + nextTree, + ); + if (matchedNode !== focusableNode) { + throw Error( + `Attempting to focus node which isn't recognized by its parent tree: ` + + `${focusableNode}.`, + ); + } + const prevNode = this.focusedNode; - if (prevNode && prevNode.getFocusableTree() !== nextTree) { - this.setNodeToPassive(prevNode); + const prevTree = prevNode?.getFocusableTree(); + if (prevNode && prevTree !== nextTree) { + this.passivelyFocusNode(prevNode, nextTree); } + // If there's a focused node in the new node's tree, ensure it's reset. const prevNodeNextTree = FocusableTreeTraverser.findFocusedNode(nextTree); const nextTreeRoot = nextTree.getRootFocusableNode(); @@ -229,9 +263,10 @@ export class FocusManager { if (nextTreeRoot !== focusableNode) { this.removeHighlight(nextTreeRoot); } + if (!this.currentlyHoldsEphemeralFocus) { // Only change the actively focused node if ephemeral state isn't held. - this.setNodeToActive(focusableNode); + this.activelyFocusNode(focusableNode, prevTree ?? null); } this.focusedNode = focusableNode; } @@ -257,6 +292,7 @@ export class FocusManager { takeEphemeralFocus( focusableElement: HTMLElement | SVGElement, ): ReturnEphemeralFocus { + this.ensureManagerIsUnlocked(); if (this.currentlyHoldsEphemeralFocus) { throw Error( `Attempted to take ephemeral focus when it's already held, ` + @@ -266,7 +302,7 @@ export class FocusManager { this.currentlyHoldsEphemeralFocus = true; if (this.focusedNode) { - this.setNodeToPassive(this.focusedNode); + this.passivelyFocusNode(this.focusedNode, null); } focusableElement.focus(); @@ -282,34 +318,124 @@ export class FocusManager { this.currentlyHoldsEphemeralFocus = false; if (this.focusedNode) { - this.setNodeToActive(this.focusedNode); + this.activelyFocusNode(this.focusedNode, null); } }; } + /** + * Ensures that the manager is currently allowing operations that change its + * internal focus state (such as via focusNode()). + * + * If the manager is currently not allowing state changes, an exception is + * thrown. + */ + private ensureManagerIsUnlocked(): void { + if (this.lockFocusStateChanges) { + throw Error( + 'FocusManager state changes cannot happen in a tree/node focus/blur ' + + 'callback.', + ); + } + } + + /** + * Defocuses the current actively focused node tracked by the manager, iff + * there's a node being tracked and the manager doesn't have ephemeral focus. + */ private defocusCurrentFocusedNode(): void { // The current node will likely be defocused while ephemeral focus is held, // but internal manager state shouldn't change since the node should be // restored upon exiting ephemeral focus mode. if (this.focusedNode && !this.currentlyHoldsEphemeralFocus) { - this.setNodeToPassive(this.focusedNode); + this.passivelyFocusNode(this.focusedNode, null); this.focusedNode = null; } } - private setNodeToActive(node: IFocusableNode): void { + /** + * Marks the specified node as actively focused, also calling related lifecycle + * callback methods for both the node and its parent tree. This ensures that + * the node is properly styled to indicate its active focus. + * + * This does not change the manager's currently tracked node, nor does it + * change any other nodes. + * + * @param node The node to be actively focused. + * @param prevTree The tree of the previously actively focused node, or null + * if there wasn't a previously actively focused node. + */ + private activelyFocusNode( + node: IFocusableNode, + prevTree: IFocusableTree | null, + ): void { + // Note that order matters here. Focus callbacks are allowed to change + // element visibility which can influence focusability, including for a + // node's focusable element (which *is* allowed to be invisible until the + // node needs to be focused). + this.lockFocusStateChanges = true; + node.getFocusableTree().onTreeFocus(node, prevTree); + node.onNodeFocus(); + this.lockFocusStateChanges = false; + + this.setNodeToVisualActiveFocus(node); + node.getFocusableElement().focus(); + } + + /** + * Marks the specified node as passively focused, also calling related + * lifecycle callback methods for both the node and its parent tree. This + * ensures that the node is properly styled to indicate its passive focus. + * + * This does not change the manager's currently tracked node, nor does it + * change any other nodes. + * + * @param node The node to be passively focused. + * @param nextTree The tree of the node receiving active focus, or null if no + * node will be actively focused. + */ + private passivelyFocusNode( + node: IFocusableNode, + nextTree: IFocusableTree | null, + ): void { + this.lockFocusStateChanges = true; + node.getFocusableTree().onTreeBlur(nextTree); + node.onNodeBlur(); + this.lockFocusStateChanges = false; + + this.setNodeToVisualPassiveFocus(node); + } + + /** + * Updates the node's styling to indicate that it should have an active focus + * indicator. + * + * @param node The node to be styled for active focus. + */ + private setNodeToVisualActiveFocus(node: IFocusableNode): void { const element = node.getFocusableElement(); 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 { + /** + * Updates the node's styling to indicate that it should have a passive focus + * indicator. + * + * @param node The node to be styled for passive focus. + */ + private setNodeToVisualPassiveFocus(node: IFocusableNode): void { const element = node.getFocusableElement(); dom.removeClass(element, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME); dom.addClass(element, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME); } + /** + * Removes any active/passive indicators for the specified node. + * + * @param node The node which should have neither passive nor active focus + * indication. + */ private removeHighlight(node: IFocusableNode): void { const element = node.getFocusableElement(); dom.removeClass(element, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME); diff --git a/core/interfaces/i_focusable_node.ts b/core/interfaces/i_focusable_node.ts index 14100d44c7f..53a432d30f4 100644 --- a/core/interfaces/i_focusable_node.ts +++ b/core/interfaces/i_focusable_node.ts @@ -25,8 +25,14 @@ export interface IFocusableNode { * and a tab index must be present in order for the element to be focusable in * the DOM). * - * It's expected the return element will not change for the lifetime of the - * node. + * The returned element must be visible if the node is ever focused via + * FocusManager.focusNode() or FocusManager.focusTree(). It's allowed for an + * element to be hidden until onNodeFocus() is called, or become hidden with a + * call to onNodeBlur(). + * + * It's expected the actual returned element will not change for the lifetime + * of the node (that is, its properties can change but a new element should + * never be returned). */ getFocusableElement(): HTMLElement | SVGElement; @@ -36,4 +42,38 @@ export interface IFocusableNode { * belongs. */ getFocusableTree(): IFocusableTree; + + /** + * Called when this node receives active focus. + * + * Note that it's fine for implementations to change visibility modifiers, but + * they should avoid the following: + * - Creating or removing DOM elements (including via the renderer or drawer). + * - Affecting focus via DOM focus() calls or the FocusManager. + */ + onNodeFocus(): void; + + /** + * Called when this node loses active focus. It may still have passive focus. + * + * This has the same implementation restrictions as onNodeFocus(). + */ + onNodeBlur(): void; +} + +/** + * Determines whether the provided object fulfills the contract of + * IFocusableNode. + * + * @param object The object to test. + * @returns Whether the provided object can be used as an IFocusableNode. + */ +export function isFocusableNode(object: any | null): object is IFocusableNode { + return ( + object && + 'getFocusableElement' in object && + 'getFocusableTree' in object && + 'onNodeFocus' in object && + 'onNodeBlur' in object + ); } diff --git a/core/interfaces/i_focusable_tree.ts b/core/interfaces/i_focusable_tree.ts index bc0c38849c8..69afa24ffdf 100644 --- a/core/interfaces/i_focusable_tree.ts +++ b/core/interfaces/i_focusable_tree.ts @@ -37,6 +37,34 @@ export interface IFocusableTree { */ getRootFocusableNode(): IFocusableNode; + /** + * Returns the IFocusableNode of this tree that should receive active focus + * when the tree itself has focus returned to it. + * + * There are some very important notes to consider about a tree's focus + * lifecycle when implementing a version of this method that doesn't return + * null: + * 1. A null previousNode does not guarantee first-time focus state as nodes + * can be deleted. + * 2. This method is only used when the tree itself is focused, either through + * tab navigation or via FocusManager.focusTree(). In many cases, the + * previously focused node will be directly focused instead which will + * bypass this method. + * 3. The default behavior (i.e. returning null here) involves either + * restoring the previous node (previousNode) or focusing the tree's root. + * + * This method is largely intended to provide tree implementations with the + * means of specifying a better default node than their root. + * + * @param previousNode The node that previously held passive focus for this + * tree, or null if the tree hasn't yet been focused. + * @returns The IFocusableNode that should now receive focus, or null if + * default behavior should be used, instead. + */ + getRestoredFocusableNode( + previousNode: IFocusableNode | null, + ): IFocusableNode | null; + /** * Returns all directly nested trees under this tree. * @@ -58,4 +86,55 @@ export interface IFocusableTree { * @param id The ID of the node's focusable HTMLElement or SVGElement. */ lookUpFocusableNode(id: string): IFocusableNode | null; + + /** + * Called when a node of this tree has received active focus. + * + * Note that a null previousTree does not necessarily indicate that this is + * the first time Blockly is receiving focus. In fact, few assumptions can be + * made about previous focus state as a previous null tree simply indicates + * that Blockly did not hold active focus prior to this tree becoming focused + * (which can happen due to focus exiting the Blockly injection div, or for + * other cases like ephemeral focus). + * + * See IFocusableNode.onNodeFocus() as implementations have the same + * restrictions as with that method. + * + * @param node The node receiving active focus. + * @param previousTree The previous tree that held active focus, or null if + * none. + */ + onTreeFocus(node: IFocusableNode, previousTree: IFocusableTree | null): void; + + /** + * Called when the previously actively focused node of this tree is now + * passively focused and there is no other active node of this tree taking its + * place. + * + * This has the same implementation restrictions and considerations as + * onTreeFocus(). + * + * @param nextTree The next tree receiving active focus, or null if none (such + * as in the case that Blockly is entirely losing DOM focus). + */ + onTreeBlur(nextTree: IFocusableTree | null): void; +} + +/** + * Determines whether the provided object fulfills the contract of + * IFocusableTree. + * + * @param object The object to test. + * @returns Whether the provided object can be used as an IFocusableTree. + */ +export function isFocusableTree(object: any | null): object is IFocusableTree { + return ( + object && + 'getRootFocusableNode' in object && + 'getRestoredFocusableNode' in object && + 'getNestedTrees' in object && + 'lookUpFocusableNode' in object && + 'onTreeFocus' in object && + 'onTreeBlur' in object + ); } diff --git a/tests/mocha/focus_manager_test.js b/tests/mocha/focus_manager_test.js index 4a3f6b3ad1f..69ecfe722a5 100644 --- a/tests/mocha/focus_manager_test.js +++ b/tests/mocha/focus_manager_test.js @@ -27,6 +27,10 @@ class FocusableNodeImpl { getFocusableTree() { return this.tree; } + + onNodeFocus() {} + + onNodeBlur() {} } class FocusableTreeImpl { @@ -46,6 +50,10 @@ class FocusableTreeImpl { return this.rootNode; } + getRestoredFocusableNode() { + return null; + } + getNestedTrees() { return this.nestedTrees; } @@ -53,6 +61,10 @@ class FocusableTreeImpl { lookUpFocusableNode(id) { return this.idToNodeMap[id]; } + + onTreeFocus() {} + + onTreeBlur() {} } suite('FocusManager', function () { @@ -293,6 +305,18 @@ suite('FocusManager', function () { assert.isTrue(isRegistered); }); + + test('for unregistered tree with other registered tree returns false', function () { + this.focusManager.registerTree(this.testFocusableTree2); + this.focusManager.registerTree(this.testFocusableTree1); + this.focusManager.unregisterTree(this.testFocusableTree1); + + const isRegistered = this.focusManager.isRegistered( + this.testFocusableTree1, + ); + + assert.isFalse(isRegistered); + }); }); suite('getFocusedTree()', function () { @@ -2067,7 +2091,7 @@ suite('FocusManager', function () { ); }); - test('registered tree focus()ed other tree node passively focused tree root now has active property', function () { + test('registered tree focus()ed other tree node passively focused tree node now has active property', function () { this.focusManager.registerTree(this.testFocusableTree1); this.focusManager.registerTree(this.testFocusableTree2); document.getElementById('testFocusableTree1.node1').focus(); @@ -2075,26 +2099,27 @@ suite('FocusManager', function () { document.getElementById('testFocusableTree1').focus(); - // This differs from the behavior of focusTree() since directly focusing a tree's root will - // coerce it to now have focus. + // Directly refocusing a tree's root should have functional parity with focusTree(). That + // means the tree's previous node should now have active focus again and its root should + // have no focus indication. const rootElem = this.testFocusableTree1 .getRootFocusableNode() .getFocusableElement(); const nodeElem = this.testFocusableTree1Node1.getFocusableElement(); assert.includesClass( - rootElem.classList, + nodeElem.classList, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME, ); assert.notIncludesClass( - rootElem.classList, + nodeElem.classList, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME, ); assert.notIncludesClass( - nodeElem.classList, + rootElem.classList, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME, ); assert.notIncludesClass( - nodeElem.classList, + rootElem.classList, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME, ); }); @@ -3879,7 +3904,7 @@ suite('FocusManager', function () { ); }); - test('registered tree focus()ed other tree node passively focused tree root now has active property', function () { + test('registered tree focus()ed other tree node passively focused tree node now has active property', function () { this.focusManager.registerTree(this.testFocusableGroup1); this.focusManager.registerTree(this.testFocusableGroup2); document.getElementById('testFocusableGroup1.node1').focus(); @@ -3887,26 +3912,27 @@ suite('FocusManager', function () { document.getElementById('testFocusableGroup1').focus(); - // This differs from the behavior of focusTree() since directly focusing a tree's root will - // coerce it to now have focus. + // Directly refocusing a tree's root should have functional parity with focusTree(). That + // means the tree's previous node should now have active focus again and its root should + // have no focus indication. const rootElem = this.testFocusableGroup1 .getRootFocusableNode() .getFocusableElement(); const nodeElem = this.testFocusableGroup1Node1.getFocusableElement(); assert.includesClass( - rootElem.classList, + nodeElem.classList, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME, ); assert.notIncludesClass( - rootElem.classList, + nodeElem.classList, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME, ); assert.notIncludesClass( - nodeElem.classList, + rootElem.classList, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME, ); assert.notIncludesClass( - nodeElem.classList, + rootElem.classList, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME, ); }); diff --git a/tests/mocha/focusable_tree_traverser_test.js b/tests/mocha/focusable_tree_traverser_test.js index b6674573ecd..d2467b6e95c 100644 --- a/tests/mocha/focusable_tree_traverser_test.js +++ b/tests/mocha/focusable_tree_traverser_test.js @@ -25,6 +25,10 @@ class FocusableNodeImpl { getFocusableTree() { return this.tree; } + + onNodeFocus() {} + + onNodeBlur() {} } class FocusableTreeImpl { @@ -44,6 +48,10 @@ class FocusableTreeImpl { return this.rootNode; } + getRestoredFocusableNode() { + return null; + } + getNestedTrees() { return this.nestedTrees; } @@ -51,6 +59,10 @@ class FocusableTreeImpl { lookUpFocusableNode(id) { return this.idToNodeMap[id]; } + + onTreeFocus() {} + + onTreeBlur() {} } suite('FocusableTreeTraverser', function () {