Enable custom elements through Surface component#506
Enable custom elements through Surface component#506shahabl wants to merge 3 commits intogoogle:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request correctly adds the enableCustomElements property to the Surface component and passes it to the underlying Root component, enabling custom element rendering when using <a2ui-surface>. The change is minimal and effective. My review focuses on improving project quality by adding tests and documentation for this new feature, as per the repository guidelines. I've also noted from the PR checklist that the changelog should be updated.
| @property({ type: Boolean }) | ||
| accessor enableCustomElements = false; |
There was a problem hiding this comment.
This new property enables key functionality for the Surface component. Based on the repository's style guide (line 17), new code changes should include tests. Please add a unit test to verify that custom elements are correctly rendered when enableCustomElements is set to true on an <a2ui-surface> component.
References
- If there are code changes, code should have tests. (link)
There was a problem hiding this comment.
The existing test suite (model.test.ts) focuses on the data processing layer and
doesn't include UI component rendering tests. UI component tests would require browser
globals (DOM, CustomEvent, HTMLElement, etc.) and a test environment setup (like
jsdom or happy-dom).
Add enableCustomElements property to Surface component and propagate it to Root component. This allows custom elements registered via componentRegistry to be rendered when using a2ui-surface. Previously, the Root component supported custom elements via the enableCustomElements property, but Surface did not expose this property, making it impossible to enable custom elements when using Surface as the top-level component.
- Update custom-components.md guide with usage instructions - Include code example showing how to enable custom elements on Surface Note: UI component tests require browser globals (DOM, CustomEvent, etc.) which would need a test environment like happy-dom or jsdom. The existing test suite only tests the data processing layer. The 4-line code change can be verified by code review.
3b36271 to
e3357c6
Compare
zeroasterisk
left a comment
There was a problem hiding this comment.
Thanks for the clean PR, @shahabl.
Before this lands, I want to flag that v0.9 of the spec (now finalized in specification/v0_9/) replaces the enableCustomElements pattern entirely. In v0.9, custom catalog support is handled through the required catalogId field on createSurface:
{
"createSurface": {
"surfaceId": "main",
"catalogId": "mycompany.com:restaurant-catalog"
}
}The renderer resolves components from whatever catalog the agent declares — no boolean flag needed. This is by design: catalog negotiation is a first-class protocol concept, not a renderer config toggle.
My recommendation: We should not merge a v0.8-only API addition that v0.9 explicitly supersedes. Instead:
- The docs addition to
custom-components.mdis valuable — but it should document the v0.9catalogIdapproach as the primary path, with v0.8enableCustomElementsnoted as legacy. - The Lit renderer change itself is fine as a v0.8 backport, but the
Surfacecomponent will need to supportcatalogId-based catalog resolution for v0.9 regardless — that's the change worth investing in.
Would you be open to updating this PR to target the v0.9 catalog mechanism? Happy to point you to the relevant spec files (specification/v0_9/json/server_to_client.json and specification/v0_9/docs/a2ui_protocol.md).
Thank you for the feedback @zeroasterisk. I'll update the PR as suggested. |
|
Thanks for this docs improvement, @shahabl! The The code change in this PR (adding |
Description
This PR adds support for custom elements when using the
<a2ui-surface>component by exposing theenableCustomElementsproperty.Problem
The A2UI
Rootcomponent has anenableCustomElementsproperty that allows custom elements registered viacomponentRegistryto be rendered. However, theSurfacecomponent (which wrapsRoot) did not expose this property. This meant that when using<a2ui-surface>as the top-level component, it was impossible to enable custom element rendering.Solution
enableCustomElementsproperty declaration to theSurfacecomponentSurfaceto theRootcomponent in the render templateThis is a minimal, non-breaking change that enables the existing custom element functionality to work through the
Surfacecomponent.Use Case
Applications using
<a2ui-surface>can now register and render custom components alongside standard A2UI components by setting:surfaceElement.enableCustomElements = true;
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.