feat: A2UI column template rendering#2638
Conversation
🦋 Changeset detectedLatest commit: e36b253 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 |
📝 WalkthroughWalkthroughPatch updates: Button gains an ChangesA2UI Component and Renderer Updates
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested Reviewers
Poem
🚥 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 |
❌ 1 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
c0cec8b to
c2eeb7d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/tame-bears-happen.md (1)
5-5: ⚡ Quick winConsider documenting all user-facing changes.
The changeset currently documents the Column template rendering and NodeRenderer fixes, but the stack context indicates this release also includes Button disabled state support (new
isValidprop) and theme/styling updates. Users reading the changelog would benefit from knowing about all changes in this release.📝 Suggested expanded description
-Fix `Column` template rendering so nested bindings keep the correct `dataContextPath`, and preserve the caller-provided context in `NodeRenderer` instead of overwriting it from the stored component snapshot. +Fix `Column` template rendering so nested bindings keep the correct `dataContextPath`, and preserve the caller-provided context in `NodeRenderer` instead of overwriting it from the stored component snapshot. Add `isValid` prop to Button for disabled state handling. Update theme variables for Button styling and image feature sizes.🤖 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 @.changeset/tame-bears-happen.md at line 5, Update the changeset text to document all user-facing changes in this release: keep the existing note about fixing Column template rendering and preserving caller context in NodeRenderer, and add concise entries mentioning Button disabled-state support via the new isValid prop (e.g., "Button: added isValid prop to control disabled state") and the theme/styling updates (briefly summarize what changed visually or theming API-wise); ensure the entries reference "Column" and "NodeRenderer" and the new "isValid" prop so users can quickly see the scope of changes.
🤖 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/src/react/A2UIRenderer.tsx`:
- Around line 185-189: The current logic only overwrites
component.dataContextPath when initialComponent.dataContextPath is defined,
leaving stale paths when initial was undefined; change the condition in the
effectiveComponent assignment to compare initialComponent.dataContextPath
directly with component.dataContextPath (i.e., if they are not equal) and, when
different, return { ...component, dataContextPath:
initialComponent.dataContextPath } so the initial value (including undefined)
replaces the current one; update the code referencing effectiveComponent,
initialComponent.dataContextPath, and component.dataContextPath accordingly.
---
Nitpick comments:
In @.changeset/tame-bears-happen.md:
- Line 5: Update the changeset text to document all user-facing changes in this
release: keep the existing note about fixing Column template rendering and
preserving caller context in NodeRenderer, and add concise entries mentioning
Button disabled-state support via the new isValid prop (e.g., "Button: added
isValid prop to control disabled state") and the theme/styling updates (briefly
summarize what changed visually or theming API-wise); ensure the entries
reference "Column" and "NodeRenderer" and the new "isValid" prop so users can
quickly see the scope of changes.
🪄 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: 6dc6e20d-e7e9-4918-a179-719c16ee4a8f
📒 Files selected for processing (7)
.changeset/tame-bears-happen.mdpackages/genui/a2ui/src/catalog/Button/index.tsxpackages/genui/a2ui/src/catalog/Column/index.tsxpackages/genui/a2ui/src/react/A2UIRenderer.tsxpackages/genui/a2ui/styles/catalog/Button.csspackages/genui/a2ui/styles/catalog/Image.csspackages/genui/a2ui/styles/theme.css
💤 Files with no reviewable changes (1)
- packages/genui/a2ui/styles/catalog/Image.css
c2eeb7d to
43e34f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/catalog/Column/index.tsx`:
- Around line 29-33: The Column component currently overwrites a child's
explicit dataContextPath; update the logic around childWithContext so it only
assigns props.dataContextPath when the child does not already provide one (i.e.,
if childPath is falsy and the child lacks dataContextPath then set
dataContextPath = props.dataContextPath), preserving any existing
child.dataContextPath; locate the assignment using the variables
childWithContext, childPath and dataContextPath and change the branching to
inherit parent dataContextPath only when the child has no predefined
dataContextPath.
- Around line 81-100: childList is memoizing actual ComponentInstance objects
from surface.components which can mutate in place and cause stale renders;
change this by stopping memoization of resolved components—either remove the
useMemo around childList and compute children each render, or have childList
memoize only IDs/paths/keys (e.g., the children array, template.componentId,
item keys and itemPath) and call buildChild/surface.components lookup during
render so the latest ComponentInstance is used; update references to
childList/buildChild accordingly and remove the now-unused useMemo import.
🪄 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: 69bf241e-2939-4647-9833-15524f6c57b2
📒 Files selected for processing (7)
.changeset/tame-bears-happen.mdpackages/genui/a2ui/src/catalog/Button/index.tsxpackages/genui/a2ui/src/catalog/Column/index.tsxpackages/genui/a2ui/src/react/A2UIRenderer.tsxpackages/genui/a2ui/styles/catalog/Button.csspackages/genui/a2ui/styles/catalog/Image.csspackages/genui/a2ui/styles/theme.css
💤 Files with no reviewable changes (1)
- packages/genui/a2ui/styles/catalog/Image.css
✅ Files skipped from review due to trivial changes (1)
- .changeset/tame-bears-happen.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/genui/a2ui/styles/theme.css
- packages/genui/a2ui/src/catalog/Button/index.tsx
- packages/genui/a2ui/src/react/A2UIRenderer.tsx
- packages/genui/a2ui/styles/catalog/Button.css
Merging this PR will degrade performance by 6.81%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 002-hello-reactLynx-destroyBackground |
670.3 µs | 916.9 µs | -26.9% |
| ⚡ | 008-many-use-state-destroyBackground |
9.5 ms | 8 ms | +18.79% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing p/a2ui-column-template-rendering (e36b253) with main (ee79eff)2
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. ↩
-
No successful run was found on
main(d55deae) during the generation of this report, so ee79eff was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
React External#1351 Bundle Size — 695.33KiB (0%).e36b253(current) vs d55deae main#1346(baseline) Bundle metrics
|
| Current #1351 |
Baseline #1346 |
|
|---|---|---|
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-column-template-rendering Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8237 Bundle Size — 237.15KiB (0%).e36b253(current) vs d55deae main#8232(baseline) Bundle metrics
|
| Current #8237 |
Baseline #8232 |
|
|---|---|---|
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 #8237 |
Baseline #8232 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.39KiB |
91.39KiB |
Bundle analysis report Branch p/a2ui-column-template-rendering Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#504 Bundle Size — 199.83KiB (0%).e36b253(current) vs d55deae main#499(baseline) Bundle metrics
Bundle size by type
|
| Current #504 |
Baseline #499 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
54.08KiB |
54.08KiB |
Bundle analysis report Branch p/a2ui-column-template-rendering Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1370 Bundle Size — 208.1KiB (0%).e36b253(current) vs d55deae main#1365(baseline) Bundle metrics
|
| Current #1370 |
Baseline #1365 |
|
|---|---|---|
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 #1370 |
Baseline #1365 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
96.86KiB |
96.86KiB |
Bundle analysis report Branch p/a2ui-column-template-rendering Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9811 Bundle Size — 901.35KiB (0%).e36b253(current) vs d55deae main#9806(baseline) Bundle metrics
Bundle size by type
|
| Current #9811 |
Baseline #9806 |
|
|---|---|---|
497.08KiB |
497.08KiB |
|
402.06KiB |
402.06KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch p/a2ui-column-template-rendering Project dashboard
Generated by RelativeCI Documentation Report issue
Summary by CodeRabbit
New Features
Bug Fixes
Style
Checklist