[Refactor/#86] 통합 대시보드 피드백 반영 - #87
Conversation
|
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로컬 Toast 컴포넌트 제거 및 sonner 통합, 렌더 키 변경(인덱스→라벨/타이틀), 차트 접근성·애니메이션 개선 및 렌더 성능 힌트 추가, 플랫폼 테이블의 클릭 네비게이션 제거, 타이포그래피·애니메이션 CSS 조정이 적용되었습니다. Changes
Sequence Diagram(s)(생성 조건에 부합하지 않아 생략합니다.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
29-37:⚠️ Potential issue | 🟠 Major
totalBudget === 0일 때 상태가 반대로 표시됩니다.분모 예외를 막는 건 좋지만,
spent > 0인 경우에도percentage를 0으로 고정해서 상단이0% / 안정으로 보입니다. 예산이 0인데 지출이 발생한 상태는 사실상 즉시위험으로 처리돼야 해서, 이 케이스를 별도 분기하지 않으면 사용자가 현재 상태를 반대로 이해하게 됩니다.수정 예시
- const percentage = - totalBudget > 0 ? Math.round((spent / totalBudget) * 100) : 0; - const remaining = Math.max(0, totalBudget - spent); - const isOverBudget = spent > totalBudget; + const balance = totalBudget - spent; + const hasBudget = totalBudget > 0; + const isOverBudget = balance < 0; + const percentage = hasBudget + ? Math.round((spent / totalBudget) * 100) + : isOverBudget + ? 100 + : 0; + const remaining = Math.max(0, balance); const getStatus = () => { + if (!hasBudget && spent > 0) return "위험"; if (percentage >= dangerThreshold) return "위험"; if (percentage >= warningThreshold) return "주의"; return "안정";🤖 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 29 - 37, The status logic misreports when totalBudget === 0 and spent > 0; update the percentage calculation and getStatus to treat a zero budget with positive spending as over-budget: change percentage to totalBudget > 0 ? Math.round((spent / totalBudget) * 100) : (spent > 0 ? 100 : 0), and in getStatus add an explicit branch that returns "위험" when totalBudget === 0 && spent > 0 (or simply use isOverBudget || (totalBudget === 0 && spent > 0)) before checking dangerThreshold and warningThreshold so getStatus, percentage, and isOverBudget/remaining reflect the zero-budget-over-spent case correctly.
🤖 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 31-32: The chart currently clamps remaining to zero (const
remaining = Math.max(0, totalBudget - spent)) and always shows ₩0 when over
budget; change logic in BudgetGaugeChart so when isOverBudget (const
isOverBudget = spent > totalBudget) you compute and display an overage amount
(e.g., overage = spent - totalBudget) and use that value for the "초과 지출"
card/label instead of remaining; update any rendering branches that show
remaining (including the block around the existing label logic and the similar
code at lines referenced 118-127) to choose remaining when not over budget and
overage when isOverBudget. Ensure formatting/currency is applied consistently.
In `@src/components/dashboard/charts/TrafficChart.tsx`:
- Around line 86-93: The chart currently only exposes an aria-label on the
container, so screen readers get the title but not the underlying data; update
the TrafficChart component to provide a true accessible alternative by adding a
visually-hidden summary and/or a hidden data table generated from the series and
options values (use the existing series and options props to build the text),
connect it to the chart via aria-describedby, and keep the visual chart
(ReactApexChart) aria-hidden if needed; ensure the summary uses your app's
"sr-only" CSS class and the table includes timestamps and click counts so
assistive tech can read the actual trend and numbers.
In `@src/components/dashboard/platform/PlatformRoasTable.tsx`:
- Around line 39-42: The row button currently calls navigate("/platform") and
drops the clicked platform context; update the onClick in PlatformRoasTable so
it passes platform.name to the destination (e.g., as a path parameter, a query
parameter, or via navigation state) instead of a static "/platform" URL; locate
the button with key platform.name and replace the navigate call so the
destination can reconstruct the selected platform (use encodeURIComponent for
safety if embedding in a URL) and ensure the target route/component reads that
param/query/state to restore the selected platform.
---
Outside diff comments:
In `@src/components/dashboard/charts/BudgetGaugeChart.tsx`:
- Around line 29-37: The status logic misreports when totalBudget === 0 and
spent > 0; update the percentage calculation and getStatus to treat a zero
budget with positive spending as over-budget: change percentage to totalBudget >
0 ? Math.round((spent / totalBudget) * 100) : (spent > 0 ? 100 : 0), and in
getStatus add an explicit branch that returns "위험" when totalBudget === 0 &&
spent > 0 (or simply use isOverBudget || (totalBudget === 0 && spent > 0))
before checking dangerThreshold and warningThreshold so getStatus, percentage,
and isOverBudget/remaining reflect the zero-budget-over-spent case correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c12e541-2030-43ab-977c-77872f91f8fd
📒 Files selected for processing (7)
src/components/common/chart/ChartLegend.tsxsrc/components/common/toast/Toast.tsxsrc/components/dashboard/charts/BudgetGaugeChart.tsxsrc/components/dashboard/charts/TrafficChart.tsxsrc/components/dashboard/platform/PlatformRoasTable.tsxsrc/pages/dashboard/overview/OverviewAiReportPanel.tsxsrc/pages/dashboard/overview/OverviewDashboard.tsx
💤 Files with no reviewable changes (1)
- src/components/common/toast/Toast.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 29-32: The status/percentage logic in BudgetGaugeChart is
inconsistent when totalBudget === 0: percentage is forced to 0 while
isOverBudget can be true, causing contradictory UI. Update the computations in
BudgetGaugeChart so the three values are computed together: determine
isOverBudget = spent > totalBudget || (totalBudget === 0 && spent > 0); compute
percentage = totalBudget > 0 ? Math.round((spent / totalBudget) * 100) :
(isOverBudget ? 100 : 0); and compute remaining = isOverBudget ? Math.abs(spent
- totalBudget) : Math.max(totalBudget - spent, 0); ensure any downstream
status/label logic uses these unified values so the badge, percent display, and
helper text remain consistent.
- Around line 73-78: The gauge currently clamps displayed percentage for
assistive tech by using Math.min(percentage, 100) for aria-valuenow and
aria-label; update BudgetGaugeChart to keep aria-valuenow as
Math.min(percentage, 100) but add an aria-valuetext that exposes the actual
percentage and when it exceeds 100 (e.g., "120% (over budget)") so screen
readers get the true value and overage status while preserving expected 0–100
range in aria-valuenow; reference the percentage variable and the element
rendering the progressbar to add aria-valuetext accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9caaa967-e750-43cb-b0f4-6061e3dce670
📒 Files selected for processing (2)
src/components/dashboard/charts/BudgetGaugeChart.tsxsrc/components/dashboard/platform/PlatformRoasTable.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/dashboard/platform/PlatformRoasTable.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/dashboard/charts/BudgetGaugeChart.tsx (1)
29-40:⚠️ Potential issue | 🟡 Minor
totalBudget === 0일 때 상태 표시 불일치 문제가 남아있습니다.현재 로직에서
totalBudget === 0이고spent > 0인 경우:
percentage = 0(분모 0 방어)isOverBudget = true(spent > 0 > totalBudget)status = "안정"(percentage가 0이므로)결과적으로 상단 뱃지는 "안정"으로 표시되지만, 하단 카드는 "초과 지출"로 표시되어 사용자에게 모순된 정보를 전달합니다. API 연동 시 예산 데이터가 0으로 들어올 수 있으므로, 이 케이스를 명시적으로 처리하는 것이 좋습니다.
수정 제안
const percentage = totalBudget > 0 ? Math.round((spent / totalBudget) * 100) : 0; - const isOverBudget = spent > totalBudget; + const isOverBudget = spent > totalBudget || (totalBudget === 0 && spent > 0); const remaining = isOverBudget ? spent - totalBudget : totalBudget - spent; const getStatus = () => { + // totalBudget이 0인데 지출이 있으면 위험 상태로 처리 + if (totalBudget === 0 && spent > 0) return "위험"; if (percentage >= dangerThreshold) return "위험"; if (percentage >= warningThreshold) return "주의"; return "안정"; };코딩 가이드라인에 따라 "에러 처리: API 실패 대응 및 사용자 피드백 적절성 검토"를 준수해야 합니다.
🤖 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 29 - 40, The status logic is inconsistent when totalBudget === 0 and spent > 0 (percentage is 0 but isOverBudget is true), so update getStatus to explicitly treat over-budget-zero-budget cases: reference totalBudget, spent, isOverBudget, percentage, getStatus and status and make getStatus return the over-budget/danger state when isOverBudget or (totalBudget === 0 && spent > 0), otherwise use the existing percentage threshold checks; ensure the computed status aligns with the remaining/isOverBudget display elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/dashboard/charts/BudgetGaugeChart.tsx`:
- Around line 29-40: The status logic is inconsistent when totalBudget === 0 and
spent > 0 (percentage is 0 but isOverBudget is true), so update getStatus to
explicitly treat over-budget-zero-budget cases: reference totalBudget, spent,
isOverBudget, percentage, getStatus and status and make getStatus return the
over-budget/danger state when isOverBudget or (totalBudget === 0 && spent > 0),
otherwise use the existing percentage threshold checks; ensure the computed
status aligns with the remaining/isOverBudget display elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cc7e3c7f-7c47-44ea-aa12-e3f06f874142
📒 Files selected for processing (1)
src/components/dashboard/charts/BudgetGaugeChart.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/dashboard/charts/TrafficChart.tsx (1)
86-96:⚠️ Potential issue | 🟠 Major차트 이름만 노출되고 실제 데이터는 여전히 읽히지 않아요.
Line 87-88의
role="img"+aria-label만으로는 스크린리더가 차트 제목만 읽고, 시간대별 클릭수/추세 자체는 전달하지 못합니다. 숨김 요약이나 숨김 테이블을 추가해서aria-describedby로 연결하고, 시각 차트는 별도 래퍼에서aria-hidden처리하는 쪽으로 마무리해 주세요.접근성 대체 표현 예시
export default function TrafficChart() { + const descriptionId = "traffic-chart-description"; + return ( <div role="img" aria-label="실시간 트래픽 변화 차트: 시간대별 클릭수 추이" + aria-describedby={descriptionId} style={{ willChange: "transform" }} > - <ReactApexChart - type="area" - options={options} - series={series} - height={360} - /> + <div id={descriptionId} className="sr-only"> + {labels + .map((label, i) => `${label} 클릭수 ${clicks[i].toLocaleString()}회`) + .join(", ")} + </div> + <div aria-hidden="true"> + <ReactApexChart + type="area" + options={options} + series={series} + height={360} + /> + </div> </div> ); }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/charts/TrafficChart.tsx` around lines 86 - 96, The visual chart currently only exposes a title via role="img" and aria-label; update the TrafficChart component to provide a hidden textual summary and/or hidden data table (describing time buckets and click counts and trend) and connect it with aria-describedby on the chart wrapper, and mark the visual chart element (the ReactApexChart wrapper) aria-hidden="true" so screen readers ignore the SVG; ensure the hidden summary/table is kept in the same component (near ReactApexChart) and references the chart's series/options data to produce the descriptive text, and use stable IDs (e.g., chartSummaryId, chartDataTableId) to link aria-describedby to the hidden element(s).
🧹 Nitpick comments (1)
src/components/dashboard/platform/PlatformComparisonChart.tsx (1)
115-122:willChange: "transform"적용에 대한 참고 사항스크롤 성능 개선을 위한
willChange적용 의도는 이해되지만, 몇 가지 고려할 점이 있습니다:
will-change는 브라우저에게 레이어 승격(compositing layer)을 요청하여 GPU 메모리를 지속적으로 점유합니다.- 정적 차트처럼 실제 transform 애니메이션이 빈번하지 않은 경우, 상시 적용보다는 스크롤/애니메이션 직전에 동적으로 적용하고 완료 후 제거하는 것이 권장됩니다.
- 현재 대시보드에 여러 차트가 있다면, 모든 차트에
will-change를 적용 시 메모리 오버헤드가 누적될 수 있습니다.현재 적용이 체감 성능 향상에 기여한다면 유지해도 괜찮지만, 실제 효과가 미미하다면 제거를 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/platform/PlatformComparisonChart.tsx` around lines 115 - 122, The inline style willChange: "transform" on the wrapper div in PlatformComparisonChart can cause persistent GPU layer allocation; either remove the static will-change or apply it dynamically only during interactions: update the PlatformComparisonChart component to stop setting style={{ willChange: "transform" }} by default, and instead add a short-lived application via a hook or event handlers (e.g., set will-change on the chart wrapper just before scroll/hover/animation and remove it after a short timeout or on mouseLeave/scroll end) using useEffect or local state tied to events around ReactApexChart to ensure the property is applied only when needed to avoid memory overhead when many charts are rendered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/dashboard/charts/TrafficChart.tsx`:
- Around line 86-96: The visual chart currently only exposes a title via
role="img" and aria-label; update the TrafficChart component to provide a hidden
textual summary and/or hidden data table (describing time buckets and click
counts and trend) and connect it with aria-describedby on the chart wrapper, and
mark the visual chart element (the ReactApexChart wrapper) aria-hidden="true" so
screen readers ignore the SVG; ensure the hidden summary/table is kept in the
same component (near ReactApexChart) and references the chart's series/options
data to produce the descriptive text, and use stable IDs (e.g., chartSummaryId,
chartDataTableId) to link aria-describedby to the hidden element(s).
---
Nitpick comments:
In `@src/components/dashboard/platform/PlatformComparisonChart.tsx`:
- Around line 115-122: The inline style willChange: "transform" on the wrapper
div in PlatformComparisonChart can cause persistent GPU layer allocation; either
remove the static will-change or apply it dynamically only during interactions:
update the PlatformComparisonChart component to stop setting style={{
willChange: "transform" }} by default, and instead add a short-lived application
via a hook or event handlers (e.g., set will-change on the chart wrapper just
before scroll/hover/animation and remove it after a short timeout or on
mouseLeave/scroll end) using useEffect or local state tied to events around
ReactApexChart to ensure the property is applied only when needed to avoid
memory overhead when many charts are rendered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a38766a-ce99-4187-ab3b-6466c8292bdd
📒 Files selected for processing (2)
src/components/dashboard/charts/TrafficChart.tsxsrc/components/dashboard/platform/PlatformComparisonChart.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/dashboard/charts/BudgetGaugeChart.tsx (1)
29-32:⚠️ Potential issue | 🟠 Major
totalBudget === 0분기가 아직도 상태 계산과 어긋납니다.Line 29-32에서는 분모 0만 피하고 있어서,
totalBudget가 0이고spent > 0이면 하단 카드만초과 지출로 바뀌고 상단은0% / 안정으로 남습니다.isOverBudget,percentage,remaining은 같은 분기에서 같이 계산하고, 상태 계산도 그 값을 기준으로 맞춰야 화면이 일관됩니다.수정 예시
- const percentage = - totalBudget > 0 ? Math.round((spent / totalBudget) * 100) : 0; - const isOverBudget = spent > totalBudget; - const remaining = isOverBudget ? spent - totalBudget : totalBudget - spent; + const isOverBudget = totalBudget > 0 ? spent > totalBudget : spent > 0; + const percentage = + totalBudget > 0 + ? Math.round((spent / totalBudget) * 100) + : isOverBudget + ? 100 + : 0; + const remaining = isOverBudget + ? Math.max(spent - totalBudget, 0) + : Math.max(totalBudget - spent, 0); const getStatus = () => { + if (isOverBudget) return "위험"; if (percentage >= dangerThreshold) return "위험"; if (percentage >= warningThreshold) return "주의"; return "안정"; };As per coding guidelines, "6. 에러 처리: API 실패 대응 및 사용자 피드백 적절성 검토."
🤖 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 29 - 32, The three derived values percentage, isOverBudget, and remaining are computed with inconsistent branching when totalBudget is 0; update BudgetGaugeChart to compute them together in a single branch: determine isOverBudget as (spent > totalBudget), and then if totalBudget <= 0 set percentage = isOverBudget ? 100 : 0 and remaining = Math.abs(spent - totalBudget) (or spent when overbudget, totalBudget - spent when not), so the top gauge and bottom card use the same state; replace the existing separate calculations of percentage, isOverBudget, and remaining with this unified logic and ensure any UI state checks reference these updated variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/dashboard/charts/BudgetGaugeChart.tsx`:
- Around line 29-32: The three derived values percentage, isOverBudget, and
remaining are computed with inconsistent branching when totalBudget is 0; update
BudgetGaugeChart to compute them together in a single branch: determine
isOverBudget as (spent > totalBudget), and then if totalBudget <= 0 set
percentage = isOverBudget ? 100 : 0 and remaining = Math.abs(spent -
totalBudget) (or spent when overbudget, totalBudget - spent when not), so the
top gauge and bottom card use the same state; replace the existing separate
calculations of percentage, isOverBudget, and remaining with this unified logic
and ensure any UI state checks reference these updated variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: afa2b96f-c39b-4ad9-a08e-77b51f54ffe3
📒 Files selected for processing (3)
src/components/auth/intro/IntroAIAnalytics.tsxsrc/components/dashboard/charts/BudgetGaugeChart.tsxsrc/index.css
|
P3: Card 컴포넌트의 제목 부분 폰트를 heading4 정도로 키워도 좋을 것 같습니다! |
|
P2: 플랫폼별 비교에서 호버시 나오는 툴팁이 막대를 가려서 막대가 잘 보이지 않습니다. 막대들의 옆쪽에 나오면 좋을 것 같습니다! |
jjjsun
left a comment
jjjsun
left a comment
There was a problem hiding this comment.
P4: 지금 실시간 차트랑 플랫폼별 비교 차트에 hover시에 Chart라고 text가 뜨는데 조금 어색해 보여서 삭제하는걸 추천드립니다!
|
차트 라이브러리 관련 |
사실 그래프 안에서만 tooltip이 표시되기 때문에 막대 그래프가 가려질 수 밖에 없습니다ㅠㅠ |
지금 react.lazy + suspense로 먼저 최적화를 하고 나중에 라이브러리를 교체하면, 최적화 작업이 두 번 이루어지는 셈이라 전체 비용이 더 커질 것 같습니다. 그래서 |
그러면 저는 ApexChart 유지하고, 이슈 확인후 판단으로 진행하는것이 좋을것같습니다! |
📚 Storybook 배포 완료
|
저도 일단 유지하고 개발 후에 판단하는 것이 좋을 것 같습니다! |
|
P4: 확인했습니다 고생하셨어요!! |
3 participants
🚨 관련 이슈
Closes #86
✨ 변경사항
✏️ 작업 내용
통합 대시보드 UI 구현
1. TrafficChart — 실시간 트래픽 차트
(1) 왼쪽: PlatformComparisonChart — Google/NAVER/kakao 바 차트
(2) 오른쪽: PlatformRoasTable — ROAS 순위 테이블
통합 대시보드 피드백 수정
스크린샷
2026-03-09.3.53.29.mov
📂 폴더 구조
😅 미완성 작업
📢 논의 사항 및 참고 사항
(자료 제출 마감이 임박해 PR 범위가 넓어졌습니다..죄송합니다ㅠㅠ)
디자인 수정 피드백 요청
UI 구현 과정에서 예산 소진 현황 카드 및 플랫폼 비교 카드 디자인 일부 수정했습니다. 함께 피드백 남겨주시면 감사하겠습니다!!!
통합 필요한 부분
아이콘 의미에 맞지 않는 네이밍 및 중복 사용되는 구간이 있는 것 같아 전체적으로 정리 필요할 것 같습니다.
KPI 카드의 trend up, down 에 따라 다른 의미를 가질 수 있으니 색상에 대해서 의미 정리 필요할 것 같습니다.
차트 라이브러리 관련 논의 내용
지금 사용하고 있는 apexcharts 라이브러리가 SVG 기반이라 스크롤 시 DOM 재계산 비용이 크고, 번들도 ~450KB로 무거워 초기 로딩 및 스크롤 속도가 느린 것 같습니다.
지금 즉시 적용 가능한 부분은 React.lazy + Suspense으로 초기 번들에서 분리하거나
그래프가 더 많아질 경우에는 아예 Canvas 기반인 Chart.js 라이브러리로 교체하는 방법이 있을 것 같습니다.
어떤 방식이 좋을지 의견 부탁드립니다!
Summary by CodeRabbit
릴리스 노트