fix(mover): Pass drag delta to dragger.onDrag in pixels
#599
Conversation
cpcallen
force-pushed
the
fix/446
branch
2 times, most recently
from
0607f32 to
6ebb919
Compare
Change the signature of sendKeyAndWait: - Export it, so it can be used directly by tests. - Allow multiple keys to be specified. - Make times parameter optional (default 1).
This test currently fails due to issue RaspberryPiFoundation#446.
BenHenning
left a comment
BenHenning
left a comment
There was a problem hiding this comment.
Thanks @cpcallen! Seems like a really nice fix. Just had some nits and one core design consideration, but otherwise the PR LGTM.
| info.dragger.onDrag( | ||
| info.fakePointerEvent('pointermove', direction), | ||
| info.totalDelta, | ||
| info.totalDelta.clone().scale(workspace.scale), |
There was a problem hiding this comment.
This makes me wonder if we should have distinct types for different coordinate spaces, e.g. ScreenCoordinate and WorkspaceCoordinate. It's not obvious from code that scaling performs the space conversion, or what coordinate system we're currently in at this point (since if totalDelta is already in screen space then scaling again would be wrong).
There was a problem hiding this comment.
This would be a great idea; I've filed RaspberryPiFoundation/blockly#9156 for future consideration.
Relatedly, one of the things I somewhat dislike about TypeScript (and also Go) is duck typing: it should be possible to give different names to the same type specification and have the compiler flag any mixed use (absent explicit casts). For object types it's pretty easy to make things different shapes (adding a dummy method to the class definition changes the type without changing the actuals shape of the objects in memory), but a classic use of nominal typing is to distinguish between strings containing HTML and ones containing plain text, and enforce calling a function to do escaping when converting the latter to the former. TypeScript doesn't let you do that—you'd have to use subclasses of the String object type rather than plain strings.
2 participants
There was an error in the initial implementation of
DragMover(in PR #317), where the drag delta was being passed in workspace coordinates instead of pixels. This lead to various problems, notably #446.Fixes #446.