Dynamically add/remove row not working as expected

Question

I would like to do a form that could add/remove row by clicking add/remove buttons. My problems now is that, the remove button can always just remove the last row, instead of the row that the remove button located. When I used the array delete operator, it can delete the desired row. Yet, this method did not really delete the object in the array and probably hard for me to do the checking in the for loop. I wonder where did I do wrong?

Updated: 1. Added jsfiddle https://jsfiddle.net/69z2wepo/84246/ 2. Save the dropdown value to parent's state

constructor(props){
  super(props)
  this.state = {
     dropdown: [
         {first: "true", hideAdd: "false", result: ""}
     ]
  }
  this.addRow = this.addRow.bind(this);
  this.removeRow = this.removeRow.bind(this);
  this.onChange = this.onChange.bind(this);
}

onChange = (key, value) => {
    let oldArray = JSON.parse(JSON.stringify(this.state.dropdown));
    oldArray[key]['result'] = value;
    this.setState({dropdown:oldArray}, function(){
        console.log(this.state.dropdown);
        //console.log(this.state.dropdown.length)
    })
}

removeRow = (e, key) => {
    e.preventDefault();

    let oldArray = JSON.parse(JSON.stringify(this.state.dropdown));

    oldArray.slice();
    oldArray.splice(key, 1);

    for ( let i=0; i<oldArray.length; i++){

        if (i==0){
            oldArray[i]['first'] = 'true';
        }

        if (oldArray.length==1){
            oldArray[i]['hideAdd'] = 'false';
        }else{
            oldArray[i]['hideAdd'] = 'true';
            if ( i == oldArray.length-1){
                oldArray[i]['hideAdd'] = 'false';
            }   
        }

    }

    this.setState({dropdown:oldArray}, function(){
        console.log(this.state.dropdown);
        //console.log(oldArray);
        //console.log(oldArray.length);
    })

}
render() {
        return (
            <Grid.Column>
                <Grid style={comStyles().gridWidth}>
                    <Grid.Row>
                        <Grid.Column width={4} textAlign='left' style={comStyles().lineHeight}>Then</Grid.Column>
                        <Grid.Column width={12}>
                            {
                            this.state.dropdown.map((item, index) => (
                               <RulesThenDropdown default={item.result} onChange={this.onChange} add={this.addRow} remove={this.removeRow} id={index} key={index} hideAdd={item.hideAdd} first={item.first} />
                            ))
                        }
                        </Grid.Column>
                    </Grid.Row>
                </Grid>
            </Grid.Column>
        )
    }

And the followings are the code from the child component

render() {
    const Options = thenOptions;

    let extra = "";

    const removeBtn = <button onClick={(e,m)=>this.props.remove(e,this.props.id)} className="circular ui icon button"><i className="icon minus"></i></button>  
    const addBtn = <button onClick={(e,m)=>this.props.add(e,this.props.id)} className="circular ui icon button"><i className="icon plus"></i></button>

    if(this.props.first==="false"){
        if(this.props.hideAdd=="true"){
            extra = removeBtn;
        }else{
            extra = <div>{addBtn}{removeBtn}</div>;
        }
    }else if(this.props.first==="true"){
        if(this.props.hideAdd!="true"){
            extra = addBtn;
        }else{
            extra = removeBtn;
        }
    }

    return (
        <div style={comStyles().buttonsMainWrapper}>
            <Form.Dropdown 
                placeholder='Then' 
                fluid 
                selection 
                options={Options} 
                defaultValue = {this.props.result}
                onChange={(e,{ value })=>this.props.onChange(this.props.id, value)}
            />
            <div style={comStyles().buttonsGroup}>
            {
                extra
            }
            </div>
        </div>
    )
}

In the end, the problem is this line `oldArray.splice(key, 1);` ? Have you tried to `console.log` and check `oldArray` value before and after ? — An Nguyen, Aug 09 '17 at 07:05
The result of `oldArray` between `splice` and `delete` are the same? What if using `delete` and clean your `oldArray` after that by `oldArray = oldArray.filter(n => n)` — An Nguyen, Aug 09 '17 at 07:24
Updated the post and it helps to remove the undefined one, but still removing the last row — HUNG, Aug 09 '17 at 10:19
What do you mean by `When I used the array delete operator, it can delete the desired row.` ? — An Nguyen, Aug 09 '17 at 10:25
for example, there are five rows and there is a remove button in each row respectively. If i click the third row's remove button, it is expected that the third row will be removed, but now, the last row was removed. — HUNG, Aug 09 '17 at 10:42
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/151481/discussion-between-firice-nguyen-and-hung). — An Nguyen, Aug 09 '17 at 10:43

score 1 · Accepted Answer · answered Aug 09 '17 at 07:26

1

The problem is you're modifying the components state directly instead of updating immutable copies with this.setState().

The culprit is this line:

let oldArray = this.state.dropdown;

which doesn't copy the state but rather obtains a reference, so now both oldArray and this.state.dropdown are both pointing to the same structure in memory.

When you subsequently slice, splice and update oldArray you're breaking the React component's contract regarding state mutability (see https://facebook.github.io/react/docs/state-and-lifecycle.html#do-not-modify-state-directly ).

To fix this, you'll need to deep clone this.state.dropdown like so:

let oldArray = JSON.parse(JSON.stringify(this.state.dropdown))

(see https://stackoverflow.com/a/5344074/501217 for details)

answered Aug 09 '17 at 07:26

Kim Burgaard

3,508
18
11

unfortunately the behaviour is still the same :( – HUNG Aug 09 '17 at 07:39
You also need to assign the result of oldArray.slice() and oldArray.splice(). Unlike array.push and array.pop, they don't side-effect on the array, but rather return a new array. Since you already deep copied oldArray, you can skip the slice statement and change `oldArray.splice(...)` to `oldArray = oldArray.splice(...)` – Kim Burgaard Aug 09 '17 at 07:49
I am sorry, i am still a bit confused. let oldArray = JSON.parse(JSON.stringify(this.state.dropdown)); I firstly do this, then you mean the oldArray here is already deep copied then I can oldArray.splice(key, 1) and continue the for loop for updating some value and finally setState? coz now still the same, i am not sure if i misunderstand any of the steps – HUNG Aug 09 '17 at 10:04
`oldArray = JSON.parse(JSON.stringify(this.state.dropdown))` creates a deep copy of `this.state.dropdown`. When you subsequently make changes to `oldArray`, these changes are not visible in `this.state.dropdown` until you call `this.setState({dropdown: oldArray})` – Kim Burgaard Aug 10 '17 at 02:48
I added the jsfiddle. Seems i was doing what you mentioned but not sure if the problem not because of this.. – HUNG Aug 10 '17 at 05:51

score 1 · Answer 2 · answered Aug 09 '17 at 10:28

1

I would like to add more to @Kim Burgaard answer, that Immutable List is a good replacement for Array in state

dropdown: List([{first: "true", hideAdd: "false", id: -1}])

answered Aug 09 '17 at 10:28

An Nguyen

1,487
10
21

Dynamically add/remove row not working as expected

2 Answers2