diff --git a/core/block_svg.ts b/core/block_svg.ts index 1b85d38ce6b..43fe163c799 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -300,9 +300,12 @@ export class BlockSvg const oldXY = this.getRelativeToSurfaceXY(); const focusedNode = getFocusManager().getFocusedNode(); - const restoreFocus = this.getSvgRoot().contains( - focusedNode?.getFocusableElement() ?? null, - ); + let element = focusedNode?.getFocusableElement() ?? null; + // Make sure the element is of Node type + if (!(element instanceof Node)) { + element = null; + } + const restoreFocus = this.getSvgRoot().contains(element); if (newParent) { (newParent as BlockSvg).getSvgRoot().appendChild(svgRoot); // appendChild() clears focus state, so re-focus the previously focused @@ -870,11 +873,12 @@ export class BlockSvg // If this block (or a descendant) was focused, focus its parent or // workspace instead. const focusManager = getFocusManager(); - if ( - this.getSvgRoot().contains( - focusManager.getFocusedNode()?.getFocusableElement() ?? null, - ) - ) { + let element = focusManager.getFocusedNode()?.getFocusableElement() ?? null; + // Make sure the element is of Node type + if (!(element instanceof Node)) { + element = null; + } + if (this.getSvgRoot().contains(element)) { let parent: BlockSvg | undefined | null = this.getParent(); if (!parent) { // In some cases, blocks are disconnected from their parents before diff --git a/core/focus_manager.ts b/core/focus_manager.ts index 02e0591070f..21716be1058 100644 --- a/core/focus_manager.ts +++ b/core/focus_manager.ts @@ -320,13 +320,18 @@ export class FocusManager { this.isUpdatingFocusedNode = true; } - // Double check that state wasn't desynchronized in the background. See: - // https://github.com/google/blockly-keyboard-experimentation/issues/87. - // This is only done for the case where the same node is being focused twice - // since other cases should automatically correct (due to the rest of the - // routine running as normal). - const prevFocusedElement = this.focusedNode?.getFocusableElement(); - const hasDesyncedState = prevFocusedElement !== document.activeElement; + // // TODO: This will break multiselect plugin as it will make the + // // selected `MultiselectDraggable` (has already assigned to `this.focusedNode`) + // // to lose focus (get changed to the actual focused one managed by Blockly core). + // // Double check that state wasn't desynchronized in the background. See: + // // https://github.com/google/blockly-keyboard-experimentation/issues/87. + // // This is only done for the case where the same node is being focused twice + // // since other cases should automatically correct (due to the rest of the + // // routine running as normal). + // const prevFocusedElement = this.focusedNode?.getFocusableElement(); + // const hasDesyncedState = prevFocusedElement !== document.activeElement; + const hasDesyncedState = false; + if (this.focusedNode === focusableNode && !hasDesyncedState) { if (mustRestoreUpdatingNode) { // Reenable state syncing from DOM events. diff --git a/core/layer_manager.ts b/core/layer_manager.ts index fd7d8fe235a..48d3053103b 100644 --- a/core/layer_manager.ts +++ b/core/layer_manager.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {getFocusManager} from './focus_manager.js'; import type {IFocusableNode} from './interfaces/i_focusable_node.js'; import {IRenderedElement} from './interfaces/i_rendered_element.js'; import * as layerNums from './layers.js'; @@ -104,11 +103,14 @@ export class LayerManager { moveToDragLayer(elem: IRenderedElement & IFocusableNode) { this.dragLayer?.appendChild(elem.getSvgRoot()); - if (elem.canBeFocused()) { - // Since moving the element to the drag layer will cause it to lose focus, - // ensure it regains focus (to ensure proper highlights & sent events). - getFocusManager().focusNode(elem); - } + // // TODO: This will break multiselect plugin, as multiselect plugin will + // // implicitly call this method as well, we should not change the focus to individual + // // block and move away from `MultiselectDraggable` at this time. + // if (elem.canBeFocused()) { + // // Since moving the element to the drag layer will cause it to lose focus, + // // ensure it regains focus (to ensure proper highlights & sent events). + // getFocusManager().focusNode(elem); + // } } /** @@ -119,11 +121,14 @@ export class LayerManager { moveOffDragLayer(elem: IRenderedElement & IFocusableNode, layerNum: number) { this.append(elem, layerNum); - if (elem.canBeFocused()) { - // Since moving the element off the drag layer will cause it to lose focus, - // ensure it regains focus (to ensure proper highlights & sent events). - getFocusManager().focusNode(elem); - } + // // TODO: This will break multiselect plugin, as multiselect plugin will + // // implicitly call this method as well, we should not change the focus to individual + // // block and move away from `MultiselectDraggable` at this time. + // if (elem.canBeFocused()) { + // // Since moving the element off the drag layer will cause it to lose focus, + // // ensure it regains focus (to ensure proper highlights & sent events). + // getFocusManager().focusNode(elem); + // } } /** diff --git a/core/utils/dom.ts b/core/utils/dom.ts index 4087984151c..d400047a7cf 100644 --- a/core/utils/dom.ts +++ b/core/utils/dom.ts @@ -76,10 +76,16 @@ export function createSvgElement( */ export function addClass(element: Element, className: string): boolean { const classNames = className.split(' '); - if (classNames.every((name) => element.classList.contains(name))) { + if ( + classNames.every( + (name) => !!element.classList && element.classList.contains(name), + ) + ) { return false; } - element.classList.add(...classNames); + if (element.classList) { + element.classList.add(...classNames); + } return true; } @@ -90,7 +96,9 @@ export function addClass(element: Element, className: string): boolean { * @param classNames A string of one or multiple class names for an element. */ export function removeClasses(element: Element, classNames: string) { - element.classList.remove(...classNames.split(' ')); + if (element.classList) { + element.classList.remove(...classNames.split(' ')); + } } /** @@ -104,10 +112,16 @@ export function removeClasses(element: Element, classNames: string) { */ export function removeClass(element: Element, className: string): boolean { const classNames = className.split(' '); - if (classNames.every((name) => !element.classList.contains(name))) { + if ( + classNames.every( + (name) => !element.classList || !element.classList.contains(name), + ) + ) { return false; } - element.classList.remove(...classNames); + if (element.classList) { + element.classList.remove(...classNames); + } return true; } @@ -119,7 +133,7 @@ export function removeClass(element: Element, className: string): boolean { * @returns True if class exists, false otherwise. */ export function hasClass(element: Element, className: string): boolean { - return element.classList.contains(className); + return !!element.classList && element.classList.contains(className); } /**