-
Notifications
You must be signed in to change notification settings - Fork 54
Refactor the custom backend to work with A2UI-based schema #410
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
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 is a solid refactoring that aligns the custom_backend example with the A2UI protocol specification. The transition from the custom uiGenerator to the standard surfaceUpdate tool, along with parsing responses into A2uiMessage objects and introducing the BeginRendering message, significantly improves protocol compliance and code structure. The tests have also been thoughtfully updated to reflect these changes. My review includes a few minor suggestions to enhance debugging practices and improve the user experience.
| print('No UI received.'); | ||
| _surfaceId = null; | ||
| setState(() { | ||
| _isLoading = false; | ||
| _errorMessage = null; | ||
| }); |
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.
When no UI is received from the backend, the app currently prints a message to the console and shows a blank space to the user. It would be a better user experience to display a message on the screen. You can achieve this by setting the _errorMessage in the state.
| print('No UI received.'); | |
| _surfaceId = null; | |
| setState(() { | |
| _isLoading = false; | |
| _errorMessage = null; | |
| }); | |
| setState(() { | |
| _isLoading = false; | |
| _errorMessage = 'No UI received from the backend.'; | |
| }); |
| return const Text('No UI 🤷♀️'); | ||
| } | ||
| return GenUiSurface(surfaceId: _surfaceId!, host: _genUi); | ||
| return GenUiSurface(surfaceId: kSurfaceId, host: _genUi); |
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 change removes the initial 'No UI 🤷♀️' message, showing a blank area instead. This might be confusing for users. To restore the original behavior and provide better feedback, you can use the defaultBuilder property of GenUiSurface.
| return GenUiSurface(surfaceId: kSurfaceId, host: _genUi); | |
| return GenUiSurface(surfaceId: kSurfaceId, host: _genUi, defaultBuilder: (context) => const Text('No UI 🤷♀️')); |
| } | ||
|
|
||
| const _title = 'Custom Backend Demo'; | ||
| const kSurfaceId = 'custom_backend_surface'; |
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.
Using prefix 'k' is debatable, that means not everyone will follow this code style.
this tradition is very old and came from C:
"c" was the tag for type "char", so it couldn't also be used for "const"; so "k" was chosen, since that's the first letter of "konstant" in German, and is widely used for constants in mathematics
https://stackoverflow.com/questions/5016622/where-does-the-k-prefix-for-constants-come-from
How about declaring genui traditions for debatable things in some central Coding Style document in the repo, to make it clear for all developers?
Added a constant for the surface ID in the Protocol class.
This PR refactors the
custom_backendexample to fully align with the A2UI streaming protocol specification.Key changes include:
uiGeneratortool. It now correctly generates asurfaceUpdatetool call that conforms to the officialA2uiSchemas.surfaceUpdateSchema.A2uiMessageobject using theA2uiMessage.fromJsonfactory.BeginRenderingImplementation: ABeginRenderingmessage is now synthetically created and sent to theGenUiManagerafter theSurfaceUpdatemessage, correctly following the A2UI progressive rendering flow.surfaceId: A constantkSurfaceIdhas been introduced to ensure consistency across the prompt, UI, and message creation.widget_test.dartfile.