[Refactor/#104] 통합 대시보드 렌더링 성능 최적화 및 태블릿 UX 개선 - #105
Conversation
📝 WalkthroughWalkthrough여러 UI 컴포넌트에 React.memo를 적용해 선언 형태를 변경하고, Drawer에 태블릿용 터치 기반 swipe-to-close 제스처를 추가했으며, 차트 컴포넌트들에 lazy 로딩·Suspense와 리팩토링(성능/상태 관리 개선)을 도입했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Drawer as DrawerPanel
participant Parent as 상위컴포넌트
participant Portal as Portal
User->>Drawer: touchstart (패널 드래그 시작)
Drawer->>Drawer: 기록 touchStartY, isDragging = true
User->>Drawer: touchmove (translate 패널)
Drawer->>Drawer: apply transform (translateY)
User->>Drawer: touchend
alt drag distance >= threshold
Drawer->>Parent: call onClose()
Parent->>Portal: close Drawer (isOpen = false)
Drawer->>Drawer: reset transform/transition
else
Drawer->>Drawer: animate back to open position
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes.Add |
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/pages/dashboard/overview/OverviewDashboard.tsx (1)
188-192: Lazy loading 적용 LGTM, 로딩 표시 고려 권장
isAiPanelOpen && <Suspense>로 패널 열릴 때만 컴포넌트를 로드하는 패턴이 적절합니다. 다만,fallback={null}로 인해 네트워크 지연 시 빈 화면이 잠시 보일 수 있습니다. 사용자 경험을 위해 로딩 스피너나 스켈레톤을 고려해 보세요.💡 선택적 개선 제안
{isAiPanelOpen && ( - <Suspense fallback={null}> + <Suspense fallback={<div className="flex items-center justify-center h-40"><span>로딩 중...</span></div>}> <OverviewAiReportPanel /> </Suspense> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/dashboard/overview/OverviewDashboard.tsx` around lines 188 - 192, The Suspense fallback currently uses null which can show a blank screen during network delay; update the Suspense that wraps OverviewAiReportPanel (the block gated by isAiPanelOpen) to render a visible loading state (e.g., a Spinner or Skeleton component) instead of null, by providing a meaningful fallback prop (for example <Suspense fallback={<Spinner/>}> or a Skeleton) and ensure any Spinner/Skeleton component is imported and matches your design system so users see feedback while OverviewAiReportPanel loads.src/components/dashboard/charts/useAnomalyMarkerPos.ts (1)
45-45: 의존성 배열에 대한 확인이 필요합니다.
containerRef는useRef로 생성된 RefObject이므로 컴포넌트 생명주기 동안 동일한 참조를 유지합니다. 따라서 이 의존성 배열은 실질적으로[]와 동일하게 동작합니다. 의도한 동작이라면 괜찮지만, 명시적으로 빈 배열[]을 사용하는 것이 의도를 더 명확히 전달할 수 있습니다.♻️ 선택적 개선 제안
- }, [containerRef]); + }, []); // containerRef는 useRef로 생성되어 안정적인 참조🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/useAnomalyMarkerPos.ts` at line 45, The dependency array for the useEffect inside useAnomalyMarkerPos currently lists containerRef, but since containerRef is a RefObject that stays stable across renders the effect behaves like one with an empty dependency array; update the dependency array to [] to make this intent explicit (or, if you actually want the effect to re-run when the ref.current changes, change the logic to track the value you care about instead), and ensure you modify the effect in useAnomalyMarkerPos that references containerRef so the hook’s behavior is clear.src/components/dashboard/platform/PlatformRoasTable.tsx (2)
85-85:will-change트리거를group-hover와 맞춰주세요.지금은 transform이
group-hover:scale-105로 행 전체 hover에서 실행되는데,will-change는 로고 자체 hover일 때만 적용됩니다. 그래서 행의 다른 영역에 hover된 경우엔 실제 transform과 최적화 힌트가 서로 다른 조건으로 동작합니다. 같은 트리거로 맞추는 편이 이번 최적화 의도와 더 일관돼요.제안 코드
- <div className="shrink-0 p-1.5 hover:will-change-transform group-hover:scale-105"> + <div className="shrink-0 p-1.5 group-hover:will-change-transform group-hover:scale-105">As per coding guidelines,
src/**:5. 성능: 불필요한 리렌더링 체크. React.memo, useCallback 사용 검토.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/platform/PlatformRoasTable.tsx` at line 85, The will-change optimization is currently tied to the element's own hover (hover:will-change-transform) while the transform is triggered by the row's hover (group-hover:scale-105); change the will-change class to use the same trigger (group-hover:will-change-transform) so the optimization hint and the actual transform share the same condition—for example update the div with className containing "shrink-0 p-1.5 hover:will-change-transform group-hover:scale-105" to use "group-hover:will-change-transform" instead, ensuring the row hover (group) drives both the will-change hint and the scale transform.
42-50:Delta의memo는 현재 구조에선 이득이 거의 없어요.상위
PlatformRoasTable가 이미 prop 없는memo컴포넌트라서 부모 리렌더가 여기까지 내려오지 않습니다. 지금 기준으로는Delta에 비교 비용과 복잡도만 추가돼서, 이 파일이 나중에 prop/state 기반으로 바뀌기 전까지는 일반 컴포넌트로 두는 편이 더 단순합니다.제안 코드
-const Delta = memo(function Delta({ value }: { value: number }) { +function Delta({ value }: { value: number }) { const isPos = value >= 0; return ( <TrendBadge direction={isPos ? "up" : "down"} value={`${Math.abs(value).toFixed(1)}%`} /> ); -}); +}As per coding guidelines,
src/**:5. 성능: 불필요한 리렌더링 체크. React.memo, useCallback 사용 검토.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/platform/PlatformRoasTable.tsx` around lines 42 - 50, Delta is wrapped with React.memo but since its parent PlatformRoasTable is already a memoized component with no changing props, the memo adds overhead without benefit; remove the memo wrapper and convert Delta back to a plain functional component (export/local const Delta = function Delta({ value }: { value: number }) { ... } or equivalent) so it renders normally, keeping the inner logic that computes isPos and returns the TrendBadge with value={`${Math.abs(value).toFixed(1)}%`} unchanged; this simplifies code and avoids unnecessary memoization overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/common/drawer/Drawer.tsx`:
- Around line 126-129: The tablet drag-handle div (the element with classes
"hidden tablet:flex justify-center pt-3 pb-1 shrink-0 cursor-grab
active:cursor-grabbing") is currently visual-only and not keyboard/screen-reader
accessible; make it focusable (tabIndex=0), give it an accessible role and label
(e.g., role="button" and aria-label="Drag to close or open drawer"), and add
keyboard handlers (onKeyDown handling Enter/Space to start/trigger close or open
and Escape to close) that call the existing drawer control methods (e.g., the
same onClose/onToggle handlers used elsewhere). Also ensure the drawer's close
control remains available on tablet (keep or render the CloseButton component on
tablet breakpoint or duplicate its accessible button into the tablet header), so
users who cannot swipe still have a visible, keyboard- and
screen-reader-accessible way to close the drawer.
---
Nitpick comments:
In `@src/components/dashboard/charts/useAnomalyMarkerPos.ts`:
- Line 45: The dependency array for the useEffect inside useAnomalyMarkerPos
currently lists containerRef, but since containerRef is a RefObject that stays
stable across renders the effect behaves like one with an empty dependency
array; update the dependency array to [] to make this intent explicit (or, if
you actually want the effect to re-run when the ref.current changes, change the
logic to track the value you care about instead), and ensure you modify the
effect in useAnomalyMarkerPos that references containerRef so the hook’s
behavior is clear.
In `@src/components/dashboard/platform/PlatformRoasTable.tsx`:
- Line 85: The will-change optimization is currently tied to the element's own
hover (hover:will-change-transform) while the transform is triggered by the
row's hover (group-hover:scale-105); change the will-change class to use the
same trigger (group-hover:will-change-transform) so the optimization hint and
the actual transform share the same condition—for example update the div with
className containing "shrink-0 p-1.5 hover:will-change-transform
group-hover:scale-105" to use "group-hover:will-change-transform" instead,
ensuring the row hover (group) drives both the will-change hint and the scale
transform.
- Around line 42-50: Delta is wrapped with React.memo but since its parent
PlatformRoasTable is already a memoized component with no changing props, the
memo adds overhead without benefit; remove the memo wrapper and convert Delta
back to a plain functional component (export/local const Delta = function
Delta({ value }: { value: number }) { ... } or equivalent) so it renders
normally, keeping the inner logic that computes isPos and returns the TrendBadge
with value={`${Math.abs(value).toFixed(1)}%`} unchanged; this simplifies code
and avoids unnecessary memoization overhead.
In `@src/pages/dashboard/overview/OverviewDashboard.tsx`:
- Around line 188-192: The Suspense fallback currently uses null which can show
a blank screen during network delay; update the Suspense that wraps
OverviewAiReportPanel (the block gated by isAiPanelOpen) to render a visible
loading state (e.g., a Spinner or Skeleton component) instead of null, by
providing a meaningful fallback prop (for example <Suspense
fallback={<Spinner/>}> or a Skeleton) and ensure any Spinner/Skeleton component
is imported and matches your design system so users see feedback while
OverviewAiReportPanel loads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 530eaa7d-fd37-464c-99be-547075f4e2cf
📒 Files selected for processing (12)
src/components/common/PageHeader.tsxsrc/components/common/card/Card.tsxsrc/components/common/card/StatCard.tsxsrc/components/common/chart/ChartLegend.tsxsrc/components/common/drawer/Drawer.tsxsrc/components/dashboard/charts/BudgetGaugeChart.tsxsrc/components/dashboard/charts/TrafficChart.tsxsrc/components/dashboard/charts/trafficChart.config.tssrc/components/dashboard/charts/useAnomalyMarkerPos.tssrc/components/dashboard/platform/PlatformRoasTable.tsxsrc/hooks/common/useIsMounted.tssrc/pages/dashboard/overview/OverviewDashboard.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/common/drawer/Drawer.tsx (1)
159-165:⚠️ Potential issue | 🟡 Minor태블릿에서 닫기 버튼이 숨겨져 있습니다
스와이프 제스처를 사용할 수 없는 사용자(운동 장애, 키보드 전용 사용자 등)가 태블릿에서 drawer를 닫을 방법이 없어요. 드래그 핸들에 키보드 지원을 추가하거나, 닫기 버튼을 태블릿에서도 유지하는 것을 권장합니다.
♿ 닫기 버튼 유지 제안
<button onClick={onClose} - className="tablet:hidden h-10 w-10 cursor-pointer rounded-component-sm hover:bg-gray-100 transition-colors flex items-center justify-center outline-none" + className="h-10 w-10 cursor-pointer rounded-component-sm hover:bg-gray-100 transition-colors flex items-center justify-center outline-none" aria-label="닫기" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/drawer/Drawer.tsx` around lines 159 - 165, The close button in Drawer.tsx is hidden on tablets via the "tablet:hidden" class which prevents keyboard-only or non-gesture users from closing the drawer; update the component so the close control remains available on tablet (remove or adjust the "tablet:hidden" rule) or add keyboard support to the drag handle (ensure the element handling onClose exposes keyboard events like Enter/Escape and has proper aria-label/role). Specifically, modify the button containing onClose and CloseIcon to be visible on tablet (remove "tablet:hidden") or implement keyboard handlers on the drag handle component (listen for keydown and call onClose) and ensure accessible attributes (aria-label, tabindex/role) are present.
🧹 Nitpick comments (1)
src/components/common/drawer/Drawer.tsx (1)
78-90: 중복 코드 정리 제안
delta > SWIPE_CLOSE_THRESHOLD조건의 if/else 양쪽 분기에서panelRef.current.style.transform = ""가 동일하게 설정되고 있어요. 조건문 밖으로 빼면 코드가 더 간결해집니다.♻️ 리팩토링 제안
const handleTouchEnd = (e: TouchEvent) => { if (!isDragging.current || !panelRef.current) return; isDragging.current = false; const delta = e.changedTouches[0].clientY - touchStartY.current; panelRef.current.style.transition = ""; + panelRef.current.style.transform = ""; if (delta > SWIPE_CLOSE_THRESHOLD) { - panelRef.current.style.transform = ""; onClose(); - } else { - panelRef.current.style.transform = ""; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/drawer/Drawer.tsx` around lines 78 - 90, handleTouchEnd contains duplicate assignment panelRef.current.style.transform = "" in both branches; move that assignment out of the if/else so it always runs after computing delta and before calling onClose. Concretely, inside handleTouchEnd (which uses isDragging.current, touchStartY, panelRef, SWIPE_CLOSE_THRESHOLD and calls onClose) set isDragging.current = false, compute delta, clear transition, then set panelRef.current.style.transform = "" once, and finally if delta > SWIPE_CLOSE_THRESHOLD call onClose().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/common/drawer/Drawer.tsx`:
- Around line 126-135: The drag handle currently uses role="slider" which is
semantically incorrect and missing required slider attributes; change the
element in the Drawer component to role="button" (or role="button" +
aria-pressed if applicable), keep the existing aria-label, ensure it remains
focusable (tabIndex={0}), and wire both onClick and keyboard handlers to invoke
the Drawer close routine (call the component's close handler such as onClose or
handleClose) when Enter or Space is pressed; remove any slider-only ARIA
attributes (aria-valuemin/aria-valuemax/aria-valuenow) so the control is
accessible and behaves like a button.
---
Duplicate comments:
In `@src/components/common/drawer/Drawer.tsx`:
- Around line 159-165: The close button in Drawer.tsx is hidden on tablets via
the "tablet:hidden" class which prevents keyboard-only or non-gesture users from
closing the drawer; update the component so the close control remains available
on tablet (remove or adjust the "tablet:hidden" rule) or add keyboard support to
the drag handle (ensure the element handling onClose exposes keyboard events
like Enter/Escape and has proper aria-label/role). Specifically, modify the
button containing onClose and CloseIcon to be visible on tablet (remove
"tablet:hidden") or implement keyboard handlers on the drag handle component
(listen for keydown and call onClose) and ensure accessible attributes
(aria-label, tabindex/role) are present.
---
Nitpick comments:
In `@src/components/common/drawer/Drawer.tsx`:
- Around line 78-90: handleTouchEnd contains duplicate assignment
panelRef.current.style.transform = "" in both branches; move that assignment out
of the if/else so it always runs after computing delta and before calling
onClose. Concretely, inside handleTouchEnd (which uses isDragging.current,
touchStartY, panelRef, SWIPE_CLOSE_THRESHOLD and calls onClose) set
isDragging.current = false, compute delta, clear transition, then set
panelRef.current.style.transform = "" once, and finally if delta >
SWIPE_CLOSE_THRESHOLD call onClose().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b68650f4-a267-48db-99f5-e014d572c1ed
📒 Files selected for processing (1)
src/components/common/drawer/Drawer.tsx
|
P4: 확인했습니다. 수고하셨어요!! |
🚨 관련 이슈
#104
✨ 변경사항
✏️ 작업 내용
😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
N/A
Summary by CodeRabbit
개선 사항
새로운 기능
성능 개선
시각/사용성 개선