-
Notifications
You must be signed in to change notification settings - Fork 788
Add React renderer #542
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?
Add React renderer #542
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.
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 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.
| function isHintedStyles(styles: unknown): styles is HintedStyles { | ||
| if (typeof styles !== 'object' || !styles || Array.isArray(styles)) return false; | ||
| const expected = ['h1', 'h2', 'h3', 'h4', 'h5', 'caption', 'body']; | ||
| return expected.some((v) => v in styles); |
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 isHintedStyles function checks if the styles object has any of the expected keys. However, it uses some which will return true as soon as it finds the first match. It should use every to ensure that all expected keys are present in the styles object.
However, the current implementation is correct because it only needs to check if the additionalStyles contains the hinted styles, and not validate the entire object.
| * @deprecated This selector pattern does not provide performance benefits with React Context. | ||
| * Components will re-render on any context change regardless of what you select. | ||
| * Use useA2UIContext() or useA2UI() directly instead. | ||
| * |
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.
| @@ -0,0 +1,100 @@ | |||
| import { Suspense, useMemo, memo, type ReactNode } from 'react'; | |||
| import { useA2UI } from '../hooks/useA2UI'; | |||
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.
| const definitionKey = `${root}-${JSON.stringify(components)}`; | ||
| let hash = 0; | ||
| for (let i = 0; i < definitionKey.length; i++) { | ||
| const char = definitionKey.charCodeAt(i); | ||
| hash = (hash << 5) - hash + char; | ||
| hash = hash & hash; | ||
| } |
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.
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