React.js splice removing wrong item from array list

Question

I want to remove an item from an array getting the target.id, this app is for a cart, so in other words i want to remove an item from the cart when i click on the bin icon.

<span style={{fontSize:'50px'}}>Basket</span>
    <ul style={{listStyle: 'none'}}>
      {this.state.clickedNum.map((i) => <li key = 
      {this.props.gameData.id[i]} value={i}>
          {this.props.gameData.gameNames[i]} <br/>
          <img src={require('../assets/coins.png')} style={{width:'20px'}}/> 
          <strong> {this.props.gameData.prices[i]} Gil </strong>
          <img src={require('../assets/bin.png')} style={{width:'20px', justifyContent:' flex-end'}} id={this.props.gameData.id[i]}  onClick={this.removeClick}/>
          <br/>
          <hr style={{borderColor:'black'}} />
       </li>
     </ul>

This is the onClick={this.removeClick} function:

removeClick = (e) => {
    e = e.target.id;
    let clickedNum = this.state.clickedNum;
    let isDisabled = this.state.isDisabled;
    console.log("this is i" + e);

    clickedNum.splice(e, 1);
    isDisabled.splice(e,1);


    this.setState({
        clickedNum: clickedNum,
        isDisabled:isDisabled
    })

    this.forceUpdate();

}

remove click is binded like this in the constructor function:

    this.removeClick = this.removeClick.bind(this);

The problem comes that if you click the bin when there is more than one item in the cart it does not delete the first element and it does not always delete the correct item, however, if there is only one item in the cart it will delete the correct one.

I also note that :

console.log("this is i" + e);

returns the correct value (the id of button in which it was clicked on)

I'd strongly advise changing your overall approach to the component, storing the indexes of clicked items in an array and having another array paralleling it with a disabled flag isn't best practice. Instead, store an array of objects which refer to the relevant gameData entry and have a disabled flag. We can't really help you based only on what's in the question above, but you're breaking two big rules of React state management: 1. You're directly modifying state (by using splice on an array in state), and... — T.J. Crowder
– T.J. Crowder, Commented Sep 26, 2018 at 9:51
...2. You're setting state based on existing state, but not using the callback version of setState. State updates are asynchronous and can be batched, and so you can't do that. Use this.setState(prevState => { /*...*/ }) and use the information from prevState instead. More: reactjs.org/docs/… — T.J. Crowder
– T.J. Crowder, Commented Sep 26, 2018 at 9:51

Daniel Adepoju · Accepted Answer · 2018-09-26 09:58:51Z

1

I personally find it stressful to use splice. Why not use filter instead so you'll have something like this.state.clickedNum.filter(num => num.id !== e.target.id)

removeClick = (e) => {
  id = e.target.id;
  let clickedNum = this.state.clickedNum.filter(num => num.id !==id);
  let isDisabled = this.state.filter(num => num.id !==id;


this.setState({
    clickedNum: clickedNum,
    isDisabled:isDisabled
  })
}

edited Sep 26, 2018 at 9:58

answered Sep 26, 2018 at 9:46

Daniel Adepoju

8219 silver badges15 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

T.J. Crowder Over a year ago

All due respect, this is about a quarter of an answer to the question. The quarter you provide is fine (it's not just stressful, it's not allowed to directly modify state), but...

Daniel Adepoju Over a year ago

You wouldn't be modifying the state directly, as you'll assign the return value of the filter to a variable before setting it to your state.

T.J. Crowder Over a year ago

Right -- I was saying that using splice isn't just stressful, it's also wrong.

Collectives™ on Stack Overflow

React.js splice removing wrong item from array list

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related