Propagate the event group when enforcing bounds on blocks#2163
Merged
RoboErikG merged 1 commit intoRaspberryPiFoundation:developfrom Dec 12, 2018
Merged
Conversation
371a88f to
13afca3
Compare
Collaborator
|
Interesting. So you're changing the group on the event after it's been fired? |
Contributor
Author
|
No? I'm setting the group for the followup move events that are triggered by the event. This is the same way the bumping gets done. |
Collaborator
|
I inverted the setGroup when I read it. Whoops. |
Collaborator
|
And if the previous event didn't have a group? |
Contributor
Author
|
As written, then neither will the followup events. Would you like a warning log statement in that case? |
Collaborator
|
Yes, that would be good. After that lgtm. |
Fixes RaspberryPiFoundation#1653 Makes the bounds checker created in inject correctly set the event group before moving blocks in bounds. When a workspace can't scroll the inject method sets up an event listener to bump blocks back into the visible bounds when dropped off the edge. The event group wasn't being used, though, causing the move to be a separate undo step. When undone, this would get you stuck in a loop where the workspace would keep pulling it back into the visible range.
13afca3 to
4aa2e50
Compare
Merged
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
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.
Fixes #1653
Makes the bounds checker created in inject correctly set the event group
before moving blocks in bounds.
The basics
The details
Resolves
#1653
Proposed Changes
Propagate the event group in the bounds checker callback.
Reason for Changes
When a workspace can't scroll the inject method sets up an event listener
to bump blocks back into the visible bounds when dropped off the edge. The
event group wasn't being used, though, causing the move to be a separate
undo step. When undone, this would get you stuck in a loop where the
workspace would keep pulling it back into the visible range.
Test Coverage
Tested on:
Desktop Chrome
Additional Information
This change is