fix: Fix positioning of move marker on blocks#9722
Conversation
mikeharv
left a comment
mikeharv
left a comment
There was a problem hiding this comment.
A couple small suggestions, but this looks like a good solution!
| renderManagement.finishQueuedRenders().then(() => { | ||
| let bounds = this.draggable?.getBoundingRectangle(); | ||
| if ( | ||
| this.draggable && |
There was a problem hiding this comment.
Is it possible that this.draggable could change before the async promise resolves? It might be worth using a local variable to capture this before the async call to be sure we're looking at the right block.
There was a problem hiding this comment.
No, it's only mutable by starting a move, and you can't do that while a move is in progress.
There was a problem hiding this comment.
Oh right. Thanks for explaining.
| // as the starting block. | ||
| while ( | ||
| targetBlock?.getBoundingRectangleWithoutChildren().getOrigin().y !== | ||
| block.getBoundingRectangleWithoutChildren().getOrigin().y |
There was a problem hiding this comment.
nit: Could we cache this before the loop so we don't have to recalculate each time through the loop?
There was a problem hiding this comment.
We could also make a getY helper to possibly make this more readable.
There was a problem hiding this comment.
Split it out of the loop, which also improved the readability.
gonfunko
changed the title
2 participants
The basics
The details
Resolves
Fixes #9702
Proposed Changes
This PR resolves two issues with move indicator placement: