-
Notifications
You must be signed in to change notification settings - Fork 43
[MCP Apps] Add styles prop to host context
#127
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
styles prop to host context
commit: |
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.
Maybe merge the spec change into this one? Spec types are really part of the spec, I think having it all atomically together is best.
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.
Agreed, I added some comments on the spec's PR (since they're more high-level) - and it would be easier to have the SDK and the spec at the same PR.
|
Nice! |
d07eb1f to
0c7bcd0
Compare
|
@liady we added more tokens and some semantic styles, but we didn't think it was realistic to be able to capture the full range of component <> variable sets (e.g. button borderradius, button primary/secondary/tertiary color, modal border radius, modal padding, etc) because there's just an endless number of components. The goal is really just to get apps to not clash with the host, not to make it look native to the host, so we think this set should be sufficient for now Also, we made it |
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <meta name="color-scheme" content="light dark"> |
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.
I made this change to all the examples so that they can properly have a transparent background. Without it, iframe sets a default black/white background
| --color-smb: #10b981; | ||
| --color-startup: #818cf8; | ||
| } | ||
| color-scheme: light dark; |
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.
Changes in this file are me updating this one example to use the host context styles (with its original styles as fallbacks)
| let colorResolver: HTMLDivElement | null = null; | ||
|
|
||
| // Resolve a CSS color value (handles light-dark() function) | ||
| function resolveColor(cssValue: string, fallback: string): string { |
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 is necessary for this particular example b/c chartJS doesn't play nicely with light-dark by default
6f490f5 to
7166f75
Compare
ochafik
left a comment
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.
Quick high-level comments, will review more thorougly shortly:
- Can we fence under a host capability?
- Can you build a table of equivalence mapping between these variables and OpenAI Apps SDK variables? (add a STYLES.md) Hope to avoid confusion for users targetting both MCP Apps and OAI Apps SDK.
antonpk1
left a comment
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.
awesome contribution!
I think it's fine to not have capability negotiation here - this is just an optional field
also we can evolve set of variables over time in follow-up commits
|
@ochafik if it's something we can follow up on i'd strongly prefer to do that. This PR needs to go out roughly the same time as another one, and that's a bit tough to coordinate |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add styles field to HostContext interface with reference to Theming section - Add Theming section documenting 36 standardized CSS variables - Document Host/App behavior for theming including light-dark() usage - Add Design Decision #4 explaining CSS variables approach - Include JSON example showing light-dark() pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Prevents iframe from automatically applying a default background color. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Updated screenshots to reflect UI changes from styles addition. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
4333ec0 to
8f591b6
Compare
|
@ochafik it can be nice if the MCP Apps React UI SDK would somehow already have the ChatGPT Apps <> MCP Apps style variable mapping, perhaps as a /*
This maps tokens from ChatGPT Apps design system to MCP Apps design system - where the latter are missing.
--mcp-apps-variable-name: var(--chatgpt-apps-variable-name);
*/
--color-background-primary: var(--color-background-primary-solid);
--color-background-secondary: var(--color-background-secondary-solid);
--color-background-tertiary: var(--color-surface-tertiary);
--color-background-inverse: var(--color-text-inverse);
--color-background-ghost: transparent;
--color-background-info: var(--color-background-info-solid);
--color-background-danger: var(--color-background-danger-solid);
--color-background-success: var(--color-background-success-solid);
--color-background-warning: var(--color-background-warning-solid);
--color-border-primary: var(--color-border-primary-outline);
--color-border-secondary: var(--color-border-secondary-outline);
--color-border-tertiary: var(--color-border-subtle);
--color-border-inverse: var(--color-text-inverse);
--color-border-ghost: transparent;
--color-border-info: var(--color-border-info-outline);
--color-border-danger: var(--color-border-danger-outline);
--color-border-success: var(--color-border-success-outline);
--color-border-warning: var(--color-border-warning-outline);
--color-ring-inverse: var(--color-text-inverse);
--border-radius-xs: var(--radius-xs);
--border-radius-sm: var(--radius-sm);
--border-radius-md: var(--radius-md);
--border-radius-lg: var(--radius-lg);
--border-radius-xl: var(--radius-xl);
--border-radius-full: var(--radius-full);
--border-width-regular: 1px;
--shadow-sm: var(--shadow-100);
--shadow-md: var(--shadow-200);
--shadow-lg: var(--shadow-300);This maps the ChatGPT Apps variables to matching MCP Apps variables (the rest of the list exists in both systems). This way app authors would be able to write MCP Apps applications and only use this compatibility layer when targeting ChatGPT Apps. |
Summary
Some choices we made in landing on this architecture / our thought process:
What if a host doesn't provide styles?
Request for input
Making a standard for styling that'll work with many hosts is a tricky thing to get right, so this definitely isn't final, and we welcome discussion on it!
Some things that we are still thinking about and would love input on: