Db12797
Db12797

Reputation: 31

React to do list, how do i get this delete button to work?

I have a simple to do app that is working fine, except for the ability to delete items from the list. I have already added the button to each of the list items. I know I want to use the .filter() method to pass the state a new array that doesn't have the deleted to-do but I'm not sure how to do something like this.

Here is the App's main component:

class App extends Component {
  constructor(props){
    super(props);
    this.state = {
      todos: [
        { description: 'Walk the cat', isCompleted: true },
        { description: 'Throw the dishes away', isCompleted: false },
        { description: 'Buy new dishes', isCompleted: false }
      ],
      newTodoDescription: ''
    };
  }

  deleteTodo(e) {
    this.setState({ })
  }

  handleChange(e) {
    this.setState({ newTodoDescription: e.target.value })
  }

  handleSubmit(e) {
    e.preventDefault();
    if (!this.state.newTodoDescription) { return }
    const newTodo = { description: this.state.newTodoDescription, 
    isCompleted: false };
    this.setState({ todos: [...this.state.todos, newTodo], 
    newTodoDescription: '' });
  }

  toggleComplete(index) {
    const todos = this.state.todos.slice();
    const todo = todos[index];
    todo.isCompleted = todo.isCompleted ? false : true;
    this.setState({ todos: todos });
  }

  render() {
    return (
      <div className="App">
        <ul>
          { this.state.todos.map( (todo, index) =>
            <ToDo key={ index } description={ todo.description } 
              isCompleted={ todo.isCompleted } toggleComplete={ () => 
              this.toggleComplete(index) } />
          )}
        </ul>
        <form onSubmit={ (e) => this.handleSubmit(e) }>
          <input type="text" value={ this.state.newTodoDescription } 
            onChange={ (e) => this.handleChange(e) } />
          <input type="submit" />
        </form>

      </div>
    );
  }
}

And then here is the To-Do's component:

class ToDo extends Component {
  render() {
    return (
      <li>
        <input type="checkbox" checked={ this.props.isCompleted } 
          onChange={ this.props.toggleComplete } />
        <button>Destroy!</button>
        <span>{ this.props.description }</span>
      </li>
    );
  }
}

Upvotes: 1

Views: 800

Answers (2)

Hemerson Carlin
Hemerson Carlin

Reputation: 7424

Event handlers to the rescue:

You can send onDelete prop to each ToDo:

const Todo = ({ description, id, isCompleted, toggleComplete, onDelete }) => 
  <li>
    <input
      type="checkbox"
      checked={isCompleted} 
      onChange={toggleComplete}
    />
    <button onClick={() => onDelete(id)}>Destroy!</button>
    <span>{description}</span>
  </li>

And from App:

<ToDo 
  // other props here
  onDelete={this.deleteTodo}
/>

As pointed by @Dakota, using index as key while mapping through a list is not a good pattern.

Maybe just change your initialState and set an id to each one of them:

this.state = {
  todos: [
    { id: 1, description: 'Walk the cat', isCompleted: true },
    { id: 2, description: 'Throw the dishes away', isCompleted: false },
    { id: 3, description: 'Buy new dishes', isCompleted: false }
  ],
  newTodoDescription: '',
}

This also makes life easier to delete an item from the array:

deleteTodo(id) {
  this.setState((prevState) => ({
    items: prevState.items.filter(item => item.id !== id),
  }))
}

Upvotes: 2

Dakota
Dakota

Reputation: 1274

Before you get any further you should never use a list index as the key for your React Elements. Give your ToDo an id and use that as the key. Sometimes you can get away with this but when you are deleting things it will almost always cause issues.

https://medium.com/@robinpokorny/index-as-a-key-is-an-anti-pattern-e0349aece318

If you don't want to read the article, just know this

Let me explain, a key is the only thing React uses to identify DOM elements. What happens if you push an item to the list or remove something in the middle? If the key is same as before React assumes that the DOM element represents the same component as before. But that is no longer true.

On another note, add an onClick to your button and pass the function you want it to run as a prop from App.

<button onClick={() => this.props.handleClick(this.props.id)} />

and App.js

...

constructor(props) {
  ...
  this.handleClick = this.handleClick.bind(this);
}

handleClick(id) {
  // Do stuff
}

<ToDo
  ...
  handleClick={this.handleClick}
/>

Upvotes: 1

Related Questions