fix: Don't repeatedly recreate the last marker block during a drag. #6875
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BeksOmega
approved these changes
Mar 1, 2023
core/insertion_marker_manager.ts
Outdated
Comment on lines
571
to
578
| if (this.firstMarker) { | ||
| eventUtils.disable(); | ||
| try { | ||
| this.firstMarker.dispose(); | ||
| } finally { | ||
| eventUtils.enable(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Oof, were we just eating memory before?
There was a problem hiding this comment.
I assume the GC would have cleaned up after us eventually since the marker would have become unreachable, but you introduced this logic in #4184 to fix an issue so I'm carrying it forward.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The basics
npm run formatandnpm run lintThe details
Resolves
Fixes #6839
Proposed Changes
#5722 introduced logic to update the available connections (and marker block) for a block being dragged as it was being dragged in order to make the marker block reflect changes to the connections of the block being dragged that happened during the drag. The insertion marker corresponding the first block in the stack being dragged was only regenerated when (a) it was shown and (b) its connections were inconsistent with those on the corresponding insertion marker block. However, the insertion marker corresponding to the last block in the stack was recreated every time the drag moved. This PR updates the insertion marker manager to only create the insertion marker block corresponding to the last block in the stack being dragged (a) at the beginning of the drag and (b) in the event of the same sort of mismatch during showing that would cause the insertion marker block corresponding to the first block in the stack being dragged to be regenerated.
Behavior Before Change
Numerous useless block creations.
Behavior After Change
Insertion marker blocks are only created on an as-needed basis.