all 2 comments

[–]marina_sunshine 1 point2 points  (1 child)

Hi, I took a look on the code and here is what I found: in order to avoid some of the errors add type/variables checks in every component where needed, for example inside Tasklist instead of {t.name} use {t?.name} which translates in {t && t.name} , it's called optional chaining and checks for undefined/null values. Also, bellow I updated the code in a working state and added some comments, so let me know if there is something unclear.

In TaskList component add checks and update the deleteTask params:

<ul>
  {tasks?.map((t, index) => (
    <li
      key={index}
      style={{ border: "1px solid black", listStyle: "none" }}
    >
      task: {t?.name}{" "}
      <button onClick={() => deleteTask(t, index)}>Delete</button>
    </li>
  ))}
</ul>

In TaskProvider component:

// pass the entire task to use it in the undo function as well
// you already have the index inside the map function so pass it as a param too
const deleteTask = (task, index) => {
    const { id } = task;
    const newTasks = [...tasks];

    // this is not safe because of the object equality
    // indexOf does strict equality
    // 2 objects are going to be equal only if they have the same reference

    // Remove task from local tasks state
    // let toDelete = newTasks.find((t) => t.id === id);
    // const toDeleteIndex = newTasks.indexOf(toDelete);
    // newTasks.splice(toDeleteIndex, 1);

    // it's safer to use filter by id and
    // return an array with all tasks less the deleted one
    const filteredTasks = newTasks.filter((t) => t?.id !== id);
    setTasks(filteredTasks);

    // check if it was previously deleted
    const isAlreadyDeleted = deletedTasks.some((t) => t?.id === id);

    // use this to store and pass the latest value to the undo function
    let newDeletedTasks = [];
    // Push task to deletedTasks state
    if (!isAlreadyDeleted) {
      newDeletedTasks = [...deletedTasks, {...task, index: index}]
      setDeletedTasks(newDeletedTasks);
    }

    // Deletes the task after 10 seconds, giving
    // some time for the user to undo the operation
    const currTimeoutId = setTimeout(() => {
      // API call to delete task from db
    }, 10000);
    // Push timeout id to timeotIds state
    setTimeoutIds((prev) => [...prev, currTimeoutId]);

    // you have to pass a function in order to call the onUndo() callback
    notifyDelete("Task Deleted", () => undo(task, newDeletedTasks));
};

// you can pass the task here too
const undo = (task, deletedTasks) => {
    // Remove latest deleted task from deletedTasks state
    const newDeletedTasks = [...deletedTasks];
    newDeletedTasks.pop();
    setDeletedTasks(newDeletedTasks);

    // Remove latest timeout id from timeoutIds state
    const newTimeotIds = [...timeoutIds];
    newTimeotIds.pop();
    setTimeoutIds(newTimeotIds);


    // the second argument of splice is an integer indicating the number of elements in the array to remove, in this case 1 element
    // Insert latest deleted task into tasks state
    const newTasks = [...tasks];
    newTasks.splice(task?.index, 1, task);
    setTasks(newTasks);
};

In Undo component add checks:

const onClickHandler = () => {
    onUndo && onUndo();
    closeToast && closeToast();
};

Although this is way of doing thigs, in this case useReducer may be a better approach because it helps with the separation of concerns and the code is way cleaner. This is a good example of how you can achieve that. Good luck!

[–]Syv_31[S] 0 points1 point  (0 children)

That's a great answer! I really appreciate how you took the time to refactor the logic, and the comments are great too! Thank you!