-
Notifications
You must be signed in to change notification settings - Fork 7
[강지현] 고급 투두리스트 (2단계) 구현 완료 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
givvemee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다. 크게 모난 곳 없이 둥글둥글 코드를 잘 작성하셨네요. 고생하셨습니다.
src/step1/BasicTodo.jsx
Outdated
| if (newTodo === "") { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (newTodo === "") { | |
| return; | |
| } | |
| if (!newTodo) return; |
아주 간략하게, 이렇게 줄여서 쓸 수도 있습니다. 빈 문자열은 falsy 한 값이기 때문이죵. 시간 되실 때 참고 링크를 보셔요.
| state: "미완료", | ||
| }; | ||
| // 스프레드 연산자로 기존 배열을 복사하고 새로운 할 일의 객체 추가 | ||
| setTodoList([...todoList, newTodoItem]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
배열의 불변성을 지키신 것 아주 굿입니다.
|
|
||
| // TODO: 4. 할 일 삭제 함수 구현 | ||
| const deleteTodo = (index) => { }; | ||
| const deleteTodo = (index) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addTodo 와 비교하여 toggleTodoState 와 deleteTodo 는 새로운 배열을 할당하지 않고 .map() 이나 .filter() 가 새 배열을 반환한다는 점을 활용하여 바로 setTodoList() 에 할당하셨네요. 두 가지의 방식이 크게 차이는 없지만 결국 배열의 불변성을 지켜야 한다는 점에서는 같은 맥락입니다. 두 방법 모두 활용해 보신 시도가 좋았어요.
| const handleKeyDown = (e) => { }; | ||
| const handleKeyDown = (e) => { | ||
| // 누른 키가 Enter 키라면 입력 | ||
| if (e.key === "Enter") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key 가 Enter 일 때만 addTodo() 를 실행하는데, 여기서도 early return 사용해 볼 수 있지 않을까요? 어떻게 하면 좋을까 생각해 봅시다.
|
|
||
| // TODO: 6. 수정된 할 일 텍스트를 저장하는 함수 구현 | ||
| const saveEdit = () => { | ||
| if (editingText === "") return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (editingText === "") return; | |
| if (!editingText) return; |
요렇게도 쓸 수 있겠지요. early return 처리 아주 좋습니다.
| }; | ||
|
|
||
| // 전체 할 일의 상태가 완료인지 확인한 결과 저장 변수 | ||
| const allTodosSelected = todoList.every((todo) => todo.state === "완료"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오! 특이하게 every() 함수를 사용하셨군요. every() 는 사실 읽기 전용 메서드라 원본 배열이나 요소를 변경하지 않습니다. 그래서 안전한 읽기 연산이에요. 단순 결과 저장 용도로 사용해 보신 것 같은데, 좋은 시도입니다. map() 이나 .filter() 같은 것도 많이 사용되지만, 이런 식으로 비교적 덜 사용되는 메서드를 사용해 보는 것도 좋아요. 잘하셨습니당.
| // 전체 할 일의 개수 | ||
| const totalCount = todoList.length; | ||
| // 진행률 | ||
| const progressPercentage = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
안녕하세요, 멘토님!
제공해주신
AdvancedTodo.jsx,TodoActions.jsx,TodoFilter.jsx의 TODO 구현을 모두 완료했습니다.각 파일에 작성된 TODO 내용과
README.md의 내용을 참고하여 구현하였습니다.로컬에서 기능 테스트를 진행한 결과, 모든 기능이 의도한 대로 작동하는 것을 확인했습니다.
작성한 코드에 대해 리뷰를 부탁드립니다.
특히 개선할 점이나 추가할 기능이 있으면 피드백 주시면 감사하겠습니다!
감사합니다.