Incorrect drag position after state has been updated from sortable nested item

[parkderick]

Hi, I found a potential bug where the position of the draggable is positioned incorrectly after updating the state while a draggable is still in action. FYI I am using react-virtualized for the lists

In my case, I have lists sorted vertically and cards within each list sorted horizontally. When I drag a card from one list to another, I update the state of the items so that the new list has the card.

But this leads to unexpected incorrect draggable positions.

Not sure what I'm doing wrong. Any advice would be much appreciated! Thank you!

import _ from "lodash";
import { useState, useCallback, useMemo } from "react";
import { createPortal } from "react-dom";
import {
  DndContext,
  closestCorners,
  PointerSensor,
  useSensor,
  useSensors,
  useDroppable,
  DragOverlay
} from "@dnd-kit/core";
import {
  arrayMove,
  SortableContext,
  verticalListSortingStrategy,
  useSortable,
  rectSwappingStrategy
} from "@dnd-kit/sortable";
import {
  List,
  AutoSizer,
  CellMeasurer,
  CellMeasurerCache
} from "react-virtualized";
import { CSS } from "@dnd-kit/utilities";

const INITIAL_ACTIVE_DND_STATE = {
  activeId: null,
  activeContainer: null,
  type: null,
  prevOverContainer: null
};

const MOCK_ITEMS = [
  {
    id: "A",
    cards: ["A1", "A2", "A3", "A4", "A5", "A6"]
  },
  {
    id: "B",
    cards: ["B1", "B2"]
  },
  {
    id: "C",
    cards: ["C1", "C2", "C3", "C4"]
  },
  {
    id: "D",
    cards: ["D1", "D2"]
  },
  {
    id: "E",
    cards: ["E1", "E2"]
  },
  {
    id: "F",
    cards: ["F1", "F2"]
  },
  {
    id: "G",
    cards: ["G1", "G2", "G3", "G4"]
  },
  {
    id: "H",
    cards: ["H1", "H2"]
  },
  {
    id: "I",
    cards: ["I1", "I2"]
  },
  {
    id: "J",
    cards: ["J1", "J2"]
  },
  {
    id: "K",
    cards: ["K1", "K2"]
  },
  {
    id: "L",
    cards: ["L1", "L2"]
  },
  {
    id: "M",
    cards: ["M1", "M2", "M3", "M4"]
  },
  {
    id: "N",
    cards: ["N1", "N2"]
  },
  {
    id: "O",
    cards: ["O1", "O2"]
  },
  {
    id: "P",
    cards: ["P1", "P2"]
  }
];

const Card = (props) => {
  return (
    <div
      style={{
        width: "13rem",
        height: "6.2rem",
        backgroundColor: "green"
      }}
    >
      {props.card}
    </div>
  );
};

const DragOverlayList = (props) => {
  return (
    <div>
      <div>{props.list.id}</div>
      <div style={{ display: "flex", flexFlow: "row wrap" }}>
        {props.list.cards.map((card) => (
          <Card key={card} card={card} />
        ))}
      </div>
    </div>
  );
};

const SortableItem = (props) => {
  const {
    attributes,
    listeners,
    setNodeRef,
    transform = "",
    transition
  } = useSortable({ id: props.id });

  const style = {
    transform: CSS.Transform.toString(transform),
    transition
  };

  if (props.sortableType === "LIST") {
    return (
      <div ref={setNodeRef} style={style} {...listeners} {...attributes}>
        <FunctionalList {...props} />
      </div>
    );
  }

  if (props.sortableType === "CARD") {
    return (
      <div ref={setNodeRef} style={style} {...listeners} {...attributes}>
        <Card {...props} card={props.card} />
      </div>
    );
  }
};

const FunctionalList = (props) => {
  return (
    <div>
      <div>LIST - {props.list.id}</div>
      <div style={{ display: "flex", flexFlow: "row wrap" }}>
        <SortableContext
          id={`LIST-${props.list.id}`}
          items={props.list.cards}
          strategy={rectSwappingStrategy}
        >
          {props.list.cards.map((cardId) => (
            <SortableItem
              key={cardId}
              sortableType="CARD"
              id={cardId}
              card={cardId}
            />
          ))}
        </SortableContext>
      </div>
    </div>
  );
};

