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.

Sign up for GitHub

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

feat: have the gesture use a dragger for blocks #7972

Merged
merged 4 commits into from
Mar 29, 2024
Merged

feat: have the gesture use a dragger for blocks #7972

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41f9a94
feat: have block use drag strategy
BeksOmega Mar 27, 2024
f2b86b5
fix: gesture to use dragger for blocks
BeksOmega Mar 28, 2024
e2b4f41
chore: register dragger
BeksOmega Mar 28, 2024
8ff7cc8
chore: remove getInsertionMarkers and pull logic into workspace
BeksOmega Mar 29, 2024
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
58 changes: 26 additions & 32 deletions core/gesture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ import type {IBlockDragger} from './interfaces/i_block_dragger.js';
import type {IBubble} from './interfaces/i_bubble.js';
import type {IFlyout} from './interfaces/i_flyout.js';
import * as internalConstants from './internal_constants.js';
import * as registry from './registry.js';
import * as Tooltip from './tooltip.js';
import * as Touch from './touch.js';
import {Coordinate} from './utils/coordinate.js';
import {WorkspaceCommentSvg} from './workspace_comment_svg.js';
import {WorkspaceDragger} from './workspace_dragger.js';
import type {WorkspaceSvg} from './workspace_svg.js';
import type {IIcon} from './interfaces/i_icon.js';
import {IDragger} from './interfaces/i_dragger.js';
import * as registry from './registry.js';

/**
* Note: In this file "start" refers to pointerdown
Expand Down Expand Up @@ -116,8 +117,7 @@ export class Gesture {
/** The object tracking a bubble drag, or null if none is in progress. */
private bubbleDragger: BubbleDragger | null = null;

/** The object tracking a block drag, or null if none is in progress. */
private blockDragger: IBlockDragger | null = null;
private dragger: IDragger | null = null;

