Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces a React renderer for A2UI, aiming for consistency with existing Lit, Angular, and Flutter renderers. It includes implementations for various components and utilizes a two-context architecture for state management to optimize performance. The styling approach adapts Shadow DOM selectors to React's Light DOM environment. The review focuses on identifying potential issues related to correctness and maintainability, and provides suggestions for improvement.
bbaefd0 to
3acfc9b
Compare
|
|
||
| let result = html; | ||
| for (const [regex, replacement] of replacements) { | ||
| result = result.replace(regex, replacement); |
There was a problem hiding this comment.
If I'm understanding this method correctly, it's looking at the rendered markdown and applying classnames to certain tag names through regexes.
This looks quite brittle and a pain to maintain! There's an example in the markdown-it on how to apply CSS classnames to elements through the renderer rules, here:
This is a similar approach to what the Lit renderer is doing here:
A2UI/renderers/lit/src/0.8/ui/directives/markdown.ts
Lines 103 to 117 in 9c35418
I'd suggest this step to be done by leveraging the existing markdown-it API, rather than doing string manipulations.
There was a problem hiding this comment.
Thank's for the suggestion! We've replaced applyMarkdownTheme with markdown-it's renderer rules API, using token.attrJoin() on _open tokens before rendering and cleaning up afterward.
This should now mirror the Lit renderer's #applyTagClassMap pattern.
renderers/react/src/styles/index.ts
Outdated
| /* ========================================================================= | ||
| * Card (from Lit card.ts static styles) | ||
| * ========================================================================= */ |
There was a problem hiding this comment.
Why are these comments part of the output string?
There was a problem hiding this comment.
Fair point, they were developer notes explaining the transformation from Lit to React
-> I moved them into the JSDoc above.
ditman
left a comment
There was a problem hiding this comment.
Thanks for the PR! I left some comments about the handling of markdown in the Text widget on my first quick look to this, but more knowledgeable people should actually dive deeper than that :)
There's some brittleness by grabbing the structural styles directly from the lit renderer, but then maintaining a list of the classnames to be added to each markdown element in this repository; I wonder if some of those might make sense to be moved to a shared package somehow (this is not for this PR, of course)
|
I'm actually working on decoupling the Markdown rendering from the lit and angular renderers here, and I'm planning to do the same to the React renderer once it lands: |
gspencergoog
left a comment
There was a problem hiding this comment.
This looks reasonable to me, but I'm not familiar with React, so I'm going to abstain from approving anything here.
4e6363d to
f806ccb
Compare
There was a problem hiding this comment.
Code Review
This is a substantial pull request that introduces a new React renderer for A2UI, along with significant enhancements to the Python agent's schema management and validation capabilities. The new React renderer is well-architected, using a two-context pattern for performance and providing a comprehensive set of components. The Python agent changes improve schema validation robustness and build process. I've identified a potential bug in a file path for schema loading, a missing CODEOWNERS entry for the new renderer, and a minor architectural improvement to address a circular dependency. Overall, this is an excellent contribution.
| potential_path = os.path.abspath( | ||
| os.path.join( | ||
| os.path.dirname(__file__), | ||
| "..", |
There was a problem hiding this comment.
The relative path used in the "Local Assets" fallback logic for loading schemas appears to be incorrect. The path is constructed using os.path.join(os.path.dirname(__file__), "..", "assets", ...).
Given that this file is at src/a2ui/inference/schema/manager.py, going up one level (..) lands in src/a2ui/inference/. The assets directory is a sibling of inference, not a child. The path should likely go up two levels (../..) to reach src/a2ui/ before descending into assets/.
This could cause the fallback loading mechanism to fail when running from source if package resources are not available.
"..",
".."| def validator(self) -> "A2uiValidator": | ||
| from .validator import A2uiValidator | ||
|
|
||
| return A2uiValidator(self) |
There was a problem hiding this comment.
The validator property creates a runtime circular dependency between A2uiCatalog and A2uiValidator by using a local import inside the method. While this pattern works, it can make the code harder to reason about and maintain.
Consider refactoring to break the cycle, for example by using dependency injection: the A2uiValidator instance could be created separately and passed to the A2uiCatalog during its initialization, or a factory function could be used to construct both objects.
ava-cassiopeia
left a comment
There was a problem hiding this comment.
Looks like there's a large number of file conflicts, could you please rebase this and fix those?
Move ensureInitialized() into A2UIProvider so users no longer need to manually call initializeDefaultCatalog() and injectStyles() before rendering. Both are idempotent so multiple providers are safe.
Prevents stale 504 errors when file: dependencies are rebuilt by setting optimizeDeps.force in both vite configs.
ditman
left a comment
There was a problem hiding this comment.
I don't see anything that stands out anymore. If @ava-cassiopeia is happy, and you guys believe the API is idiomatic enough for React JS, I think this can land!
Thanks for all the updates! Please, take into consideration that I did a small update to the react rendering on web_core, so once packages publish, we might need to do an update to the dependencies of the test apps here! (I left a link to my PR with the change in comments below)
| // provide the theme directly on a component that renders a2ui-surface as a child | ||
| @customElement('themed-a2ui-surface') | ||
| class ThemedA2UISurface extends LitElement { | ||
| @provide({ context: UI.Context.themeContext }) |
There was a problem hiding this comment.
I changed a little bit the renderMarkdown method (made it async), this might break slightly until all the packages are at the right revisions. If using a NPM dependency, we should be fine for now! Once the new web_core and markdown-it packages update, we might need a dependency on markdown-it 0.0.2 which has the async implementation of the renderMarkdown function.
Apologies for this change, I realized too late that I need the renderMarkdown to be asynchronous for google's markdown rendering to work properly 🤦
See this:
(Only dep changes would be needed, nothing on the actual code of the app)
|
I'm going to take one more pass at this, but I think I'm pretty happy with the state of this, and it looks like pretty idiomatic React to me. |
ava-cassiopeia
left a comment
There was a problem hiding this comment.
There's a few small changes I want to make to this before we merge it - this needs Google copyright notices in the source code files, and I'll run a few of these markdown files through our formatter.
Otherwise, LGTM. Thanks for your contributions and your patience while we get this landed!
|
Oh darn, looks like there's a merge conflict on the codeowners now. @lukasmoschitz can you sort that out so I don't have to create another PR on Copilotkit (lol)? |
- Replace require('gts') with ESM import in eslint.config.js
- Disable prefer-arrow-callback to preserve React.memo display names
- Exclude .d.ts files from linting
- Fix floating promise in A2UIProvider dispatch
React renderer PR google#542 is still open and not by CopilotKit. Composer was contributed by maxkorp (@CopilotKit).
- React renderer PR google#542 merged — update status to Stable across all pages - Add React section to client-setup guide - Expand community renderer submission with step-by-step instructions - Replace community showcase placeholder with concrete guidance + sample links - Collapse duplicate partner info in community.md (points to a2ui-in-the-world) - Fix typo (signiciant → significant) - Tighten how-to-use wording, fix Angular install (add @a2ui/web-lib)
…dates (#776) * docs: reorganize files to match nav sections - Move catalogs, transports, client_to_server_actions → concepts/ - Move renderers, agents → reference/ - Move where-is-it-used, community → ecosystem/ - Rewrite agent-ui-ecosystem as concise 'How Does A2UI Compare?' intro page - Move third-party integrations (json-render, OpenClaw) to ecosystem - Fix all cross-references for moved files - Remove duplicate nav entries (each page appears once) * docs: refine How Does A2UI Compare page - native-first → declarative UI, clarify host controls styling - MCP Apps: clarify remote controls content, config via tool calling - AG UI: credit CopilotKit team, React renderer, Composer contributions - ChatKit: note shared design philosophy (basic components, declarative) * docs: restore comparison table as At a Glance section * docs: rename 'Where is A2UI Used?' → 'A2UI in the World' * docs: rename where-is-it-used → a2ui-in-the-world, MCP SEP-1865 citation * docs: add Google ADK section to A2UI in the World ADK Web has native A2UI rendering and A2A↔A2UI message conversion. Links to ADK docs, adk-web repo, and agent development guide. * docs: fix CopilotKit attribution — Composer yes, React renderer no React renderer PR #542 is still open and not by CopilotKit. Composer was contributed by maxkorp (@CopilotKit). * docs: React renderer status — In Review, link to PR #542 * docs: add redirects for all moved files Old URLs now redirect to new locations: - catalogs, transports, actions → concepts/ - renderers, agents → reference/ - community → ecosystem/ - where-is-it-used → ecosystem/a2ui-in-the-world * docs: React renderer merged, contribution guide, writing fixes - React renderer PR #542 merged — update status to Stable across all pages - Add React section to client-setup guide - Expand community renderer submission with step-by-step instructions - Replace community showcase placeholder with concrete guidance + sample links - Collapse duplicate partner info in community.md (points to a2ui-in-the-world) - Fix typo (signiciant → significant) - Tighten how-to-use wording, fix Angular install (add @a2ui/web-lib) * docs: add web_core shared library section to renderer development guide Explains what @a2ui/web-lib (web_core) provides, how Lit/Angular/React renderers use it, and why new web renderers should start from it rather than reimplementing message processing and state management. * docs: link renderer names to source in client-setup table * docs: remove stars and last-updated columns from community renderers table * docs: fix ChatKit link to point to actual docs instead of openai.com homepage * docs: add live shields.io badges for stars and last-commit to community renderers * docs: fix broken React sample link, fix relative path in ecosystem --------- Co-authored-by: Zaf <zaf@0-a.org>
|
@lukasmoschitz shouldn’t this be published on npm? The README says |
Thanks @timoslanitis-aisera for the heads up. We still had a few issues to sort out, but the npm package should be available soon. |
|
@lukasmoschitz |
|
I'm planning on publishing this to NPM today (US time, sometime within the next 8 hours after this comment's timestamp, lol) |
|
Published: https://www.npmjs.com/package/@a2ui/react |
React Renderer for A2UI
Summary
Adds a React renderer implementation for A2UI alongside the existing Lit and Angular renderers, that brings the protocol's declarative UI capabilities to React applications.
Approach
React.memo()for additional performance optimization.:hostselectors to scoped class selectors, allowing visual parity while working in React's Light DOM environment.Components
Core Modules
Demo images