Kendiproject1#38
Conversation
| } | ||
| return todos.map((todo, index) => ( | ||
| <div | ||
| className={todo.isCpmplete ? "todo-row complete" : "todo-row"} |
There was a problem hiding this comment.
You have a typo here, I think this will not work
| <div className="icons"> | ||
| <RiCloseCircleLine | ||
| onClick={() => removeTodo(todo.id)} | ||
| className="delete - icon" |
There was a problem hiding this comment.
with the spacing in the className, this styling will probably not be applied properly
| if (props.editMode) setUpdate(e.target.value) | ||
| else setInput(e.target.value); |
There was a problem hiding this comment.
if you use return statements, the else is redundant here.
| if (props.editMode) setUpdate(e.target.value) | |
| else setInput(e.target.value); | |
| if (props.editMode) return setUpdate(e.target.value) | |
| setInput(e.target.value); |
| }); | ||
| const toggleColorScheme = () => { | ||
| setColorScheme(colorScheme === 'dark' ? 'light' : 'dark'); | ||
| console.log(colorScheme); |
There was a problem hiding this comment.
remove console logs before pushing your code
| // if (!newValue || /^|s*$/.test(newValue.text)) { | ||
| // return; | ||
| // } |
| }; | ||
|
|
||
| const removeTodo = (id) => { | ||
| const removeArr = [...todos].filter((todo) => todo.id !== id); |
There was a problem hiding this comment.
| const removeArr = [...todos].filter((todo) => todo.id !== id); | |
| const removeArr = todos.filter((todo) => todo.id !== id); |
making a copy of todos here is redundant, as filter will return a new array anyways
| if (todo.id === id) { | ||
| todo.isComplete = !todo.isComplete; | ||
| } |
There was a problem hiding this comment.
By doing this, you change the value of the original todos. See https://bootcamp.rocketacademy.co/0-foundations/0.4-javascript/0.4.3-reference-vs-value
There was a problem hiding this comment.
I would suggest making a new copy of the todo if you want to get a new array with the inverted values
| }; | ||
|
|
||
| const completeTodo = (id) => { | ||
| let updatedTodos = todos.map((todo) => { |
There was a problem hiding this comment.
Since you just want to complete a single Todo, it might make more sense to use .find() here or a normal for loop, as you don't need to update every single todo in the array. Would like to know why you are using a map here
| if (edit.id) { | ||
| return <TodoForm edit={edit} editMode={editMode} setEditMode={setEditMode} onSubmit={submitUpdate} />; | ||
| } | ||
| return todos.map((todo, index) => ( |
There was a problem hiding this comment.
it is odd to render a list of todos in the Todo component. This component should define what a single todo looks like only
There was a problem hiding this comment.
In the TodoList component I would do something like this:
....
return todos.map((todo, index) => <Todo todo={todo} key={index} />
....
| @@ -3,8 +3,13 @@ | |||
| "version": "0.1.0", | |||
There was a problem hiding this comment.
the README file is missing any content about your project. The planning documents are also missing
2 participants
No description provided.