-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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.
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: snap immediately #7745
fix: snap immediately #7745
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I should ultimately only need to review the second of the two commits in this PR, correct?
if (this.workspace.isDragging()) { | ||
return; // Don't bump blocks during a drag. | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to remove this check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah there's no reason this should be called while the workspace is dragging. It only gets called when we click a block in the flyout to create it, or when we finish dragging a block.
c9610ca
to
c80bf82
Compare
The basics
The details
Resolves
Work on #5146 #4288
Proposed Changes
Makes it so that snapping to the grid happens immediately instead of after a delay.
Reason for Changes
I am working on refactoring how block initialization works. As part of it, I need to have
bumpNeighbors
be triggered by the render management system, instead of being triggered by each individual change that might change the shape of the block.But before
bumpNeighbors
gets called, I need to make sure the snap has happened. So I'm getting rid of the delay.Test Coverage
Manually tested that snapping works properly, and looks correct when dropping after dragging.
Documentation
N/A
Additional Information
Dependent on #7743