Refactor: Universal App Shell, Theming, Profile Pic Fix, and Shell Re… #242
Conversation
dmandar
requested review from
ava-cassiopeia,
gspencergoog,
jacobsimionato and
paullewis
jacobsimionato
left a comment
jacobsimionato
left a comment
There was a problem hiding this comment.
Hey this looks great! It's a huge step forward to remove the duplicated demos.
Personally, I'm in favor of just using the same theme for all the demos for now, but provide the CSS APIs you created to show developers how they can modify the appearance in their own clients. But this seems like a clear improvement, so we can keep discussing this later.
| * Shadows and elevations | ||
| * Typography scales and weights (using **Outfit** font) | ||
| * Animations (hover states, transitions) | ||
| * **Key Mechanism**: It consumes **CSS Variables** (e.g., `var(--primary-color)`, `var(--bg-gradient)`) instead of hardcoding colors. |
There was a problem hiding this comment.
Is there a list of these variables somewhere, so it's clear what levers you can control?
There was a problem hiding this comment.
I added them to styling.md
| // Overriding the primary colors to give it a distinct look | ||
| '--primary-color': 'light-dark(#10b981, #34d399)', // Emerald 500/400 | ||
| '--primary-gradient': 'linear-gradient(135deg, light-dark(#10b981, #34d399) 0%, light-dark(#059669, #059669) 100%)', | ||
| '--button-text': '#ffffff', |
There was a problem hiding this comment.
How does this interact with the use of primary vs secondary buttons? I would expect secondary buttons may be light colored, and so this white text may not be visible on them?
There was a problem hiding this comment.
These kinds of interactions are why I think it would be helpful to list out the variables somewhere and what impact they have.
There was a problem hiding this comment.
Good catch! Currently, the A2UI Button component doesn't support variants (primary/secondary)—every button gets the primary-gradient background, so white text is always safe.
When we add secondary button support to the schema/component, we'll definitely need to introduce a separate variable (e.g., --button-secondary-text) to handle contrast on lighter backgrounds.
| 'Almost there...' | ||
| ], | ||
| serverUrl: 'http://localhost:10002', | ||
| theme: { |
There was a problem hiding this comment.
Could it potentially work to just use the same theme for all agents, to emphasize that they're interchangeable? Maybe the agents themselves include hero images etc in their layouts, to help identify their content?
There was a problem hiding this comment.
I would rather we create another example to use as a starting point if people wish. This is pretty modular now so devs could experiment with different themes easily.
| heroImage: '/hero.png', | ||
| heroImageDark: '/hero-dark.png', | ||
| placeholder: 'Top 5 Chinese restaurants in New York.', | ||
| loadingText: [ |
There was a problem hiding this comment.
I wonder if in the future we could build an agent-driven loading experience, e.g. have the agent stream back layouts containing messages like this while it's thinking.
|
|
||
| // Load config from URL | ||
| const urlParams = new URLSearchParams(window.location.search); | ||
| const appKey = urlParams.get('app') || 'restaurant'; |
There was a problem hiding this comment.
In order to really emphasize that a single client codebase can talk to many agents, what if we had a menu at the top of the UI, where you can choose from the known demos, or else even type in your own URL and use some custom server, with a default theme etc?
I put a UX mock for something like this here: https://docs.google.com/document/d/1y6JLGp-1CTKQfT7CdnSVVjPN3qhl9-Ga3V3JpHQHNR0/edit?tab=t.0#bookmark=kix.aw09l4rlm8
There was a problem hiding this comment.
We could ..just that i didn't want to complicate the app in case devs just use this as a starting point.
|
The title itself implies that there are multiple discrete changes in this PR. Can you break it up so it's a little easier to review and follow? |
There was a problem hiding this comment.
I think in the main this all makes sense. Where I think it makes less sense is the ThemeManager. I don't think that's necessary. With the light-dark CSS in play it should be possible to put the following in the global CSS file:
@media (prefers-color-scheme: dark) {
:root {
color-scheme: dark;
}
}And everything should work. If it doesn't for some reason then that particular bit of CSS would be best placed in the the colors.ts so that it gets imported by the components and used directly. Except if that's the case :root should be set to :host.
|
dmandar
dismissed
paullewis’s stale review
Paul will work on his recommended changes tonight
4 participants
…name
Refactors the client application into a Universal App Shell (app.ts) capable of dynamically loading configurations for different agents (Restaurant, Contacts).
Universal Shell: Replaced restaurant.ts with app.ts and added AppConfig support via configs/.
Theming: Implemented ThemeManager for global style injection and renamed elegant.css to styles.css (See doc in PR)
.
Core Components: Updated
Image component in renderers/lit to support fit (object-fit) and usageHint for better styling control.
Python Agents: Refactored restaurant_finder and contact_lookup to use external a2ui_examples.py for cleaner prompt generation.
Fixes: Resolved oversized profile pictures in Contact Lookup by implementing usageHint="avatar" and fit="cover".