-
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
feat: have the gesture use a dragger for blocks #7972
feat: have the gesture use a dragger for blocks #7972
Conversation
72b1c24
to
e2b4f41
Compare
core/gesture.ts
Outdated
@@ -1235,9 +1230,6 @@ export class Gesture { | |||
* @internal | |||
*/ | |||
getInsertionMarkers(): BlockSvg[] { |
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.
I'm probably missing context, but: why does this function continue to exist but always return an empty list? Will you re-implement it later?
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.
Mmm I left this like this because I wasn't quite sure what to do with it, since the draggable doesn't have APIs for getting insertion markers, cuz that's super specific to blocks. But I think I can reimplements this by just walking the workspace and checking for any blocks that are flagged as insertion markers. I'll check where this is getting called from and whether that's going to be a performance killer or not.
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.
So it turns out that getInsertionMarkers
is only called from workspace.render
, so I just moved the relevant logic into there. Performance impact shouldn't be an issue, because we don't actually call that method. And if people are calling it and run into problems, I can give them more performant ways of triggering rerenders.
The basics
The details
Resolves
Work on #7844
Proposed Changes
Upgrades the gesture class to use a
IDragger
for dragging blocks instead of using aIBlockDragger
. This conforms to our new design for handling dragging.Test Coverage
Manually tested with all of the previous PRs, and everything seems to wire together correctly! Once this is in I can hammer it more.
Documentation
N/A
Additional Information
As I write the other
IDraggable
s I'll migrate the gesture to pass those to the dragger instead of using special draggers. (i.e. theworkspaceDragger
andbubbleDragger
)