React-sortable-hoc: Uncaught TypeError: Cannot read property 'top' of undefined(…)

[sahilchaddha]

Initally Dragging seems fine, i am re sorting my array and re setting the state on onSortEnd.
After 1-2 drags, I am able to drag single element but all other elements in list is stuck.Other Elements does not scroll.
onSortEnd: function (newProps) { var oldGroups = this.state.groups; oldGroups.move(newProps.oldIndex, newProps.newIndex); this.setState({groups: oldGroups}); },

[clauderic]

Hmm, I'd need to see more of your code. Would you mind sharing the full code snippet or setting up a jsfiddle?

[sahilchaddha]

const SortableItem = SortableElement(({value,clickEvent,selected}) => {
  return (
      <ListCard selectedGroup={selected} clickEvent={clickEvent} data={value}/>
  );
});

const SortableList = SortableContainer(({items,onClick,selectedGroup}) => {
    return (
        <ul className="padding-none">
            {items.map((value, index) =>
                <SortableItem selected={selectedGroup} clickEvent={onClick} key={`item-${index}`} index={index} value={value} />
            )}
        </ul>
    );
});

var MessengerList = React.createClass({
    onSortEnd: function(oldIndex) {
      this.props.dragEnd(oldIndex);
    },
    render: function () {
        return (
            <div className="chat-list-head-wrapper user-list-heading messenger-user-list-left-panel" id="user-list-wrapper">
              <UserListHeading currentState={this.props.currentState} stateChange={this.props.stateChange} />
                <div className="user-list-messenger">
                <ul className="user-list col-xs-12 padding-none" id="user-list">
                  <SortableList axis='y' lockToContainerEdges={true} selectedGroup={this.props.selectedGroup} onClick={this.userClicked} items={this.props.groups} onSortEnd={this.onSortEnd} pressDelay={500} />
                </ul>
                </div>
            </div>
        );
    }
});

Note : All the elements in the same list are different.Some cards are similar
Dragging similar type elements does not create any problem but dragging different types of elements breaks the List.
All elements have same height but different content,css and sub elements

Initially i am able to drag drop and scroll easily.
Steps to reproduce
1.Drag and drop an element from middle of the list to below elements.
2.Try dragging second last element to top, it is draggable but the other list does not sort/scroll

[clauderic]

Hey @sahilchaddha,

Thanks for providing some code for me to look at and help you debug this. At first look, everything looks fine in terms of setup.

I believe the issue might be with your this.props.dragEnd(oldIndex) method. Would you be able to share that implementation as well?

[TimHollies]

I've encountered the same error - it looks like the 'refs' in the manager class end up pointing at unmounted elements. I've been attempting to create a JSfiddle that reproduces it, but have thus far been unsuccessful. The common thing I can see between @sahilchaddha's code and mine is that the list is stored in state in a level above the component so that both the data and the 'onSortEnd' function are passed as props. I'll keep trying on the JSFiddle.

[TimHollies]

I still haven't reproduced it in JSFiddle - but I think I've found the source of the bug and the solution, in my project at least.

The issue was that components wrapped by SortableElement were being re-rendered but the wrapping SortableElement component was not. This meant that the componentWillUnmount event on SortableElement was not firing so the refs in the manager class were not being updated, resulting in the refs pointing at DOM nodes that had been removed by the re-render.

The fix was to set the key prop of the SortableElement component to a string dependent on the information displayed within the wrapped component. This means that if the data changes such that the wrapped component needs to re-render, the wrapping SortableElement will have it's key changed causing it to re-render as well which will trigger the lifecycle events that will keep the manager class refs in sync.

[sahilchaddha]

@TimHollies Thanks a ton, Solved my problem.

[jamespero]

@TimHollies Would you be able to provide a code example. I'm having the same issue and I don't understand your solution.

[TimHollies]

So we've got a wrapped SortableElement:

const Card = SortableElement(MyElement);

And an array containing the data for each card which we want to map over to generate the list. React will complain if the sibling items in the list do not have a unique key set - the solution I usually use for this is to use the array index to build the key:

{cards.map((card, index) => {
  return (
    <Card
      key={`card-${index}`}
      data={card}
    />
  );
})}

