-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(fonts): family options #13037
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
base: main
Are you sure you want to change the base?
feat(fonts): family options #13037
Conversation
✅ Deploy Preview for astro-docs-2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
delucis
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.
Thanks @florian-lefebvre! Changes seem fine to me. I left some comments, but then realised I maybe had a future idea of the API in mind from our discussions and not actually what is implemented, so maybe my comments would only make sense for the future rather than now.
| </TabItem> | ||
|
|
||
| </Tabs> | ||
| Only available when resolving the new Material Symbols icons. |
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 seems important, so might be better to move it close to the heading?
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.
Not sure how to blend it properly with the existing sentences, but sure! FYI I copied it from unifont docs, where if was below the codeblock
| ### Supporting a 3rd-party unifont provider | ||
| You can define an Astro font provider using a unifont provider under the hood: |
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.
Probably not high priority given we’ve concluded almost none of these actually exist, but it looks like we could potentially provide a built-in “adapter” rather than people having to wire it up manually.
provider: unifontToAstro(acmeProvider()),Just a thought.
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.
We talked about this in various places FYI, quoting myself from the concerned PR:
I considered exposing some kind of unifontToAstroFontProvider() util but I don't think it's worth doing. I'm not aware of any public 3rd-party unifont providers so I don't see the benefit for us to maintain such thing when we can just show an example in docs (deploy-preview-13025--astro-docs-2.netlify.app/en/reference/experimental-flags/fonts#supporting-a-3rd-party-unifont-provider)
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'm not aware of any public 3rd-party unifont providers so I don't see the benefit for us to maintain such thing
I wonder if there’s any point in us maintaining docs as well then? Doesn’t have to be decided in this PR, but I did read this section thinking a bit that no-one is ever going to be faced with needing to do this. (And if they do want to, it’s not impossible to figure it out just with the existing provider docs.)
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.
We talked about it during last T&D, and the conclusion was (IIRC): there are only 2 main usecases for this API which are private registries and unifont providers. Both are gonna be pretty niche anyways.
I personally think it's worth keeping (unifont is great!), it requires basically no maintenance from us.
|
I think the comments about the local provider make sense but yeah it indeed isn't the right PR for them. So I'm gonna mark them as resolved for this PR and look at them when I unify providers later on this week or next week |
Description (required)
Related issues & labels (optional)
For Astro version:
5.16.10. See astro PR #15175.