[Refactor/#450] 타임라인 페이지 훅/UI 분리 - #451
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough타임라인 페이지의 기간, 패널, 편집 모달, 요약 폴링 상태를 전용 훅으로 분리했습니다. 툴바, 캔버스, 삭제 모달을 새 컴포넌트로 추가하고 Changes타임라인 페이지 리팩터링
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/hooks/timeline/useTimelinePeriod.ts (1)
5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value인터페이스 이름 오타를 고쳐주세요.
IUserTimelinePeriodParams는IUseTimelinePeriodParams의 오타로 보입니다. 훅 이름과 파라미터 타입 이름을 일치시키면 검색과 유지보수가 쉬워집니다.♻️ 제안 수정
-interface IUserTimelinePeriodParams { +interface IUseTimelinePeriodParams { scrollRef: RefObject<HTMLDivElement | null>; hasNoTimelines: boolean; } export function useTimelinePeriod({ scrollRef, hasNoTimelines, -}: IUserTimelinePeriodParams) { +}: IUseTimelinePeriodParams) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/timeline/useTimelinePeriod.ts` around lines 5 - 8, Rename the interface IUserTimelinePeriodParams to IUseTimelinePeriodParams and update all references to this parameter type so it matches the useTimelinePeriod hook naming.src/hooks/timeline/useTimelinePanel.ts (1)
17-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
panelData는 파생 값이므로useMemo로 계산하는 방식을 검토해 주세요.현재는
detail을useState+useEffect로 복제합니다. 이 패턴은 렌더가 한 번 더 발생하고, 상태 동기화 버그가 생기기 쉽습니다.useTimelineDetail의 쿼리 키에timelineId가 포함되므로, 선택한 바가 바뀌면detail은 다시undefined가 됩니다. 따라서handleBarClick의setPanelData(null)없이도 이전 패널 데이터가 남지 않습니다.♻️ 제안 리팩터
-import { useEffect, useState } from "react"; +import { useMemo, useState } from "react"; @@ const [isPanelOpen, setIsPanelOpen] = useState(false); const [selectedBarId, setSelectedBarId] = useState<number | null>(null); - const [panelData, setPanelData] = useState<ITimelineSummaryPanelData | null>( - null, - ); const { data: detail } = useTimelineDetail(selectedBarId); - useEffect(() => { - if (!detail) return; - setPanelData(buildTimelineSummaryPanel(detail)); - }, [detail]); + const panelData: ITimelineSummaryPanelData | null = useMemo( + () => (detail ? buildTimelineSummaryPanel(detail) : null), + [detail], + );
handleBarClick에서setPanelData(null)호출도 함께 제거하면 됩니다.경로 지침의 "Hook 사용: useEffect 의존성 배열 및 불필요한 사용 검토"에 따른 제안입니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/timeline/useTimelinePanel.ts` around lines 17 - 26, Replace the derived panelData useState/useEffect synchronization with a useMemo that computes buildTimelineSummaryPanel(detail) when detail exists and otherwise returns null. Update the component to use the memoized value, and remove the handleBarClick setPanelData(null) call since detail reset from useTimelineDetail now clears the derived data.Source: Path instructions
src/components/timeline/TimelineToolbar.tsx (1)
7-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value새로 추가한 파일들이 상대 경로 임포트를 사용합니다. 코딩 가이드라인은 모든 임포트에
@/alias를 요구합니다. 상대 경로는 파일 이동 시 깨지기 쉽고, 기존 코드와 스타일이 어긋납니다.
src/components/timeline/TimelineToolbar.tsx#L7-L13:./TimelineFilterSortMenus,./TimelinePeriodSelector,./TimelineStatusLegend,../common/button/Button을@/components/...경로로 바꾸세요.src/components/timeline/TimelineCanvas.tsx#L9-L11:./TimelineAxis,./TimelineBar,./TimelineGrid를@/components/timeline/...경로로 바꾸세요.src/components/timeline/TimelineDeleteModal.tsx#L1-L2:../common/modal/Modal,../common/modal/ModalContent를@/components/common/modal/...경로로 바꾸세요.src/hooks/timeline/useTimelinePanel.ts#L8:./useTimelineDetail을@/hooks/timeline/useTimelineDetail로 바꾸세요.src/hooks/timeline/useTimelineEditModal.ts#L4:./useTimelineDetail을@/hooks/timeline/useTimelineDetail로 바꾸세요.src/hooks/timeline/useTimelineSummaryPolling.ts#L4-L5:./useRequestTimelineSummary,./useTimelineDetail을@/hooks/timeline/...경로로 바꾸세요.코딩 가이드라인의 "Use
@/alias for all imports"에 따른 제안입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/timeline/TimelineToolbar.tsx` around lines 7 - 13, Replace all listed relative imports with the configured `@/` aliases: in src/components/timeline/TimelineToolbar.tsx lines 7-13, use `@/components/`... paths for TimelineFilterSortMenus, TimelinePeriodSelector, TimelineStatusLegend, and Button; in src/components/timeline/TimelineCanvas.tsx lines 9-11, use `@/components/timeline/`... for TimelineAxis, TimelineBar, and TimelineGrid; in src/components/timeline/TimelineDeleteModal.tsx lines 1-2, use `@/components/common/modal/`...; in src/hooks/timeline/useTimelinePanel.ts line 8 and useTimelineEditModal.ts line 4, use `@/hooks/timeline/useTimelineDetail`; and in src/hooks/timeline/useTimelineSummaryPolling.ts lines 4-5, use `@/hooks/timeline/`... for both imports.Source: Coding guidelines
src/hooks/timeline/useTimelineEditModal.ts (1)
36-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win로딩 토스트에 고정 ID를 사용하고 로딩 종료 시 닫아 주세요.
toast.loading(message, { id })와toast.dismiss(id)를 사용하면 중복 토스트와 잔류 토스트를 방지할 수 있습니다.isEditDetailLoading이false가 되거나editTimelineId가null이면 고정 ID의 토스트를 닫고, 컴포넌트가 해제될 때도 정리해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/timeline/useTimelineEditModal.ts` around lines 36 - 39, Update the loading toast effect in useTimelineEditModal to use a fixed toast ID with toast.loading, dismiss that ID when isEditDetailLoading becomes false or editTimelineId is null, and also dismiss it during effect cleanup on unmount to prevent duplicate or lingering toasts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/timeline/TimelineDeleteModal.tsx`:
- Around line 13-18: Assign a descriptive function name to the default-exported
component in TimelineDeleteModal, such as TimelineDeleteModal, while preserving
its existing props and behavior. Match the named default export pattern used by
TimelineToolbar and TimelineCanvas.
In `@src/hooks/timeline/useTimelineSummaryPolling.ts`:
- Around line 10-21: Reset summary polling state whenever selectedBarId changes,
including becoming null, by adding the effect in useTimelineSummaryPolling that
clears isAwaitingSummary and summaryPollStartedAt; this is the preferred
root-cause fix. In src/hooks/timeline/useTimelineSummaryPolling.ts lines 10-21,
update the hook accordingly. In src/pages/dashboard/timeline/Timeline.tsx lines
99-117, handlePanelClose requires no direct change because the hook-level reset
covers panel closing and other selection changes.
---
Nitpick comments:
In `@src/components/timeline/TimelineToolbar.tsx`:
- Around line 7-13: Replace all listed relative imports with the configured `@/`
aliases: in src/components/timeline/TimelineToolbar.tsx lines 7-13, use
`@/components/`... paths for TimelineFilterSortMenus, TimelinePeriodSelector,
TimelineStatusLegend, and Button; in src/components/timeline/TimelineCanvas.tsx
lines 9-11, use `@/components/timeline/`... for TimelineAxis, TimelineBar, and
TimelineGrid; in src/components/timeline/TimelineDeleteModal.tsx lines 1-2, use
`@/components/common/modal/`...; in src/hooks/timeline/useTimelinePanel.ts line 8
and useTimelineEditModal.ts line 4, use `@/hooks/timeline/useTimelineDetail`; and
in src/hooks/timeline/useTimelineSummaryPolling.ts lines 4-5, use
`@/hooks/timeline/`... for both imports.
In `@src/hooks/timeline/useTimelineEditModal.ts`:
- Around line 36-39: Update the loading toast effect in useTimelineEditModal to
use a fixed toast ID with toast.loading, dismiss that ID when
isEditDetailLoading becomes false or editTimelineId is null, and also dismiss it
during effect cleanup on unmount to prevent duplicate or lingering toasts.
In `@src/hooks/timeline/useTimelinePanel.ts`:
- Around line 17-26: Replace the derived panelData useState/useEffect
synchronization with a useMemo that computes buildTimelineSummaryPanel(detail)
when detail exists and otherwise returns null. Update the component to use the
memoized value, and remove the handleBarClick setPanelData(null) call since
detail reset from useTimelineDetail now clears the derived data.
In `@src/hooks/timeline/useTimelinePeriod.ts`:
- Around line 5-8: Rename the interface IUserTimelinePeriodParams to
IUseTimelinePeriodParams and update all references to this parameter type so it
matches the useTimelinePeriod hook naming.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0bc33294-43b4-4669-a50d-2da33c540ff3
📒 Files selected for processing (8)
src/components/timeline/TimelineCanvas.tsxsrc/components/timeline/TimelineDeleteModal.tsxsrc/components/timeline/TimelineToolbar.tsxsrc/hooks/timeline/useTimelineEditModal.tssrc/hooks/timeline/useTimelinePanel.tssrc/hooks/timeline/useTimelinePeriod.tssrc/hooks/timeline/useTimelineSummaryPolling.tssrc/pages/dashboard/timeline/Timeline.tsx
🚨 관련 이슈
Closed #450
✨ 변경사항
✏️ 작업 내용
Timeline.tsx에서 기간/패널/요약 폴링/수정 모달 로직을 커스텀 훅으로 분리😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
Timeline.tsx기존 약 450줄에서 대략 260줄로 코드 가독성 높였습니다Summary by CodeRabbit
새 기능
사용성 개선