dnd-kit icon indicating copy to clipboard operation
dnd-kit copied to clipboard

Add verticalSortableList collision detection strategy

Open ceddlyburge opened this issue 2 years ago • 20 comments

Fixes #804

Probably this will want a little demo or something added, if you are happy with it, maybe in Stories?

ceddlyburge avatar Jun 16 '22 16:06 ceddlyburge

⚠️ No Changeset found

Latest commit: cef8e5e0a80a9438002730427d6b0b987052ec28

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

changeset-bot[bot] avatar Jun 16 '22 16:06 changeset-bot[bot]

I spent a few hours trying to understand the weird behavior we were expecting here until I finally found this issue. I can confirm that this collision strategy makes vertical lists that don't use drag overlay work properly.

It would be really helpful to have this as a building collision strategy to avoid this boilerplate code everywhere.

marcospassos avatar Jan 02 '23 00:01 marcospassos

One thing to note is that it doesn't work with autoscrolling (scrolling the container while dragging). We have been using this code with the autoscrolling option turned off for a while now, and it's been working fine, so I think it's solid.

ceddlyburge avatar Jan 11 '23 09:01 ceddlyburge

any update on this one? could it possibly be related to an issue that looks like this dnd-kit-variable-height

TheMikeyRoss avatar Nov 08 '23 00:11 TheMikeyRoss

You could copy the code in to your project and see if it fixes the problem?

ceddlyburge avatar Nov 08 '23 09:11 ceddlyburge

Thanks @ceddlyburge for your reply,

May I ask how do I use verticalSortableListCollisionDetection in my code?

TheMikeyRoss avatar Nov 08 '23 17:11 TheMikeyRoss

Its surprisingly easy!

Just copy all the code in this PR somewhere, and then pass the function in to the DndContext.

<DndContext
      collisionDetection={verticalSortableListCollisionDetection}
      autoScroll={false}
      ...
>

ceddlyburge avatar Nov 08 '23 18:11 ceddlyburge

Awesome, thanks @ceddlyburge 👍

TheMikeyRoss avatar Nov 09 '23 01:11 TheMikeyRoss

@ceddlyburge It works wonderfully! I really appreciate it ♥ Here's a demo below 👇

Before

dnd-kit-variable-height-before

After

dnd-kit-variable-height-after

TheMikeyRoss avatar Nov 09 '23 03:11 TheMikeyRoss

No worries, glad you like it :) Also, nice demo :)

ceddlyburge avatar Nov 09 '23 09:11 ceddlyburge

@ceddlyburge Hello Cedd, I have a similar problem too. Actually, the only difference is that my structure is horizontal. I refactored your code according to horizontal style, but it did not work as I wanted. Can you help me too?

https://github.com/clauderic/dnd-kit/assets/42464623/3da72bf3-931f-4d01-9c67-2cea7fdf1a1e

import { CollisionDetection, DroppableContainer } from '@dnd-kit/core';
import { sortBy } from 'lodash';

export const horizontalSortableListCollisionDetection: CollisionDetection = (
  args
) => {
  if (args.collisionRect.left < (args.active.rect.current?.initial?.left ?? 0)) {
    return leftmostDroppableContainerMajorityCovered(args);
  } else {
    return rightmostDroppableContainerMajorityCovered(args);
  }
};

const leftmostDroppableContainerMajorityCovered: CollisionDetection = ({
  droppableContainers,
  collisionRect,
}) => {
  const ascendingDroppableContainers = sortBy(
    droppableContainers,
    (c) => c?.rect.current?.left
  );

  for (const droppableContainer of ascendingDroppableContainers) {
    const {
      rect: { current: droppableRect },
    } = droppableContainer;

    if (droppableRect) {
      const coveredPercentage =
        (droppableRect.left + droppableRect.width - collisionRect.left) /
        droppableRect.width;

      if (coveredPercentage > 0.5) {
        return [collision(droppableContainer)];
      }
    }
  }

  return [collision(ascendingDroppableContainers[0])];
};

const rightmostDroppableContainerMajorityCovered: CollisionDetection = ({
  droppableContainers,
  collisionRect,
}) => {
  const descendingDroppableContainers = sortBy(
    droppableContainers,
    (c) => c?.rect.current?.left
  ).reverse();

  for (const droppableContainer of descendingDroppableContainers) {
    const {
      rect: { current: droppableRect },
    } = droppableContainer;

    if (droppableRect) {
      const coveredPercentage =
        (collisionRect.right - droppableRect.left) / droppableRect.width;

      if (coveredPercentage > 0.5) {
        return [collision(droppableContainer)];
      }
    }
  }

  return [collision(descendingDroppableContainers[0])];
};

const collision = (droppableContainer?: DroppableContainer) => {
  return {
    id: droppableContainer?.id ?? '',
    value: droppableContainer,
  };
};

ozdemircibaris avatar Nov 23 '23 14:11 ozdemircibaris

Hi Baris, I'll take a look when I get some time. I can't remember all the details of this, but hopefully it will come back to me fairly quickly :)

ceddlyburge avatar Nov 23 '23 14:11 ceddlyburge

I am waiting for your return, thank you very much

ozdemircibaris avatar Nov 23 '23 14:11 ozdemircibaris

HI Baris, this looks like it should work to me, do you have some example code I can debug? Thanks, Cedd

ceddlyburge avatar Nov 24 '23 08:11 ceddlyburge

Hi Cedd, I will prepare it quickly and share the link Thanks

ozdemircibaris avatar Nov 24 '23 09:11 ozdemircibaris

Hey Cedd, Sorry for the late reply, I don't have any problems right now, my above code works.

ozdemircibaris avatar Nov 24 '23 15:11 ozdemircibaris

Ah great, glad to hear it! Maybe you could add the horizontalSortableListCollisionDetection to this PR?

ceddlyburge avatar Nov 24 '23 15:11 ceddlyburge

Yes of course!

ozdemircibaris avatar Nov 24 '23 15:11 ozdemircibaris

I've added horizontalSortableListCollisionDetection to this PR now, hopefully it will be useful for someone in the future.

ceddlyburge avatar Nov 28 '23 18:11 ceddlyburge

Amazon work @ceddlyburge 👍

@clauderic could this be merged? or is it gonna have some potential conflict with your planned refactoring mentioned [here] ?(https://github.com/clauderic/dnd-kit/issues/1194#issuecomment-1696704815)

TheMikeyRoss avatar Nov 28 '23 18:11 TheMikeyRoss