16

I'm doing a simple todo app with React, just to practise. How can I delete a list item, when clicking on it?

Here is my todos.js

export default class Todos extends Component {
    constructor(props) {
        super(props);
        this.state = { todos: [], text: '' };
    }

    addTodo(e) {
        e.preventDefault();
        this.setState({ todos: [ this.state.text, ...this.state.todos ] });
        this.setState({ text: ''});
    }

    updateValue(e) {
        this.setState({ text: [e.target.value]})
    }

    render() {
        return(
            <div>
                <form onSubmit = {(e) => this.addTodo(e)}>
                    <input
                        placeholder="Add Todo"
                        value={this.state.text}
                        onChange={(e) => {this.updateValue(e)}}
                    />
                    <button type="submit">Add Todo</button>
                </form>
                <TodoList todos={this.state.todos}/>
            </div>
        );
    }
}

And here is the TodoList.js, where I'm trying to remove a list item from.

import React, { Component } from 'react';
import { connect } from 'react-redux';

export default class TodoList extends Component {
    removeItem(e) {
        // splice this.props.todos??
    }
    render() {
        return(
            <ul>
                { this.props.todos.map((todo) => {
                    return <li onClick={(e) => { this.removeItem(e)}} key={todo}>{ todo }</li>
                })}
            </ul>
        );
    }
}
userden
  • 1,615
  • 6
  • 26
  • 50

7 Answers7

22

To delete the todo items, first pass a function from parent component:

<TodoList todos={this.state.todos} removeTodo={this.removeTodo}/>

Bind this function in the constructor:

this.removeTodo = this.removeTodo.bind(this);

Define this function in parent component, it will delete that item from state variable:

removeTodo(name){
    this.setState({
        todo: this.state.todo.filter(el => el !== name)
    })
}

Then inside child component call this method to delete todo:

export default class TodoList extends Component {
    removeItem(e) {
        this.props.removeTodo(item);
    }
    render() {
        return(
            <ul>
                { this.props.todos.map((todo) => {
                    return <li onClick={() => { this.removeItem(todo)}} key={todo}>{ todo }</li>
                })}
            </ul>
        );
    }
}

Suggestion:

Don't call setState multiple time within a function if you want to set multiple state values then write it like this:

this.setState({
    a: value1,
    b: value2,
    c: value3
})

Working example:

class Todos extends React.Component {
    constructor(props) {
        super(props);
        this.state = { todos: [], text: '' };
        this.removeTodo = this.removeTodo.bind(this);
    }

    addTodo(e) {
        e.preventDefault();
        this.setState({ 
         todos: [ this.state.text, ...this.state.todos ],
         text: ''
        });
    }

    removeTodo(name, i){
        let todos = this.state.todos.slice();
        todos.splice(i, 1);
        this.setState({
            todos
        });
    }

    updateValue(e) {
        this.setState({ text: e.target.value})
    }

    render() {
        return(
            <div>
                <form onSubmit = {(e) => this.addTodo(e)}>
                    <input
                        placeholder="Add Todo"
                        value={this.state.text}
                        onChange={(e) => {this.updateValue(e)}}
                    />
                    <button type="submit">Add Todo</button>
                </form>
                <TodoList todos={this.state.todos} removeTodo={this.removeTodo}/>
            </div>
        );
    }
}

class TodoList extends React.Component {

    removeItem(item, i) {
        this.props.removeTodo(item, i);
    }

    render() {
        return(
            <ul>
                { this.props.todos.map((todo,i) => {
                    return <li onClick={() => { this.removeItem(todo, i)}} key={i}>{ todo }</li>
                })}
            </ul>
        );
    }
}

ReactDOM.render(<Todos/>, document.getElementById('app'))
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.min.js"></script>

<div id='app'/>

Update:

This is for @whs.bsmith doubt, the code that i suggested will work properly in the case where user will add the unique items in the todo list, if he will try to add the same item it will not reflect in ui because OP is using the todo items name as the key and key should be unique.

To solve that issue:

