react1-week2/gizem #282
react1-week2/gizem #282
Conversation
|
|
||
| useEffect(()=>{ | ||
| const timer = setInterval(()=>{ | ||
| setSeconds(prevSeconds => { |
There was a problem hiding this comment.
Awesome work with the split into minutes and seconds! 🎉 Much more appealing to look at.
There is a small issue with the updater function for seconds: it is not pure! It has a side-effect since it calls setMinutes. You might have seen when running in development mode, that your minute counter increments twice every time 60 seconds pass. This is due to it running it strict mode where it runs functions supposed to be pure twice to provoke purity issues during development. You can read more here.
Can you think of a way to do this with only one simple state and not two? Then it becomes a lot easier to manager. 😄
There was a problem hiding this comment.
I created one state elapsedTime. Can you check from my commits:Fix Timer bug? Thank you for helping with this bug🙏
| ); | ||
| } | ||
|
|
||
| TodoItem.propTypes = { |
There was a problem hiding this comment.
Very nice to set some types and the like so we can catch pesky type errors and missing props! 👍 Deadline, Task, and TodoList are missing them though. They are not required, but consistency is key! 😄
There was a problem hiding this comment.
Thank you🙏 I just used PropTypes for Deadline, Task and TodoList.
| } | ||
|
|
||
| const handleRemoveClick = () => { | ||
| removeTodo(todo.id); |
There was a problem hiding this comment.
Good use of callback patterns! ⭐ I would suggest going with the convention of onSomeAction to make it clear this is akin to listening to an event in the component.
There was a problem hiding this comment.
but React documentation has examples with handleStuff. Sorry, I couldn't understand what to do🙈
Demo.mov