Revamp and expand the theming documentation based on learnings.#809
Revamp and expand the theming documentation based on learnings.#809jgindin wants to merge 6 commits intogoogle:mainfrom
Conversation
![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 provides a significant and valuable expansion of the theming documentation. The new guide is much more detailed and provides a clearer step-by-step process for theming. My review includes a few minor suggestions to improve formatting and fix a potentially broken link, ensuring the documentation is as clear and accurate as possible.
Note: Security Review has been skipped due to the limited scope of the PR.
| relevant to the specific app or brand (e.g., colors or typography). | ||
| > | ||
| > ```typescript | ||
| > import { cloneDefaultTheme } from "../theme/default-theme.js"; |
There was a problem hiding this comment.
The relative path ../theme/default-theme.js is likely to result in a broken link when this documentation is rendered on a website. It's also inconsistent with other source code links in this document that use full GitHub URLs. Please replace it with a placeholder or a full URL to the file if it exists in the repository.
| > import { cloneDefaultTheme } from "../theme/default-theme.js"; | |
| import { cloneDefaultTheme } from "path/to/your/default-theme.js"; |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| passed down as **Component Overrides** via the `additionalStyles` record. | ||
|
|
||
| **Web (Lit):** | ||
| ### Dynamic Styles via BeginRenderingMessage |
There was a problem hiding this comment.
Isn't this the same as above? The 'styles' / 'theme' property on beginRendering / createSurface is defined by the catalog and it's what the agent uses to express theme.
There was a problem hiding this comment.
The beginRendering's styles are limited to what's in the catalog's theme?
Hmmm... That's not something I had realized.
Please confirm this and then I'll update the doc.
There was a problem hiding this comment.
But...I thought that the catalog's theme was meant to provide "hints" for rendering all the components from this catalog, and the beginRendering styles were meant as hints for this particular surface.
E.g., if this surface is rendering some sort of interactive UI for Coca-cola, it would want to use, e.g., it's own color scheme.
| styles. On mobile native clients like Flutter, these semantic hints are mapped | ||
| directly to global or widget-specific `ThemeData` configurations instead of CSS. | ||
|
|
||
| The `Types.Theme` object consists of structurally predefined properties: |
There was a problem hiding this comment.
I actually think that we should restructure this for v0.9. Having a monolithic theme object for the basic catalog which then has sections for each component makes things less modular. Now if I want to take some components from the basic catalog and use them elsewhere, I need to define some new theme object. Instead, these theme settings should ideally be passed into the constructor of a Component instance when you instantiate it in an app's configuration. E.g. imagine something like:
myNewCatalog = createCatalog({
components: [
BasicCatalog.Text(defaultSize: 10, color: 0xFFF)
]
})
WDYT?
There was a problem hiding this comment.
Or maybe some CSS mapping should be passed in.
There was a problem hiding this comment.
I absolutely agree that theming should be rethought. This is confusing.
I think we should have a brainstorming session on this ASAP.
| A2UI follows a **client-controlled styling** approach, meaning: | ||
|
|
||
| - **Agents describe _what_ to show** (components and structure). | ||
| - **Clients decide _how_ it looks** (colors, fonts, spacing). |
There was a problem hiding this comment.
This is up the catalog in a way, no?
A catalog could expose color, font, and spacing params to its components if it wants to.
There was a problem hiding this comment.
See my follow up comment here: https://github.com/google/A2UI/pull/809/changes#r2918598387
There was a problem hiding this comment.
The catalog can provide theme data, but it's still up to the client to decide how it actually gets applied.
| } | ||
|
|
||
| // ❌ Bad: Visual properties (not supported) | ||
| // ❌ Bad: Visual properties. Breaks responsiveness and brand consistency. |
There was a problem hiding this comment.
I guess this addresses my comment above: https://github.com/google/A2UI/pull/809/changes#r2918577043
Here we say you CANNOT do what I suggested there. But this isn't enforced, right? We're just strongly suggesting it?
There was a problem hiding this comment.
Right -- this is bad, but we can't stop someone from building a component that allows these sorts of properties.
| `a2ui_client_capabilities.json`). The catalog definition dictates what | ||
| components and APIs an agent can use. | ||
|
|
||
| **Why would an agent need to send theme properties?** While the client |
There was a problem hiding this comment.
Is this using BeginRenderingMessage.styles?
There was a problem hiding this comment.
Most likely, but it could also be the catalog's theme data.
4 participants
Description
Add a lot more documentation, going into more detail on how theming actually works.
Pre-launch Checklist