fix: Fixes #8764 by moving the event grouping calls up to dragger.ts #8781
Conversation
This file contains hidden or 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
RoboErikG
changed the title
…lls up to dragger.ts
RoboErikG
force-pushed
the
undo-groups
branch
from
ce2f776 to
983f1d9
Compare
Contributor
Author
|
The failing test is the flakey connection test which was fixed in develop but hasn't been merged into V12 yet. |
gonfunko
approved these changes
Feb 20, 2025
RoboErikG
merged commit b343a13
into
RaspberryPiFoundation:rc/v12.0.0
7 of 8 checks passed
RoboErikG
added
the
breaking change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
The details
Resolves
Fixes #8764
Proposed Changes
Moves the event group management up to dragger.ts instead of being handled in the various *_drag_strategy.ts files. Conceptually, this makes it the responsibility of the top level dragger to manage event groups and ensures that the group wraps all side effects of the drag.
In vanilla Blockly this shouldn't have any effect, but is necessary for the multi-select plugin used by AppInventor.
Also fixes a bug where snapping to the grid adds a move event that isn't part of the group.
Reason for Changes
AppInventor has a multi-select plugin that creates multiple BlockDragStrategy instances. This requires that endDrag be called on each one, which was causing the event group to be reset after the first one. Moving it up a level ensures all of the block drags finish before the event group is reset.
Test Coverage
Screen.recording.2025-02-19.12.39.26.PM.webm
Documentation
No documentation changes, but we should include it in the release notes that the behavior has changed and anyone overriding dragger.ts should make this change if they aren't calling through to the super.
Additional Information