react1-week2/olesia #272
react1-week2/olesia #272
Conversation
There was a problem hiding this comment.
You are not using this AddTodo component in your app, so this file is safe to remove
| useEffect(() => { | ||
| setTimeout(() => { | ||
| setSecond((prev) => prev + 1); | ||
| }, 1000); | ||
| }); |
There was a problem hiding this comment.
While technically valid, using useEffect without a dependency array might lead to unexpected behavior, especially in reusable components (maybe what you saw when trying to render it in thw TodoList?). Every time the component re-renders, the code within the useEffect hook will run. If TimeCount is used inside a component that triggers frequent re-renders, such as TodoList, the timer may behave unpredictably. To ensure consistent behavior and prevent external factors from affecting the timer, consider adding the 'seconds' variable to the dependency array. This ensures that the timer continues updating based solely on its own state, unaffected by external re-renders.
useEffect(() => { setTimeout(() => { setSecond((prev) => prev + 1); }, 1000); }, [seconds]);
| const [itemStatus, setItemStatus] = useState(props.status); | ||
|
|
||
| function toggleStatus(e) { | ||
| setItemStatus(!itemStatus); | ||
| } |
There was a problem hiding this comment.
Have you noticed that after checking the item, if you interact with your list (add or delete something) the checkmark disappears? That is because your original 'todos' array lives in the TodoList component and TodoItem can not transfer data upstream to mutate it, so if a re-render is triggered you fallback to an array in the TodoList. To be able to change the item status in your todos array you will need to repeat what you have succesfully done with the 'onDeleteClick' function - have the editing function in the same file as your data.
const toggleItemStatus = (id) => setTodoList(todoList.map(todo => todo.id === id ? { ...todo, status: !todo.status } : todo));
Adding function like this to the TodoList component, and when passing it the same way as you've done with 'onDeleteClick' will do the trick 😉
| checked={itemStatus} | ||
| onChange={toggleStatus} | ||
| /> | ||
| <label htmlFor="item">{props.name}</label> |
There was a problem hiding this comment.
You are missing the task to add a line-through decoration to checked item from the homework. You can conditionally add a classname like this
<label htmlFor="item" className={props.status ? 'done' : ''}>
This way the label element will only have 'done' class when the props.status is true. Now only need to create CSS for it!
| import { useState } from "react"; | ||
|
|
||
| function AppTodo() { | ||
| // const data = [...todos]; |
There was a problem hiding this comment.
remove commented out code when submitting a PR
| const [todolist, setTodoList] = useState([...todos]); | ||
|
|
||
| const addTodo = () => { | ||
| const newTodo = {id:new Date(), name: "new todo", date: new Date(), status: false }; |
There was a problem hiding this comment.
You are never using the date value from this object. Did you want to name it 'deadline' as in your initial todo array? If so, also note that new Date() returns a date object, so you would need to turn it into a string using .toLocaleDateString() function
|
|
||
| const addTodo = () => { | ||
| const newTodo = {id:new Date(), name: "new todo", date: new Date(), status: false }; | ||
| setTodoList((todolist) => { return [...todolist, newTodo]; }); |
There was a problem hiding this comment.
State setter function updates the state with the value you pass to it, so you can simplify this snippet like
setTodoList([...todolist, newTodo])
| setTodoList(todolist.filter(todo => todo.id !== id)); | ||
| } | ||
|
|
||
| const TodoList = (props) => { |
There was a problem hiding this comment.
The react pattern is that every component has it's own file. In your case, the TodoList component is not reusable as it is declared inside the AppTodo component, and can only be called here. If that was intended, you could move lines 21-40 to line 48, and use it directly in the AppTodo JSX without creating a separate component. Otherwise, if you want the TodoList component to be reusable outside of AppTodo - create a separate file for it, and import the component into this one.
| {props.todos.length > 0 ? ( | ||
| props.todos.map((todo) => { | ||
| return ( | ||
| <TodoItem |
There was a problem hiding this comment.
Always add a unique key prop when using map to render a list of items in react
key={todo.id}
| return ( | ||
| <div> | ||
| <h2>My To-Do List</h2> | ||
| {/* <TimeCount /> */} |
No description provided.