From e34a9690eddf4512f682aa80c697211123326a7c Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 13 May 2025 14:37:58 -0700 Subject: [PATCH] fix: Ensure selection stays when dragging blocks (#9034) ## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #9027 ### Proposed Changes Ensure that a block being dragged is properly focused mid-drag. ### Reason for Changes Focus seems to be lost due to the block being moved to the drag layer, so re-focusing the block ensures that it remains both actively focused and selected while dragging. The regression was likely caused when block selection was moved to be fully synced based on active focus. ### Test Coverage This has been manually verified in Core's simple playground. At the time of the PR being opened, this couldn't be tested in the test environment for the experimental keyboard navigation plugin since there's a navigation connection issue there that needs to be resolved to test movement. It would be helpful to add a new test case for the underlying problem (i.e. ensuring that the block holds focus mid-drag) as part of resolving #8915. ### Documentation No new documentation should need to be added. ### Additional Information This was found during the development of https://github.com/google/blockly-keyboard-experimentation/pull/511. --- core/layer_manager.ts | 14 ++++++++++++-- core/renderers/zelos/path_object.ts | 12 ++++++++++++ tests/mocha/layering_test.js | 9 +++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/core/layer_manager.ts b/core/layer_manager.ts index a7cb57934..1d5afdd74 100644 --- a/core/layer_manager.ts +++ b/core/layer_manager.ts @@ -4,6 +4,8 @@ * 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'; import {Coordinate} from './utils/coordinate.js'; @@ -99,8 +101,12 @@ export class LayerManager { * * @internal */ - moveToDragLayer(elem: IRenderedElement) { + moveToDragLayer(elem: IRenderedElement & IFocusableNode) { this.dragLayer?.appendChild(elem.getSvgRoot()); + + // 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); } /** @@ -108,8 +114,12 @@ export class LayerManager { * * @internal */ - moveOffDragLayer(elem: IRenderedElement, layerNum: number) { + moveOffDragLayer(elem: IRenderedElement & IFocusableNode, layerNum: number) { this.append(elem, layerNum); + + // 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/renderers/zelos/path_object.ts b/core/renderers/zelos/path_object.ts index f40426483..3c304fd6b 100644 --- a/core/renderers/zelos/path_object.ts +++ b/core/renderers/zelos/path_object.ts @@ -8,6 +8,7 @@ import type {BlockSvg} from '../../block_svg.js'; import type {Connection} from '../../connection.js'; +import {FocusManager} from '../../focus_manager.js'; import type {BlockStyle} from '../../theme.js'; import * as dom from '../../utils/dom.js'; import {Svg} from '../../utils/svg.js'; @@ -91,6 +92,17 @@ export class PathObject extends BasePathObject { if (!this.svgPathSelected) { this.svgPathSelected = this.svgPath.cloneNode(true) as SVGElement; this.svgPathSelected.classList.add('blocklyPathSelected'); + // Ensure focus-specific properties don't overlap with the block's path. + dom.removeClass( + this.svgPathSelected, + FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME, + ); + dom.removeClass( + this.svgPathSelected, + FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME, + ); + this.svgPathSelected.removeAttribute('tabindex'); + this.svgPathSelected.removeAttribute('id'); this.svgRoot.appendChild(this.svgPathSelected); } } else { diff --git a/tests/mocha/layering_test.js b/tests/mocha/layering_test.js index efc3ef3d6..1ef0ee697 100644 --- a/tests/mocha/layering_test.js +++ b/tests/mocha/layering_test.js @@ -24,6 +24,15 @@ suite('Layering', function () { const g = Blockly.utils.dom.createSvgElement('g', {}); return { getSvgRoot: () => g, + getFocusableElement: () => { + throw new Error('Unsupported.'); + }, + getFocusableTree: () => { + throw new Error('Unsupported.'); + }, + onNodeFocus: () => {}, + onNodeBlur: () => {}, + canBeFocused: () => false, }; }