Skip to content
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: dragging and disposing of shadows #8172

Merged
merged 2 commits into from
May 28, 2024

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented May 22, 2024

The basics

The details

Resolves

Fixes #8171

Background

This handles shadow block dragging behavior in a very non-generalized but backwards compatible way. As far as I can think, there's no way for it to be both generalized and backwards compatible, so I chose backwards compatibility.

Our requirements are:

  1. We need shadows to be the thing being dragged, so that custom draggables can duplicate instead of dragging shadows.
  2. We need non-customized shadow blocks to drag their parent blocks.
  3. We need non-customized shadow blocks to dispose of their root blocks when they're dragged over a delete area.

Our constraints are:

  1. We can't change the signature of dispose in IDeletable because it would with the block dispose parameters.
  2. We can't change anything else about IDeletable either because because it would be breaking.
  3. We can't change what we expect to be returned from IDeleteAreas. It is not technically binary breaking, but changing what we expect could be behaviorally breaking, the worst kind of breaking.

So unless we want to break something, our only option is to special case for blocks for now, and reevaluate for V12.

Proposed changes

So what we're doing is passing the root block to any delete area callbacks, and we're also disposing of the root block. Any draggable-specific hooks are still called on the draggable itself, not the root.

If the draggable is not a block, we just always use the draggable.

Testing

Manually tested:

  • Dragging shadows
  • Dropping dragged shadows over delete areas
  • Setting a duplicate-on-drag drag strategy on a shadow blocks duplicates the shadow on drag

@github-actions github-actions bot added the PR: fix Fixes a bug label May 22, 2024
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 22, 2024
@BeksOmega BeksOmega marked this pull request as ready for review May 22, 2024 18:15
@BeksOmega BeksOmega requested a review from a team as a code owner May 22, 2024 18:15
@BeksOmega BeksOmega requested a review from NeilFraser May 22, 2024 18:15
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 22, 2024
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 28, 2024
@rachel-fenichel rachel-fenichel requested review from rachel-fenichel and removed request for NeilFraser May 28, 2024 17:20
@BeksOmega BeksOmega requested review from NeilFraser and removed request for NeilFraser May 28, 2024 17:20
Copy link
Collaborator

@rachel-fenichel rachel-fenichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why you don't like the conflict between backwards compatibility and generality, but this looks like a reasonable solution for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot apply custom drag strategy to shadow in v11
3 participants