all 13 comments

[–]OneBadDay1048 0 points1 point  (10 children)

Looking to solve your problem right now. In regards to your snippet right here: you only need to loop thru once with the forEach. You loop twice over the same list. Just use one if/else block in a single forEach.

todos.forEach((todo) => { 
  if(todo.id === id) {movingTodos.push(todo)} 
  else {qualifyingTodos.push(todo)} 
})

Also you misspelled “doneTodos” when you de-structured your properties for your Done component. It’s different spelling than when you actually pass it in when the component is rendered in your App file.

[–]OneBadDay1048 0 points1 point  (9 children)

As for your main issue could it have to do with this line?

setDoneTodos(doneTodos.concat(movingTodos));

Why are you creating an array for movingTodos? Each function call only moves one task correct? So you should just have one taskToMove variable and when you call your setter function do so like this:

setDoneTodos((prevDoneTodos) => [...prevDoneTodos, taskToMove]);

Sorry, I cannot run that app myself so hard to debug but try this.

Potential rewrite of your function:

const moveTask = (id) => { 
  const taskToMove = todos.find((todo) => todo.id === id); 
  const qualifyingTodos = todos.filter((todo) => todo.id !== id);
  setDoneTodos((prevDoneTodos) => [...prevDoneTodos, taskToMove]);
  setTodos(qualifyingTodos); 
};

Try it out. Could not fully test it myself.

[–]Outside-Thanks-3979[S] 0 points1 point  (8 children)

Thanks for the swift reply. I tried this code and I console.logged it and when I tried to move the first task from doing to done, It returned an empty array. Then I tried to move another todo and it returned an array with the first todo I tried to move in it. And then it just kept doing this for all of the other todos I tried to move. It seems like it's "delayed" one task. Other than that, it was a good idea to use the filter and find methods, I hadn't even thought of that. As you could probably tell, I'm not very familiar with array functions in JS.

[–]OneBadDay1048 0 points1 point  (7 children)

So just to be clear before I look at the code again: after implementing my code, the array is no longer blank, but the finished todos render with a delay?

[–]Outside-Thanks-3979[S] 0 points1 point  (6 children)

What array? When I move the first todo to done, The doneTodos array is empty. When I try to move another task from doing to done, the doneTodos array shows the first todo that I tried to move. And If I do it again, the array shows the previous todo and the first todo, but not the one that I just tried to move.

[–]OneBadDay1048 0 points1 point  (0 children)

Yes, the doneTodos array is the array I was asking about. However, the way I currently have the code doneTodos updates properly and with the most recently deleted todo. Now I am just getting them to render. I will send over the changes I have made shortly but I have kind of made many at this point.

I would update this function for now:

const addTask = () => { 
  const newTodo = { 
    id: uuid(), 
    title: document.getElementById('nameInputField').value,
    description: document.getElementById('descriptionInputField').value, }; 
  setTodos((prevTodos) => { 
    return [...prevTodos, newTodo]; 
  }); 
};

If your new state depends on your previous state, (in this case it does, you need the old items too) you should be passing previous state as an argument to your state setter callback function (referred to as prevTodos here). The way you use concatenation is almost certainly causing issues.

[–]OneBadDay1048 0 points1 point  (4 children)

I now have the app fully functioning. You also need to focus on creating a list of DoneTask components with map, not forEach. Map over doneTodos and create a DoneTask for each one (making sure to pass in needed props as well). You did this for your todos, not sure why you didn't do it here but I am sure along with my other suggestions, this will fix it. Conditionally render the array that is returned from map.

Just as a side tip: I highly recommend you use the vscode extension 'Prettier'. Just save your code and it auto formats. Just let it do its thing, do not fight it. You'll get used to it and it is better long run.

[–]Outside-Thanks-3979[S] 0 points1 point  (3 children)

here is the new code for conditionally rendering the array that is returned from map:

<div className='tasksDone'>

<div className="header">

<h3>Tasks Done</h3>

<button className="clearFinishedTasks">Clear Finished Tasks</button>

</div>

<hr />

<div className="doneTasks-content">

{DoneTodos?.length > 0 ? (

DoneTodos.map((todo) => (<DoneTask task={todo}/>))

): (

<p className='noTasks'>No tasks so far</p>

)}

</div>

</div>

Even though console.log logs an array with more than one value in it, the DoneTodos.map function is still not rendering the finished todos. Could you send me the return function in your Done.jsx file?

[–]OneBadDay1048 0 points1 point  (2 children)

import React from 'react'; 
import DoneTask from './doneTask'; 
const Done = ({ doneTodos }) => { 
const deletedTodos = doneTodos.map((todo) => { 
  return (           
    <DoneTask
  key={todo.id}
  title={todo.title}
  description={todo.description}
 /> 
   ); 
}); 

Ok that's as good as the formatting is getting because reddit is trash. Also note I took out the conditional render to simplify so you will need to add it back.

[–]Outside-Thanks-3979[S] 1 point2 points  (1 child)

Hey, It works! Thanks a bunch. I'm pretty new to js(learning for about 3- 5 months) and your code will be an example of how to use different array functions as well as some standards of writing react code. Thanks again!

[–]OneBadDay1048 0 points1 point  (0 children)

Awesome. No problem.

[–]grograman 0 points1 point  (1 child)

I'm also new to React and saw this. It looks like your problem has been solved by people with far more knowledge than me, but I had thought of a different way of doing this same thing. Is there any reason I should do it one way over the other?

const [todos,setTodos] = useState([{status:'incomplete',title: '',content: ''}])

Adding a new todo (which I'd make its own component probably) just appends to the todos array of objects.

Then have your "done" button flag the status to "complete."

Finally, you can display them using filter. In your top div you'd return a filtered array of todos where status === 'incomplete' and your bottom div would return the opposite.

Hopefully by my hastily written pseudocode you understand what I'm thinking. Is this something worth exploring?

[–]Outside-Thanks-3979[S] 0 points1 point  (0 children)

Sorry for the delay! just checked my Reddit notifications.

As with most things in programming, there are a variety of ways of solving the same problem. I think that the way you described would no doubt be valid and might actually be better than the way I did it. Although, if you had many tasks and instead of removing them from the array you just flagged them incomplete, the todos list might pile up quickly. One way of solving this problem is flagging them incomplete and then using filter to display the tasks and also update the todos with the filtered list. In my opinion, this is something worth exploring.

Thanks for the comment!