Skip to content

Commit e34a969

Browse files
authored
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 RaspberryPiFoundation/blockly-keyboard-experimentation#511.
1 parent 556ee39 commit e34a969

File tree

3 files changed

+33
-2
lines changed

3 files changed

+33
-2
lines changed

core/layer_manager.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7+
import {getFocusManager} from './focus_manager.js';
8+
import type {IFocusableNode} from './interfaces/i_focusable_node.js';
79
import {IRenderedElement} from './interfaces/i_rendered_element.js';
810
import * as layerNums from './layers.js';
911
import {Coordinate} from './utils/coordinate.js';
@@ -99,17 +101,25 @@ export class LayerManager {
99101
*
100102
* @internal
101103
*/
102-
moveToDragLayer(elem: IRenderedElement) {
104+
moveToDragLayer(elem: IRenderedElement & IFocusableNode) {
103105
this.dragLayer?.appendChild(elem.getSvgRoot());
106+
107+
// Since moving the element to the drag layer will cause it to lose focus,
108+
// ensure it regains focus (to ensure proper highlights & sent events).
109+
getFocusManager().focusNode(elem);
104110
}
105111

106112
/**
107113
* Moves the given element off of the drag layer.
108114
*
109115
* @internal
110116
*/
111-
moveOffDragLayer(elem: IRenderedElement, layerNum: number) {
117+
moveOffDragLayer(elem: IRenderedElement & IFocusableNode, layerNum: number) {
112118
this.append(elem, layerNum);
119+
120+
// Since moving the element off the drag layer will cause it to lose focus,
121+
// ensure it regains focus (to ensure proper highlights & sent events).
122+
getFocusManager().focusNode(elem);
113123
}
114124

115125
/**

core/renderers/zelos/path_object.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import type {BlockSvg} from '../../block_svg.js';
1010
import type {Connection} from '../../connection.js';
11+
import {FocusManager} from '../../focus_manager.js';
1112
import type {BlockStyle} from '../../theme.js';
1213
import * as dom from '../../utils/dom.js';
1314
import {Svg} from '../../utils/svg.js';
@@ -91,6 +92,17 @@ export class PathObject extends BasePathObject {
9192
if (!this.svgPathSelected) {
9293
this.svgPathSelected = this.svgPath.cloneNode(true) as SVGElement;
9394
this.svgPathSelected.classList.add('blocklyPathSelected');
95+
// Ensure focus-specific properties don't overlap with the block's path.
96+
dom.removeClass(
97+
this.svgPathSelected,
98+
FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME,
99+
);
100+
dom.removeClass(
101+
this.svgPathSelected,
102+
FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME,
103+
);
104+
this.svgPathSelected.removeAttribute('tabindex');
105+
this.svgPathSelected.removeAttribute('id');
94106
this.svgRoot.appendChild(this.svgPathSelected);
95107
}
96108
} else {

tests/mocha/layering_test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ suite('Layering', function () {
2424
const g = Blockly.utils.dom.createSvgElement('g', {});
2525
return {
2626
getSvgRoot: () => g,
27+
getFocusableElement: () => {
28+
throw new Error('Unsupported.');
29+
},
30+
getFocusableTree: () => {
31+
throw new Error('Unsupported.');
32+
},
33+
onNodeFocus: () => {},
34+
onNodeBlur: () => {},
35+
canBeFocused: () => false,
2736
};
2837
}
2938

0 commit comments

Comments
 (0)