diff --git a/core/comments/rendered_workspace_comment.ts b/core/comments/rendered_workspace_comment.ts index 2903bff4bce..59e462c9507 100644 --- a/core/comments/rendered_workspace_comment.ts +++ b/core/comments/rendered_workspace_comment.ts @@ -23,7 +23,6 @@ import {IFocusableNode} from '../interfaces/i_focusable_node.js'; import type {IFocusableTree} from '../interfaces/i_focusable_tree.js'; import {IRenderedElement} from '../interfaces/i_rendered_element.js'; import {ISelectable} from '../interfaces/i_selectable.js'; -import * as layers from '../layers.js'; import * as commentSerialization from '../serialization/workspace_comments.js'; import {Coordinate} from '../utils/coordinate.js'; import * as dom from '../utils/dom.js'; @@ -346,7 +345,7 @@ export class RenderedWorkspaceComment onNodeFocus(): void { this.select(); // Ensure that the comment is always at the top when focused. - this.workspace.getLayerManager()?.append(this, layers.BLOCK); + this.getSvgRoot().parentElement?.appendChild(this.getSvgRoot()); this.workspace.scrollBoundsIntoView(this.getBoundingRectangle()); } diff --git a/core/dragging/block_drag_strategy.ts b/core/dragging/block_drag_strategy.ts index 76020f90b5b..0fb6d531eea 100644 --- a/core/dragging/block_drag_strategy.ts +++ b/core/dragging/block_drag_strategy.ts @@ -14,8 +14,10 @@ import {ConnectionType} from '../connection_type.js'; import type {BlockMove} from '../events/events_block_move.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; +import type {IBubble} from '../interfaces/i_bubble.js'; import {IConnectionPreviewer} from '../interfaces/i_connection_previewer.js'; import {IDragStrategy} from '../interfaces/i_draggable.js'; +import {IHasBubble, hasBubble} from '../interfaces/i_has_bubble.js'; import * as layers from '../layers.js'; import * as registry from '../registry.js'; import {finishQueuedRenders} from '../render_management.js'; @@ -120,6 +122,34 @@ export class BlockDragStrategy implements IDragStrategy { } this.block.setDragging(true); this.workspace.getLayerManager()?.moveToDragLayer(this.block); + this.getVisibleBubbles(this.block).forEach((bubble) => { + this.workspace.getLayerManager()?.moveToDragLayer(bubble, false); + }); + } + + /** + * Returns an array of visible bubbles attached to the given block or its + * descendants. + * + * @param block The block to identify open bubbles on. + * @returns An array of all currently visible bubbles on the given block or + * its descendants. + */ + private getVisibleBubbles(block: BlockSvg): IBubble[] { + return block + .getDescendants(false) + .flatMap((block) => block.getIcons()) + .filter((icon) => hasBubble(icon) && icon.bubbleIsVisible()) + .map((icon) => (icon as unknown as IHasBubble).getBubble()) + .filter((bubble) => !!bubble) // Convince TS they're non-null. + .sort((a, b) => { + // Sort the bubbles by their position in the DOM in order to maintain + // their relative z-ordering when moving between layers. + const position = a.getSvgRoot().compareDocumentPosition(b.getSvgRoot()); + if (position & Node.DOCUMENT_POSITION_PRECEDING) return 1; + if (position & Node.DOCUMENT_POSITION_FOLLOWING) return -1; + return 0; + }); } /** @@ -393,6 +423,13 @@ export class BlockDragStrategy implements IDragStrategy { this.workspace .getLayerManager() ?.moveOffDragLayer(this.block, layers.BLOCK); + + this.getVisibleBubbles(this.block).forEach((bubble) => + this.workspace + .getLayerManager() + ?.moveOffDragLayer(bubble, layers.BUBBLE, false), + ); + this.block.setDragging(false); } @@ -462,6 +499,12 @@ export class BlockDragStrategy implements IDragStrategy { this.workspace .getLayerManager() ?.moveOffDragLayer(this.block, layers.BLOCK); + this.getVisibleBubbles(this.block).forEach((bubble) => + this.workspace + .getLayerManager() + ?.moveOffDragLayer(bubble, layers.BUBBLE, false), + ); + // Blocks dragged directly from a flyout may need to be bumped into // bounds. bumpObjects.bumpIntoBounds( diff --git a/core/layer_manager.ts b/core/layer_manager.ts index fd7d8fe235a..1142bcf58d2 100644 --- a/core/layer_manager.ts +++ b/core/layer_manager.ts @@ -99,12 +99,15 @@ export class LayerManager { * Moves the given element to the drag layer, which exists on top of all other * layers, and the drag surface. * + * @param elem The element to move onto the drag layer. + * @param focus Whether or not to focus the element post-move. + * * @internal */ - moveToDragLayer(elem: IRenderedElement & IFocusableNode) { + moveToDragLayer(elem: IRenderedElement & IFocusableNode, focus = true) { this.dragLayer?.appendChild(elem.getSvgRoot()); - if (elem.canBeFocused()) { + if (focus && 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); @@ -114,12 +117,22 @@ export class LayerManager { /** * Moves the given element off of the drag layer. * + * @param elem The element to move off of the drag layer. + * @param layerNum The identifier of the layer to move the element onto. + * Should be a constant from layers.ts. + * @param focus Whether or not the element should be focused once moved onto + * the destination layer. + * * @internal */ - moveOffDragLayer(elem: IRenderedElement & IFocusableNode, layerNum: number) { + moveOffDragLayer( + elem: IRenderedElement & IFocusableNode, + layerNum: number, + focus = true, + ) { this.append(elem, layerNum); - if (elem.canBeFocused()) { + if (focus && 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); @@ -202,4 +215,13 @@ export class LayerManager { getBubbleLayer(): SVGGElement { return this.layers.get(layerNums.BUBBLE)!; } + + /** + * Returns the drag layer. + * + * @internal + */ + getDragLayer(): SVGGElement | undefined { + return this.dragLayer; + } } diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index ce0085793c7..22d865d790a 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -2883,4 +2883,50 @@ suite('Blocks', function () { assert.equal(block.inputList[1].fieldRow[0].getValue(), 'Row2'); }); }); + + suite('Dragging', function () { + setup(function () { + this.workspace = Blockly.inject('blocklyDiv'); + this.blocks = createTestBlocks(this.workspace, false); + for (const block of Object.values(this.blocks)) { + block.initSvg(); + block.render(); + } + }); + test('Bubbles are moved to drag layer along with their blocks', async function () { + this.blocks.A.setCommentText('a'); + this.blocks.B.setCommentText('b'); + this.blocks.C.setCommentText('c'); + const v1 = this.blocks.A.getIcon( + Blockly.icons.IconType.COMMENT, + ).setBubbleVisible(true); + const v2 = this.blocks.B.getIcon( + Blockly.icons.IconType.COMMENT, + ).setBubbleVisible(true); + const v3 = this.blocks.C.getIcon( + Blockly.icons.IconType.COMMENT, + ).setBubbleVisible(true); + + this.clock.tick(1000); + await Promise.all([v1, v2, v3]); + + this.blocks.B.startDrag(); + + // Block A stays put and should have its comment stay on the bubble layer. + assert.equal( + this.blocks.A.getIcon(Blockly.icons.IconType.COMMENT) + .getBubble() + .getSvgRoot().parentElement, + this.blocks.A.workspace.getLayerManager()?.getBubbleLayer(), + ); + + // Block B moves to the drag layer and its comment should follow. + assert.equal( + this.blocks.B.getIcon(Blockly.icons.IconType.COMMENT) + .getBubble() + .getSvgRoot().parentElement, + this.blocks.B.workspace.getLayerManager()?.getDragLayer(), + ); + }); + }); }); diff --git a/tests/mocha/workspace_comment_test.js b/tests/mocha/workspace_comment_test.js index bb87ad82ac6..fd4c94f6238 100644 --- a/tests/mocha/workspace_comment_test.js +++ b/tests/mocha/workspace_comment_test.js @@ -168,8 +168,10 @@ suite('Workspace comment', function () { this.workspace.id, ); }); + }); - test('focuses the workspace when deleted', function () { + suite('Focus', function () { + test('moves to the workspace when deleted', function () { const comment = new Blockly.comments.RenderedWorkspaceComment( this.workspace, ); @@ -178,5 +180,18 @@ suite('Workspace comment', function () { comment.view.getCommentBarButtons()[1].performAction(); assert.equal(Blockly.getFocusManager().getFocusedNode(), this.workspace); }); + + test('does not change the layer', function () { + const comment = new Blockly.comments.RenderedWorkspaceComment( + this.workspace, + ); + + this.workspace.getLayerManager()?.moveToDragLayer(comment); + Blockly.getFocusManager().focusNode(comment); + assert.equal( + comment.getSvgRoot().parentElement, + this.workspace.getLayerManager()?.getDragLayer(), + ); + }); }); });