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

Multiple sortable items are skipped when a keyboard is used to sort vertical lists.

Open siarheikarabko opened this issue 3 years ago • 2 comments

It seems that the default sortableKeyboardCoordinates coordinateGetter for KeyboardSensor is not working properly when items in a vertical sortable list have different heights. The issue could be reproduced using this story. To reproduce: try to sort an item using a keyboard, especially an item with 'large' height from the bottom to the top.

https://user-images.githubusercontent.com/22076932/111517412-185b9080-8766-11eb-9238-9e4b91c000d3.mp4

Maybe I didn't get it right but I tried to debug the source code and think that getEdgeOffset should respect the fact that a draggable item might be translated using CSS (while sorting is active) and it results in differences between actual topOffset/leftOffset and what node.topOffset node.leftOffset return. I tried to update getEdgeOffset to respect the CSS translation but got stuck into the issues in finalTransform calculating in useSortable hook.

siarheikarabko avatar Mar 17 '21 18:03 siarheikarabko

You can fix this by writing a custom sortableKeyboardCoordinates method that is optimized for vertical lists, something along these lines...

import {closestCenter, KeyboardCode, rectIntersection} from '@dnd-kit/core';
import type {CollisionDetection, KeyboardCoordinateGetter} from '@dnd-kit/core';
import {arrayMove} from '@dnd-kit/sortable';

enum Direction {
  Down = 1,
  Up = -1,
}

const keycodes: string[] = [
  KeyboardCode.Down,
  KeyboardCode.Right,
  KeyboardCode.Up,
  KeyboardCode.Left,
];

export const collisionDetection: CollisionDetection = (entries, target) => {
  return rectIntersection(entries, target) ?? closestCenter(entries, target);
};

export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = (
  event,
  {context: {active, over, droppableContainers, translatedRect}},
) => {
  if (keycodes.includes(event.code)) {
    event.preventDefault();

    if (!translatedRect || !active || !active.data || !active.data.current) {
      return undefined;
    }

    const items: string[] = active.data.current.sortable.items;
    const overId = over?.id || active.id;
    const overIndex = items.indexOf(overId);
    const activeIndex = items.indexOf(active.id);
    let nextIndex = overIndex;
    let direction = Direction.Down;

    switch (event.code) {
      case KeyboardCode.Right:
      case KeyboardCode.Down:
        nextIndex = Math.min(overIndex + 1, items.length - 1);
        break;
      case KeyboardCode.Left:
      case KeyboardCode.Up:
        nextIndex = Math.max(0, overIndex - 1);
        direction = Direction.Up;
        break;
    }

    let nextId: string | null = null;

    if (overIndex !== nextIndex) {
      nextId = items[nextIndex];
    }

    if (nextId) {
      const activeNode = droppableContainers[active.id]?.node.current;
      const sortedItems = arrayMove(items, activeIndex, overIndex);
      const currentNodeIdAtNextIndex = sortedItems[nextIndex];
      const newNode =
        droppableContainers[currentNodeIdAtNextIndex]?.node.current;

      if (newNode && activeNode) {
        const activeRect = activeNode.getBoundingClientRect();
        const newRect = newNode.getBoundingClientRect();
        const offset =
          direction === Direction.Down
            ? newRect.top - activeRect.bottom
            : activeRect.top - newRect.bottom;
        const newCoordinates = {
          x: translatedRect.left,
          y: activeRect.top + direction * (newRect.height + offset),
        };

        return newCoordinates;
      }
    }
  }

  return undefined;
};

clauderic avatar Jun 30 '21 15:06 clauderic

@clauderic are there any plans to have the above workaround maintained as part of the library it seems generally quite useful, it fixed my use-case of re-ordering table rows. Prior to this Getter when using the keyboard pressing up or down wouldn't move a full row height, dropping this in fixed that. Just a bit of a shame to put all of this code in (without changes) straight into the codebase when it seems to be a useful out of the box util.

Totally understand if not though! that's what code comments with urls to this git issue is for

Georgegriff avatar Dec 01 '21 17:12 Georgegriff