-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 체크리스트 항목에 saveCount 및 isSaved 속성 추가, 기타 코드 개선 #341
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
Conversation
Walkthrough이번 변경은 저장 수(saveCount)와 저장 여부(isSaved)를 항목/할 일 데이터에 옵션 필드로 추가하고, UI에서 항목별 저장 수를 표시하도록 타입과 렌더링을 보완했습니다. 또한 OtherTodoListPage의 useEffect 의존성이 location.pathname으로 수정되어 경로 변경 시 태그 액션이 재실행됩니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant Router as Router/Location
participant Page as OtherTodoListPage
participant Tag as ReactTagManager
participant Hook as useJobQuery
participant Card as OtherTodoCard
participant UI as CheckList (saveCount 표시)
User->>Router: 경로 변경
Router-->>Page: location.pathname 업데이트
Note over Page: useEffect [location.pathname]
Page->>Tag: action(...)
Page->>Hook: eachTodos 조회
Hook-->>Page: todos[{..., saveCount?, isSaved?}]
Page->>Card: props 전달({..., saveCount, isSaved})
Card->>UI: 항목 렌더링 요청
UI-->>User: 각 항목의 saveCount 표시
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Chasyuss
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.
확인했어요!
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hook/useJobQuery.ts (2)
161-168: 튜플로 선언된 todos 타입을 배열로 수정 필요
JobOtherList.todos가 현재 튜플([ { ... }, ])로 선언되어 있어 다수 항목을 담는 실제 응답/사용 패턴과 불일치합니다. 컴파일 타임에 불필요한 제약을 만들고 IDE 타입 추론도 왜곡됩니다. 일반 배열로 변경하세요.- todos: [ - { - todoId: number; - title: string; - completed: boolean; - }, - ]; + todos: { + todoId: number; + title: string; + completed: boolean; + }[];
202-213: EachTodos.todos도 배열로 타입 정정동일하게
EachTodos.todos또한 튜플로 선언되어 있습니다. 배열로 바꾸어 실제 데이터 형태와 일치시키세요.- todos: [ - { - todoId: number; - title: string; - completed: boolean; - isMemoExist: boolean; - isPublic: boolean; - saveCount?: number; - isSaved?: boolean; - }, - ]; + todos: { + todoId: number; + title: string; + completed: boolean; + isMemoExist: boolean; + isPublic: boolean; + saveCount?: number; + isSaved?: boolean; + }[];
🧹 Nitpick comments (7)
src/hook/useJobQuery.ts (1)
215-241: 타입/값 네임 충돌(가독성 저하) — 함수명 변경 권장타입
EachTodos와 함수EachTodos가 동일 이름입니다. TS에선 가능하지만 읽기/검색성이 떨어집니다.fetchEachTodos등으로 함수명을 변경하세요.-const EachTodos = async (todoGroupId: number) => { +const fetchEachTodos = async (todoGroupId: number) => { ... -export const useEachTodosQuery = (todoGroupId: number) => { - return useQuery<EachTodos>({ +export const useEachTodosQuery = (todoGroupId: number) => { + return useQuery<EachTodos>({ queryKey: ['EachTodos', todoGroupId], - queryFn: () => EachTodos(todoGroupId), + queryFn: () => fetchEachTodos(todoGroupId), }); };src/common/CheckList.tsx (2)
24-25: 사용되지 않는 prop 제거 제안: saveCount
CheckListProps의saveCount?: number는 컴포넌트 내부에서 사용되지 않습니다. 혼동을 줄이기 위해 제거하세요.interface CheckListProps { lists: ChecklistItem[]; checkedIds?: number[]; className?: string; onChange?: (checkedIds: number[]) => void; showAddButton?: boolean; - saveCount?: number; }
268-270: Nullish 병합 사용 적절하며 일관성 유지 권장
{item.saveCount ?? 0}는 의도에 맞습니다. 동일 패턴을OtherTodoCard의saveCount표시에도 적용해 일관성을 맞추면 좋습니다.src/pages/otherTodoList/components/OtherTodoCard.tsx (2)
112-116: 리스트 key에 index 대신 todoId 사용재정렬/삽입 시 키 충돌을 방지하려면 안정적인 식별자 사용이 필요합니다.
- <li - key={index} + <li + key={item.todoId} className={`flex items-center justify-between px-2 py-1`}
140-145: saveCount 표기: || 대신 ?? 사용으로 의미 명확화0을 유효값으로 취급하므로 null/undefined만 대체하세요。
CheckList와도 일관적입니다.- <span className="text-sm font-B03-SB"> - {item.saveCount || 0} - </span> + <span className="text-sm font-B03-SB"> + {item.saveCount ?? 0} + </span>src/pages/otherTodoList/OtherTodoListPage.tsx (2)
101-109: 신규 필드 전달 OK + 소소한 안정성/일관성 제안
saveCount,isSaved전달은 카드 컴포넌트 요구사항에 맞습니다. 다만 라우트 파라미터가 비정상인 경우Number(todoGroupId)가NaN이 될 수 있어 초기 API 호출이 실패할 여지가 있습니다.useEachTodosQuery에enabled옵션을 도입하거나(권장) 페이지에서 유효성 검사 후 호출되도록 개선을 고려해주세요.원하시면
useEachTodosQuery에enabled를 지원하도록 훅 시그니처와 호출부를 리팩터링하는 패치를 제공하겠습니다.
113-119: 불필요한 Number 캐스팅 제거 제안
eachTodos.jobId가 이미number라면Number(...)는 불필요합니다.- jobId={Number(eachTodos.jobId)} + jobId={eachTodos.jobId}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/common/CheckList.tsx(3 hunks)src/hook/useJobQuery.ts(1 hunks)src/pages/otherTodoList/OtherTodoListPage.tsx(2 hunks)src/pages/otherTodoList/components/OtherTodoCard.tsx(1 hunks)
🔇 Additional comments (5)
src/hook/useJobQuery.ts (1)
209-213: todos 항목에 새 필드 추가 OK
saveCount?: number,isSaved?: boolean추가는 기존 소비 코드와의 하위 호환을 깨지지 않습니다.src/common/CheckList.tsx (2)
16-16: 타입 확장 방향 OK
ChecklistItem에saveCount?: number추가는 UI 요구사항을 자연스럽게 반영합니다.
37-39: 문자 입력 정규화 시 기본값 0 지정 적절문자 리스트를
{ text, saveCount: 0 }로 정규화하는 처리는 렌더링 안정성과 일관성에 도움 됩니다. LGTM.src/pages/otherTodoList/components/OtherTodoCard.tsx (1)
107-111: isAdded 계산에 nullish 병합 사용 LGTM서버 기본값 미존재 시 의도대로 동작합니다.
src/pages/otherTodoList/OtherTodoListPage.tsx (1)
22-22: 경로 변경 트래킹 재실행 의존성 변경 LGTM
[location.pathname]으로 GTM 이벤트가 경로 전환 시 재발화됩니다. 의도에 부합합니다.
🚀 풀 리퀘스트 제안
📋 작업 내용
📸 스크린샷 (선택 사항)
📄 기타
Summary by CodeRabbit