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: dragging and disposing of shadows #8172

Merged
merged 2 commits into from
May 28, 2024
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: 0 additions & 11 deletions core/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -675,17 +675,6 @@ export class Block implements IASTNodeLocation {
return block;
}

/**
* Returns this block if it is a shadow block, or the first non-shadow parent.
*
* @internal
*/
getFirstNonShadowBlock(): this {
if (!this.isShadow()) return this;
// We can assert the parent is non-null because shadows must have parents.
return this.getParent()!.getFirstNonShadowBlock();
}

/**
* Find all the blocks that are directly nested inside this one.
* Includes value and statement inputs, as well as any following statement.
Expand Down
49 changes: 47 additions & 2 deletions core/dragging/block_drag_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,24 @@ export class BlockDragStrategy implements IDragStrategy {

private dragging = false;

/**
* If this is a shadow block, the offset between this block and the parent
* block, to add to the drag location. In workspace units.
*/
private dragOffset = new Coordinate(0, 0);

constructor(private block: BlockSvg) {
this.workspace = block.workspace;
}

/** Returns true if the block is currently movable. False otherwise. */
isMovable(): boolean {
if (this.block.isShadow()) {
return this.block.getParent()?.isMovable() ?? false;
}

return (
this.block.isOwnMovable() &&
!this.block.isShadow() &&
!this.block.isDeadOrDying() &&
!this.workspace.options.readOnly &&
// We never drag blocks in the flyout, only create new blocks that are
Expand All @@ -77,6 +86,11 @@ export class BlockDragStrategy implements IDragStrategy {
* from any parent blocks.
*/
startDrag(e?: PointerEvent): void {
if (this.block.isShadow()) {
this.startDraggingShadow(e);
return;
}

this.dragging = true;
if (!eventUtils.getGroup()) {
eventUtils.setGroup(true);
Expand Down Expand Up @@ -106,6 +120,22 @@ export class BlockDragStrategy implements IDragStrategy {
this.workspace.getLayerManager()?.moveToDragLayer(this.block);
}

/** Starts a drag on a shadow, recording the drag offset. */
private startDraggingShadow(e?: PointerEvent) {
const parent = this.block.getParent();
if (!parent) {
throw new Error(
'Tried to drag a shadow block with no parent. ' +
'Shadow blocks should always have parents.',
);
}
this.dragOffset = Coordinate.difference(
parent.getRelativeToSurfaceXY(),
this.block.getRelativeToSurfaceXY(),
);
parent.startDrag(e);
}

/**
* Whether or not we should disconnect the block when a drag is started.
*
Expand Down Expand Up @@ -174,6 +204,11 @@ export class BlockDragStrategy implements IDragStrategy {

/** Moves the block and updates any connection previews. */
drag(newLoc: Coordinate): void {
if (this.block.isShadow()) {
this.block.getParent()?.drag(Coordinate.sum(newLoc, this.dragOffset));
return;
}

this.block.moveDuringDrag(newLoc);
this.updateConnectionPreview(
this.block,
Expand Down Expand Up @@ -317,7 +352,12 @@ export class BlockDragStrategy implements IDragStrategy {
* Cleans up any state at the end of the drag. Applies any pending
* connections.
*/
endDrag(): void {
endDrag(e?: PointerEvent): void {
if (this.block.isShadow()) {
this.block.getParent()?.endDrag(e);
return;
}

this.fireDragEndEvent();
this.fireMoveEvent();

Expand Down Expand Up @@ -373,6 +413,11 @@ export class BlockDragStrategy implements IDragStrategy {
* including reconnecting connections.
*/
revertDrag(): void {
if (this.block.isShadow()) {
this.block.getParent()?.revertDrag();
return;
}

this.startChildConn?.connect(this.block.nextConnection);
if (this.startParentConn) {
switch (this.startParentConn.type) {
Expand Down
44 changes: 25 additions & 19 deletions core/dragging/dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,25 +42,25 @@ export class Dragger implements IDragger {
*/
onDrag(e: PointerEvent, totalDelta: Coordinate) {
this.moveDraggable(e, totalDelta);
const root = this.getRoot(this.draggable);

// Must check `wouldDelete` before calling other hooks on drag targets
// since we have documented that we would do so.
if (isDeletable(this.draggable)) {
this.draggable.setDeleteStyle(
this.wouldDeleteDraggable(e, this.draggable),
);
if (isDeletable(root)) {
root.setDeleteStyle(this.wouldDeleteDraggable(e, root));
}
this.updateDragTarget(e);
}

/** Updates the drag target under the pointer (if there is one). */
protected updateDragTarget(e: PointerEvent) {
const newDragTarget = this.workspace.getDragTarget(e);
const root = this.getRoot(this.draggable);
if (this.dragTarget !== newDragTarget) {
this.dragTarget?.onDragExit(this.draggable);
newDragTarget?.onDragEnter(this.draggable);
this.dragTarget?.onDragExit(root);
newDragTarget?.onDragEnter(root);
}
newDragTarget?.onDragOver(this.draggable);
newDragTarget?.onDragOver(root);
this.dragTarget = newDragTarget;
}

Expand All @@ -80,7 +80,7 @@ export class Dragger implements IDragger {
*/
protected wouldDeleteDraggable(
e: PointerEvent,
draggable: IDraggable & IDeletable,
rootDraggable: IDraggable & IDeletable,
) {
const dragTarget = this.workspace.getDragTarget(e);
if (!dragTarget) return false;
Expand All @@ -92,50 +92,56 @@ export class Dragger implements IDragger {
);
if (!isDeleteArea) return false;

return (dragTarget as IDeleteArea).wouldDelete(draggable);
return (dragTarget as IDeleteArea).wouldDelete(rootDraggable);
}

/** Handles any drag cleanup. */
onDragEnd(e: PointerEvent) {
const origGroup = eventUtils.getGroup();
const dragTarget = this.workspace.getDragTarget(e);
const root = this.getRoot(this.draggable);

if (dragTarget) {
this.dragTarget?.onDrop(this.draggable);
this.dragTarget?.onDrop(root);
}

if (this.shouldReturnToStart(e, this.draggable)) {
if (this.shouldReturnToStart(e, root)) {
this.draggable.revertDrag();
}

const wouldDelete =
isDeletable(this.draggable) &&
this.wouldDeleteDraggable(e, this.draggable);
const wouldDelete = isDeletable(root) && this.wouldDeleteDraggable(e, root);

// TODO(#8148): use a generalized API instead of an instanceof check.
if (wouldDelete && this.draggable instanceof BlockSvg) {
blockAnimations.disposeUiEffect(this.draggable);
blockAnimations.disposeUiEffect(this.draggable.getRootBlock());
}

this.draggable.endDrag(e);

if (wouldDelete && isDeletable(this.draggable)) {
if (wouldDelete && isDeletable(root)) {
// We want to make sure the delete gets grouped with any possible
// move event.
const newGroup = eventUtils.getGroup();
eventUtils.setGroup(origGroup);
this.draggable.dispose();
root.dispose();
eventUtils.setGroup(newGroup);
}
}

// We need to special case blocks for now so that we look at the root block
// instead of the one actually being dragged in most cases.
private getRoot(draggable: IDraggable): IDraggable {
return draggable instanceof BlockSvg ? draggable.getRootBlock() : draggable;
}

/**
* Returns true if we should return the draggable to its original location
* at the end of the drag.
*/
protected shouldReturnToStart(e: PointerEvent, draggable: IDraggable) {
protected shouldReturnToStart(e: PointerEvent, rootDraggable: IDraggable) {
const dragTarget = this.workspace.getDragTarget(e);
if (!dragTarget) return false;
return dragTarget.shouldPreventMove(draggable);
return dragTarget.shouldPreventMove(rootDraggable);
}

protected pixelsToWorkspaceUnits(pixelCoord: Coordinate): Coordinate {
Expand Down
2 changes: 1 addition & 1 deletion core/gesture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ export class Gesture {
// If the gesture already went through a bubble, don't set the start block.
if (!this.startBlock && !this.startBubble) {
this.startBlock = block;
common.setSelected(this.startBlock.getFirstNonShadowBlock());
common.setSelected(this.startBlock);
if (block.isInFlyout && block !== block.getRootBlock()) {
this.setTargetBlock(block.getRootBlock());
} else {
Expand Down
Loading