Conversation
…ging on-the-wire a2UI JSON and actions
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… into md-componentslistagent
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the A2UI spec to version 0.8.2 and introduces new functionality to the MultipleChoice component, adding support for 'chips' display type and filtering. The changes are well-implemented and include a comprehensive new component gallery sample to demonstrate the new features. My review includes suggestions for code simplification, removal of unused imports for better code hygiene, and a correction for what appears to be a copy-paste error in the new client sample setup.
renderers/lit/src/0.8/ui/checkbox.ts
Outdated
|
|
||
|
|
There was a problem hiding this comment.
These blank lines are unnecessary and can be removed to improve code cleanliness.
| <svg class="chip-icon" xmlns="http://www.w3.org/2000/svg" viewBox="0 -960 960 960"> | ||
| <path d="M382-240 154-468l57-57 171 171 367-367 57 57-424 424Z"/> | ||
| </svg> |
There was a problem hiding this comment.
The SVG for the checkmark icon is hardcoded directly within the render method. To improve readability and reusability, consider extracting this into a constant or a separate private render method (e.g., #renderCheckIcon()).
| from starlette.middleware.cors import CORSMiddleware | ||
| from dotenv import load_dotenv | ||
|
|
||
| from agent import ComponentGalleryAgent |
There was a problem hiding this comment.
The ComponentGalleryAgent is imported but not used in this file. It can be removed to keep the imports clean.
|
Maybe make a pass through the Gemini review comments? They seem important, some of them. Maybe you covered them in the other PR? I can re-review after this one is rebased, perhaps. |
jacobsimionato
left a comment
jacobsimionato
left a comment
There was a problem hiding this comment.
+1 can we remove the gallery app from this PR?
…ctOncomponentslist
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
Some minor remarks/questions.
Please, remember to udpate the description of this PR following the instructions there (add a paragraph describing the change and a link to the issue that this PR resolves), we'll need that to remember why things happened later!
There was a problem hiding this comment.
Why do we want to commit the lockfiles?
4 participants
Description
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. For larger changes, raising an issue first helps reduce redundant work.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.