However, that doesn't work in this case. Sometimes the array index will stay the same, but the component needs to be re-rendered for react-sortable-hoc to work. If the data prop changes the wrapped component is re-rendered, but the wrapping component is not!

That's what causes the weird behavior, all of the components whose index changed work fine - the ones where the index stayed the same no longer work. You can check out this behavior in React-Devtools.

The solution I've been using is to make it so that the key is dependent on the data in card. This will force react to re-render the wrapping component whenever the data changes, rather than when it's index changes:

{cards.map((card) => {
  return (
    <Card
      key={`card-${card.title}-${card.content}`}
      data={card}
     />
   );
})}

I hope that helps :)

[mmela]

Great, thanks @TimHollies! I had the same issue with SortableElement state being swapped between the dragged elements. Changing the key to an element specific value instead of using index did the trick.

[p4bloch]

Thank's for taking the time of documenting this @TimHollies. Made my day.

[erisnuts]

I think timestamp would be better
key: ${index}-${Math.floor(Date.now() / 1000)}

[clauderic]

That's not something I would recommend @erisnuts. This will needlessly cause many nodes to unmount and re-mount. You should always use predictable, consistent keys where possible.

https://facebook.github.io/react/docs/reconciliation.html#tradeoffs

[kloseliebej]

I have a totally opposite situation, when I use index as the key they all work fine, but after I changing to use item-related content as key they stuck in the list after initial move. No idea why is that.

Edit: Problem solved. Must use something that changes during the sorting process as the key.
And if you use the index as the key, that's another problem, every wrapped element is new when sorting, so their states won't persist.

[jacksonhardy]

I still haven't reproduced it in JSFiddle - but I think I've found the source of the bug and the solution, in my project at least.

The issue was that components wrapped by SortableElement were being re-rendered but the wrapping SortableElement component was not. This meant that the componentWillUnmount event on SortableElement was not firing so the refs in the manager class were not being updated, resulting in the refs pointing at DOM nodes that had been removed by the re-render.

The fix was to set the key prop of the SortableElement component to a string dependent on the information displayed within the wrapped component. This means that if the data changes such that the wrapped component needs to re-render, the wrapping SortableElement will have it's key changed causing it to re-render as well which will trigger the lifecycle events that will keep the manager class refs in sync.

^^^ that ^^^

I was having the same issue and this just solved it perfectly - thank you!

[amirjanyanIT]

I have same issue in my library, i tried all that was said at the top but i can't solve that issue

https://github.com/amirjanyanIT/react-form-designer

if someone can help me, please check index.js in src folder

[katy6514]

hello! I also am having the same issue and the above suggestions did not help, still getting the original error

It seems like things go wrong when I delete an item, then the UI and the data are out of sync, so it sounds like @TimHollies suggestion above about the wrapped components being re-rendered but not the wrapping component, however I wasn't able to get the suggestion to work

Any help or guidance sincerely appreciated!

const SortableItem = SortableElement(({ item, itemIndex, ...rest }) => (
  <ListItem
    key={item.itemId}
    useDragHandle
    item={item}
    index={itemIndex}
    onClickCheckBox={() => rest.onClickCheckBox(itemIndex)}
    onItemTextUpdate={value =>
      rest.onItemTextUpdate(value, item.itemId, itemIndex)
    }
    onDelete={() => rest.onDelete(item.itemId)}
  >
    {item.text}
  </ListItem>
))

const SortableList = SortableContainer(({ items, ...rest }) => (
  <StyledItemList>
    {items.map((item, index) => (
      <SortableItem
        key={`item-${item.itemId}-${item.text}`}
        itemIndex={index}
        index={index}
        item={item}
        {...rest}
      />
    ))}
  </StyledItemList>
))

class MobileListItems extends Component {
  state = {
    items: this.props.listItems,
  }

  onSortEnd = ({ oldIndex, newIndex }) => {
    this.setState(({ items }) => ({
      items: arrayMove(items, oldIndex, newIndex),
    }))
  }

  render() {
    const { onClickCheckBox, onDelete, onItemTextUpdate } = this.props

    return (
      <SortableList
        onClickCheckBox={index => onClickCheckBox(index)}
        onDelete={onDelete}
        onItemTextUpdate={onItemTextUpdate}
        items={this.state.items}
        onSortEnd={this.onSortEnd}
      />
    )
  }
}