export default function App() {
  const [items, setItems] = useState(MOCK_ITEMS);
  const [clonedItems, setClonedItems] = useState([]);
  const [activeDnD, setActiveDnD] = useState({});

  const sensors = useSensors(
    useSensor(PointerSensor, {
      activationConstraint: {
        distance: 5
      }
    })
  );

  const onDragStart = (event) => {
    const containerId = event.active.data.current.sortable.containerId;
    const isDraggingList = containerId === "LISTS_SECTION";
    const activeContainerId = isDraggingList
      ? containerId
      : containerId.substring(5);
    setClonedItems(items);
    setActiveDnD({
      activeId: event.active.id,
      activeContainer: activeContainerId,
      type: isDraggingList ? "LIST" : "CARD"
    });
  };

  const onDragOver = (event) => {
    const { active, over } = event;

    // Only update item state when card drags out of one list to another
    if (!active?.data?.current?.sortable || !over?.data?.current?.sortable) {
      return;
    }
    const isDraggingList =
      active.data.current.sortable.containerId === "LISTS_SECTION";
    // activeDnD.prevOverContainer - If card has already been picked up and dragged over container/list, activeDnD.prevOverContainer is the containerId/listId
    const activeContainer =
      activeDnD.prevOverContainer ||
      active.data.current.sortable.containerId.substring(5);
    const overContainer = over.data.current.sortable.containerId.substring(5);
    const isCardDraggedWithinSameList = activeContainer === overContainer;

    if (isDraggingList || isCardDraggedWithinSameList) {
      return;
    }
    console.group('onDragOver')
    console.log('activeContainer',activeContainer)
    console.log('overContainer',overContainer)
    console.groupEnd()

    setItems((prev) => {
      const activeItems = _.find(prev, { id: activeContainer }).cards;
      const overItems = _.find(prev, { id: overContainer }).cards;

      const activeIndex = activeItems.indexOf(active.id);
      const overIndex = overItems.indexOf(over.id);

      const activeCard = activeItems[activeIndex];
      const result = prev.map((list) => {
        // Is listId the same as the activeContainerId where card has been dragged from
        // Is listId the same as the prevOverContainerId where card has previously been hovering over
        // Is overContainerId same as the prevOverContainerId
        const isListActiveContainer = list.id === activeContainer;
        const isListPreviousOverContainer =
          list.id === activeDnD.prevOverContainer;
        const isOverContainerSameAsThePreviousOverContainer =
          overContainer === activeDnD.prevOverContainer;
        if (
          isListActiveContainer ||
          (isListPreviousOverContainer &&
            !isOverContainerSameAsThePreviousOverContainer)
        ) {
          return {
            ...list,
            cards: list.cards.filter((cardId) => cardId !== active.id)
          };
        }

        const isListOverContainer = list.id === overContainer;
        if (isListOverContainer) {
          return {
            ...list,
            cards: [
              ...list.cards.slice(0, overIndex),
              activeCard,
              ...list.cards.slice(overIndex, list.cards.length)
            ]
          };
        }
        return list;
      });
      return result;
    });

    setActiveDnD((prev) => ({
      ...prev,
      prevOverContainer: overContainer
    }));
  };

  const onDragEnd = (event) => {
    const { active, over } = event;

    if (!over?.data?.current) {
      setItems(clonedItems);
      return;
    }

    const isList = activeDnD.activeContainer === "LISTS_SECTION";
    const isCard = activeDnD.activeContainer !== "LISTS_SECTION";

    if (isList) {
      setItems((prev) => {
        const currentIndex = _.findIndex(prev, { id: event.active.id });
        const overIndex = _.findIndex(prev, { id: event.over.id });
        return arrayMove(prev, currentIndex, overIndex);
      });
    }

    if (isCard) {
      setItems((prev) => {
        const activeContainer =
          activeDnD.prevOverContainer ||
          active.data.current.sortable.containerId.substring(5);
        const overContainer = over.data.current.sortable.containerId.substring(
          5
        );

        const activeItems = _.find(prev, { id: activeContainer }).cards;
        const overItems = _.find(prev, { id: overContainer }).cards;

        // Find the indexes for the items
        const activeIndex = activeItems.indexOf(active.id);
        const overIndex = overItems.indexOf(over.id);

        const activeCard = activeItems[activeIndex];
        const result = prev.map((list) => {
          // FOR CARD IN SAME LIST
          const isWithinSameList =
            activeContainer === list.id && overContainer === list.id;
          if (isWithinSameList) {
            return {
              ...list,
              cards: arrayMove(list.cards, activeIndex, overIndex)
            };
          }

          // Is listId the same as the activeContainerId where card has been dragged from
          // Is listId the same as the prevOverContainerId where card has previously been hovering over
          // Is overContainerId same as the prevOverContainerId
          const isListActiveContainer = list.id === activeContainer;
          const isListPreviousOverContainer =
            list.id === activeDnD.prevOverContainer;
          const isOverContainerSameAsThePreviousOverContainer =
            overContainer === activeDnD.prevOverContainer;
          if (
            isListActiveContainer ||
            (isListPreviousOverContainer &&
              !isOverContainerSameAsThePreviousOverContainer)
          ) {
            return {
              ...list,
              cards: list.cards.filter((cardId) => cardId !== active.id)
            };
          }

          const isListOverContainer = list.id === overContainer;
          if (isListOverContainer) {
            return {
              ...list,
              cards: [
                ...list.cards.slice(0, overIndex),
                activeCard,
                ...list.cards.slice(overIndex, list.cards.length)
              ]
            };
          }

          return list;
        });
        return result;
      });
    }
    setActiveDnD(INITIAL_ACTIVE_DND_STATE);
  };

  const sortableIds = useMemo(() => items.map((l) => l.id), [items]);

  const cache = useMemo(
    () =>
      new CellMeasurerCache({
        defaultWidth: 200,
        minWidth: 75,
        defaultHeight: 200,
        fixedWidth: true,
        keyMapper: (index) => (items[index] ? items[index].id : index)
      }),
    [items]
  );

  const handleRowRender = useCallback(
    ({ index: itemIndex, parent, style }) => {
      const _list = items[itemIndex];
      // We are rendering an extra item for the placeholder
      if (!_list) return null;

      return (
        <div style={style} key={_list.id}>
          <CellMeasurer
            cache={cache}
            parent={parent}
            rowIndex={itemIndex}
            columnIndex={0}
          >
            <SortableItem sortableType="LIST" id={_list.id} list={_list} />
          </CellMeasurer>
        </div>
      );
    },
    [items, cache]
  );

  const { setNodeRef } = useDroppable({
    id: "LIST_SECTION"
  });

  return (
    <div
      className="App"
      style={{
        width: "100vw",
        height: "100vh",
        backgroundColor: "pink"
      }}
    >
      <DndContext
        sensors={sensors}
        collisionDetection={closestCorners}
        onDragEnd={onDragEnd}
        onDragStart={onDragStart}
        onDragOver={onDragOver}
      >
        <SortableContext
          id={"LISTS_SECTION"}
          items={sortableIds}
          strategy={verticalListSortingStrategy}
        >
          <div
            ref={setNodeRef}
            style={{ width: "100%", height: "100%", flexGrow: 0 }}
          >
            <AutoSizer>
              {({ width, height }) => {
                return (
                  <List
                    role="list"
                    rowRenderer={handleRowRender}
                    height={height}
                    overscanRowCount={0}
                    deferredMeasurementCache={cache}
                    rowCount={items.length}
                    width={width}
                    rowHeight={cache.rowHeight}
                    estimatedRowSize={20}
                  />
                );
              }}
            </AutoSizer>
          </div>
        </SortableContext>
        {createPortal(
          <DragOverlay>
            {activeDnD.activeId
              ? (() => {
                  if (activeDnD.type === "LIST") {
                    const activeList = _.find(items, {
                      id: activeDnD.activeId
                    });
                    return <DragOverlayList list={activeList} />;
                  }
                  if (activeDnD.type === "CARD") {
                    const activeList =
                      _.find(items, {
                        id:
                          activeDnD.prevOverContainer ||
                          activeDnD.activeContainer
                      }) || {};
                    const activeCard = _.find(
                      activeList.cards,
                      (card) => card === activeDnD.activeId
                    );
                    return <Card card={activeCard} />;
                  }
                })()
              : null}
          </DragOverlay>,
          document.body
        )}
      </DndContext>
    </div>
  );
}

