react-sortablejs icon indicating copy to clipboard operation
react-sortablejs copied to clipboard

I cannot update the list using this.setState()

Open andrepadeti opened this issue 5 years ago • 3 comments

I'm trying to build a 'put the words in order' typical language learning activity. The user is supposed to start by dragging the first word of a sentence to its right place and move on from there. Meanwhile, I'm checking whether the next word is in the right place. If so, that word/item changes CSS class and becomes not draggable and of a different background colour.

For this CSS class change to happen, I clone the draggable list of words, make the necessary changes and then I setState() with the new updated list. That's when I'm having problems. The list and the component will update occasionally, seems to be at random. I must be doing something wrong when I try to update the list of words in the handleDragEnd() method. Here's part of the code:

export default class Sentence extends Component {
  constructor(props) {
    super (props)
  
    // prepare this.state.list to be used by SortableJS
    let list = this.props.sentenceShuffled.map((word, index) => {
      return { id: (++index).toString(), name: word, class: 'sortable' }
    })

    this.state = {
      currentCorrectWord: 0, // stores how many words are correct already while student is sorting
      complete: false, // becomes true when sentence is sorted
      list: list // this object is used by SortableJS and is calculated in componentWillMount
    }
  
    this.handleDragEnd = this.handleDragEnd.bind(this)
  }

  handleDragEnd() {
    let currentCorrectWord = this.state.currentCorrectWord
    
    while ( this.state.list[currentCorrectWord].name === this.props.sentenceInOrder[currentCorrectWord]) {
      let newList = _.cloneDeep(this.state.list)
      newList[currentCorrectWord].class = 'notSortable'
      currentCorrectWord++

      /*
         this is where (I think) the problem is: setSate won't update states nor re-render component.
      */

      this.setState({ currentCorrectWord: currentCorrectWord })
      this.setState({ list: newList })

      ... more code here
    }
  }

  render() {
    return (
      ...more code here
            <ReactSortable
              list={this.state.list}
              setList={newState => this.setState({ list: newState })}
              filter=".notSortable"
            >
              {this.state.list.map(item => (
                <MDBBox 
                    className={item.class + ' eachWord'} 
                    key={item.id} 
                    onDragEnd={this.handleDragEnd} 
                    >{item.name}
                </MDBBox>
              ))}
            </ReactSortable>
      ...more code here
    )
  }
}

What am I doing wrong?

andrepadeti avatar Sep 03 '20 05:09 andrepadeti

I was able to throw this together in a code sandbox. It does a pretty decent job as far as I can tell. Part of the problem is that even if an option in your list is disabled, it can't be dragged, but it can still be moved if you drag another item past it. If you mess around with onChange, you might be able to work something out there to prevent that from happening.

Also, I'm pretty sure onDragEnd should just be onEnd, and should be a prop of <ReactSortable> and not it's children.

https://codesandbox.io/s/sortable-js-bug-forked-k3ih1?file=/src/App.js

dagreatbrendino avatar Oct 05 '20 17:10 dagreatbrendino

Thanks for your reply!

If I use let testList = [...this.state.list]; instead of let newList = _.cloneDeep(this.state.list) (we're using different variable names. lestList is the same as newList), this.state.list updates immediately after I newList[currentCorrectWord].class = 'notSortable' (in your version of the code, that would be testList[index].class = "notSortable";), even before I this.setState({ list: newList }), which makes me wonder if let testList = [...this.state.list]; is not simply copying the pointer instead of cloning the whole object.

On the other hand, if I let newList = _.cloneDeep(this.state.list), then change one of the elements in the array with newList[currentCorrectWord].class = 'notSortable' and then this.setState({ list: newList }), this.state.list won't necessarily update the list.

andrepadeti avatar Oct 06 '20 02:10 andrepadeti

No problem! I just learned something new thanks your reply...

So, using the spread operator like I did does copy the array and not the pointer. However, apparently the objects contained in the array only have their references copied and not the objects themselves.

I updated my sandbox so now it maps over the array and uses the spread operator on the objects of the array into the temp copy. I tested to make sure it wasn't modifying the objects until the setState this time.

Thanks again for teaching me something new! I've got a lot of code to update now 😓 https://codesandbox.io/s/sortable-js-bug-forked-k3ih1?file=/src/App.js

dagreatbrendino avatar Oct 06 '20 18:10 dagreatbrendino