Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Always draw dragged blocks atop others in the workspace. #6874

Merged
merged 2 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions core/block_dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,10 @@ export class BlockDragger implements IBlockDragger {
}
this.fireDragStartEvent_();

// Mutators don't have the same type of z-ordering as the normal workspace
// during a drag. They have to rely on the order of the blocks in the SVG.
// For performance reasons that usually happens at the end of a drag,
// but do it at the beginning for mutators.
if (this.workspace_.isMutator) {
this.draggingBlock_.bringToFront();
}
// The z-order of blocks depends on their order in the SVG, so move the
// block being dragged to the front so that it will appear atop other blocks
// in the workspace.
this.draggingBlock_.bringToFront(true);

// During a drag there may be a lot of rerenders, but not field changes.
// Turn the cache on so we don't do spurious remeasures during the drag.
Expand Down
14 changes: 12 additions & 2 deletions core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,14 @@ export class BlockSvg
} else if (oldParent) {
// If we are losing a parent, we want to move our DOM element to the
// root of the workspace.
this.workspace.getCanvas().appendChild(svgRoot);
const draggingBlock = this.workspace
.getCanvas()
.querySelector('.blocklyDragging');
if (draggingBlock) {
this.workspace.getCanvas().insertBefore(svgRoot, draggingBlock);
Comment on lines +331 to +335
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gonfunko do you remember what case this was handling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not offhand :/ Judging by the comment maybe it's when a block is disconnected from a stack?

} else {
this.workspace.getCanvas().appendChild(svgRoot);
}
this.translate(oldXY.x, oldXY.y);
}

Expand Down Expand Up @@ -1146,9 +1153,11 @@ export class BlockSvg
* order that they are in the DOM. By placing this block first within the
* block group's <g>, it will render on top of any other blocks.
*
* @param blockOnly: True to only move this block to the front without
* adjusting its parents.
* @internal
*/
bringToFront() {
bringToFront(blockOnly = false) {
/* eslint-disable-next-line @typescript-eslint/no-this-alias */
let block: this | null = this;
do {
Expand All @@ -1159,6 +1168,7 @@ export class BlockSvg
if (childNodes[childNodes.length - 1] !== root) {
parent!.appendChild(root);
}
if (blockOnly) break;
block = block.getParent();
} while (block);
}
Expand Down