In working snippet i used indexes in place of todo items name for key, that will work properly and it will allow the user to add same item multiple times and on deletion, it will delete only that specific item not all the item having that same name, But it's not a good idea to use indexes as the key.

Mayank Shukla
  • 100,735
  • 18
  • 158
  • 142
  • 1
    this won't work in all cases because you may have to identical todos, and this kind of filter will always remove them both – whs.bsmith Jun 29 '17 at 18:09
  • @whs.bsmith it will work unless he's having and ID or unique key per todo. and it will be filtered by that unique key – Nicholas Jun 29 '17 at 18:11
  • actually, in your example, you can't put identical todos in the first place – whs.bsmith Jun 29 '17 at 18:13
  • @whs.bsmith check the updated answer, it was not allowing to add the same item because OP was using the name of todo as the key, now by using index we can add multiple todos of same name, and it will delete specific item also but its not a good idea to use the **index as the key**. – Mayank Shukla Jun 29 '17 at 18:18
  • OK, but I think originally you were using the todo itself to do the filtering – whs.bsmith Jun 29 '17 at 18:22
  • and your code in the snippet is different than the code in the answer – whs.bsmith Jun 29 '17 at 18:43
  • @whs.bsmith check the update section it will clear all your doubt :) – Mayank Shukla Jun 29 '17 at 18:51
  • I'm just saying your answer should match your snippet. But, I agree, it's not good to use indexes for something like this. Better to use an id. – whs.bsmith Jun 29 '17 at 18:54
3

You have called setState two times in the addTodo function. You can set todos and text in a single setState function like this:

addTodo(e) {
    e.preventDefault();
    this.setState({ todos: [ this.state.text, ...this.state.todos ], text: '' });
}

Do not write removeItem function in TodoList Component as it is purely working on props. Pass a removeItem function from Todos to it and remove that item in Todos's removeItem function like this:

import React, {Component} from 'react'
export default class Todos extends Component {
  constructor(props) {
    super(props);
    this.state = { todos: [], text: '' };
    this.removeItem = this.removeItem.bind(this)
  }

  addTodo(e) {
    e.preventDefault();
    this.setState({ todos: [ this.state.text, ...this.state.todos ], text: '' });
  }


  updateValue(e) {
    this.setState({ text: [e.target.value]})
  }
  removeItem(index) {
    const todos = this.state.todos.filter((todo, todoIndex) => {
      return todoIndex !== index
    })
    this.setState({ todos })
  }
  render() {
    return(
      <div>
        <form onSubmit = {(e) => this.addTodo(e)}>
          <input
            placeholder="Add Todo"
            value={this.state.text}
            onChange={(e) => {this.updateValue(e)}}
            />
          <button type="submit">Add Todo</button>
        </form>
        <TodoList todos={this.state.todos} removeItem={this.removeItem} />
      </div>
    );
  }
}

class TodoList extends Component {
  render() {
    return(
      <ul>
        { this.props.todos.map((todo, index) => {
          return <li onClick={(e) => { this.props.removeItem(index)}} key={todo}>{ todo }</li>
        })}
      </ul>
    );
  }
}

Hope, it helps.

Ritesh Bansal
  • 3,108
  • 2
  • 16
  • 23
1
class TodoList extend React.Component{
    constructor(props){
        super(props);
        this.state = {
            todos: [],
            todo: ''
        }
        this.changeTodo = this.changeTodo.bind(this);
        this.addTodo = this.addTodo.bind(this);
        this.removeTodo = this.removeTodo.bind(this);
    }

    changeTodo(event){
        this.setState({
            todo: event.target.value
        })
    }

    addTodo(){
        let { todo, todos } = this.state;
        this.setState({
            todo: '',
            todos: [...todos, todo]
        })
    }

    removeTodo(index){
        let { todos } = this.state;
        todos.splice(index, 1);
        this.setState({
            todos: todos
        })
    }

    render(){
        let { todo, todos } = this.state;
        return <div>
            <input value={todo} onChange={this.changeTodo}/>
            <button onClick={this.addTodo}>Add Todo</button>
            {
                todos.map((todo, index)=>{
                    return <h1 onClick={this.removeTodo.bind(undefined, index)} key={index}>{todo}</h1>
                })
            }
        </div>
    }
}

