Skip to content

Propagate the event group when enforcing bounds on blocks#2163

Merged
RoboErikG merged 1 commit intoRaspberryPiFoundation:developfrom
RoboErikG:roboerikg-moveBounds
Dec 12, 2018
Merged

Propagate the event group when enforcing bounds on blocks#2163
RoboErikG merged 1 commit intoRaspberryPiFoundation:developfrom
RoboErikG:roboerikg-moveBounds

Conversation

@RoboErikG
Copy link
Contributor

@RoboErikG RoboErikG commented Dec 12, 2018

Fixes #1653

Makes the bounds checker created in inject correctly set the event group
before moving blocks in bounds.

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

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 Reviewable

@rachel-fenichel
Copy link
Collaborator

Interesting. So you're changing the group on the event after it's been fired?

@RoboErikG
Copy link
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.

@rachel-fenichel
Copy link
Collaborator

I inverted the setGroup when I read it. Whoops.

@rachel-fenichel
Copy link
Collaborator

And if the previous event didn't have a group?

@RoboErikG
Copy link
Contributor Author

As written, then neither will the followup events. Would you like a warning log statement in that case?

@rachel-fenichel
Copy link
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.
@RoboErikG RoboErikG force-pushed the roboerikg-moveBounds branch from 13afca3 to 4aa2e50 Compare December 12, 2018 19:55
@RoboErikG RoboErikG merged commit e49d07d into RaspberryPiFoundation:develop Dec 12, 2018
@RoboErikG RoboErikG deleted the roboerikg-moveBounds branch October 8, 2019 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants