JellyBear97 / personalProj3_todolist

리덕스 이용해서 전역 상태관리 시도해봅니다
0 stars 0 forks source link

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

Open yeongsbook opened 1 year ago

yeongsbook commented 1 year ago

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

yeongsbook commented 1 year ago

Router.js

https://github.com/JellyBear97/personalProj3_todolist/blob/1f20cbca34b0148f3d752cf754241a11447d6525/src/shared/Router.js#L12-L13

아래와 같이 자식이 없으면 자기 자신을 닫는 형태의 태그를 사용할 수 있습니다.

<Route path="/" element={<Home />} />
<Route path="/tododetail/:id" element={<TodoDetail />} />
yeongsbook commented 1 year ago

module/todos.js

https://github.com/JellyBear97/personalProj3_todolist/blob/1f20cbca34b0148f3d752cf754241a11447d6525/src/redux/modules/todos.js#L7-L12

객체의 키와 값이 같으면 아래와 같이 줄여서 사용할 수 있습니다.

 export const addTodo = payload => { 
   return { 
     type: ADD_TODO, 
     payload, 
   }; 
 }; 

https://github.com/JellyBear97/personalProj3_todolist/blob/1f20cbca34b0148f3d752cf754241a11447d6525/src/redux/modules/todos.js#L42-L56

개인적으로 리듀서만으로도 어떤 액션인지 알 수 있는 코드가 좋다고 생각합니다. payload로 새로운 state(여기선 배열)를 받기보다는 id를 받고 내부에서 새로운 state를 return 해주면 어떨까요? 제 방법이 정답은 아닙니다!

yeongsbook commented 1 year ago

TodoDetail.jsx

https://github.com/JellyBear97/personalProj3_todolist/blob/1f20cbca34b0148f3d752cf754241a11447d6525/src/pages/TodoDetail.jsx#L40-L46

  1. params.id와 일치하는 todo는 1개이기 때문에 filter를 사용할 필요 없습니다. Array의 find 메서드를 쓰시면 일치하는 1개의 엘리먼트를 찾을 수 있습니다.

    const targetTodo = todos.find(todo => todo.id === params.id);
    const { title, content } = targetTodo;
  2. filter를 쓰실거면 구조분해할당을 사용하시는 걸 추천드립니다.

    const [ targetTodo ] = todos.filter(todo => todo.id === params.id);
    const { title, content } = targetTodo;
yeongsbook commented 1 year ago

TodoCard.jsx

https://github.com/JellyBear97/personalProj3_todolist/blob/1f20cbca34b0148f3d752cf754241a11447d6525/src/components/TodoCard.jsx#L95-L98 할일 추가, 카드 삭제 버튼은 사용하지 않는 버튼 같습니다. 삭제해주세요.

https://github.com/JellyBear97/personalProj3_todolist/blob/1f20cbca34b0148f3d752cf754241a11447d6525/src/components/TodoCard.jsx#L112-L128

https://github.com/JellyBear97/personalProj3_todolist/blob/1f20cbca34b0148f3d752cf754241a11447d6525/src/components/TodoCard.jsx#L134-L150

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