-
Notifications
You must be signed in to change notification settings - Fork 49
[MCP Apps] Add way to pass custom fonts #159
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
3be22a5 to
4d9cc91
Compare
4d9cc91 to
6397ec1
Compare
commit: |
| /** CSS blocks that apps can inject */ | ||
| css?: { | ||
| /** CSS for font loading (@font-face rules or @import statements) */ | ||
| fonts?: 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 not conceptually different than having an arbitrary css chunk that can be injected, which we didn't want to introduce.
Perhaps we need to make it more restricted somehow?
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.
Yes this was considered, but I think it's worth it b/c this is the most ergonomic and flexible approach by far. Also, the host can already just inject arbitrary css directly into the app HTML anyways. So this doesn't change the "security" at all. We kind of have to trust hosts b/c they're already hosting the app and could do any number of things to it
|
|
||
| ```typescript | ||
| if (hostContext.styles?.css?.fonts) { | ||
| applyHostFonts(hostContext.styles.css.fonts); |
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.
Do we want to refer to specific implementations in the abstract spec itself?
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.
Wdym refer to specific implementations? I have more specific examples of how hosts can specify font files elsewhere in this file already if that's what you're asking!
|
|
||
| // Check if already loaded | ||
| if (document.getElementById(styleId)) { | ||
| return; |
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.
nit: probably better to update the style tag content instead of returning early
Motivation and Context
<link>tag or a@font-faceCSS declarationstyles.css.fontsCSS string to Host Context, which apps will inject into a script tag usingapplyHostFontsoruseHostFontsHow Has This Been Tested?
Yes, manually tested
Breaking Changes
None
Types of changes
Checklist
Additional context