Home > Software engineering >  Why isn't the todo being deleted?
Why isn't the todo being deleted?

Time:12-22

I am working through a tutorial for a course I'm taking. The lab I'm working on walks through creating a to-do app. I'm on step 3, which asks us to create a button that deletes a task. I feel ridiculous, because I know I can figure it out but...well, I haven't yet! I will post the code to see if there are any initial issues, and then update with the methods I've already tried. Any help is greatly appreciated. Thank you!

import React, { useState } from "react";
import "./App.css";

const App = () => {
  const [todos, setTodos] = useState([]);
  const [todo, setTodo] = useState("");

  const handleSubmit = (e) => {
      e.preventDefault();

      const newTodo = {
          id: new Date().getTime(),
          text: todo.trim(),
          completed: false,
      };

      if (newTodo.text.length > 0) {

          setTodos([...todos].concat(newTodo));
          setTodo("");

      } else {

          alert("Enter Valid Task");
          setTodo("");
      }
  }

  const deleteTodo = (id) => {
      let updatedTodos = [...todos].filter((todo) => todo.id !== id);
      setTodos(updatedTodos);
  }

  const button = <button onClick={() => deleteTodo(todo.id)}>Delete</button>

return (
    <div>
        <h1>To-do List</h1>
            <form onSubmit={handleSubmit}>
                <input
                    type="text"
                    onChange={(e) => setTodo(e.target.value)}
                    placeholder="Add a new task"
                    value={todo}
                />
                <button type="submit">Add Todo</button>
            </form>
            {todos.map((todo) => <div>ID: {todo.id} Task: {todo.text} {button}</div>)}
    </div>
    );
};

export default App;

I didn't just copy and paste, so it's possible that I messed something up while typing. I'm expecting the deleteTodo() function to accept a todo.id and filter the list of todos, excluding the one I want to delete. I'm thinking that the issue may be cause by the way I've created the button? Again, I'm not sure why I can't figure it out. TIA.

CodePudding user response:

The button cannot access which todo it has I think you should put the code from the const button where you are referring to it or by changing it to const button = (todo) => <button onClick={ () => deleteTodo(todo.id); }>Delete</button> and access it by doing {button()}

CodePudding user response:

const button = <button onClick={() => deleteTodo(todo.id)}>Delete</button>

You're creating this button element just once, and the todo variable it refers to is the todo state, which is a string (usually an empty string). Since todo is a string, todo.id is undefined, and deleteTodo can't do anything with that.

You need to create separate buttons for each item, so you should move this code down into your .map:

{todos.map((todo) => (
  <div>
    ID: {todo.id} Task: {todo.text}
    <button onClick={() => deleteTodo(todo.id)}>Delete</button>
  </div>
))}

Now each item has its own button, with its own onClick function. And in those functions, todo is the item of the array.

CodePudding user response:

const button = <button onClick={() => deleteTodo(todo.id)}>Delete</button>

This has the same callBack for each todo, you should move this inside your map so that todo.id refers to the iterator of the map():

{todos.map((todo) => (
    <React.Fragment>
        <div>ID: {todo.id} Task: {todo.text}</div>
        <button onClick={() => deleteTodo(todo.id)}>Delete</button>
    </React.Fragment>
))}

Updated Demo:

const { useState } = React;

const App = () => {
  const [todos, setTodos] = useState([]);
  const [todo, setTodo] = useState("");

  const handleSubmit = (e) => {
      e.preventDefault();

      const newTodo = {
          id: new Date().getTime(),
          text: todo.trim(),
          completed: false,
      };

      if (newTodo.text.length > 0) {

          setTodos([...todos].concat(newTodo));
          setTodo("");

      } else {

          alert("Enter Valid Task");
          setTodo("");
      }
  }

  const deleteTodo = (id) => {
      let updatedTodos = [...todos].filter((todo) => todo.id !== id);
      setTodos(updatedTodos);
  }


    return (
        <div>
            <h1>To-do List</h1>
                <form onSubmit={handleSubmit}>
                    <input
                        type="text"
                        onChange={(e) => setTodo(e.target.value)}
                        placeholder="Add a new task"
                        value={todo}
                    />
                    <button type="submit">Add Todo</button>
                </form>
                {todos.map((todo) => (
                    <React.Fragment>
                        <div>ID: {todo.id} Task: {todo.text}</div>
                        <button onClick={() => deleteTodo(todo.id)}>Delete</button>
                    </React.Fragment>
                ))}
        </div>
    );
};

ReactDOM.render(<App />, document.getElementById("react"));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/17.0.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/17.0.1/umd/react-dom.production.min.js"></script>
<div id="react"></div>

CodePudding user response:

Try this:

const button = (t) => <button onClick={() => deleteTodo(t.id)}>Delete</button>

and then, in the map

{todos.map((todo) => <div>ID: {todo.id} Task: {todo.text} {button(todo)}</div>)}

this way, the "delete todo" button will be bound to the specific todo ID, avoiding being bound to whatever the current value of todo is in the app.

CodePudding user response:

  1. Its better to update the state using setState. Muting the state directly breaks the primary principle of React's data flow (which is made to be unidirectional), making your app very fragile and basically ignoring the whole component lifecycle.
  2. Also You need to change the delete from string to function and pass the id or place the jsx directly inside map function.
 import React, { useState } from 'react'

const App = () => {
  const [todos, setTodos] = useState([])
  const [todo, setTodo] = useState('')

  const handleSubmit = e => {
    e.preventDefault()

    const newTodo = {
      id: new Date().getTime(),
      text: todo.trim(),
      completed: false,
    }

    if (newTodo.text.length > 0) {
      setTodos([...todos].concat(newTodo))
      setTodo('')
    } else {
      alert('Enter Valid Task')
      setTodo('')
    }
  }

   /*
   * Changed Here
   */
  const deleteTodo = id => {
  
    setTodos(prevState => {
      return prevState.filter(todo => todo?.id != id)
    })
  }

  const button = id => <button onClick={() => 
  deleteTodo(id)}>Delete</button>

  return (
    <div>
      <h1>To-do List</h1>
      <form onSubmit={handleSubmit}>
        <input
          type="text"
          onChange={e => setTodo(e.target.value)}
          placeholder="Add a new task"
          value={todo}
        />
        <button type="submit">Add Todo</button>
      </form>
      {todos.map(todo => (
        <div key={todo.id}>
          ID: {todo.id} Task: {todo.text} {button(todo.id)}
        </div>
      ))}
    </div>
  )
}

export default App
  • Related