fix: compute collision against DragOverlay child bounding rect

[wmain]

Fixes #401

This PR corrects what is, I believe, an improper assumption made in DnDContext. Defending this decision is most expeditiously done by providing an example of bad behavior on the master branch:

In this gif the black box with the text "1 item" is the first child of the DragOverlay, the blue box is the drag overlay itself, and the red box is the final computed collision rectangle. The drag overlay has the snapToCenter modifier applied, and has standard display: flex + centering styles applied to center the overlay child within the DragOverlay. The blue/red rectangle backgrounds exist purely for debugging/demonstration purposes and would not be visible to the end user. In practice, the user would visualize their cursor and the "1 item" element as the thing driving collision, but the current approach improperly (I believe) ignores the positioning of the overlay child and only adopts its width/height. If we remove the calculation of rect and just use draggingNodeRect the expected behavior occurs and the collision rectangle is positioned appropriately.

It's not immediately obvious to me why this logic existed in the first place, so I wholly expect that there's a case I might be missing/am not thinking of, but ultimately this feels like the correct behavior and the behavior I would expect as a developer making use of the library. I ultimately want collision to be measured against the bounding rect of the first child of DragOverlay. If I want that to be equivalent to the active node then all I have to do is not modify the position of the first child of the overlay and its bounding box will be the same.

As an aside, working through this has made me think that there might be long term value in providing some configuration option on DragOverlay(or a new component like PointerOverlay) that foregoes the relationship with the active node and instead just aligns with the cursor position. It would make this case a lot simpler to implement and wouldn't require the use of the snapToCenter modifier.

[changeset-bot]

🦋 Changeset detected

Latest commit: b253d36

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@dnd-kit/core	Minor
@dnd-kit/modifiers	Major
@dnd-kit/sortable	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[wmain]

@clauderic Are the e2e tests flaky historically? I've gotten a bunch of successful runs locally.

[clauderic]

The e2e tests are sometimes flaky but they seem to be consistently failing here. I vaguely remember attempting this change a while back and running into similar issues

[wmain]

Any chance you remember what you investigated there? Sort of unclear at first glance why this would make the results flaky between environments.

[clauderic]

Okay I found the problem after a bit of digging.

When dragging without a drag overlay, we need to compute a delta between the draggable node's initial position and new position if it is reparented.

This logic isn't relevant when using the DragOverlay. It was working before because we were only using the width and height of the drag overlay in the collision rect.

As an aside, working through this has made me think that there might be long term value in providing some configuration option on DragOverlay(or a new component like PointerOverlay) that foregoes the relationship with the active node and instead just aligns with the cursor position. It would make this case a lot simpler to implement and wouldn't require the use of the snapToCenter modifier.

That's an interesting idea. I would assume most consumers would still want most of the features of DragOverlay (drop animation for example), so I'd be a bit hesitant to create a brand new concept.

I'd lean towards extending DragOverlay to support this type of use-case.

[wmain]

@clauderic The additional commit actually breaks the functionality achieved without it for my case. I'm looking into why.

[wmain]

Adding back the willRecomputeLayouts flag to the useClientRect call for overlayNodeRect restores things to good working order. Will open a PR for discussion there as it's not obvious to me why it was removed.