[Feature/#88] 통합 대시보드 디자인 개선 및 Custom Tooltip 적용 - #96
Merged
Conversation
5 tasks
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough카드 컴포넌트 스타일 조정, StatCard에 TrendBadge 분리, BudgetGaugeChart에 상태 타입화·인사이트 메시지 추가, TrafficChart에 이상 마커 호버/툴팁과 config·위치 훅 도입, 플랫폼 ROAS 테이블 재구성 및 관련 mock/훅 추가/삭제가 적용되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Container as TrafficChart Container
participant Hook as useAnomalyMarkerPos
participant Chart as ApexCharts SVG
participant Bubble as AnomalyBubble
User->>Container: mouse move / focus
Container->>Hook: 요청 (containerRef)
Hook->>Chart: SVG에서 annotation marker 쿼리
Chart-->>Hook: marker 화면 좌표 반환
Hook-->>Container: container-local markerPos
Container->>Container: 거리 계산 → HOVER_RADIUS 비교
alt within HOVER_RADIUS
Container->>Bubble: render / show (rgba(255,165,0,0.5))
Bubble-->>User: 툴팁 표시
else outside HOVER_RADIUS
Container->>Bubble: hide
end
User->>Container: mouse leave
Container->>Bubble: hide
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 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 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 |
📚 Storybook 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/components/common/card/StatCard.tsx (1)
25-27: 접근성 레이블 방식은aria-label보다 구조적 텍스트 제공이 더 안전합니다.Line 26처럼 일반
span에aria-label만 두기보다,sr-only로 “상승/하락” 텍스트를 같이 렌더링하는 쪽이 스크린리더 호환성이 더 좋습니다.접근성 개선 예시
export function TrendBadge({ direction, value }: ITrend) { + const trendText = direction === "up" ? "상승" : "하락"; return ( <span - aria-label={`${value} ${direction === "up" ? "상승" : "하락"}`} className={twMerge( "inline-flex items-center gap-1 px-2 py-1 rounded-full font-caption w-fit", trendClasses[direction], )} > {direction === "up" ? ( <TrendUpIcon aria-hidden className="w-4 h-4" /> ) : ( <TrendDownIcon aria-hidden className="w-4 h-4" /> )} + <span className="sr-only">{trendText}</span> {value} </span> ); }As per coding guidelines, "7. 접근성: 시맨틱 HTML, ARIA 속성 사용 확인."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/card/StatCard.tsx` around lines 25 - 27, The span in the StatCard component currently relies only on an aria-label for the direction; instead render the direction text as visually-hidden content so screen readers get real DOM text (e.g., add a sibling or inside the same element a span with the existing "sr-only" / visually-hidden class containing direction === "up" ? "상승" : "하락") while keeping the visible value unchanged; update the span that currently has aria-label to include the sr-only element (or remove aria-label if redundant) so screen readers read the structured text rather than only an aria-label.src/components/dashboard/charts/TrafficChart.tsx (2)
42-67: AnomalyBubble 내용이 하드코딩되어 있습니다.현재 "구글-캠페인 A-광고 1", "부정 클릭 의심" 등의 텍스트가 하드코딩되어 있는데, API 연동 시 동적 데이터로 교체가 필요해 보입니다.
♻️ Props로 데이터 전달하는 구조 제안
-function AnomalyBubble({ x, y }: { x: number; y: number }) { +interface AnomalyBubbleProps { + x: number; + y: number; + title?: string; + description?: string; +} + +function AnomalyBubble({ + x, + y, + title = "클릭 이상 징후 감지", + description = "구글-캠페인 A-광고 1\n부정 클릭 의심" +}: AnomalyBubbleProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/TrafficChart.tsx` around lines 42 - 67, AnomalyBubble currently renders hardcoded strings; change its signature to accept props (e.g., anomaly: { title: string; detail: string } or separate props like title and detail) and use those props instead of the fixed texts ("구글-캠페인 A-광고 1", "부정 클릭 의심"); update any callers (the component that renders AnomalyBubble in TrafficChart) to pass the API-provided anomaly data so the bubble displays dynamic content and remains positioned using the existing x/y props.
102-104: 조건부 클래스 처리에twMerge또는clsx사용을 권장합니다.현재 문자열 연결로 조건부 클래스를 처리하고 있는데, 다른 파일에서는
twMerge를 사용하고 있어 일관성을 위해 동일한 방식을 사용하면 좋겠습니다.♻️ twMerge 사용 제안
+import { twMerge } from "tailwind-merge"; <div id={CHART_CONTAINER_ID} ref={containerRef} role="img" aria-label="실시간 트래픽 변화 차트: 시간대별 클릭수 추이" - className={`relative [&_.apexcharts-toolbar]:hidden${isAnomalyHovered ? " [&_.apexcharts-tooltip]:hidden!" : ""}`} + className={twMerge( + "relative [&_.apexcharts-toolbar]:hidden", + isAnomalyHovered && "[&_.apexcharts-tooltip]:hidden!" + )} onMouseMove={handleMouseMove} onMouseLeave={handleMouseLeave} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/TrafficChart.tsx` around lines 102 - 104, Replace the inline string concatenation used in the className prop of the TrafficChart component with a call to the project's class merging utility (twMerge) to match other files: import/use twMerge and build the class name by merging the static string "relative [&_.apexcharts-toolbar]:hidden" with the conditional part that depends on isAnomalyHovered so the conditional classes are applied cleanly; update the JSX element that currently uses className={`...${isAnomalyHovered ? " ...": ""}`} (the same element with onMouseMove={handleMouseMove} and onMouseLeave={handleMouseLeave}) to call twMerge instead.src/components/dashboard/charts/trafficChart.config.ts (2)
17-18: 모듈 로드 시점에 날짜가 고정되는 점 참고해 주세요.
TODAY상수가 모듈 로드 시 한 번만 평가되므로, 장기 실행 시 날짜가 업데이트되지 않습니다. 파일명 용도이므로 큰 문제는 아니지만, 필요시 함수로 래핑하여 호출 시점에 날짜를 생성할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/trafficChart.config.ts` around lines 17 - 18, TODAY is computed once at module load (const TODAY = new Date().toISOString().slice(0, 10)) so it will not update during long-running processes; change to a function (e.g., getToday()) or replace usages of TODAY with a call that computes new Date().toISOString().slice(0,10) at call time so the generated filename uses the current date; update all references to TODAY in this module (trafficChart.config related exports/functions) to call the new function.
83-97: 하드코딩된 annotation 값이 mock 데이터와 동기화되지 않을 수 있습니다.현재
x: 11,y: 53000값이 직접 하드코딩되어 있는데,trafficChartMock.clicks[ANOMALY_INDEX]값이 변경되면 annotation이 잘못된 위치에 표시될 수 있습니다.♻️ mock 데이터와 연동하는 방식 제안
+import { ANOMALY_INDEX } from "./trafficChart.config"; + // 이상 징후 위치에 빨간 점 표시 annotations: { points: [ { - x: 11, - y: 53000, + x: ANOMALY_INDEX, + y: trafficChartMock.clicks[ANOMALY_INDEX], marker: { size: 5, fillColor: "#ff4560", strokeColor: "#ff4560", strokeWidth: 1, }, }, ], },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/trafficChart.config.ts` around lines 83 - 97, The annotations block currently hardcodes x: 11 and y: 53000 which can desync from the mock; replace those literals inside the annotations.points entry in trafficChart.config.ts with values derived from trafficChartMock.clicks[ANOMALY_INDEX] (use ANOMALY_INDEX for the x position or pull the corresponding x/timestamp if your mock stores x/y pairs) and the click value for y, and add a safe fallback if the mock entry is undefined so the chart doesn't break; update the annotations object (the points array) to compute x and y from trafficChartMock and ANOMALY_INDEX rather than using hardcoded numbers.src/components/dashboard/charts/useAnomalyMarkerPos.ts (1)
35-44: 300ms 딜레이가 차트 렌더링 시간에 의존합니다.현재 하드코딩된 300ms 딜레이는 ApexCharts가 렌더링을 완료하기를 기다리는 것으로 보입니다. 네트워크 지연이나 저사양 기기에서는 이 시간이 충분하지 않을 수 있습니다.
더 안정적인 방법으로는
MutationObserver를 사용하여.apexcharts-point-annotation-marker요소가 DOM에 추가될 때 위치를 계산하거나,BASE_OPTIONS.chart.events.mounted콜백을 활용하는 방안이 있습니다.♻️ MutationObserver 활용 예시
useEffect(() => { - const timer = setTimeout(updateMarkerPos, 300); + // 마커 요소가 DOM에 추가될 때까지 관찰 + const mutationObserver = new MutationObserver(() => { + if (containerRef.current?.querySelector(".apexcharts-point-annotation-marker")) { + updateMarkerPos(); + mutationObserver.disconnect(); + } + }); + if (containerRef.current) { + mutationObserver.observe(containerRef.current, { childList: true, subtree: true }); + } + const observer = new ResizeObserver(updateMarkerPos); if (containerRef.current) observer.observe(containerRef.current); + return () => { - clearTimeout(timer); + mutationObserver.disconnect(); observer.disconnect(); }; }, [updateMarkerPos, containerRef]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/useAnomalyMarkerPos.ts` around lines 35 - 44, The useEffect in useAnomalyMarkerPos currently waits a hardcoded 300ms before calling updateMarkerPos which is brittle; replace the timeout approach by observing DOM mutations or ApexCharts mount event: use a MutationObserver on containerRef.current that watches for additions of elements matching ".apexcharts-point-annotation-marker" and call updateMarkerPos when they appear (and disconnect the observer in the cleanup), or alternatively invoke updateMarkerPos from BASE_OPTIONS.chart.events.mounted (or chart instance mounted callback) so the position is calculated exactly when charts finish rendering; keep the existing ResizeObserver (observer.observe(containerRef.current)) but remove the setTimeout and ensure both observers are properly disconnected in the return cleanup.src/components/dashboard/platform/PlatformRoasTable.tsx (1)
40-54: 테이블 구조의 접근성 개선을 고려해 주세요.현재
div기반의 그리드 레이아웃으로 테이블을 구현하고 있는데, 스크린 리더 사용자를 위해 테이블 관련 ARIA 속성을 추가하면 좋겠습니다.♻️ ARIA 테이블 역할 추가 제안
- <div className="flex flex-col h-full font-pretendard w-full"> - <div className="flex flex-col flex-1 min-w-0"> + <div className="flex flex-col h-full font-pretendard w-full" role="table" aria-label="플랫폼별 ROAS 비교"> + <div className="flex flex-col flex-1 min-w-0" role="rowgroup"> {/* 헤더 */} <div - className={`grid ${COL} gap-x-4 px-4 pt-2 pb-4 font-caption text-[`#8B95A1`] font-medium tracking-wider uppercase border-b border-[`#F2F4F6`]`} + role="row" + className={`grid ${COL} gap-x-4 px-4 pt-2 pb-4 font-caption text-[`#8B95A1`] font-medium tracking-wider uppercase border-b border-[`#F2F4F6`]`} > - <span className="text-center">#</span> + <span role="columnheader" className="text-center">#</span>As per coding guidelines: "접근성: 시맨틱 HTML, ARIA 속성 사용 확인"
🤖 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 40 - 54, PlatformRoasTable currently renders a visual grid with divs (using the COL constant) but lacks semantic table/ARIA roles; update the component to expose proper table semantics by adding role="table" and an accessible name (aria-label or aria-labelledby) to the outer container, wrap header group with role="rowgroup" and the header row div with role="row", and mark each header cell spans as role="columnheader" (or add scope="col" if you convert to actual <th> later); similarly ensure data rows use role="row" and cells use role="cell" so screen readers can interpret the structure (make these changes in PlatformRoasTable around the header div and the corresponding row/cell elements that use COL).src/pages/dashboard/overview/OverviewDashboard.tsx (1)
45-55:useMemo의존성 배열에 대한 lint 경고 가능성이 있습니다.
budgetGaugeChartMock이 상수이므로 현재 로직은 정확하지만, ESLintreact-hooks/exhaustive-deps규칙에서 경고가 발생할 수 있습니다. 의도적인 최적화임을 명시하거나, 상수 참조를 의존성에 추가하는 것을 고려해 주세요.♻️ 의도 명시 또는 의존성 추가
const budgetStatusBadge = useMemo(() => { const { totalBudget, spent, warningThreshold, dangerThreshold } = budgetGaugeChartMock; const pct = totalBudget > 0 ? Math.round((spent / totalBudget) * 100) : 0; const status = getBudgetStatus(pct, warningThreshold, dangerThreshold); return ( <Badge variant={statusBadgeVariant[status]} size="sm" className="px-2"> {status} </Badge> ); -}, []); +}, []); // budgetGaugeChartMock is a module-level constant또는 API 연동 시 실제 데이터를 사용하게 되면 적절한 의존성을 추가:
}, [budgetData]); // 실제 API 데이터로 교체 시🤖 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 45 - 55, The useMemo for budgetStatusBadge currently has an empty dependency array which may trigger react-hooks/exhaustive-deps lint warnings; update the dependency array to include the values used (e.g., budgetGaugeChartMock or its fields) or explicitly document intent by adding an eslint comment. Concretely, modify the useMemo call (budgetStatusBadge) to use dependencies like [budgetGaugeChartMock] or [budgetGaugeChartMock.totalBudget, budgetGaugeChartMock.spent, budgetGaugeChartMock.warningThreshold, budgetGaugeChartMock.dangerThreshold], or if this is intentionally constant, add a single-line // eslint-disable-next-line react-hooks/exhaustive-deps above the useMemo to suppress the warning and keep getBudgetStatus and statusBadgeVariant usage unchanged.
🤖 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/dashboard/platform/PlatformRoasTable.tsx`:
- Around line 16-17: The calculation for maxRoas in PlatformRoasTable.tsx can
produce 0 or -Infinity (if platformRoasRanking is empty) leading to division by
zero/NaN; update the logic that computes maxRoas (the
Math.max(...platformRoasRanking.map((p) => p.roas)) expression) to safely handle
empty arrays and zero values by providing a sensible fallback (e.g., treat empty
or all-zero results as 1) before using it for ratio/division operations; ensure
any downstream usage of maxRoas (in bar width/percentage calculations) uses this
guarded value.
---
Nitpick comments:
In `@src/components/common/card/StatCard.tsx`:
- Around line 25-27: The span in the StatCard component currently relies only on
an aria-label for the direction; instead render the direction text as
visually-hidden content so screen readers get real DOM text (e.g., add a sibling
or inside the same element a span with the existing "sr-only" / visually-hidden
class containing direction === "up" ? "상승" : "하락") while keeping the visible
value unchanged; update the span that currently has aria-label to include the
sr-only element (or remove aria-label if redundant) so screen readers read the
structured text rather than only an aria-label.
In `@src/components/dashboard/charts/trafficChart.config.ts`:
- Around line 17-18: TODAY is computed once at module load (const TODAY = new
Date().toISOString().slice(0, 10)) so it will not update during long-running
processes; change to a function (e.g., getToday()) or replace usages of TODAY
with a call that computes new Date().toISOString().slice(0,10) at call time so
the generated filename uses the current date; update all references to TODAY in
this module (trafficChart.config related exports/functions) to call the new
function.
- Around line 83-97: The annotations block currently hardcodes x: 11 and y:
53000 which can desync from the mock; replace those literals inside the
annotations.points entry in trafficChart.config.ts with values derived from
trafficChartMock.clicks[ANOMALY_INDEX] (use ANOMALY_INDEX for the x position or
pull the corresponding x/timestamp if your mock stores x/y pairs) and the click
value for y, and add a safe fallback if the mock entry is undefined so the chart
doesn't break; update the annotations object (the points array) to compute x and
y from trafficChartMock and ANOMALY_INDEX rather than using hardcoded numbers.
In `@src/components/dashboard/charts/TrafficChart.tsx`:
- Around line 42-67: AnomalyBubble currently renders hardcoded strings; change
its signature to accept props (e.g., anomaly: { title: string; detail: string }
or separate props like title and detail) and use those props instead of the
fixed texts ("구글-캠페인 A-광고 1", "부정 클릭 의심"); update any callers (the component
that renders AnomalyBubble in TrafficChart) to pass the API-provided anomaly
data so the bubble displays dynamic content and remains positioned using the
existing x/y props.
- Around line 102-104: Replace the inline string concatenation used in the
className prop of the TrafficChart component with a call to the project's class
merging utility (twMerge) to match other files: import/use twMerge and build the
class name by merging the static string "relative
[&_.apexcharts-toolbar]:hidden" with the conditional part that depends on
isAnomalyHovered so the conditional classes are applied cleanly; update the JSX
element that currently uses className={`...${isAnomalyHovered ? " ...": ""}`}
(the same element with onMouseMove={handleMouseMove} and
onMouseLeave={handleMouseLeave}) to call twMerge instead.
In `@src/components/dashboard/charts/useAnomalyMarkerPos.ts`:
- Around line 35-44: The useEffect in useAnomalyMarkerPos currently waits a
hardcoded 300ms before calling updateMarkerPos which is brittle; replace the
timeout approach by observing DOM mutations or ApexCharts mount event: use a
MutationObserver on containerRef.current that watches for additions of elements
matching ".apexcharts-point-annotation-marker" and call updateMarkerPos when
they appear (and disconnect the observer in the cleanup), or alternatively
invoke updateMarkerPos from BASE_OPTIONS.chart.events.mounted (or chart instance
mounted callback) so the position is calculated exactly when charts finish
rendering; keep the existing ResizeObserver
(observer.observe(containerRef.current)) but remove the setTimeout and ensure
both observers are properly disconnected in the return cleanup.
In `@src/components/dashboard/platform/PlatformRoasTable.tsx`:
- Around line 40-54: PlatformRoasTable currently renders a visual grid with divs
(using the COL constant) but lacks semantic table/ARIA roles; update the
component to expose proper table semantics by adding role="table" and an
accessible name (aria-label or aria-labelledby) to the outer container, wrap
header group with role="rowgroup" and the header row div with role="row", and
mark each header cell spans as role="columnheader" (or add scope="col" if you
convert to actual <th> later); similarly ensure data rows use role="row" and
cells use role="cell" so screen readers can interpret the structure (make these
changes in PlatformRoasTable around the header div and the corresponding
row/cell elements that use COL).
In `@src/pages/dashboard/overview/OverviewDashboard.tsx`:
- Around line 45-55: The useMemo for budgetStatusBadge currently has an empty
dependency array which may trigger react-hooks/exhaustive-deps lint warnings;
update the dependency array to include the values used (e.g.,
budgetGaugeChartMock or its fields) or explicitly document intent by adding an
eslint comment. Concretely, modify the useMemo call (budgetStatusBadge) to use
dependencies like [budgetGaugeChartMock] or [budgetGaugeChartMock.totalBudget,
budgetGaugeChartMock.spent, budgetGaugeChartMock.warningThreshold,
budgetGaugeChartMock.dangerThreshold], or if this is intentionally constant, add
a single-line // eslint-disable-next-line react-hooks/exhaustive-deps above the
useMemo to suppress the warning and keep getBudgetStatus and statusBadgeVariant
usage unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1cdad15a-c1a1-4e89-a69f-b787ea9afc09
⛔ Files ignored due to path filters (3)
.claude/settings.jsonis excluded by none and included by none.gitignoreis excluded by none and included by nonesrc/assets/icon/common/alert-circle.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (12)
src/components/common/card/Card.tsxsrc/components/common/card/StatCard.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/PlatformComparison.tsxsrc/components/dashboard/platform/PlatformComparisonChart.tsxsrc/components/dashboard/platform/PlatformRoasTable.tsxsrc/components/dashboard/platform/platformComparison.mock.tssrc/hooks/common/useIsMounted.tssrc/pages/dashboard/overview/OverviewDashboard.tsx
💤 Files with no reviewable changes (2)
- src/components/dashboard/platform/PlatformComparison.tsx
- src/components/dashboard/platform/PlatformComparisonChart.tsx
📚 Storybook 배포 완료
|
📚 Storybook 배포 완료
|
Collaborator
|
P3: 정보들이 변경된 디자인에서 훨씬 한눈에 잘 보이는 것 같습니다! |
YermIm
approved these changes
Mar 13, 2026
jjjsun
reviewed
Mar 13, 2026
Collaborator
Author
넵! 추후 태블릿 기준 반응형 적용 시에 적용하겠습니다! |
Collaborator
제 로컬에서는 깨지는 것 같습니다! |
Collaborator
제 로컬에서도 깨지는것같습니다! |
Collaborator
Author
모바일 반응형은 우선순위가 낮아 추후 일정에 따라 진행 예정입니다. 현재는 데스크탑·태블릿 기준 반응형만 검토 부탁드립니다. |
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/components/dashboard/platform/PlatformRoasTable.tsx (3)
16-17: API 연동 시maxRoas계산을 컴포넌트 내부로 이동해야 합니다.현재
maxRoas가 모듈 레벨에서 계산되어 있어서, mock 데이터 기준으로는 문제없지만 API 연동 후에는 데이터가 변경되어도maxRoas가 갱신되지 않습니다.♻️ API 연동 대비 구조 제안
-// 바 차트 비율 계산용 최대 ROAS -const maxRoas = Math.max(...platformRoasRanking.map((p) => p.roas)); export default function PlatformRoasTable() { + // API 연동 후: const { data: platformRoasRanking } = useQuery(...) + const maxRoas = useMemo( + () => Math.max(...platformRoasRanking.map((p) => p.roas), 1), + [platformRoasRanking] + ); + return (Also applies to: 41-42
🤖 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 16 - 17, The module-level constant maxRoas is computed from platformRoasRanking at import time, so it won't update after API data changes; move the calculation into the PlatformRoasTable component (e.g., inside the component body or a useMemo) and reference platformRoasRanking prop/state there so maxRoas is recomputed whenever platformRoasRanking changes; update the other occurrences noted (lines ~41-42) the same way to ensure all ROAS-derived values depend on current props/state rather than a frozen module-level value.
27-35:value === 0일 때 중립 상태 표시를 고려해보세요.현재
value >= 0이면 상승(up)으로 표시되어, 0% 변화도 파란색 상승 화살표로 렌더링됩니다. 사용자에게 "변화 없음"을 명확히 전달하려면 중립 상태를 별도로 처리하는 것이 좋을 것 같아요.♻️ 중립 상태 처리 제안
function Delta({ value }: { value: number }) { - const isPos = value >= 0; + if (value === 0) { + return <span className="text-text-disabled font-caption">-</span>; + } + const isPos = value > 0; return ( <TrendBadge direction={isPos ? "up" : "down"} value={`${Math.abs(value).toFixed(1)}%`} /> ); }🤖 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 27 - 35, The Delta component treats value === 0 as "up"; update it to explicitly handle neutral/flat values by using a three-way condition and passing a neutral direction to TrendBadge (e.g., direction={value > 0 ? "up" : value < 0 ? "down" : "neutral"}), keeping the displayed value as `${Math.abs(value).toFixed(1)}%` (or `"0.0%"`) so 0% renders a clear neutral state; adjust the TrendBadge usage if it expects a different neutral token (e.g., "flat" or undefined) accordingly.
43-61: 테이블 구조에 접근성(a11y) 속성 추가를 권장드려요.현재
div+grid기반으로 테이블을 구현하셨는데, 스크린 리더 사용자에게는 테이블 구조로 인식되지 않습니다. 코딩 가이드라인에서 시맨틱 HTML 및 ARIA 속성 사용을 권장하고 있어요.ARIA role을 추가하면 현재 스타일을 유지하면서 접근성을 개선할 수 있습니다:
♿ ARIA role 적용 제안
- <div className="@container flex flex-col h-full font-pretendard w-full"> + <div className="@container flex flex-col h-full font-pretendard w-full" role="table" aria-label="플랫폼별 ROAS 비교"> <div className="flex flex-col flex-1 min-w-0"> {/* 헤더 */} <div - className={`grid ${COL} ...`} + className={`grid ${COL} ...`} + role="row" > - <span className="text-center">순위</span> + <span className="text-center" role="columnheader">순위</span> ... </div> - <div className="flex flex-col pb-2 divide-y divide-[`#F2F4F6`]"> + <div className="flex flex-col pb-2 divide-y divide-[`#F2F4F6`]" role="rowgroup"> {platformRoasRanking.map((platform, index) => ( <div key={platform.name} - className={`group grid ${COL} ...`} + className={`group grid ${COL} ...`} + role="row" > + {/* 각 셀에 role="cell" 추가 */}As per coding guidelines, 시맨틱 HTML, ARIA 속성 사용 확인이 필요합니다.
🤖 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 43 - 61, The grid-based table lacks semantic/ARIA roles, so update PlatformRoasTable: add role="table" and an accessible aria-label or aria-labelledby to the outer container div (the one with class "@container flex..."), mark the header div (the one with class including `${COL} gap-x-4 ...`) as role="rowgroup" and change each header span to role="columnheader" (add aria-sort where relevant), mark the body wrapper (the div with "flex flex-col pb-2 divide-y...") as role="rowgroup", and for each mapped row inside platformRoasRanking (the div created in map with `key={platform.name}` and `className="group grid..."`) add role="row" and change each data span inside that row to role="cell"; ensure keyboard focus if rows are interactive by adding tabIndex and appropriate aria-selected/aria-pressed attributes as needed.src/components/dashboard/charts/trafficChart.config.ts (1)
121-124:BASE_OPTIONS를 데이터 의존 팩토리로 분리하는 게 안전합니다.Line [123]이 mock 데이터에 고정되어 있어 API 데이터 연동 시 y축 스케일이 실제 시리즈와 어긋날 가능성이 큽니다.
createTrafficChartOptions(clicks)형태로 분리해 주세요.
As per coding guidelines,src/**: "2. 구조와 책임 분리: 페이지에 비즈니스 로직이 과도하지 않은지 확인. 커스텀 훅으로의 분리 여부 검토."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/trafficChart.config.ts` around lines 121 - 124, The BASE_OPTIONS currently hardcodes yaxis max using trafficChartMock.clicks which will misalign when real API data is used; refactor BASE_OPTIONS into a factory function createTrafficChartOptions(clicks) that accepts the clicks series and computes yaxis.max as Math.ceil(Math.max(...clicks) / 10000) * 10000 (preserve min: 0 and tickAmount: 6), update any usages to call createTrafficChartOptions(series) instead of referencing BASE_OPTIONS, and keep other base properties intact so the chart scales correctly for dynamic data.src/components/dashboard/charts/BudgetGaugeChart.tsx (2)
66-85: 기간 진행률 기준 시각은 상위에서 내려받는 쪽이 더 안정적입니다.이 카드 안에서
new Date()로 기간 진행률을 계산하면, 대시보드 헤더의 기준 시각이나 추후 API 기준일과 쉽게 분리됩니다. 히스토리 조회, 타임존 차이, 자정 이후 재렌더 같은 경우에 같은 화면 안에서 서로 다른 날짜 기준이 섞일 수 있어서referenceDate나 이미 계산된 기간 정보(elapsedDays,totalDays)를 prop으로 받는 편이 좋아 보입니다.Also applies to: 155-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/BudgetGaugeChart.tsx` around lines 66 - 85, The logic that computes periodElapsedRate/percentage inside BudgetGaugeChart should not call new Date() locally; instead accept a stable referenceDate or precomputed elapsedDays/totalDays via props and use those to derive periodElapsedRate and percentage so all cards share the same time basis. Update the BudgetGaugeChart component signature to add either referenceDate or elapsedDays/totalDays props, remove any local new Date() usage that feeds periodElapsedRate, and use the incoming values when evaluating isOverBudget, dangerThreshold, warningThreshold and building insightDesc so the conditions and messages remain consistent across the dashboard.
18-25: 상태 계산 기준을 한 곳으로 모아두는 편이 안전해 보여요.지금은
getBudgetStatus()가 절대 소진율만 보고,insightDesc는 기간 진행률까지 같이 보고 있습니다. 이 구조면 월초처럼percentage는 아직 낮지만periodElapsedRate + 15를 넘는 케이스에서 진행 바/배지는안정인데 하단 문구는 경고로 나올 수 있어요.status와insightDesc를 같은 평가 함수에서 함께 계산하면 이런 엇갈림을 막기 쉽습니다.정리 예시
- const status = getBudgetStatus(percentage, warningThreshold, dangerThreshold); - - let insightDesc = ""; - if (isOverBudget) { - ... - } else if (...) { - ... - } + const { status, insightDesc } = evaluateBudgetState({ + percentage, + periodElapsedRate, + warningThreshold, + dangerThreshold, + isOverBudget, + });Also applies to: 64-85, 123-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/BudgetGaugeChart.tsx` around lines 18 - 25, Unify the status and description calculation so both use the same rules: replace separate logic in getBudgetStatus and wherever insightDesc is computed by creating/adjusting a single function (e.g., getBudgetStatus or a new getBudgetEvaluation) that accepts percentage and periodElapsedRate plus warningThreshold and dangerThreshold and returns a consistent TBudgetStatus and the insight description together; update callers to use this single result so the gauge/label and the insight text never diverge (affects getBudgetStatus and the locations computing insightDesc around the other referenced blocks).
🤖 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/dashboard/charts/BudgetGaugeChart.tsx`:
- Around line 90-93: The progress bar(s) in BudgetGaugeChart lack accessible
names; update the progress element(s) in the BudgetGaugeChart component to
include an accessible name by either adding aria-label="이번 달 사용 예산 소진율" (or a
similar descriptive label) or by giving the title span a unique id and
referencing it from the progress with aria-labelledby (apply the same change for
the second progress instance around lines 107-112); ensure the progress elements
(the bar element(s) in BudgetGaugeChart) receive the new aria attribute so
screen readers announce what the value represents.
In `@src/components/dashboard/charts/trafficChart.config.ts`:
- Line 15: ANOMALY_INDEX is hardcoded in trafficChart.config.ts which decouples
marker position from the data and will point to the wrong place when the dataset
changes; remove the exported ANOMALY_INDEX constant and instead compute the
anomaly marker index from the actual chart data where markers are rendered
(e.g., replace usages of ANOMALY_INDEX with a runtime lookup such as
data.findIndex(d => d.isAnomaly) or by matching a timestamp/id field), or expose
the anomaly index alongside the data payload so the marker uses that value;
update any code referencing ANOMALY_INDEX to use the computed/index-from-data
value (search for ANOMALY_INDEX and the marker-rendering logic in
trafficChart.config.ts and the chart component).
In `@src/components/dashboard/charts/TrafficChart.tsx`:
- Around line 104-105: The chart currently only exposes hover handlers
(handleMouseMove, handleMouseLeave) which blocks keyboard and touch users from
seeing anomaly info; update TrafficChart to add keyboard and pointer
accessibility paths by: make the interactive element focusable (add tabindex and
appropriate role/aria attributes), wire onFocus to the same logic as
handleMouseMove (or a new handleFocus) and onBlur to handleMouseLeave (or
handleBlur), and add onPointerMove/onPointerLeave handlers that reuse
handleMouseMove/handleMouseLeave so touch/pointer devices receive the same
feedback; ensure any dynamic text uses aria-describedby or an aria-live region
so screen readers announce the anomaly details.
In `@src/components/dashboard/platform/PlatformRoasTable.tsx`:
- Around line 70-72: When rendering platform logos in PlatformRoasTable,
accessing platformLogoMap[platform.name] is unsafe and can render undefined for
unknown platforms; update the render to use a type-guarded lookup and a
fallback: check whether platform.name exists as a key in platformLogoMap (or use
Object.prototype.hasOwnProperty.call) and if not render a default/fallback logo
or placeholder component (e.g., a generic icon or initials) instead of
undefined; ensure this logic is applied where platformLogoMap and platform.name
are used in the component so new/unknown platform names do not produce empty UI.
---
Nitpick comments:
In `@src/components/dashboard/charts/BudgetGaugeChart.tsx`:
- Around line 66-85: The logic that computes periodElapsedRate/percentage inside
BudgetGaugeChart should not call new Date() locally; instead accept a stable
referenceDate or precomputed elapsedDays/totalDays via props and use those to
derive periodElapsedRate and percentage so all cards share the same time basis.
Update the BudgetGaugeChart component signature to add either referenceDate or
elapsedDays/totalDays props, remove any local new Date() usage that feeds
periodElapsedRate, and use the incoming values when evaluating isOverBudget,
dangerThreshold, warningThreshold and building insightDesc so the conditions and
messages remain consistent across the dashboard.
- Around line 18-25: Unify the status and description calculation so both use
the same rules: replace separate logic in getBudgetStatus and wherever
insightDesc is computed by creating/adjusting a single function (e.g.,
getBudgetStatus or a new getBudgetEvaluation) that accepts percentage and
periodElapsedRate plus warningThreshold and dangerThreshold and returns a
consistent TBudgetStatus and the insight description together; update callers to
use this single result so the gauge/label and the insight text never diverge
(affects getBudgetStatus and the locations computing insightDesc around the
other referenced blocks).
In `@src/components/dashboard/charts/trafficChart.config.ts`:
- Around line 121-124: The BASE_OPTIONS currently hardcodes yaxis max using
trafficChartMock.clicks which will misalign when real API data is used; refactor
BASE_OPTIONS into a factory function createTrafficChartOptions(clicks) that
accepts the clicks series and computes yaxis.max as
Math.ceil(Math.max(...clicks) / 10000) * 10000 (preserve min: 0 and tickAmount:
6), update any usages to call createTrafficChartOptions(series) instead of
referencing BASE_OPTIONS, and keep other base properties intact so the chart
scales correctly for dynamic data.
In `@src/components/dashboard/platform/PlatformRoasTable.tsx`:
- Around line 16-17: The module-level constant maxRoas is computed from
platformRoasRanking at import time, so it won't update after API data changes;
move the calculation into the PlatformRoasTable component (e.g., inside the
component body or a useMemo) and reference platformRoasRanking prop/state there
so maxRoas is recomputed whenever platformRoasRanking changes; update the other
occurrences noted (lines ~41-42) the same way to ensure all ROAS-derived values
depend on current props/state rather than a frozen module-level value.
- Around line 27-35: The Delta component treats value === 0 as "up"; update it
to explicitly handle neutral/flat values by using a three-way condition and
passing a neutral direction to TrendBadge (e.g., direction={value > 0 ? "up" :
value < 0 ? "down" : "neutral"}), keeping the displayed value as
`${Math.abs(value).toFixed(1)}%` (or `"0.0%"`) so 0% renders a clear neutral
state; adjust the TrendBadge usage if it expects a different neutral token
(e.g., "flat" or undefined) accordingly.
- Around line 43-61: The grid-based table lacks semantic/ARIA roles, so update
PlatformRoasTable: add role="table" and an accessible aria-label or
aria-labelledby to the outer container div (the one with class "@container
flex..."), mark the header div (the one with class including `${COL} gap-x-4
...`) as role="rowgroup" and change each header span to role="columnheader" (add
aria-sort where relevant), mark the body wrapper (the div with "flex flex-col
pb-2 divide-y...") as role="rowgroup", and for each mapped row inside
platformRoasRanking (the div created in map with `key={platform.name}` and
`className="group grid..."`) add role="row" and change each data span inside
that row to role="cell"; ensure keyboard focus if rows are interactive by adding
tabIndex and appropriate aria-selected/aria-pressed attributes as needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5565e823-9a68-4708-9bbe-bba2f88ea4ca
📒 Files selected for processing (5)
src/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.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/dashboard/charts/useAnomalyMarkerPos.ts
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/dashboard/charts/BudgetGaugeChart.tsx (1)
48-65:⚠️ Potential issue | 🟡 Minor퍼센트/임계값 경계값 방어가 부족해서 시각 표현이 깨질 수 있어요.
percentage가 음수일 때 Line 130의scaleX(...)가 음수가 되어 바가 반전 렌더링될 수 있고, 임계값도 0~100 범위를 벗어나면 마커 위치가 카드 밖으로 나갈 수 있습니다. 시각화용 값은 한 번 정규화(clamp)해서 재사용하는 게 안전합니다.경계값 방어 예시
const percentage = totalBudget > 0 ? Math.round((spent / totalBudget) * 100) : 0; + const normalizedPercentage = Math.min(Math.max(percentage, 0), 100); + const normalizedWarning = Math.min(Math.max(warningThreshold, 0), 100); + const normalizedDanger = Math.min(Math.max(dangerThreshold, 0), 100); - const status = getBudgetStatus(percentage, warningThreshold, dangerThreshold); + const status = getBudgetStatus( + normalizedPercentage, + normalizedWarning, + normalizedDanger, + ); <div className="absolute top-0 bottom-0 w-0.5 bg-white/60 z-10" - style={{ left: `${warningThreshold}%` }} + style={{ left: `${normalizedWarning}%` }} /> <div className="absolute top-0 bottom-0 w-0.5 bg-white/60 z-10" - style={{ left: `${dangerThreshold}%` }} + style={{ left: `${normalizedDanger}%` }} /> ... - transform: `scaleX(${mounted ? Math.min(percentage, 100) / 100 : 0})`, + transform: `scaleX(${mounted ? normalizedPercentage / 100 : 0})`, }}Also applies to: 106-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/BudgetGaugeChart.tsx` around lines 48 - 65, Clamp all visualization values before use: ensure percentage is bounded to 0–100 (use the computed percentage from spent/totalBudget but clamp negatives and >100), clamp warningThreshold and dangerThreshold into 0–100 and handle swapped/out-of-order thresholds, guard periodTotalDays from zero and clamp periodElapsedRate to 0–100, and compute remaining/isOverBudget independently of clamped display values; update usages of percentage, periodElapsedRate, warningThreshold and dangerThreshold (and any place that calls scaleX(...) with percentage) to use the clamped variants so markers and bars never render outside the card or invert.
♻️ Duplicate comments (1)
src/components/dashboard/platform/PlatformRoasTable.tsx (1)
29-31:⚠️ Potential issue | 🟡 Minor
maxRoas분모 0 케이스를 가드해 주세요.Line 30 계산값이 0이면 Line 100에서
NaN%너비가 만들어져 ROAS 바가 깨질 수 있습니다(실 API에서 전부 0인 데이터일 때 재현됩니다).수정 제안
-const maxRoas = Math.max(...platformRoasRanking.map((p) => p.roas)); +const maxRoas = Math.max(...platformRoasRanking.map((p) => p.roas), 1);Also applies to: 99-101
🤖 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 29 - 31, Guard against a zero denominator when computing bar widths: in PlatformRoasTable replace the current maxRoas calculation using Math.max(...platformRoasRanking.map(p => p.roas)) with a safe max that never equals 0 (for example Math.max(...platformRoasRanking.map(p => p.roas), 1) or conditionally set to 1 if result is 0), and update any width calculations that use (p.roas / maxRoas) * 100 so they rely on this non-zero maxRoas; ensure the same fix is applied to the other ROAS width computation sites that currently divide by maxRoas.
🧹 Nitpick comments (3)
src/components/dashboard/charts/BudgetGaugeChart.tsx (1)
16-38: 상태 계산/매핑 로직은 컴포넌트 밖 유틸로 분리하는 게 좋아요.지금처럼
BudgetGaugeChart.tsx에서getBudgetStatus,statusBadgeVariant를 export하면, 상위 페이지가 “차트 컴포넌트 구현”에 의존하게 됩니다. 재사용 로직은budgetStatus.ts같은 별도 유틸로 옮기고, 컴포넌트는 consume만 하도록 분리해 두는 편이 유지보수/의존성 관리에 유리합니다.권장 리팩터링 예시
-// src/components/dashboard/charts/BudgetGaugeChart.tsx -export type TBudgetStatus = "안정" | "주의" | "위험"; -export function getBudgetStatus(...) { ... } -export const statusBadgeVariant: Record<TBudgetStatus, TBadgeVariant> = { ... } +// src/components/dashboard/charts/utils/budgetStatus.ts +import { type TBadgeVariant } from "@/components/common/badge/Badge"; + +export type TBudgetStatus = "안정" | "주의" | "위험"; + +export function getBudgetStatus( + percentage: number, + warningThreshold: number, + dangerThreshold: number, +): TBudgetStatus { + if (percentage >= dangerThreshold) return "위험"; + if (percentage >= warningThreshold) return "주의"; + return "안정"; +} + +export const statusBadgeVariant: Record<TBudgetStatus, TBadgeVariant> = { + 안정: "success", + 주의: "syncing", + 위험: "inactive", +};As per coding guidelines, "구조와 책임 분리: 페이지에 비즈니스 로직이 과도하지 않은지 확인. 커스텀 훅으로의 분리 여부 검토."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/BudgetGaugeChart.tsx` around lines 16 - 38, Move the budget status logic (TBudgetStatus type, getBudgetStatus function, statusBadgeVariant and statusPointClasses maps) out of BudgetGaugeChart.tsx into a new utility module (e.g., budgetStatus.ts), export them from that module, and update BudgetGaugeChart.tsx to import these symbols instead of defining them inline; ensure getBudgetStatus signature and exported names remain unchanged so callers don't need changes, and update any tests or imports that referenced the original exports to the new module.src/components/dashboard/charts/TrafficChart.tsx (1)
70-75: 마커 좌표 훅 실행 타이밍을isMounted와 동기화하면 초기 미검출 케이스를 줄일 수 있습니다.지금은 Line [74]에서 훅이 항상 실행되는데,
src/components/dashboard/charts/useAnomalyMarkerPos.ts는 첫 탐색 실패 시 재시도 트리거가 제한적입니다. 마운트 이후에만 좌표 탐색 effect가 돌도록enabled인자를 추가하면 안정성이 좋아집니다.리팩터링 예시
- const markerPos = useAnomalyMarkerPos(containerRef); + const markerPos = useAnomalyMarkerPos(containerRef, isMounted);// src/components/dashboard/charts/useAnomalyMarkerPos.ts export function useAnomalyMarkerPos( containerRef: RefObject<HTMLDivElement | null>, enabled: boolean, ) { useEffect(() => { if (!enabled) return; // 기존 로직 }, [enabled, updateMarkerPos, containerRef]); }As per coding guidelines,
src/**: "3. Hook 사용: useEffect 의존성 배열 및 불필요한 사용 검토."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/TrafficChart.tsx` around lines 70 - 75, The hook useAnomalyMarkerPos is called unconditionally which can cause initial-detection misses; update its signature to accept an enabled boolean and guard its internal effect with that flag (e.g., add parameter enabled and return early in the effect if !enabled), then call it from TrafficChart as useAnomalyMarkerPos(containerRef, isMounted) so the markerPos calculation only runs after isMounted is true; keep existing refs/state (containerRef, markerPos, isAnomalyHovered, setIsAnomalyHovered) unchanged except for the new enabled arg.src/components/dashboard/charts/trafficChart.config.ts (1)
19-19: 파일명 날짜가 UTC 기준이라 로컬 날짜와 하루 어긋날 수 있습니다.Line [19]의
toISOString()는 UTC 기준이라, KST 자정 근처에는 파일명이 전날로 저장될 수 있어요. 로컬 기준 포맷으로 바꾸는 걸 추천드립니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/charts/trafficChart.config.ts` at line 19, TODAY is derived via toISOString() which yields UTC dates and can be off by a day in local time; change the TODAY constant assignment so it formats the current local date (YYYY-MM-DD) instead of using toISOString(). Update the TODAY definition in trafficChart.config.ts (the TODAY constant) to generate a local-date string (for example using a local-safe formatter such as building from new Date().getFullYear()/getMonth()+1/getDate() with zero-padding or using toLocaleDateString with a YYYY-MM-DD locale like 'sv' or 'en-CA') so filenames reflect local (KST) dates rather than UTC.
🤖 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/dashboard/charts/trafficChart.config.ts`:
- Line 31: The forEach callback used on
chartContext.el.querySelectorAll("title") is written as an implicit-return arrow
((el) => el.remove()) which violates the useIterableCallbackReturn lint rule;
change the callback to a block-bodied arrow function so it explicitly contains
the statement (e.g., (el) => { el.remove(); }) in the call to
chartContext.el.querySelectorAll("title").forEach(...) to eliminate the linter
error.
In `@src/components/dashboard/charts/TrafficChart.tsx`:
- Around line 79-84: The container div using role="img" (id CHART_CONTAINER_ID,
ref containerRef, attribute data-hide-tooltip) currently wraps interactive
children (e.g., the chart button rendered later), which can hide those controls
from assistive tech; change the chart wrapper to use role="group" or no role
(leave as a plain div) and move role="img" to the non-interactive
decorative/image-only layer instead so interactive elements remain in the
accessibility tree and the data-hide-tooltip logic (data-hide-tooltip) continues
to apply to the wrapper.
In `@src/components/dashboard/platform/PlatformRoasTable.tsx`:
- Around line 56-68: Add ARIA roles to make the grid behave like a semantic
table: on the outer wrapper in PlatformRoasTable (the div with class "@container
flex...") add role="table" (and optionally aria-label/aria-labelledby), change
the header row div (the one building the column headers using COL) to
role="row", mark each header span as role="columnheader", and ensure all
rendered data rows (the rows generated later in the component between lines
~72-138) are role="row" with their child cells marked role="cell" (and add
aria-rowindex/aria-colindex if required for ordering). Update the header spans
for CTR/CVR and the final sales/ad span similarly so screen readers see
header→cell relationships.
---
Outside diff comments:
In `@src/components/dashboard/charts/BudgetGaugeChart.tsx`:
- Around line 48-65: Clamp all visualization values before use: ensure
percentage is bounded to 0–100 (use the computed percentage from
spent/totalBudget but clamp negatives and >100), clamp warningThreshold and
dangerThreshold into 0–100 and handle swapped/out-of-order thresholds, guard
periodTotalDays from zero and clamp periodElapsedRate to 0–100, and compute
remaining/isOverBudget independently of clamped display values; update usages of
percentage, periodElapsedRate, warningThreshold and dangerThreshold (and any
place that calls scaleX(...) with percentage) to use the clamped variants so
markers and bars never render outside the card or invert.
---
Duplicate comments:
In `@src/components/dashboard/platform/PlatformRoasTable.tsx`:
- Around line 29-31: Guard against a zero denominator when computing bar widths:
in PlatformRoasTable replace the current maxRoas calculation using
Math.max(...platformRoasRanking.map(p => p.roas)) with a safe max that never
equals 0 (for example Math.max(...platformRoasRanking.map(p => p.roas), 1) or
conditionally set to 1 if result is 0), and update any width calculations that
use (p.roas / maxRoas) * 100 so they rely on this non-zero maxRoas; ensure the
same fix is applied to the other ROAS width computation sites that currently
divide by maxRoas.
---
Nitpick comments:
In `@src/components/dashboard/charts/BudgetGaugeChart.tsx`:
- Around line 16-38: Move the budget status logic (TBudgetStatus type,
getBudgetStatus function, statusBadgeVariant and statusPointClasses maps) out of
BudgetGaugeChart.tsx into a new utility module (e.g., budgetStatus.ts), export
them from that module, and update BudgetGaugeChart.tsx to import these symbols
instead of defining them inline; ensure getBudgetStatus signature and exported
names remain unchanged so callers don't need changes, and update any tests or
imports that referenced the original exports to the new module.
In `@src/components/dashboard/charts/trafficChart.config.ts`:
- Line 19: TODAY is derived via toISOString() which yields UTC dates and can be
off by a day in local time; change the TODAY constant assignment so it formats
the current local date (YYYY-MM-DD) instead of using toISOString(). Update the
TODAY definition in trafficChart.config.ts (the TODAY constant) to generate a
local-date string (for example using a local-safe formatter such as building
from new Date().getFullYear()/getMonth()+1/getDate() with zero-padding or using
toLocaleDateString with a YYYY-MM-DD locale like 'sv' or 'en-CA') so filenames
reflect local (KST) dates rather than UTC.
In `@src/components/dashboard/charts/TrafficChart.tsx`:
- Around line 70-75: The hook useAnomalyMarkerPos is called unconditionally
which can cause initial-detection misses; update its signature to accept an
enabled boolean and guard its internal effect with that flag (e.g., add
parameter enabled and return early in the effect if !enabled), then call it from
TrafficChart as useAnomalyMarkerPos(containerRef, isMounted) so the markerPos
calculation only runs after isMounted is true; keep existing refs/state
(containerRef, markerPos, isAnomalyHovered, setIsAnomalyHovered) unchanged
except for the new enabled arg.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 09744442-2342-44a6-8f5f-9c1a8e04ba70
📒 Files selected for processing (4)
src/components/dashboard/charts/BudgetGaugeChart.tsxsrc/components/dashboard/charts/TrafficChart.tsxsrc/components/dashboard/charts/trafficChart.config.tssrc/components/dashboard/platform/PlatformRoasTable.tsx
📚 Storybook 배포 완료
|
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/dashboard/charts/TrafficChart.tsx`:
- Around line 75-77: The anomaly trigger lacks keyboard/touch/assistive support:
update the button element in TrafficChart.tsx to use onFocus/onBlur alongside
onPointerEnter/onPointerLeave to set isAnomalyFocused and isAnomalyHovered (use
setIsAnomalyFocused/setIsAnomalyHovered), ensure CSS no longer forces opacity-0
when focused (add a focus-visible style to make it visible), add an onClick
handler so touch/click opens the tooltip, link the button and tooltip via
aria-controls on the button and aria-expanded reflecting the tooltip state, give
the tooltip role="status" or aria-live="polite" so screen readers announce
updates, and add a keydown handler (Escape) that closes the tooltip by clearing
the focused/hovered state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6dec68e3-aa7d-45a3-bb7a-3cf00d20c2aa
📒 Files selected for processing (2)
src/components/dashboard/charts/TrafficChart.tsxsrc/components/dashboard/charts/trafficChart.config.ts
This was referenced Mar 18, 2026
9 tasks
8 tasks
This was referenced May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚨 관련 이슈
#88
✨ 변경사항
✏️ 작업 내용
스크린샷
default.mov
2026-03-13.3.59.05.mov
폴더구조
😅 미완성 작업
API 연동 예정
📢 논의 사항 및 참고 사항
디자인 변경 내용 백엔드한테 전달해야해서 디자인 피드백도 함께 부탁드립니다!
💬 리뷰어 가이드 (P-Rules)
Summary by CodeRabbit
새로운 기능
개선
리팩터