This is a small example for TodoList. Go through this code to understand remove todo in your TodoList app.

Vasi
  • 1,147
  • 9
  • 16
0

Only thing to do is to move todos to state in TodoList and pass the index of the current todo in to the removeItem method. Then splice it as you suggested.

In TodoList:

constructor(props) {
    super(props);

    this.state = { todos: props.todos };
}

removeItem(index) {
    this.setState({
        todos: this.state.todos.filter((_, i) => i !== index)
    });
}

render() {
    return(
        <ul>
            { this.state.todos.map((todo, index) => {
                return <li onClick={(e) => { this.removeItem(index)}} key={todo}>{ todo }</li>
            })}
        </ul>
    );
}

Remove item courtesy of Removing element from array in component state

woodpav
  • 1,917
  • 2
  • 13
  • 26
0

First, you would want the removeItem method to exist in the Todos class, since that is where that state is stored. Then, you can either use filter or slice

export default class Todos extends Component {
    constructor(props) {
        super(props);
        this.state = { todos: [], text: '' };
    }

    addTodo(e) {
        e.preventDefault();
        this.setState({ todos: [ this.state.text, ...this.state.todos ] });
        this.setState({ text: ''});
    }

    updateValue(e) {
        this.setState({ text: [e.target.value]})
    }

    removeItem = index => {
        //either use filter
        const { todos } = this.state;
        const newTodos = todos.filter((todo, i) => i !== index);
        this.setState({ todos: newTodos});

        //or use slice
        const slicedNewTodos = todos.slice(0, index).concat(todos.slice(index + 1));
        this.setState({ todos: slicedNewTodos});
    }

    render() {
        return(
            <div>
                <form onSubmit = {(e) => this.addTodo(e)}>
                    <input
                        placeholder="Add Todo"
                        value={this.state.text}
                        onChange={(e) => {this.updateValue(e)}}
                    />
                    <button type="submit">Add Todo</button>
                </form>
                <TodoList removeItem={this.removeItem} todos={this.state.todos}/>
            </div>
        );
    }
}

import React, { Component } from 'react';
import { connect } from 'react-redux';

class TodoList extends Component {
    render() {
        return(
            <ul>
                { this.props.todos.map((todo, index) => {
                    return <li onClick={() => this.props.removeItem(index)} key={todo}>{ todo }</li>
                })}
            </ul>
        );
    }
}
whs.bsmith
  • 386
  • 1
  • 2
  • 12
0

Everyone has given a good big picture overview. I find this to be a good way to delete the todo from the todolist

todolist.splice(index, 1);

Splice removes 1 element from the startIndex of index (in the todolist array)

Rahul Makhija
  • 381
  • 4
  • 7
0

We can set a flag for the todo data and on initializing that data we can set the flag as true and when we delete the todo list we can set the flag as false. Please refer the below code.

const TodoComponent = ({ dataArray }) => {

  const [todos, setTodos] = useState(dataArray)

  // initial loading of todo items.
  useEffect(() =>{
    if (dataArray.length) {
      dataArray.forEach(todo =>{
        todo.show = true
      })
      setTodos(dataArray)
    }
  }, [dataArray])

 // Close the todo Item.
  const closeTodoItem = (todo, index) =>{
    todo.show = false
    todos[index] = todo
    setTodos([...todos])
  }

  return (
    todos.filter(todo => todo.show === true).length >0  &&
    (
      <Col span={24} className={style.todoBg} style={compStyle}>
        {todos.map((todo,index) => (
          (todo.show && (
            <Col span={24} className={style.todo}>
              <Col className={style.todoInner}>
                <p> {todo.content} </p>
              </Col>
              <Icon onClick={() => closeTodoItem(todo, index)} className={style.close}>close</Icon>
            </Col>
          ))
        ))}
      </Col>
    )
  )}

The above approach worked for me.

Senthuran
  • 1,583
  • 2
  • 15
  • 19