chore: fix a2ui catalog styles#2542
Conversation
|
📝 WalkthroughWalkthroughThe pull request consolidates built-in catalog component CSS assets from component-local directories into a centralized Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
PupilTong
changed the title
PupilTong
requested review from
HuJean,
Sherry-hue,
fzx2666-fz and
gaoachao
as code owners
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/genui/a2ui/src/catalog/Column/index.tsx (1)
29-35:⚠️ Potential issue | 🟠 MajorFix
{ componentId, path }binding handling forchildren(currently ignored).
ColumnProps.childrensupportsstring[] | { componentId: string; path: string }, but the implementation only rendersArray.isArray(children)and otherwise uses[], producing an empty column.Update to resolve the
{ componentId, path }form usinguseDataBinding(as implemented in the List component) to look up bound props againstsurfacewithdataContextPathsupport, then render the resolved child IDs/components.Also applies to the Row component, which has the identical issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/Column/index.tsx` around lines 29 - 35, Column currently ignores children when it's the binding form { componentId, path } because it only handles Array.isArray(children) and falls back to [], so update the Column component to detect the object-binding form for ColumnProps.children and resolve it via useDataBinding (same approach as List): call useDataBinding(surface, dataContextPath, children) or similar to fetch the bound array of child IDs/components, map those into explicitChildren for rendering (preserving existing justify/align handling), and ensure the same fix is applied to the Row component which has identical logic; reference the variables children, surface, dataContextPath, explicitChildren and the hook useDataBinding when making the change.packages/genui/a2ui/src/catalog/RadioGroup/index.tsx (1)
34-41:⚠️ Potential issue | 🟠 MajorResolve
{ path }bindings foritemsandvalueprops.
RadioGroupComponentPropsexplicitly allowsitems: string[] | { path: string }andvalue: string | { path: string }, but the component currently discards bindings: it convertsitemsto[]when it's a{ path }object and castsvaluetostringwithout resolving.Destructure
surfaceanddataContextPathfrom props, then resolve these bindings viasurface.store.getSignal()with relative path support (prependdataContextPathif the path doesn't start with/). Pass resolved values to the component logic and RadioGroupRoot.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/RadioGroup/index.tsx` around lines 34 - 41, Destructure surface and dataContextPath from props, then when items or value are objects with a { path } property resolve them via surface.store.getSignal(resolvedPath) before using; resolvedPath should be the path as-is if it starts with '/' otherwise prepend dataContextPath + '/' (or dataContextPath if empty) to form a relative path. Replace the current assignments to value and explicitItems so: if value is a { path } object read the signal and use its current string, and if items is a { path } object read the signal and use its current array (fall back to []), keeping usageHint and setValue behavior the same, and pass these resolved values into the RadioGroupRoot/render logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui/src/catalog/RadioGroup/index.tsx`:
- Line 8: The RadioGroup component must resolve items and value when they are
binding objects ({path: string}) by querying Surface.store and supporting
relative paths against dataContextPath: modify the code in
packages/genui/a2ui/src/catalog/RadioGroup/index.tsx so that when props.items is
an object with a path you call Surface.store.get(resolvedPath, dataContextPath)
(resolve relative paths by joining the binding path with dataContextPath) and
ensure the result is coerced to an array (fallback to [] if null/undefined), and
when props.value is a binding object you similarly resolve it via
Surface.store.get(resolvedPath, dataContextPath) and coerce/convert to string
safely; update the logic that currently converts non-array items to [] and the
direct cast of value to string to use these resolved values instead.
---
Outside diff comments:
In `@packages/genui/a2ui/src/catalog/Column/index.tsx`:
- Around line 29-35: Column currently ignores children when it's the binding
form { componentId, path } because it only handles Array.isArray(children) and
falls back to [], so update the Column component to detect the object-binding
form for ColumnProps.children and resolve it via useDataBinding (same approach
as List): call useDataBinding(surface, dataContextPath, children) or similar to
fetch the bound array of child IDs/components, map those into explicitChildren
for rendering (preserving existing justify/align handling), and ensure the same
fix is applied to the Row component which has identical logic; reference the
variables children, surface, dataContextPath, explicitChildren and the hook
useDataBinding when making the change.
In `@packages/genui/a2ui/src/catalog/RadioGroup/index.tsx`:
- Around line 34-41: Destructure surface and dataContextPath from props, then
when items or value are objects with a { path } property resolve them via
surface.store.getSignal(resolvedPath) before using; resolvedPath should be the
path as-is if it starts with '/' otherwise prepend dataContextPath + '/' (or
dataContextPath if empty) to form a relative path. Replace the current
assignments to value and explicitItems so: if value is a { path } object read
the signal and use its current string, and if items is a { path } object read
the signal and use its current array (fall back to []), keeping usageHint and
setValue behavior the same, and pass these resolved values into the
RadioGroupRoot/render logic.
🪄 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: bc74cd63-ae7c-4d75-86f8-8c352b2cc4c1
📒 Files selected for processing (27)
.github/a2ui-catalog.instructions.mdpackages/genui/a2ui/src/catalog/Button/index.tsxpackages/genui/a2ui/src/catalog/Card/index.tsxpackages/genui/a2ui/src/catalog/CheckBox/index.tsxpackages/genui/a2ui/src/catalog/Column/index.tsxpackages/genui/a2ui/src/catalog/Divider/index.tsxpackages/genui/a2ui/src/catalog/Image/index.tsxpackages/genui/a2ui/src/catalog/List/index.tsxpackages/genui/a2ui/src/catalog/RadioGroup/index.tsxpackages/genui/a2ui/src/catalog/Row/index.tsxpackages/genui/a2ui/src/catalog/Text/index.tsxpackages/genui/a2ui/styles/catalog/Button.csspackages/genui/a2ui/styles/catalog/Card.csspackages/genui/a2ui/styles/catalog/CheckBox.csspackages/genui/a2ui/styles/catalog/Column.csspackages/genui/a2ui/styles/catalog/Divider.csspackages/genui/a2ui/styles/catalog/Image.csspackages/genui/a2ui/styles/catalog/List.csspackages/genui/a2ui/styles/catalog/RadioGroup.csspackages/genui/a2ui/styles/catalog/Row.csspackages/genui/a2ui/styles/catalog/Text.csspackages/genui/a2ui/styles/catalog/luna-dark.csspackages/genui/a2ui/styles/catalog/luna-light.csspackages/genui/a2ui/styles/catalog/luna.csspackages/genui/a2ui/styles/catalog/lunaris-dark.csspackages/genui/a2ui/styles/catalog/lunaris-light.csspackages/genui/a2ui/turbo.json
Merging this PR will not alter performance
Comparing 
Footnotes
|
React MTF Example#839 Bundle Size — 196.54KiB (-0.02%).80d8e84(current) vs caadd3b main#834(baseline) Bundle metrics
|
| Current #839 |
Baseline #834 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
 Cache Invalidation |
43.42% |
43.4% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
173(-0.57%) |
174 |
| Duplicate Modules |
66 |
66 |
| Duplicate Code |
44.08%(+0.09%) |
44.04% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 
1 improvement
| Current #839 |
Baseline #834 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
85.31KiB (-0.05%) |
85.35KiB |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-sty... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9280 Bundle Size — 900.02KiB (0%).80d8e84(current) vs caadd3b main#9275(baseline) Bundle metrics
|
| Current #9280 |
Baseline #9275 |
|
|---|---|---|
| Initial JS |
44.46KiB |
44.46KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
229(-0.43%) |
230 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.28% |
27.28% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9280 |
Baseline #9275 |
|
|---|---|---|
| JS |
495.88KiB |
495.88KiB |
| Other |
401.92KiB |
401.92KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-sty... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7707 Bundle Size — 225.38KiB (-0.02%).80d8e84(current) vs caadd3b main#7702(baseline) Bundle metrics
|
| Current #7707 |
Baseline #7702 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
35.34% |
35.33% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
179(-0.56%) |
180 |
| Duplicate Modules |
69 |
69 |
| Duplicate Code |
44.57%(+0.07%) |
44.54% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 improvement
| Current #7707 |
Baseline #7702 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
79.63KiB (-0.05%) |
79.67KiB |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-sty... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#822 Bundle Size — 680.27KiB (-0.02%).80d8e84(current) vs caadd3b main#818(baseline) Bundle metrics
|
| Current #822 |
Baseline #818 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
39.71% |
39.7% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 improvement
| Current #822 |
Baseline #818 |
|
|---|---|---|
| Other |
680.27KiB (-0.02%) |
680.41KiB |
Bundle analysis report Branch PupilTong:codex/a2ui-catalog-sty... Project dashboard
Generated by RelativeCI Documentation Report issue
2 participants
Summary
src/catalogintostyles/catalog.dist/catalog/*/index.jsxresolves../../../styles/catalog/*.cssinstead of broken./style.csspaths.styles/**to the a2ui Turbo inputs and document the catalog CSS placement rule.Root Cause
TypeScript preserves the catalog component CSS side-effect imports when emitting JSX. With CSS files living under
src/catalog/<Component>/style.css, the emitteddist/catalog/<Component>/index.jsxstill imported./style.css, but no matching CSS file exists beside the emitted JSX.Validation
pnpm install --frozen-lockfilepnpm -C packages/genui/a2ui-catalog-extractor buildpnpm exec tsc --build packages/genui/a2ui/tsconfig.json --pretty falsepnpm -C packages/genui/a2ui buildpnpm dprint fmt .github/a2ui-catalog.instructions.md packages/genui/a2ui/turbo.json packages/genui/a2ui/src/catalog/*/index.tsx packages/genui/a2ui/styles/catalog/*.csspnpm biome check .github/a2ui-catalog.instructions.md packages/genui/a2ui/turbo.json packages/genui/a2ui/src/catalog/*/index.tsx packages/genui/a2ui/styles/catalog/*.cssgit diff --checkSummary by CodeRabbit