Philip R
Philip R

Reputation: 21

React - deleting array elements stored in state deletes wrong element

I've been beating my head against this for long enough that I'm not sure I'd even see an obvious problem.

The App has rows, and each row has cells of varying sizes. Adding rows works. Adding cells works. Deleting rows however, does not. If I click the delete button for row 0, row 2 disappears. If I click the button for row 1, row 2 disappears. Looking at the react dev tools, it appears that under the hood the right row is being deleted (in state), the right row is selected for deletion, but no matter what it doesn't render the way I would expect (if I hit delete for row 0, row 0 disappears, for example).

If I delete from the highest row down it works, but if I start at the zeroth row it won't let itself be deleted.

deleteRowButtonHandler of the App class is where I've been focusing my efforts.

deleteRowButtonHandler( e )
        {
            const targetRow = parseInt( e.target.id ) 
console.log( "TARGETING: ", targetRow )

            const currentRows = this.state.rows

            let newArray = []
            currentRows.forEach( entry =>
            {
                console.log( "Looking at ", entry.id )
                if( entry.id == targetRow )
                    console.log( 'skipping entry with id', entry.id )
                else
                    newArray.push( entry )
            })

            console.log( newArray )
            this.setState( {rows: newArray})
            //let newRows = currentRows.filter( thisRow => {console.log( thisRow.id + ' and the target ' +  targetRow);
             //   if( thisRow.id == targetRow )
             //   {
             //       console.log( thisRow )
             //       console.log( '...is the row that we are trying to delete' )
            //    }

            //    return thisRow.id !== targetRow} 
            //) 
            //this.setState( {rows: newArray}, console.log( "NEW STATE:", this.state ))
        }

I've tried a few variations (filter, splice, doing it in a few lines, doing it in a lot of lines, etc) so I'm not particularly proud of that chunk of code at the moment.

https://codepen.io/philipvdg/pen/mdmmWLG is the code; any pointers in the right direction would be greatly appreciated.

Update Speaking generally, the problem was that I (being a newbie) didn't understand that the constructor was only run once. In the constructor of a child component I had this.cells = this.props.cells and then, in the render method, called this.cells.map( ... ). Since the constructor only ever ran once, this.cells was never updated after a state change.

Upvotes: 1

Views: 351

Answers (2)

Aleksandr Smyshliaev
Aleksandr Smyshliaev

Reputation: 420

It get duplicate ids somehow.

Looking at  3
    pen.js:74 Looking at  4
    pen.js:81 (4) [{…}, {…}, {…}, {…}]
    pen.js:225 TEMPLATE PROPS:  {rows: Array(4)}rows: Array(4)0: {cells: Array(0), id: 0}1: {cells: Array(1), id: 3}2: {cells: Array(0), id: 3}3: {cells: Array(0), id: 4}length: 4__proto__: Array(0)__proto__: Object
    pen.js:232 row 0 is {cells: Array(0), id: 0}
    pen.js:232 row 1 is {cells: Array(1), id: 3}
    pen.js:232 row 2 is {cells: Array(0), id: 3}
    pen.js:232 row 3 is {cells: Array(0), id: 4}

// in newRowButtonHandler
this.setState( { rows: [ ...this.state.rows, {cells:[], id: this.state.rows.length} ] } )

// try this
    this.setState( { rows: [ ...this.state.rows, {cells:[], id: Math.round(Math.random() * 1000000)} ] }) )

UPD Problem with id's, render and delete function use different set of values. https://codepen.io/Asmyshlyaev177/pen/yLbXBGo?editors=0010

Should not use e.target.id, should be single source of truth e.g. react state.

Upvotes: 1

Konstantin Modin
Konstantin Modin

Reputation: 1333

In Controls render you should add line const id = row.id; so it will be like this:

this.props.rows.map( 
   ( row ) => {
     const id = row.id; 
    let rowId = 'controlRow' + id 

Using array idx is wrong. You should use idx from data structure.

Upvotes: 1

Related Questions