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

fix: Don't repeatedly recreate the last marker block during a drag. #6875

Merged
merged 2 commits into from
Mar 1, 2023
Merged

fix: Don't repeatedly recreate the last marker block during a drag. #6875

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db47884
fix: Don't repeatedly recreate the last marker block during a drag.
gonfunko Mar 1, 2023
f8cd695
refactor: Reorganize insertion marker initialization and disposal.
gonfunko Mar 1, 2023
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
61 changes: 34 additions & 27 deletions core/insertion_marker_manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ export class InsertionMarkerManager {
this.firstMarker = this.createMarkerBlock(this.topBlock);

this.availableConnections = this.initAvailableConnections();

if (this.lastOnStack) {
this.lastMarker =
this.createMarkerBlock(this.lastOnStack.getSourceBlock());
}
}

/**
Expand All @@ -142,18 +147,8 @@ export class InsertionMarkerManager {
*/
dispose() {
this.availableConnections.length = 0;

eventUtils.disable();
try {
if (this.firstMarker) {
this.firstMarker.dispose();
}
if (this.lastMarker) {
this.lastMarker.dispose();
}
} finally {
eventUtils.enable();
}
this.disposeInsertionMarker(this.firstMarker);
this.disposeInsertionMarker(this.lastMarker);
}

/**
Expand Down Expand Up @@ -289,10 +284,8 @@ export class InsertionMarkerManager {
}

/**
* Populate the list of available connections on this block stack. This
* should only be called once, at the beginning of a drag. If the stack has
* more than one block, this function will populate lastOnStack and create
* the corresponding insertion marker.
* Populate the list of available connections on this block stack. If the
* stack has more than one block, this function will also update lastOnStack.
*
* @returns A list of available connections.
*/
Expand All @@ -303,15 +296,6 @@ export class InsertionMarkerManager {
if (lastOnStack && lastOnStack !== this.topBlock.nextConnection) {
available.push(lastOnStack);
this.lastOnStack = lastOnStack;
if (this.lastMarker) {
eventUtils.disable();
try {
this.lastMarker.dispose();
} finally {
eventUtils.enable();
}
}
this.lastMarker = this.createMarkerBlock(lastOnStack.getSourceBlock());
}
return available;
}
Expand Down Expand Up @@ -561,8 +545,17 @@ export class InsertionMarkerManager {
// probably recreate the marker block (e.g. in getCandidate_), which is
// called more often during the drag, but creating a block that often
// might be too slow, so we only do it if necessary.
this.firstMarker = this.createMarkerBlock(this.topBlock);
insertionMarker = isLastInStack ? this.lastMarker : this.firstMarker;
if (isLastInStack && this.lastOnStack) {
this.disposeInsertionMarker(this.lastMarker);
this.lastMarker =
this.createMarkerBlock(this.lastOnStack.getSourceBlock());
insertionMarker = this.lastMarker;
} else {
this.disposeInsertionMarker(this.firstMarker);
this.firstMarker = this.createMarkerBlock(this.topBlock);
insertionMarker = this.firstMarker;
}

if (!insertionMarker) {
throw new Error(
'Cannot show the insertion marker because there is no insertion ' +
Expand Down Expand Up @@ -728,6 +721,20 @@ export class InsertionMarkerManager {
}
return result;
}

/**
* Safely disposes of an insertion marker.
*/
private disposeInsertionMarker(marker: BlockSvg|null) {
if (marker) {
eventUtils.disable();
try {
marker.dispose();
} finally {
eventUtils.enable();
}
}
}
}

export namespace InsertionMarkerManager {
Expand Down