Screen.Recording.2021-11-29.at.12.14.35.PM.mov

[clauderic]

Can you please share a reproduction in something like CodeSandbox?

[parkderick]

@clauderic Hi, thank you for getting back to me!

Here's my codepen: https://codesandbox.io/s/react-virtualized-list-yq1h7

On codepen, it seems to work as expected, but in my project environment it doesn't work as expected. I compared all the dependency version to the codesandbox and they are all the same. Any advice on what I should be checking?

[MasterBroki]

@parkderick Hi Derick, I had the same issue and after investigation, it was caused by the items provided was always the same array. That caused the SortableContext to not be able to refresh the underlying list of items and provide the current index (then defaulting to -1)

Before:
<SortableContext items={items}

After
<SortableContext items={[...items]}

I don't think it's a bug, but it might be worth documenting this somewhere

[developernazari]

Thank you; your comment saved my time. That sure needs to be in the documentation.

I was using a property to sort an array of objects with the js sort() method, and this problem occurred.

It seems I was reordering only the presentation loop and did not update the items property of SortableContext. so it was always the same array.

The wrong code for me was:

<SortableContext
    items={pages.map(page => page.id)}
>
    {
        pages.sort((a, b) => a.order - b.order).map( page =>
            <Item
                 key={page.id} 
             />
        )
    }
</SortableContext>

I solved it by changing the items property in SortableContext like this:

<SortableContext
    items={pages.sort((a, b) => a.order - b.order).map(page=> page.id)}
>