feat(a2ui): add piechart#2682
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR introduces a new PieChart catalog component with reusable chart utilities, refactors existing LineChart to use those utilities, and integrates PieChart into the playground with full documentation and example data. ChangesPieChart Component and Shared Utilities
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The PR introduces a substantial new component with 300+ lines of SVG rendering logic and geometry computation, plus refactoring of an existing component. However, the changes follow consistent patterns: shared utilities are straightforward helpers, PieChart implementation is self-contained, LineChart updates are mechanical replacements, and integration is standard wiring across consistent layers. 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/genui/a2ui/src/catalog/utils/shared.ts (1)
5-29: ⚡ Quick winConsolidate duplicated chart helpers into one source-of-truth.
shared.tscurrently re-implements the same helpers asutils/chart.ts, which can drift over time. Re-export from the canonical file instead.Proposed simplification
-export const DEFAULT_CHART_COLORS = [ - '`#0057d9`', - '`#0a8f8f`', - '`#8a5cf6`', - '`#d92d20`', - '`#2d6a4f`', - '`#b26a00`', -] as const; - -export function escapeXml(value: string): string { - return value - .replace(/&/g, '&') - .replace(/</g, '<') - .replace(/>/g, '>') - .replace(/"/g, '"') - .replace(/'/g, '''); -} - -export function formatValue(value: number): string { - if (!Number.isFinite(value)) return '0'; - const rounded = Math.abs(value) >= 1000 - ? Math.round(value) - : Number(value.toFixed(1)); - return String(rounded); -} +export { + DEFAULT_CHART_COLORS, + escapeXml, + formatValue, +} from './chart.js';🤖 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 `@packages/genui/a2ui/src/catalog/utils/shared.ts` around lines 5 - 29, The three helpers DEFAULT_CHART_COLORS, escapeXml, and formatValue are duplicates of the canonical implementations in utils/chart.ts; remove their local implementations from shared.ts and instead re-export them from the canonical module (import then export DEFAULT_CHART_COLORS, escapeXml, formatValue from utils/chart.ts) so there is a single source-of-truth and no behavior drift.
🤖 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 `@packages/genui/a2ui-playground/src/utils/renderUrl.ts`:
- Around line 35-46: The code currently always appends inline payloads via
url.searchParams.set for messages and actionMocks (using
encodeBase64Url(JSON.stringify(init.messages)) and
encodeBase64Url(JSON.stringify(init.actionMocks))), which makes ?demo=<id> URLs
large; change this so the inline fallback is only added when explicitly allowed
(e.g., when running in dev: process.env.NODE_ENV === 'development' or when a new
flag/param like allowInlineFallback is true) or when no demoId is present—wrap
the two url.searchParams.set blocks in that conditional and leave demoId-based
URLs short and unchanged otherwise.
In `@packages/genui/a2ui/src/catalog/PieChart/index.tsx`:
- Around line 155-159: sliceAngle can be exactly 360 for a single-slice pie
which makes arc start/end coincide and SVG fail; before computing
gapForSlice/startAngle/endAngle or before calling describeSlicePath, detect a
full-circle case (e.g., sliceAngle === 360 or value === total) and clamp
sliceAngle to just under 360 (subtract a tiny epsilon like 1e-6) so startAngle
and endAngle differ slightly; update the logic around sliceAngle, gapForSlice,
startAngle, endAngle and then call describeSlicePath with the adjusted angle to
ensure the arc renders.
---
Nitpick comments:
In `@packages/genui/a2ui/src/catalog/utils/shared.ts`:
- Around line 5-29: The three helpers DEFAULT_CHART_COLORS, escapeXml, and
formatValue are duplicates of the canonical implementations in utils/chart.ts;
remove their local implementations from shared.ts and instead re-export them
from the canonical module (import then export DEFAULT_CHART_COLORS, escapeXml,
formatValue from utils/chart.ts) so there is a single source-of-truth and no
behavior drift.
🪄 Autofix (Beta)
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
Run ID: f20f6037-2096-4366-805d-4bd9d34fa810
📒 Files selected for processing (14)
packages/genui/a2ui-playground/lynx-src/a2ui/App.tsxpackages/genui/a2ui-playground/src/catalog/a2ui.tspackages/genui/a2ui-playground/src/demos.tspackages/genui/a2ui-playground/src/mock/basic/pie-chart-china-ev-share.jsonpackages/genui/a2ui-playground/src/utils/renderUrl.tspackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/LineChart/index.tsxpackages/genui/a2ui/src/catalog/PieChart/index.tsxpackages/genui/a2ui/src/catalog/README.mdpackages/genui/a2ui/src/catalog/index.tspackages/genui/a2ui/src/catalog/utils/chart.tspackages/genui/a2ui/src/catalog/utils/shared.tspackages/genui/a2ui/src/index.tspackages/genui/a2ui/styles/catalog/PieChart.css
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will not alter performance
Comparing Footnotes
|
React External#1636 Bundle Size — 698.01KiB (0%).a49d405(current) vs 4222846 main#1633(baseline) Bundle metrics
|
| Current #1636 |
Baseline #1633 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch p/a2ui-pie Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1654 Bundle Size — 208.75KiB (0%).a49d405(current) vs 4222846 main#1651(baseline) Bundle metrics
|
| Current #1654 |
Baseline #1651 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
195 |
195 |
|
77 |
77 |
|
44.17% |
44.17% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #1654 |
Baseline #1651 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
97.52KiB |
97.52KiB |
Bundle analysis report Branch p/a2ui-pie Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#790 Bundle Size — 202.16KiB (0%).a49d405(current) vs 4222846 main#787(baseline) Bundle metrics
|
| Current #790 |
Baseline #787 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
100 |
100 |
|
30 |
30 |
|
39.22% |
39.22% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #790 |
Baseline #787 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
56.41KiB |
56.41KiB |
Bundle analysis report Branch p/a2ui-pie Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#10095 Bundle Size — 903.53KiB (0%).a49d405(current) vs 4222846 main#10092(baseline) Bundle metrics
Bundle size by type
|
| Current #10095 |
Baseline #10092 |
|
|---|---|---|
499.15KiB |
499.15KiB |
|
402.16KiB |
402.16KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch p/a2ui-pie Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8521 Bundle Size — 237.81KiB (0%).a49d405(current) vs 4222846 main#8518(baseline) Bundle metrics
|
| Current #8521 |
Baseline #8518 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
200 |
200 |
|
80 |
80 |
|
44.68% |
44.68% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #8521 |
Baseline #8518 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
92.05KiB |
92.05KiB |
Bundle analysis report Branch p/a2ui-pie Project dashboard
Generated by RelativeCI Documentation Report issue
Summary by CodeRabbit
Checklist