-
Notifications
You must be signed in to change notification settings - Fork 3
[DEV-32] 모든 할 일 컴포넌트, 무한 스크롤 훅 구현 #70
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
Co-authored-by: xorms571 <[email protected]>
Walkthrough이번 PR은 여러 컴포넌트와 설정 파일에 대한 변경사항을 포함합니다. Home 컴포넌트에서 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant LT as ListTodo 컴포넌트
participant IS as useInfiniteScroll 훅
participant API as 서버 API
U->>LT: 스크롤 이벤트 발생
LT->>IS: 대상 엘리먼트 전달
IS->>IS: Intersection Observer 감지
IS->>API: fetchNextPage() 호출
API-->>IS: todo 데이터 응답
IS-->>LT: 추가 데이터 반환
LT-->>U: UI 업데이트
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
🧹 Nitpick comments (2)
src/components/list-todo.tsx (2)
172-180
: 중복되는 input id로 인해 접근성 문제가 발생할 수 있습니다.
모든 항목에 대해 같은id="todo-check"
를 사용하면 스크린 리더 등에서는 혼동이 있을 수 있습니다. 방지하려면 각 항목마다 고유한id
를 부여하거나, label과 input 타이핑 방식을 변경해 주세요.- <label htmlFor="todo-check" ...> - <input type="checkbox" id="todo-check" /> + <label htmlFor={`todo-check-${todo.id}`} ...> + <input type="checkbox" id={`todo-check-${todo.id}`} />
221-221
: 스크롤 로딩 상태 표시를 고려하세요.
무한 스크롤 영역(div ref={setTarget}
)에 스크롤이 끝에 도달했을 때 로딩 상태를 시각적으로 표시하면 사용자 경험이 향상됩니다. 예를 들어 로딩 스피너나 "로딩 중" 텍스트를 추가하는 방법을 고려해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/page.tsx
(2 hunks)src/components/list-todo.tsx
(1 hunks)src/hooks/useInfiniteScroll.ts
(1 hunks)src/styles/globals.css
(1 hunks)
🔇 Additional comments (6)
src/app/page.tsx (3)
6-6
: ListTodo 컴포넌트 추가 확인
새 컴포넌트ListTodo
를 import한 부분은 정상적으로 보입니다.
29-29
: SentryCheck 위치 이동
SentryCheck
가 메인 콘텐츠 영역 안으로 이동되어, 전체 UI 흐름상 더욱 자연스러워졌습니다. 문제 없어 보입니다.
34-34
: ListTodo 컴포넌트 렌더링 검토
ListTodo
를Suspense
내부에서 렌더링함으로써, 비동기 로직이 별도의 로딩 상태 관리와 함께 처리됩니다. 적절한 구성입니다.src/hooks/useInfiniteScroll.ts (2)
19-28
: 무한 스크롤 이벤트 조건 검토
entry.isIntersecting && hasNextPage
조건으로 페이지 종료 시점에만 데이터를 가져오도록 되어 있습니다. 다만,entry
가 지속적으로 관찰될 경우 동일 API 호출이 여러 번 발생하지는 않는지 확인해 주세요(React Query가 중복 요청을 잘 처리하지만, 의도치 않게 중복 요청될 수도 있습니다).
31-44
: Intersection Observer 인스턴스 관리
useEffect
내에서observer.observe(target)
와observer.unobserve(target)
를 확실히 관리하고 있어, 메모리 누수 방지 및 불필요한 감시가 해제되는 구조가 좋습니다.src/styles/globals.css (1)
18-19
: 접근성 검증이 필요합니다.텍스트 색상과 배경색이 서로 바뀌었습니다. WCAG 기준에 따른 충분한 대비(contrast ratio)가 유지되는지 확인해 주세요.
❌ Verification inconclusive
🌐 Web query:
What is the minimum contrast ratio required for WCAG AA compliance?
💡 Result:
The minimum contrast ratio required for WCAG AA compliance is 4.5:1 for standard-sized text and 3:1 for large text (defined as at least 18-point or 14-point bold)[1][3][5][7]. This standard ensures readability for users with visual impairments or age-related contrast sensitivity loss[3][9].
Key Details:
- Standard text (Level AA):
4.5:1 contrast ratio between foreground (text) and background[1][5][9].- Large text exceptions:
3:1 ratio applies to:
- Text ≥18pt (~24px)
- Bold text ≥14pt (~18.67px)[1][5][7].
- Non-text elements:
3:1 ratio required for UI components (buttons, icons) and graphical objects[7].Exemptions:
- Incidental/decorative text
- Logos
- Inactive UI elements[3][5].
For stricter AAA compliance, ratios increase to 7:1 (standard text) and 4.5:1 (large text)[2][7]. Tools like WebAIM’s Contrast Checker help validate these ratios during design[5][7].
Citations:
- 1: https://www.washington.edu/accesstech/checklist/contrast/
- 2: https://accessiblyapp.com/web-accessibility/color-contrast/
- 3: https://www.w3.org/WAI/WCAG20/Understanding/contrast-minimum
- 4: https://www.w3.org/WAI/WCAG22/Understanding/contrast-minimum.html
- 5: https://www.oregon.gov/ode/accessibility/checklist/pages/contrast.aspx
- 6: https://adasitecompliance.com/ada-color-contrast-checker/
- 7: https://accessibleweb.com/color-contrast-checker/
- 8: https://www.maine.edu/content-management/accessibility/color-ratio/
- 9: https://webaim.org/articles/contrast/
- 10: https://www.boia.org/blog/designing-for-color-contrast-guidelines-for-accessibility
글로벌 CSS 파일 색상 대비 확인 요청
현재 src/styles/globals.css 파일의 18-19번째 줄에서 텍스트 색상과 배경색에 사용되는 변수가 서로 바뀌어 할당되고 있습니다. 이로 인해 실제로 적용되는 색상 값이 WCAG AA 기준(일반 텍스트의 경우 최소 대비 4.5:1, 큰 텍스트의 경우 3:1)을 충족하는지 반드시 확인할 필요가 있습니다. 실제 변수 값인
var(--background)
와var(--foreground)
의 색상이 충분한 대비를 제공하는지 검토해 주세요.
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: 4
🧹 Nitpick comments (2)
src/components/list-todo.tsx (2)
34-48
: 목 데이터 구현을 개선하세요.목 데이터 구현에 다음과 같은 개선이 필요합니다:
- 페이지 크기를 상수로 분리하여 재사용성을 높이세요.
- 지연 시간을 상수로 분리하여 테스트 환경에서 조정 가능하게 만드세요.
- 테스트를 위해 결정적인(deterministic) 데이터 생성 방식을 고려하세요.
다음과 같이 개선해보세요:
+const PAGE_SIZE = 40; +const MOCK_DELAY = 500; + const mockFetchTodos = async (pageParam = 1) => { return new Promise<{ todos: Todo[]; nextPage?: number }>((resolve) => { - setTimeout(() => { - const todos: Todo[] = Array.from({ length: 40 }, (_, i) => ({ + setTimeout(() => { + const todos: Todo[] = Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `todo-${pageParam}-${i}`, title: `${pageParam}-${i + 1} ${getRandomNote()}`, - status: Math.random() > 0.5 ? "todo" : "done", + status: i % 2 === 0 ? "todo" : "done", // 결정적인 패턴 hasNote: Math.random() > 0.5 ? getRandomNote() : null, hasLink: Math.random() > 0.5, hasFile: Math.random() > 0.5, })); resolve({ todos, nextPage: pageParam < 3 ? pageParam + 1 : undefined }); - }, 500); + }, MOCK_DELAY); }); };
58-61
: 상태 관리를 최적화하세요.여러 개의 useState를 사용하는 대신 useReducer를 사용하여 상태 관리를 단순화할 수 있습니다.
상태 로직을 reducer로 분리하여 관리하는 것을 추천드립니다. 예시 코드를 생성해드릴까요?
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
🧹 Nitpick comments (4)
src/components/list-todo.tsx (3)
248-248
: 하드코딩된 min-height 값을 개선하세요.고정된 min-height 값은 다양한 화면 크기와 컨텐츠 길이에 대응하기 어렵습니다. 컨텐츠에 따라 자동으로 조절되도록 수정하는 것이 좋습니다.
-<div className="mx-auto min-h-[2080px] w-full max-w-2xl rounded-xl bg-white p-6 text-sm text-slate-800"> +<div className="mx-auto min-h-screen w-full max-w-2xl rounded-xl bg-white p-6 text-sm text-slate-800">
44-68
: 목 데이터를 별도의 파일로 분리하세요.목 데이터를 컴포넌트 파일에 직접 포함시키면 코드의 가독성과 유지보수성이 저하됩니다. 별도의 mock 디렉토리나 파일로 분리하는 것이 좋습니다.
174-185
: toggleStatus 함수를 단순화하세요.현재 구현은 Map을 사용하여 불필요하게 복잡합니다. Array.map을 사용하여 더 간단하게 구현할 수 있습니다.
-const toggleStatus = (id: string) => { - setTodos((prevTodos) => { - const updatedTodos = new Map(prevTodos.map((todo) => [todo.id, todo])); - if (updatedTodos.has(id)) { - updatedTodos.set(id, { - ...updatedTodos.get(id)!, - status: updatedTodos.get(id)!.status === "todo" ? "done" : "todo", - }); - } - return Array.from(updatedTodos.values()); - }); -}; +const toggleStatus = (id: string) => { + setTodos((prevTodos) => + prevTodos.map((todo) => + todo.id === id + ? { ...todo, status: todo.status === "todo" ? "done" : "todo" } + : todo + ) + ); +};.eslintrc.json (1)
118-120
: ESLint 규칙 문서화를 개선하세요.규칙에 대한 설명 주석이 일관성 있게 작성되어야 합니다. 다른 규칙들처럼 영어로 작성하거나, 모든 주석을 한글로 통일하는 것이 좋습니다.
"@typescript-eslint/no-throw-literal": "off", -// 중첩 삼항 연산자 허용 +// Allow nested ternary operators "eslint no-nested-ternary": "off"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.eslintrc.json
(1 hunks)src/components/list-todo.tsx
(1 hunks)
🔇 Additional comments (4)
src/components/list-todo.tsx (4)
72-72
: QueryClient 인스턴스화 위치를 변경하세요.컴포넌트 외부에서 QueryClient를 인스턴스화하면 테스트와 SSR 환경에서 문제가 발생할 수 있습니다.
148-159
: 쿼리 에러 처리를 추가하세요.useInfiniteQuery의 에러 상태와 로딩 상태 처리가 누락되었습니다.
86-102
: 체크박스의 접근성을 개선하세요.체크박스 구현에 접근성 관련 속성이 누락되었습니다.
166-172
: 필터 로직이 잘 구현되었습니다.타입 안전성이 보장되며, 필터링 로직이 명확하게 구현되었습니다.
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
🧹 Nitpick comments (3)
src/components/list-todo.tsx (3)
15-43
: 타입과 인터페이스에 문서화를 추가하세요.각 타입과 인터페이스의 목적과 사용 사례를 설명하는 JSDoc 주석을 추가하면 코드의 가독성과 유지보수성이 향상될 것입니다.
+/** + * Todo 아이템의 기본 구조를 정의합니다. + */ type Todo = { id: string; title: string; status: "todo" | "done"; hasNote: string | null; hasLink: boolean; hasFile: boolean; };
44-68
: 목 데이터를 별도의 파일로 분리하세요.목 데이터와 관련 유틸리티 함수를 별도의 파일(예:
mocks/todoData.ts
)로 분리하면 다음과 같은 이점이 있습니다:
- 코드 구조의 개선
- 테스트 용이성 향상
- 재사용성 증가
248-248
: 하드코딩된 높이 값을 개선하세요.
min-h-[2080px]
와 같이 하드코딩된 높이 값은 다양한 화면 크기에서 레이아웃 문제를 일으킬 수 있습니다. 컨텐츠에 따라 동적으로 조정되는 높이를 사용하는 것이 좋습니다.-<div className="mx-auto min-h-[2080px] w-full max-w-2xl rounded-xl bg-white p-20 text-sm text-slate-800"> +<div className="mx-auto min-h-screen w-full max-w-2xl rounded-xl bg-white p-20 text-sm text-slate-800">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (5)
declarations.d.ts
(1 hunks)package.json
(1 hunks)src/components/list-todo.tsx
(1 hunks)tailwind.config.ts
(1 hunks)tsconfig.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- declarations.d.ts
🔇 Additional comments (6)
src/components/list-todo.tsx (3)
69-78
: getQueryClient를 사용하도록 수정하세요.이전 리뷰 피드백에 따라, 새로운 QueryClient를 생성하는 대신 getQueryClient를 사용하는 것이 좋습니다. 이는 다음과 같은 이점이 있습니다:
- 기본 옵션 활용 가능
- 일관된 QueryClient 인스턴스 사용
- 향후 옵션 관리 용이성
79-142
: 체크박스의 접근성을 개선하세요.이전 리뷰에서 지적된 대로, 체크박스의 접근성을 개선해야 합니다. ARIA 속성을 추가하여 스크린 리더 사용자의 경험을 향상시키세요.
148-159
: 쿼리 에러 처리를 추가하세요.useInfiniteQuery에 에러 상태와 로딩 상태 처리가 누락되었습니다. 사용자에게 적절한 피드백을 제공하기 위해 에러 처리를 구현하세요.
tailwind.config.ts (1)
1-6
: px to rem 변환 프리셋 추가가 적절합니다!px를 rem 단위로 자동 변환하는 프리셋을 추가한 것은 다음과 같은 이점이 있습니다:
- 반응형 디자인 구현이 용이해집니다
- 사용자의 브라우저 설정을 존중하는 접근성이 향상됩니다
- 일관된 단위 시스템을 유지할 수 있습니다
tsconfig.json (1)
31-32
: 타입 선언 파일 포함이 적절합니다!declarations.d.ts 파일을 include 배열에 추가한 것은 새로 도입된 tailwindcss-preset-px-to-rem 모듈의 타입 지원을 위해 필요한 변경사항입니다.
package.json (1)
36-37
: 의존성 버전 호환성을 확인해주세요.tailwindcss-preset-px-to-rem 패키지의 최신 버전과 호환성을 확인하시기 바랍니다.
또한 다음 스크립트를 실행하여 tailwind 관련 패키지들 간의 호환성을 확인해주세요:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check compatibility between tailwind-related packages # Get installed versions npm list tailwindcss tailwind-merge tailwindcss-preset-px-to-remLength of output: 167
🌐 Web query:
What is the latest version of tailwindcss-preset-px-to-rem?
💡 Result:
Based on the provided search results, there is no explicit mention of a version number for
tailwindcss-preset-px-to-rem
in any of the documentation excerpts or package listings[1][6].The most relevant information found shows:
- A GitHub discussion about px/rem conversion without mentioning specific versions[3]
- A Yarn package page describing installation/usage but no version history[1]
- Other px-to-rem conversion plugins like
tailwindcss-convert-px-to-rem
(v1.0.4)[2][6][9]For authoritative version information, you would need to check:
- The official npm registry listing
- The project's GitHub repository (if public)
- The
package.json
file in the distributed packageThe search results don't contain sufficient data to determine this package's current version as of February 2025[1][6].
Citations:
- 1: https://classic.yarnpkg.com/en/package/tailwindcss-preset-px-to-rem
- 2: https://www.npmjs.com/package/tailwindcss-convert-px-to-rem
- 3: Conversion from px to rem tailwindlabs/tailwindcss#7785
- 4: https://github.com/odestry/tailwindcss-rem-to-px
- 5: https://tailwindcss.com/docs/upgrade-guide
- 6: https://classic.yarnpkg.com/en/package/tailwindcss-convert-px-to-rem
- 7: Allow configuration for using px instead of rem tailwindlabs/tailwindcss#1232
- 8: https://tailwindcss.com/blog/tailwindcss-v4
- 9: https://github.com/eugenepro2/tailwindcss-convert-px-to-rem/blob/main/package.json
- 10: https://www.npmjs.com/search?q=keywords%3Aconvert+px+to+rem
의존성 버전 호환성 확인 결과
- 현재 설치된 tailwindcss-preset-px-to-rem 버전은 1.2.1이며, 이는 프로젝트의 [email protected]와 함께 정상적으로 동작하는 것으로 보입니다.
- 웹 검색 결과에서 최신 버전에 대한 명시적 정보는 확인되지 않았으나, 현재 사용 중인 버전으로 특별한 호환성 문제는 발견되지 않았습니다.
- tailwind-merge의 경우 package.json에
^3.0.1
로 명시되어 있으므로, 다른 tailwind 관련 패키지와의 버전 호환성에도 문제가 없어 보입니다.따라서, 현재 상태로 보아 의존성 간 호환성에 특별한 이슈는 없습니다.
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
♻️ Duplicate comments (3)
src/components/list-todo.tsx (3)
140-151
:⚠️ Potential issue에러 처리가 누락되었습니다.
useInfiniteQuery의 에러 상태와 로딩 상태에 대한 처리가 필요합니다.
다음과 같이 에러 처리를 추가해주세요:
const { data, fetchNextPage, hasNextPage, + isError, + error, + isLoading, } = useInfiniteQuery({ queryKey: ["todos", filter], queryFn: ({ pageParam = 1 }) => fetchTodos(pageParam), getNextPageParam: (lastPage) => lastPage.nextPage, initialPageParam: 1, }); +if (isLoading) return <div>로딩 중...</div>; +if (isError) return <div>에러가 발생했습니다: {error.message}</div>;
82-88
: 🛠️ Refactor suggestion체크박스의 접근성 개선이 필요합니다.
시각적으로 보이지 않는 체크박스에 대한 접근성 속성이 누락되었습니다.
다음과 같이 ARIA 속성을 추가해주세요:
<input type="checkbox" id={`todo-check-${index}`} checked={todo.status === "done"} onChange={() => toggleStatus(todo.id)} - className="peer absolute hidden" + className="peer absolute hidden" + aria-label={`할 일 완료: ${todo.title}`} />
53-67
: 🛠️ Refactor suggestion실제 API 통합을 위한 준비가 필요합니다.
현재는 목 데이터만 사용하도록 구현되어 있어, 실제 API 통합 시 수정이 필요합니다.
다음과 같이 props를 활성화하고 기본값을 설정해주세요:
-export default function ListTodo() { +export default function ListTodo({ fetchTodos = mockFetchTodos }: Partial<ListTodoProps>) { return ( - <ListTodoStructure fetchTodos={mockFetchTodos} /> + <ListTodoStructure fetchTodos={fetchTodos} /> ); }
🧹 Nitpick comments (1)
src/components/list-todo.tsx (1)
262-262
: 무한 스크롤 타겟 요소의 접근성 개선이 필요합니다.무한 스크롤의 타겟 요소에 대한 설명이 없어 스크린 리더 사용자가 이해하기 어려울 수 있습니다.
다음과 같이 설명을 추가해주세요:
-<div ref={setTarget} className="h-[.5px]" /> +<div + ref={setTarget} + className="h-[.5px]" + role="progressbar" + aria-label="할 일 목록 더 불러오는 중" +/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/layout.tsx
(2 hunks)src/app/page.tsx
(2 hunks)src/components/list-todo.tsx
(1 hunks)
🔇 Additional comments (3)
src/app/layout.tsx (1)
1-1
: 적절한 위치로 이동된 SentryCheck 컴포넌트!SentryCheck 컴포넌트를 layout으로 이동한 것은 좋은 변경사항입니다. 이는 모든 페이지에서 공통적으로 사용되는 provider의 적절한 위치입니다.
Also applies to: 21-21
src/app/page.tsx (1)
5-5
: 적절한 ListTodo 컴포넌트 구현!ListTodo 컴포넌트를 Suspense 경계 내에 배치한 것은 적절합니다. 이는 데이터 로딩 중에도 좋은 사용자 경험을 제공할 수 있게 해줍니다.
Also applies to: 29-29
src/components/list-todo.tsx (1)
13-41
: 타입 정의가 명확하고 체계적입니다!각 컴포넌트의 props 타입이 잘 분리되어 있고, 기본 타입을 확장하여 재사용성을 높인 것이 좋습니다.
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.
컴포넌트에 상태가 너무 많을 경우에는 context api나 전역 상태 관리 (zustand)를 사용하면 더 편할거에요.
아직 수정할 부분이 남아있어서, 어느정도 수정되면 Approve드릴게요
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: 1
♻️ Duplicate comments (2)
src/components/list-todo.tsx (2)
139-150
:⚠️ Potential issue에러 및 로딩 상태 처리를 추가하세요.
사용자에게 데이터 로딩 상태와 에러 상태를 표시해야 합니다.
const { data, fetchNextPage, hasNextPage, + isError, + error, + isLoading, }: InfiniteQueryObserverResult< InfiniteData<{ todos: Todo[]; nextPage?: number }> > = useInfiniteQuery({ queryKey: ["todos", filter], queryFn: ({ pageParam = 1 }) => fetchTodos(pageParam), getNextPageParam: (lastPage) => lastPage.nextPage, initialPageParam: 1, }); +if (isLoading) return <div className="text-center py-20">로딩 중...</div>; +if (isError) return <div className="text-center py-20 text-red-500">에러가 발생했습니다: {error.message}</div>;
68-97
: 🛠️ Refactor suggestion체크박스 컴포넌트의 접근성을 개선하세요.
스크린 리더 사용자를 위한 ARIA 속성이 누락되었습니다.
<input type="checkbox" id={`todo-check-${index}`} checked={todo.status === "done"} onChange={() => toggleStatus(todo.id)} className="peer absolute hidden" + aria-label={`할 일 완료: ${todo.title}`} /> <div className="flex h-20 w-20 items-center justify-center rounded-md border peer-checked:border-purple-500 peer-checked:bg-purple-500"> - <span className="absolute h-full w-full text-center text-sm font-bold text-slate-50"> + <span className="absolute h-full w-full text-center text-sm font-bold text-slate-50" aria-hidden="true"> ✓ </span> </div>
🧹 Nitpick comments (2)
src/components/list-todo.tsx (2)
13-20
: Todo 타입에 생성 및 수정 시간 필드를 추가하세요.할 일 항목의 생성 시간과 마지막 수정 시간을 추적하면 정렬 및 필터링에 유용할 수 있습니다.
export type Todo = { id: string; title: string; status: "todo" | "done"; hasNote: string | null; hasLink: boolean; hasFile: boolean; + createdAt: Date; + updatedAt: Date; };
53-67
: mockFetchTodos에 에러 시뮬레이션을 추가하세요.실제 API 호출에서 발생할 수 있는 다양한 에러 상황을 테스트하기 위해 에러 시뮬레이션이 필요합니다.
const mockFetchTodos = async (pageParam = 1) => { + // 랜덤하게 에러 발생 시뮬레이션 + if (Math.random() < 0.1) { + throw new Error("Failed to fetch todos"); + } + return new Promise<{ todos: Todo[]; nextPage?: number }>((resolve) => { setTimeout(() => { const todos: Todo[] = Array.from({ length: 40 }, (_, i) => ({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/app/layout.tsx
(2 hunks)src/app/page.tsx
(2 hunks)src/components/list-todo.tsx
(1 hunks)tailwind.config.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/layout.tsx
- tailwind.config.ts
- src/app/page.tsx
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: 3
♻️ Duplicate comments (2)
src/components/list-todo.tsx (2)
141-152
:⚠️ Potential issue쿼리 에러 처리와 로딩 상태를 추가하세요.
const { data, fetchNextPage, hasNextPage, + isError, + error, + isLoading, + isFetchingNextPage, }: InfiniteQueryObserverResult< InfiniteData<{ todos: Todo[]; nextPage?: number }> > = useInfiniteQuery({ queryKey: ["todos", filter], queryFn: ({ pageParam = 1 }) => fetchTodos(pageParam), getNextPageParam: (lastPage) => lastPage.nextPage, initialPageParam: 1, }); +if (isError) { + return ( + <div role="alert" className="text-center text-red-500"> + 데이터를 불러오는 중 오류가 발생했습니다: {error.message} + </div> + ); +}
75-98
: 🛠️ Refactor suggestion체크박스 컴포넌트의 접근성을 개선하세요.
키보드 접근성과 스크린 리더 지원을 위해 다음과 같이 수정하세요:
<div className="flex items-center gap-10"> <label htmlFor={`todo-check-${index}`} className="relative flex cursor-pointer items-center" + aria-label={`할 일 체크박스: ${todo.title}`} > <input type="checkbox" id={`todo-check-${index}`} checked={todo.status === "done"} onChange={() => toggleStatus(todo.id)} - className="peer absolute hidden" + className="peer absolute opacity-0" + aria-checked={todo.status === "done"} /> <div className="flex h-20 w-20 items-center justify-center rounded-md border peer-checked:border-purple-500 peer-checked:bg-purple-500" + role="presentation" > <span className="absolute h-full w-full text-center text-sm font-bold text-slate-50" + aria-hidden="true" > ✓ </span> </div> </label> <span className={`${todo.status === "done" ? "line-through" : ""}`} + aria-label={todo.status === "done" ? "완료된 할 일" : "진행중인 할 일"} > {todo.title} </span> </div>
🧹 Nitpick comments (2)
src/components/list-todo.tsx (2)
14-21
: 타입 정의를 개선하세요.타입 안전성을 높이기 위해 다음과 같은 개선사항을 제안합니다:
+type TodoStatus = "todo" | "done"; + export type Todo = { id: string; title: string; - status: "todo" | "done"; + status: TodoStatus; - hasNote: string | null; + hasNote: { + content: string; + createdAt: Date; + } | null; hasLink: boolean; hasFile: boolean; };
54-68
: 목 데이터 구현을 개선하세요.상수를 추출하고 더 현실적인 API 시뮬레이션을 구현하세요:
+const MOCK_CONFIG = { + ITEMS_PER_PAGE: 40, + MAX_PAGES: 3, + DELAY_MS: 500, + ERROR_RATE: 0.1, // 10% 확률로 에러 발생 +} as const; + const mockFetchTodos = async (pageParam = 1) => { return new Promise<{ todos: Todo[]; nextPage?: number }>((resolve) => { + // 실제 API와 유사한 에러 시뮬레이션 + if (Math.random() < MOCK_CONFIG.ERROR_RATE) { + throw new Error("Failed to fetch todos"); + } + setTimeout(() => { - const todos: Todo[] = Array.from({ length: 40 }, (_, i) => ({ + const todos: Todo[] = Array.from({ length: MOCK_CONFIG.ITEMS_PER_PAGE }, (_, i) => ({ id: `todo-${pageParam}-${i}`, title: `${pageParam}-${i + 1} ${getRandomNote()}`, status: Math.random() > 0.5 ? "todo" : "done", hasNote: Math.random() > 0.5 ? getRandomNote() : null, hasLink: Math.random() > 0.5, hasFile: Math.random() > 0.5, })); - resolve({ todos, nextPage: pageParam < 3 ? pageParam + 1 : undefined }); - }, 500); + resolve({ + todos, + nextPage: pageParam < MOCK_CONFIG.MAX_PAGES ? pageParam + 1 : undefined + }); + }, MOCK_CONFIG.DELAY_MS); }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.json
is excluded by!**/package-lock.json
src/assets/note.svg
is excluded by!**/*.svg
📒 Files selected for processing (2)
src/components/input.tsx
(1 hunks)src/components/list-todo.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/input.tsx
const statusMap = (["all", "todo", "done"] as const).map((status) => ( | ||
<li | ||
key={status} | ||
className={`${ | ||
status === filter ? "border-purple-500 bg-purple-500 text-slate-50" : "" | ||
} cursor-pointer rounded-3xl border`} | ||
> | ||
<button | ||
className="h-full w-full px-10 py-2" | ||
onClick={() => setFilter(status)} | ||
> | ||
{statusLabels[status]} | ||
</button> | ||
</li> | ||
)); |
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.
🛠️ Refactor suggestion
필터 버튼의 접근성과 사용자 경험을 개선하세요.
const statusMap = (["all", "todo", "done"] as const).map((status) => (
<li
key={status}
- className={`${
- status === filter ? "border-purple-500 bg-purple-500 text-slate-50" : ""
- } cursor-pointer rounded-3xl border`}
+ className="rounded-3xl"
>
<button
- className="h-full w-full px-10 py-2"
+ className={`h-full w-full px-10 py-2 rounded-3xl border transition-colors ${
+ status === filter
+ ? "border-purple-500 bg-purple-500 text-slate-50"
+ : "border-gray-300 hover:border-purple-300"
+ }`}
onClick={() => setFilter(status)}
+ aria-pressed={status === filter}
+ aria-label={`${statusLabels[status]} 할 일 보기`}
>
{statusLabels[status]}
+ {status !== "all" && (
+ <span className="ml-2 text-xs">
+ ({todos.filter(t => t.status === status).length})
+ </span>
+ )}
</button>
</li>
));
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const statusMap = (["all", "todo", "done"] as const).map((status) => ( | |
<li | |
key={status} | |
className={`${ | |
status === filter ? "border-purple-500 bg-purple-500 text-slate-50" : "" | |
} cursor-pointer rounded-3xl border`} | |
> | |
<button | |
className="h-full w-full px-10 py-2" | |
onClick={() => setFilter(status)} | |
> | |
{statusLabels[status]} | |
</button> | |
</li> | |
)); | |
const statusMap = (["all", "todo", "done"] as const).map((status) => ( | |
<li | |
key={status} | |
className="rounded-3xl" | |
> | |
<button | |
className={`h-full w-full px-10 py-2 rounded-3xl border transition-colors ${ | |
status === filter | |
? "border-purple-500 bg-purple-500 text-slate-50" | |
: "border-gray-300 hover:border-purple-300" | |
}`} | |
onClick={() => setFilter(status)} | |
aria-pressed={status === filter} | |
aria-label={`${statusLabels[status]} 할 일 보기`} | |
> | |
{statusLabels[status]} | |
{status !== "all" && ( | |
<span className="ml-2 text-xs"> | |
({todos.filter(t => t.status === status).length}) | |
</span> | |
)} | |
</button> | |
</li> | |
)); |
src/components/list-todo.tsx
Outdated
const TodoEditAndDeleteAndIcons = ({ | ||
todo, | ||
index, | ||
activeKebab, | ||
handleKebabClick, | ||
}: TodoEditAndDeleteAndIconsProps) => { | ||
return ( | ||
<div className="flex items-center gap-15"> | ||
{todo.hasLink && <p>링크</p>} | ||
{todo.hasFile && <p>파일</p>} | ||
<div className="relative"> | ||
<button className="px-3" onClick={() => handleKebabClick(index)}> | ||
⋮ | ||
</button> | ||
<div | ||
className={`${activeKebab !== index ? "hidden" : "flex"} absolute -left-70 z-10 w-80 flex-col items-center gap-10 rounded-lg bg-slate-50 p-10 shadow-md`} | ||
> | ||
<button>수정하기</button> | ||
<button>삭제하기</button> | ||
</div> | ||
</div> | ||
</div> | ||
); | ||
}; |
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.
🛠️ Refactor suggestion
수정/삭제 기능의 에러 처리와 사용자 경험을 개선하세요.
사용자 실수 방지와 에러 처리를 위해 다음과 같이 수정하세요:
const TodoEditAndDeleteAndIcons = ({
todo,
index,
activeKebab,
handleKebabClick,
+ onEdit,
+ onDelete,
}: TodoEditAndDeleteAndIconsProps) => {
+ const [isLoading, setIsLoading] = useState(false);
+
+ const handleEdit = async () => {
+ if (!window.confirm("수정하시겠습니까?")) return;
+ setIsLoading(true);
+ try {
+ await onEdit(todo.id);
+ } catch (error) {
+ console.error("수정 중 오류가 발생했습니다:", error);
+ alert("수정에 실패했습니다. 다시 시도해주세요.");
+ } finally {
+ setIsLoading(false);
+ }
+ };
+
+ const handleDelete = async () => {
+ if (!window.confirm("정말 삭제하시겠습니까?")) return;
+ setIsLoading(true);
+ try {
+ await onDelete(todo.id);
+ } catch (error) {
+ console.error("삭제 중 오류가 발생했습니다:", error);
+ alert("삭제에 실패했습니다. 다시 시도해주세요.");
+ } finally {
+ setIsLoading(false);
+ }
+ };
return (
<div className="flex items-center gap-15">
{todo.hasLink && <p>링크</p>}
{todo.hasFile && <p>파일</p>}
<div className="relative">
<button
className="px-3"
onClick={() => handleKebabClick(index)}
+ disabled={isLoading}
+ aria-label="할 일 메뉴"
>
⋮
</button>
<div
className={`${
activeKebab !== index ? "hidden" : "flex"
} absolute -left-70 z-10 w-80 flex-col items-center gap-10 rounded-lg bg-slate-50 p-10 shadow-md`}
+ role="menu"
>
- <button>수정하기</button>
- <button>삭제하기</button>
+ <button
+ onClick={handleEdit}
+ disabled={isLoading}
+ className="w-full"
+ role="menuitem"
+ >
+ {isLoading ? "처리중..." : "수정하기"}
+ </button>
+ <button
+ onClick={handleDelete}
+ disabled={isLoading}
+ className="w-full text-red-500"
+ role="menuitem"
+ >
+ {isLoading ? "처리중..." : "삭제하기"}
+ </button>
</div>
</div>
</div>
);
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const TodoEditAndDeleteAndIcons = ({ | |
todo, | |
index, | |
activeKebab, | |
handleKebabClick, | |
}: TodoEditAndDeleteAndIconsProps) => { | |
return ( | |
<div className="flex items-center gap-15"> | |
{todo.hasLink && <p>링크</p>} | |
{todo.hasFile && <p>파일</p>} | |
<div className="relative"> | |
<button className="px-3" onClick={() => handleKebabClick(index)}> | |
⋮ | |
</button> | |
<div | |
className={`${activeKebab !== index ? "hidden" : "flex"} absolute -left-70 z-10 w-80 flex-col items-center gap-10 rounded-lg bg-slate-50 p-10 shadow-md`} | |
> | |
<button>수정하기</button> | |
<button>삭제하기</button> | |
</div> | |
</div> | |
</div> | |
); | |
}; | |
const TodoEditAndDeleteAndIcons = ({ | |
todo, | |
index, | |
activeKebab, | |
handleKebabClick, | |
onEdit, | |
onDelete, | |
}: TodoEditAndDeleteAndIconsProps) => { | |
const [isLoading, setIsLoading] = useState(false); | |
const handleEdit = async () => { | |
if (!window.confirm("수정하시겠습니까?")) return; | |
setIsLoading(true); | |
try { | |
await onEdit(todo.id); | |
} catch (error) { | |
console.error("수정 중 오류가 발생했습니다:", error); | |
alert("수정에 실패했습니다. 다시 시도해주세요."); | |
} finally { | |
setIsLoading(false); | |
} | |
}; | |
const handleDelete = async () => { | |
if (!window.confirm("정말 삭제하시겠습니까?")) return; | |
setIsLoading(true); | |
try { | |
await onDelete(todo.id); | |
} catch (error) { | |
console.error("삭제 중 오류가 발생했습니다:", error); | |
alert("삭제에 실패했습니다. 다시 시도해주세요."); | |
} finally { | |
setIsLoading(false); | |
} | |
}; | |
return ( | |
<div className="flex items-center gap-15"> | |
{todo.hasLink && <p>링크</p>} | |
{todo.hasFile && <p>파일</p>} | |
<div className="relative"> | |
<button | |
className="px-3" | |
onClick={() => handleKebabClick(index)} | |
disabled={isLoading} | |
aria-label="할 일 메뉴" | |
> | |
⋮ | |
</button> | |
<div | |
className={`${ | |
activeKebab !== index ? "hidden" : "flex" | |
} absolute -left-70 z-10 w-80 flex-col items-center gap-10 rounded-lg bg-slate-50 p-10 shadow-md`} | |
role="menu" | |
> | |
<button | |
onClick={handleEdit} | |
disabled={isLoading} | |
className="w-full" | |
role="menuitem" | |
> | |
{isLoading ? "처리중..." : "수정하기"} | |
</button> | |
<button | |
onClick={handleDelete} | |
disabled={isLoading} | |
className="w-full text-red-500" | |
role="menuitem" | |
> | |
{isLoading ? "처리중..." : "삭제하기"} | |
</button> | |
</div> | |
</div> | |
</div> | |
); | |
}; |
src/components/list-todo.tsx
Outdated
return ( | ||
<div className="mx-auto min-h-[2080px] w-full max-w-2xl rounded-xl border-slate-300 bg-slate-50 p-20 text-sm text-slate-800"> | ||
<ul className="mb-20 flex gap-10">{statusMap}</ul> | ||
<ul className="space-y-15"> | ||
{todos.map((todo, index) => ( | ||
<li key={todo.id}> | ||
<div className="flex items-center justify-between"> | ||
<TodoTitleAndCheckBox | ||
index={index} | ||
todo={todo} | ||
toggleStatus={toggleStatus} | ||
/> | ||
<TodoEditAndDeleteAndIcons | ||
activeKebab={activeKebab} | ||
handleKebabClick={handleKebabClick} | ||
index={index} | ||
todo={todo} | ||
/> | ||
</div> | ||
<Note todo={todo} noteIcon={<NoteIcon />} /> | ||
</li> | ||
))} | ||
</ul> | ||
<div ref={setTarget} className="h-[.5px]" /> | ||
</div> | ||
); |
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.
🛠️ Refactor suggestion
성능 최적화와 로딩 상태를 개선하세요.
return (
<div className="mx-auto min-h-[2080px] w-full max-w-2xl rounded-xl border-slate-300 bg-slate-50 p-20 text-sm text-slate-800">
<ul className="mb-20 flex gap-10">{statusMap}</ul>
+ {isLoading ? (
+ <div className="flex justify-center py-20">
+ <div className="animate-spin rounded-full h-32 w-32 border-t-2 border-b-2 border-purple-500" />
+ </div>
+ ) : (
<ul className="space-y-15">
{todos.map((todo, index) => (
- <li key={todo.id}>
+ <li
+ key={todo.id}
+ className="animate-fade-in"
+ >
<div className="flex items-center justify-between">
<TodoTitleAndCheckBox
index={index}
todo={todo}
toggleStatus={toggleStatus}
/>
<TodoEditAndDeleteAndIcons
activeKebab={activeKebab}
handleKebabClick={handleKebabClick}
index={index}
todo={todo}
/>
</div>
<Note todo={todo} noteIcon={<NoteIcon />} />
</li>
))}
</ul>
+ )}
<div ref={setTarget} className="h-[.5px]" />
+ {isFetchingNextPage && (
+ <div className="text-center py-10 text-gray-500">
+ 추가 항목을 불러오는 중...
+ </div>
+ )}
</div>
);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
return ( | |
<div className="mx-auto min-h-[2080px] w-full max-w-2xl rounded-xl border-slate-300 bg-slate-50 p-20 text-sm text-slate-800"> | |
<ul className="mb-20 flex gap-10">{statusMap}</ul> | |
<ul className="space-y-15"> | |
{todos.map((todo, index) => ( | |
<li key={todo.id}> | |
<div className="flex items-center justify-between"> | |
<TodoTitleAndCheckBox | |
index={index} | |
todo={todo} | |
toggleStatus={toggleStatus} | |
/> | |
<TodoEditAndDeleteAndIcons | |
activeKebab={activeKebab} | |
handleKebabClick={handleKebabClick} | |
index={index} | |
todo={todo} | |
/> | |
</div> | |
<Note todo={todo} noteIcon={<NoteIcon />} /> | |
</li> | |
))} | |
</ul> | |
<div ref={setTarget} className="h-[.5px]" /> | |
</div> | |
); | |
return ( | |
<div className="mx-auto min-h-[2080px] w-full max-w-2xl rounded-xl border-slate-300 bg-slate-50 p-20 text-sm text-slate-800"> | |
<ul className="mb-20 flex gap-10">{statusMap}</ul> | |
{isLoading ? ( | |
<div className="flex justify-center py-20"> | |
<div className="animate-spin rounded-full h-32 w-32 border-t-2 border-b-2 border-purple-500" /> | |
</div> | |
) : ( | |
<ul className="space-y-15"> | |
{todos.map((todo, index) => ( | |
<li | |
key={todo.id} | |
className="animate-fade-in" | |
> | |
<div className="flex items-center justify-between"> | |
<TodoTitleAndCheckBox | |
index={index} | |
todo={todo} | |
toggleStatus={toggleStatus} | |
/> | |
<TodoEditAndDeleteAndIcons | |
activeKebab={activeKebab} | |
handleKebabClick={handleKebabClick} | |
index={index} | |
todo={todo} | |
/> | |
</div> | |
<Note todo={todo} noteIcon={<NoteIcon />} /> | |
</li> | |
))} | |
</ul> | |
)} | |
<div ref={setTarget} className="h-[.5px]" /> | |
{isFetchingNextPage && ( | |
<div className="text-center py-10 text-gray-500"> | |
추가 항목을 불러오는 중... | |
</div> | |
)} | |
</div> | |
); |
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: 1
🧹 Nitpick comments (3)
src/hooks/useInfiniteScroll.ts (3)
52-60
: 인터페이스와 훅 매개변수의 주석을 개선하세요.인터페이스와 매개변수에 대한 설명이 더 자세히 필요합니다. TypeScript의 JSDoc 주석을 사용하여 각 매개변수의 목적과 사용법을 명확하게 문서화하는 것이 좋습니다.
다음과 같이 수정해보세요:
interface useInfiniteScrollProps { + /** 다음 페이지의 존재 여부를 나타내는 플래그 */ hasNextPage: boolean | undefined; + /** 다음 페이지의 데이터를 불러오는 함수 */ fetchNextPage: () => Promise<InfiniteQueryObserverResult>; } export const useInfiniteScroll = ({ - hasNextPage, // 다음 페이지 여부 - fetchNextPage, // 다음페이지 데이터 가져오기 + hasNextPage, + fetchNextPage, }: useInfiniteScrollProps) => {
63-74
: 스크롤 이벤트 핸들링 최적화가 필요합니다.현재 구현은 잘 작동하지만, 스크롤 이벤트가 너무 자주 발생할 수 있습니다. 성능 최적화를 위해 throttle이나 debounce를 적용하는 것이 좋습니다.
lodash의 throttle을 사용하여 다음과 같이 최적화할 수 있습니다:
+import { throttle } from 'lodash'; + const handleScroll = useCallback(() => { const { scrollTop, scrollHeight, clientHeight } = document.documentElement; if ( scrollHeight - scrollTop <= clientHeight * 1.2 && hasNextPage && !isFetching ) { setIsFetching(true); fetchNextPage().finally(() => setIsFetching(false)); } -}, [hasNextPage, fetchNextPage, isFetching]); +}, [hasNextPage, fetchNextPage, isFetching]); + +const throttledHandleScroll = throttle(handleScroll, 300);
1-47
: 주석 처리된 코드를 제거하세요.이전 구현이 주석 처리되어 있습니다. 버전 관리 시스템을 사용하고 있으므로, 이전 구현은 git 히스토리에서 확인할 수 있습니다. 코드의 가독성을 위해 주석 처리된 코드는 제거하는 것이 좋습니다.
주석 처리된 코드(1-47줄)를 완전히 제거해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/assets/checkbox_off.svg
is excluded by!**/*.svg
src/assets/checkbox_on.svg
is excluded by!**/*.svg
src/assets/file.svg
is excluded by!**/*.svg
src/assets/kebab.svg
is excluded by!**/*.svg
src/assets/link.svg
is excluded by!**/*.svg
📒 Files selected for processing (4)
package.json
(1 hunks)src/components/SentryCheck.tsx
(1 hunks)src/components/list-todo.tsx
(1 hunks)src/hooks/useInfiniteScroll.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- package.json
- src/components/SentryCheck.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/list-todo.tsx
src/hooks/useInfiniteScroll.ts
Outdated
useEffect(() => { | ||
window.addEventListener("scroll", handleScroll); | ||
return () => { | ||
window.removeEventListener("scroll", handleScroll); | ||
}; | ||
}, [handleScroll]); |
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.
🛠️ Refactor suggestion
에러 처리를 추가하세요.
fetchNextPage 호출 시 발생할 수 있는 에러에 대한 처리가 없습니다. 사용자에게 적절한 피드백을 제공하고 에러 상태를 관리할 수 있도록 에러 처리를 추가하는 것이 좋습니다.
다음과 같이 에러 처리를 추가해보세요:
+ const [error, setError] = useState<Error | null>(null);
+
const handleScroll = useCallback(() => {
const { scrollTop, scrollHeight, clientHeight } = document.documentElement;
if (scrollHeight - scrollTop <= clientHeight * 1.2 && hasNextPage && !isFetching) {
setIsFetching(true);
- fetchNextPage().finally(() => setIsFetching(false));
+ fetchNextPage()
+ .catch((err) => {
+ setError(err);
+ console.error('Failed to fetch next page:', err);
+ })
+ .finally(() => setIsFetching(false));
}
}, [hasNextPage, fetchNextPage, isFetching]);
- return {};
+ return { error };
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useEffect(() => { | |
window.addEventListener("scroll", handleScroll); | |
return () => { | |
window.removeEventListener("scroll", handleScroll); | |
}; | |
}, [handleScroll]); | |
import { useState, useCallback, useEffect } from 'react'; | |
const useInfiniteScroll = () => { | |
const [error, setError] = useState<Error | null>(null); | |
// Assume other states and variables like hasNextPage, isFetching, and fetchNextPage are defined here. | |
const handleScroll = useCallback(() => { | |
const { scrollTop, scrollHeight, clientHeight } = document.documentElement; | |
if (scrollHeight - scrollTop <= clientHeight * 1.2 && hasNextPage && !isFetching) { | |
setIsFetching(true); | |
fetchNextPage() | |
.catch((err) => { | |
setError(err); | |
console.error('Failed to fetch next page:', err); | |
}) | |
.finally(() => setIsFetching(false)); | |
} | |
}, [hasNextPage, fetchNextPage, isFetching]); | |
useEffect(() => { | |
window.addEventListener("scroll", handleScroll); | |
return () => { | |
window.removeEventListener("scroll", handleScroll); | |
}; | |
}, [handleScroll]); | |
return { error }; | |
}; | |
export default useInfiniteScroll; |
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
🧹 Nitpick comments (7)
src/hooks/useInfiniteScroll.ts (2)
5-9
: 인터페이스 이름을 PascalCase로 수정하세요.TypeScript 네이밍 컨벤션에 따라 인터페이스 이름은 PascalCase를 사용해야 합니다.
-interface useIntersectionObserverProps { +interface UseIntersectionObserverProps { threshold?: number; hasNextPage: boolean | undefined; fetchNextPage: () => Promise<InfiniteQueryObserverResult>; }
27-34
: 옵저버 생성 시 에러 처리를 추가하세요.IntersectionObserver 생성이 실패할 수 있는 상황(예: 브라우저 지원 문제)에 대한 처리가 필요합니다.
useEffect(() => { if (!ref.current) return; - const observer = new IntersectionObserver(observerCallback, { - threshold, - }); + try { + const observer = new IntersectionObserver(observerCallback, { + threshold, + }); + observer.observe(ref.current); + return () => observer.disconnect(); + } catch (error) { + console.error('IntersectionObserver 생성 실패:', error); + return; + } - observer.observe(ref.current); - return () => observer.disconnect(); }, [observerCallback, threshold]);src/components/list-todo.tsx (5)
20-50
: 인터페이스에 JSDoc 문서를 추가하세요.각 인터페이스와 타입에 대한 설명을 추가하면 코드의 가독성과 유지보수성이 향상됩니다.
+/** + * 할 일 항목을 나타내는 인터페이스 + */ interface Todo { id: string; title: string; status: "todo" | "done"; hasNote: string | null; hasLink: boolean; hasFile: boolean; } +/** + * 서버 응답 데이터 형식을 정의하는 인터페이스 + */ interface TodosResponse { todos: Todo[]; nextCursor?: string; }
62-76
: 목 데이터에 에러 시나리오를 추가하세요.실제 API 호출 시 발생할 수 있는 다양한 에러 상황을 시뮬레이션하면 더 견고한 에러 처리가 가능합니다.
const mockFetchTodos = async (pageParam = 1) => { return new Promise<{ todos: Todo[]; nextPage?: number }>((resolve, reject) => { setTimeout(() => { + // 랜덤하게 에러 발생 시뮬레이션 + if (Math.random() < 0.1) { + reject(new Error('데이터 로딩 실패')); + return; + } const todos: Todo[] = Array.from({ length: 40 }, (_, i) => ({ id: `todo-${pageParam}-${i}`, title: `${pageParam}-${i + 1} ${getRandomNote()}`, status: Math.random() > 0.5 ? "todo" : "done", hasNote: Math.random() > 0.5 ? getRandomNote() : null, hasLink: Math.random() > 0.5, hasFile: Math.random() > 0.5, })); resolve({ todos, nextPage: pageParam < 3 ? pageParam + 1 : undefined }); }, 500); }); };
77-101
: 체크박스의 접근성을 개선하세요.스크린 리더 사용자를 위한 ARIA 속성을 추가하면 접근성이 향상됩니다.
<input type="checkbox" id={`todo-check-${index}`} checked={isDone} onChange={() => toggleStatus(todo.id)} className="peer absolute hidden" + aria-label={`할 일 완료: ${todo.title}`} />
164-166
: 로딩 상태 UI를 개선하세요.단순한 "Loading..." 텍스트 대신 더 시각적인 로딩 인디케이터를 사용하면 사용자 경험이 향상됩니다.
- if (isPending) return <div>Loading...</div>; + if (isPending) return ( + <div className="flex justify-center items-center min-h-[200px]"> + <div className="animate-spin rounded-full h-12 w-12 border-t-2 border-b-2 border-purple-500" /> + <span className="sr-only">로딩 중...</span> + </div> + );
167-168
: 에러 처리를 더 자세하게 구현하세요.사용자가 이해하기 쉬운 에러 메시지와 재시도 옵션을 제공하면 좋습니다.
- if (isError) return <div>Error: {error.message}</div>; + if (isError) return ( + <div className="flex flex-col items-center justify-center gap-4 p-6 text-center"> + <p className="text-red-500">데이터를 불러오는 중 문제가 발생했습니다</p> + <p className="text-sm text-gray-600">{error.message}</p> + <button + onClick={() => queryClient.invalidateQueries({ queryKey: ["todos", filter] })} + className="mt-4 rounded-md bg-purple-500 px-4 py-2 text-white hover:bg-purple-600" + > + 다시 시도 + </button> + </div> + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/list-todo.tsx
(1 hunks)src/hooks/useInfiniteScroll.ts
(1 hunks)
🔇 Additional comments (1)
src/hooks/useInfiniteScroll.ts (1)
18-25
: 구현이 잘 되었습니다!콜백 함수의 메모이제이션과 의존성 배열이 올바르게 처리되었습니다.
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.
모든 할 일 컴포넌트, 무한 스크롤 훅 구현 수고하셨습니다!!👍
파일 분리와 재사용성에 대한 부분은 아직 부족하지만, 저희가 이 기능을 사용하는데 큰 문제 없어 보이니까, 병합하셔도 될 것 같습니다.
* feat: jest, sentry, testing library setup * feat: jest, sentry, testing library setup * feat: jest, sentry, testing library setup (#30) Co-authored-by: xorms571 <[email protected]> * feat: jest, sentry, testing library setup * [DEV-32] refactor: 함수 수정 * [DEV-32] refactor: 리팩토링 * [DEV-32] refactor: pxToRem 적용 * [DEV-32] refactor: sentryCheck컴포넌트 layout으로 옮김, getQueryClient그냥 지움 * [DEV-73] feat: add tailwind config (color, screen) & add Pretendard font local * [DEV-32] refactor: 무한스크롤 기반 및 ref 수정 --------- Co-authored-by: hvrain <[email protected]> Co-authored-by: hvrain <[email protected]>
📑 작업 개요
✨ 작업 이유
📌 작업 내용
🖥️ 실행 화면
🤝🏻 해당 부분을 중점적으로 리뷰해주세요!
🎟️ 관련 이슈
close:
Summary by CodeRabbit
새로운 기능
스타일
버그 수정