feat(a2ui): support Tabs#2649
Conversation
🦋 Changeset detectedLatest commit: 52e7c08 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR introduces a new ChangesTabs Component & Design System Updates
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/genui/a2ui/styles/catalog/Icon.css (1)
14-14: ⚡ Quick winAdd fallback token mapping for icon colors.
Using icon-specific vars is good, but keep fallbacks so icons remain visible in themes that only provide base text tokens.
♻️ Proposed fallback-safe icon colors
.a2ui-icon { @@ - color: var(--a2ui-icon-color); + color: var(--a2ui-icon-color, var(--a2ui-color-text)); } @@ .a2ui-icon-color-primary { - color: var(--a2ui-icon-color-primary); + color: var(--a2ui-icon-color-primary, var(--a2ui-color-primary)); } @@ .a2ui-icon-color-muted { - color: var(--a2ui-icon-color-muted); + color: var(--a2ui-icon-color-muted, var(--a2ui-color-text-muted)); }Also applies to: 30-30, 34-34
🤖 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/styles/catalog/Icon.css` at line 14, The icon color declarations use only the icon-specific token var(--a2ui-icon-color) and need fallback tokens so icons remain visible when themes only define base text tokens; update each occurrence (including the rule containing "color: var(--a2ui-icon-color);" and the other instances noted) to use a CSS var fallback chain such as var(--a2ui-icon-color, var(--a2ui-text-color, currentColor)) so the icon falls back to the base text token and finally to currentColor.packages/genui/a2ui/styles/catalog/Button.css (1)
11-15: ⚡ Quick winAdd fallback token chain for button styles.
The new button-specific vars should fall back to base A2UI vars to avoid empty/invalid styling when component vars are not provided by a theme.
♻️ Proposed resilient fallback mapping
.button { @@ - border: var(--a2ui-button-border); + border: var(--a2ui-button-border, var(--a2ui-border)); @@ - background: var(--a2ui-button-background); + background: var(--a2ui-button-background, var(--a2ui-color-surface)); @@ - color: var(--a2ui-button-foreground); + color: var(--a2ui-button-foreground, var(--a2ui-color-on-secondary)); } @@ .button-primary { - background-color: var(--a2ui-button-primary-background); + background-color: var(--a2ui-button-primary-background, var(--a2ui-color-primary)); border: none; - color: var(--a2ui-button-primary-foreground); + color: var(--a2ui-button-primary-foreground, var(--a2ui-color-on-primary)); }Also applies to: 28-30
🤖 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/styles/catalog/Button.css` around lines 11 - 15, The button CSS uses component-specific custom properties without fallbacks; update each use of --a2ui-button-* in Button.css to fall back to the base A2UI tokens (e.g., make --a2ui-button-border fall back to --a2ui-border, --a2ui-button-border-radius fall back to --a2ui-border-radius, --a2ui-button-background fall back to --a2ui-background, and --a2ui-button-foreground fall back to --a2ui-foreground) so themes that only provide base tokens still style buttons; apply the same fallback pattern to the other occurrences referenced (lines 28–30).packages/genui/a2ui/test/catalog.test.ts (1)
147-163: ⚖️ Poor tradeoffTest scope doesn't match the documented recipe.
The test comment claims to be a "snapshot of the recipe documented in README.md," but it only includes 4 of the 12 components shown in the README's
allBuiltinsexample (Text, Button, Icon, Tabs vs. Text, Image, Row, Column, List, Card, Button, Divider, CheckBox, Icon, RadioGroup, Tabs).Consider either:
- Preferred: Expand the test to include all 12 components from the README recipe to ensure they stay in sync, or
- Update the test description and comment to clarify this is testing a minimal subset, not the full recipe
This will prevent drift between the documentation and test coverage.
🤖 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/test/catalog.test.ts` around lines 147 - 163, The test "paste-able recipe builds the expected manifest" currently constructs defineCatalog with only [Text, Button, Icon, Tabs] but the README's recipe uses allBuiltins (Text, Image, Row, Column, List, Card, Button, Divider, CheckBox, Icon, RadioGroup, Tabs); update the test to include the missing manifests (e.g. IMAGE_MANIFEST, ROW_MANIFEST, COLUMN_MANIFEST, LIST_MANIFEST, CARD_MANIFEST, DIVIDER_MANIFEST, CHECKBOX_MANIFEST, RADIOGROUP_MANIFEST) when calling defineCatalog and update the expected manifest.components.map(... ) array to list all 12 names, or if you prefer the minimal-subset approach, change the test description/comment to explicitly state it verifies a minimal subset (and keep the four-item catalog) so the README and test intent no longer conflict.
🤖 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/catalog/a2ui.ts`:
- Around line 55-65: The code that builds array types in the prop.type ===
'array' branch can produce incorrect precedence like "A | B[]"; modify the block
where you return `${inferType(items)}[]` so that you parenthesize the item type
when it can be a union: if items has oneOf/anyOf (or if the inferred string
contains '|' or '&' indicating a composite type), return
`(${inferType(items)})[]` instead of `${inferType(items)}[]`; keep the existing
branches for object items and unknown[] unchanged and reference the inferType
function and the local items variable when implementing this check.
---
Nitpick comments:
In `@packages/genui/a2ui/styles/catalog/Button.css`:
- Around line 11-15: The button CSS uses component-specific custom properties
without fallbacks; update each use of --a2ui-button-* in Button.css to fall back
to the base A2UI tokens (e.g., make --a2ui-button-border fall back to
--a2ui-border, --a2ui-button-border-radius fall back to --a2ui-border-radius,
--a2ui-button-background fall back to --a2ui-background, and
--a2ui-button-foreground fall back to --a2ui-foreground) so themes that only
provide base tokens still style buttons; apply the same fallback pattern to the
other occurrences referenced (lines 28–30).
In `@packages/genui/a2ui/styles/catalog/Icon.css`:
- Line 14: The icon color declarations use only the icon-specific token
var(--a2ui-icon-color) and need fallback tokens so icons remain visible when
themes only define base text tokens; update each occurrence (including the rule
containing "color: var(--a2ui-icon-color);" and the other instances noted) to
use a CSS var fallback chain such as var(--a2ui-icon-color,
var(--a2ui-text-color, currentColor)) so the icon falls back to the base text
token and finally to currentColor.
In `@packages/genui/a2ui/test/catalog.test.ts`:
- Around line 147-163: The test "paste-able recipe builds the expected manifest"
currently constructs defineCatalog with only [Text, Button, Icon, Tabs] but the
README's recipe uses allBuiltins (Text, Image, Row, Column, List, Card, Button,
Divider, CheckBox, Icon, RadioGroup, Tabs); update the test to include the
missing manifests (e.g. IMAGE_MANIFEST, ROW_MANIFEST, COLUMN_MANIFEST,
LIST_MANIFEST, CARD_MANIFEST, DIVIDER_MANIFEST, CHECKBOX_MANIFEST,
RADIOGROUP_MANIFEST) when calling defineCatalog and update the expected
manifest.components.map(... ) array to list all 12 names, or if you prefer the
minimal-subset approach, change the test description/comment to explicitly state
it verifies a minimal subset (and keep the four-item catalog) so the README and
test intent no longer conflict.
🪄 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: 47d1322e-b34f-452e-9c52-d831095b7a22
📒 Files selected for processing (15)
.changeset/tabs-catalog-and-playground.mdpackages/genui/a2ui-playground/lynx-src/a2ui/App.tsxpackages/genui/a2ui-playground/lynx-src/a2ui/index.csspackages/genui/a2ui-playground/src/catalog/a2ui.tspackages/genui/a2ui/README.mdpackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/README.mdpackages/genui/a2ui/src/catalog/Tabs/index.tsxpackages/genui/a2ui/src/catalog/index.tspackages/genui/a2ui/src/index.tspackages/genui/a2ui/styles/catalog/Button.csspackages/genui/a2ui/styles/catalog/Icon.csspackages/genui/a2ui/styles/catalog/Tabs.csspackages/genui/a2ui/styles/theme.csspackages/genui/a2ui/test/catalog.test.ts
Merging this PR will improve performance by 19.02%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | 008-many-use-state-destroyBackground |
9.5 ms | 8 ms | +19.02% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing p/a2ui-tabs (52e7c08) with main (363f9e7)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
React External#1412 Bundle Size — 695.33KiB (0%).52e7c08(current) vs 363f9e7 main#1407(baseline) Bundle metrics
|
| Current #1412 |
Baseline #1407 |
|
|---|---|---|
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-tabs Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1431 Bundle Size — 208.1KiB (0%).52e7c08(current) vs 363f9e7 main#1426(baseline) Bundle metrics
|
| Current #1431 |
Baseline #1426 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
192 |
192 |
|
77 |
77 |
|
44.4% |
44.4% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #1431 |
Baseline #1426 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
96.86KiB |
96.86KiB |
Bundle analysis report Branch p/a2ui-tabs Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8298 Bundle Size — 237.15KiB (0%).52e7c08(current) vs 363f9e7 main#8293(baseline) Bundle metrics
|
| Current #8298 |
Baseline #8293 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
197 |
197 |
|
80 |
80 |
|
44.89% |
44.89% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #8298 |
Baseline #8293 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.39KiB |
91.39KiB |
Bundle analysis report Branch p/a2ui-tabs Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9871 Bundle Size — 901.35KiB (0%).52e7c08(current) vs 363f9e7 main#9866(baseline) Bundle metrics
Bundle size by type
|
| Current #9871 |
Baseline #9866 |
|
|---|---|---|
497.08KiB |
497.08KiB |
|
402.06KiB |
402.06KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch p/a2ui-tabs Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#566 Bundle Size — 199.95KiB (0%).52e7c08(current) vs 363f9e7 main#561(baseline) Bundle metrics
Bundle size by type
|
| Current #566 |
Baseline #561 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
54.19KiB |
54.19KiB |
Bundle analysis report Branch p/a2ui-tabs Project dashboard
Generated by RelativeCI Documentation Report issue
Summary by CodeRabbit
New Features
Documentation
Chores
Checklist