/**
* The object tracking a workspace or flyout workspace drag, or null if none
Expand Down Expand Up @@ -212,9 +212,6 @@ export class Gesture {
}
this.boundEvents.length = 0;

if (this.blockDragger) {
this.blockDragger.dispose();
}
if (this.workspaceDragger) {
this.workspaceDragger.dispose();
}
Expand All @@ -230,7 +227,7 @@ export class Gesture {
const changed = this.updateDragDelta(currentXY);
// Exceeded the drag radius for the first time.
if (changed) {
this.updateIsDragging();
this.updateIsDragging(e);
Touch.longStop();
}
this.mostRecentEvent = e;
Expand Down Expand Up @@ -334,17 +331,17 @@ export class Gesture {
*
* @returns True if a block is being dragged.
*/
private updateIsDraggingBlock(): boolean {
private updateIsDraggingBlock(e: PointerEvent): boolean {
if (!this.targetBlock) {
return false;
}
if (this.flyout) {
if (this.updateIsDraggingFromFlyout()) {
this.startDraggingBlock();
this.startDraggingBlock(e);
return true;
}
} else if (this.targetBlock.isMovable()) {
this.startDraggingBlock();
this.startDraggingBlock(e);
return true;
}
return false;
Expand Down Expand Up @@ -384,7 +381,7 @@ export class Gesture {
* the drag radius is exceeded. It should be called no more than once per
* gesture.
*/
private updateIsDragging() {
private updateIsDragging(e: PointerEvent) {
// Sanity check.
if (this.calledUpdateIsDragging) {
throw Error('updateIsDragging_ should only be called once per gesture.');
Expand All @@ -397,28 +394,26 @@ export class Gesture {
return;
}
// Then check if it was a block drag.
if (this.updateIsDraggingBlock()) {
if (this.updateIsDraggingBlock(e)) {
return;
}
// Then check if it's a workspace drag.
this.updateIsDraggingWorkspace();
}

/** Create a block dragger and start dragging the selected block. */
private startDraggingBlock() {
const BlockDraggerClass = registry.getClassFromOptions(
registry.Type.BLOCK_DRAGGER,
private startDraggingBlock(e: PointerEvent) {
this.dragging = true;

const DraggerClass = registry.getClassFromOptions(
registry.Type.DRAGGER,
this.creatorWorkspace.options,
true,
);

this.dragging = true;
this.blockDragger = new BlockDraggerClass!(
this.targetBlock,
this.startWorkspace_,
);
this.blockDragger!.startDrag(this.currentDragDeltaXY, this.healStack);
this.blockDragger!.drag(this.mostRecentEvent, this.currentDragDeltaXY);
this.dragger = new DraggerClass!(this.targetBlock!, this.startWorkspace_!);
this.dragger.onDragStart(e);
this.dragger.onDrag(e, this.currentDragDeltaXY);
}

/** Create a bubble dragger and start dragging the selected bubble. */
Expand Down Expand Up @@ -587,8 +582,8 @@ export class Gesture {
this.updateFromEvent(e);
if (this.workspaceDragger) {
this.workspaceDragger.drag(this.currentDragDeltaXY);
} else if (this.blockDragger) {
this.blockDragger.drag(this.mostRecentEvent, this.currentDragDeltaXY);
} else if (this.dragger) {
this.dragger.onDrag(this.mostRecentEvent, this.currentDragDeltaXY);
} else if (this.bubbleDragger) {
this.bubbleDragger.dragBubble(
this.mostRecentEvent,
Expand Down Expand Up @@ -631,8 +626,8 @@ export class Gesture {
// not matter, because the three types of dragging are exclusive.
if (this.bubbleDragger) {
this.bubbleDragger.endBubbleDrag(e, this.currentDragDeltaXY);
} else if (this.blockDragger) {
this.blockDragger.endDrag(e, this.currentDragDeltaXY);
} else if (this.dragger) {
this.dragger.onDragEnd(e, this.currentDragDeltaXY);
} else if (this.workspaceDragger) {
this.workspaceDragger.endDrag(this.currentDragDeltaXY);
} else if (this.isBubbleClick()) {
Expand Down Expand Up @@ -797,8 +792,8 @@ export class Gesture {
this.mostRecentEvent,
this.currentDragDeltaXY,
);
} else if (this.blockDragger) {
this.blockDragger.endDrag(this.mostRecentEvent, this.currentDragDeltaXY);
} else if (this.dragger) {
this.dragger.onDragEnd(this.mostRecentEvent, this.currentDragDeltaXY);
} else if (this.workspaceDragger) {
this.workspaceDragger.endDrag(this.currentDragDeltaXY);
}
Expand Down Expand Up @@ -1235,9 +1230,6 @@ export class Gesture {
* @internal
*/
getInsertionMarkers(): BlockSvg[] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm probably missing context, but: why does this function continue to exist but always return an empty list? Will you re-implement it later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mmm I left this like this because I wasn't quite sure what to do with it, since the draggable doesn't have APIs for getting insertion markers, cuz that's super specific to blocks. But I think I can reimplements this by just walking the workspace and checking for any blocks that are flagged as insertion markers. I'll check where this is getting called from and whether that's going to be a performance killer or not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So it turns out that getInsertionMarkers is only called from workspace.render, so I just moved the relevant logic into there. Performance impact shouldn't be an issue, because we don't actually call that method. And if people are calling it and run into problems, I can give them more performant ways of triggering rerenders.

if (this.blockDragger) {
return this.blockDragger.getInsertionMarkers();
}
return [];
}

Expand All @@ -1249,7 +1241,9 @@ export class Gesture {
* progress.
*/
getCurrentDragger(): WorkspaceDragger | BubbleDragger | IBlockDragger | null {
return this.blockDragger ?? this.workspaceDragger ?? this.bubbleDragger;
// TODO: Change this to return the `dragger`, when we get rid of the last
// other dragger.
return this.workspaceDragger ?? this.bubbleDragger;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions core/registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {ToolboxItem} from './toolbox/toolbox_item.js';
import type {IPaster} from './interfaces/i_paster.js';
import type {ICopyData, ICopyable} from './interfaces/i_copyable.js';
import type {IConnectionPreviewer} from './interfaces/i_connection_previewer.js';
import type {IDragger} from './interfaces/i_dragger.js';

/**
* A map of maps. With the keys being the type and name of the class we are
Expand Down Expand Up @@ -97,6 +98,8 @@ export class Type<_T> {

static BLOCK_DRAGGER = new Type<IBlockDragger>('blockDragger');

static DRAGGER = new Type<IDragger>('dragger');

/** @internal */
static SERIALIZER = new Type<ISerializer>('serializer');

Expand Down