-
Notifications
You must be signed in to change notification settings - Fork 757
Enable custom elements through Surface component #506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.