search

NickYOOO / TodoList.ver2

https://todo-list-ver2.vercel.app
0 stars 1 forks source link

프로젝트 피드백 - 작성자: 오창영 튜터 #2

Open yeongsbook opened 1 year ago

yeongsbook commented 1 year ago

추가적으로 아래에 파일들에 대한 리뷰 남기겠습니다.

yeongsbook commented 1 year ago

modules/todos.js

https://github.com/NickYOOO/TodoList.ver2/blob/88e418d8f265eccc13d040e8b399f1d4d2bd6e24/src/redux/modules/todos.js#L60-L70

코드 잘 작성해주셨습니다. 안쓰는 코드를 주석처리한 것은 지우는 것이 좋습니다. 내용 설명에 대한 주석은 안지우셔도 됩니다!

yeongsbook commented 1 year ago

Detail.jsx

https://github.com/NickYOOO/TodoList.ver2/blob/88e418d8f265eccc13d040e8b399f1d4d2bd6e24/src/pages/Detail.jsx#L27-L39

일반적으로 Detail 페이지에서 param.id와 같은 todo아이템은 1개여야합니다. 그런데 로컬스토리지로 인해 id가 겹치는 이슈가 발생하여 map으로 여러 아이템이 노출됩니다. 원래는 Array의 find 메서드로 하나만 가져올 수 있어야 합니다.

yeongsbook commented 1 year ago

List.jsx

https://github.com/NickYOOO/TodoList.ver2/blob/88e418d8f265eccc13d040e8b399f1d4d2bd6e24/src/components/List.jsx#L31-L55

https://github.com/NickYOOO/TodoList.ver2/blob/88e418d8f265eccc13d040e8b399f1d4d2bd6e24/src/components/List.jsx#L59-L83

두 코드에 공통 요소가 많습니다. 하나의 컴포넌트로 만들면 어떨까요?

yeongsbook commented 1 year ago

Form.jsx

https://github.com/NickYOOO/TodoList.ver2/blob/88e418d8f265eccc13d040e8b399f1d4d2bd6e24/src/components/Form.jsx#L15

위 코드와 로컬스토리지 조합으로 인해 중복되는 id가 발생하여 비정상적으로 동작합니다. uuid를 사용하시면 쉽게 해결하실 수 있습니다.