[next] stable fonts #13107
[next] stable fonts #13107
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
|
github-actions
bot
added
the
i18n
sarah11918
added
improve or update documentation
sarah11918
left a comment
sarah11918
left a comment
There was a problem hiding this comment.
So exciting! As discussed, the guides page can be a rewrite focusing on just the Fonts API as now the only documented way to do fonts in Astro! 🎉
So, I only looked through the other pages to see what the content split looked like.That leaves us with:
- Guide page: pretend the existing content never existed, and just guide people to go from "no font" to "font" with this new feature. Use the Related recipe component if you want to salvage existing content and make them separate individual recipe pages.
- v6 upgrade: ✅
- font provider reference: ✅ (minor comments)
- config reference: The opening is way too long; most of that is the stuff that should be incorporated into the guide page (see my comment) because it's explanatory, tells people what they need to know about the feature but links out to the heavy lifting details.
| Astro exports built-in font providers from `astro/config`: | ||
|
|
||
| ```js | ||
| import { fontProviders } from 'astro/config' |
There was a problem hiding this comment.
Does this mean we need an entry on the astro:config page for fontProviders @ArmandPhilippot
There was a problem hiding this comment.
Another ping that GitHub saw fit not to notify me of... 🙄
I guess we could have a new section for astro/config imports but this one is a bit tricky because it contains a mix of things documented elsewhere:
import { defineConfig, envField, fontProviders, getViteConfig, mergeConfig, passthroughImageService, sharpImageService } from 'astro/config'I'm not sure how helpful it is? Maybe as an overview of what is available for someone who don't know how to find the appropriate page?
But since we are not displaying the astro/config exports at the moment, we probably should do that in another PR, with the others, so that it doesn't seem strange to have only one export here.
There was a problem hiding this comment.
Should we create an issue for this so we don't forget?
There was a problem hiding this comment.
I added that to my TODO list, so I'll get to it at some point (maybe next week). So, not necessarily? But if we want to give someone who wants to help a chance, yes, we can create one!
sarah11918
added
the
merge-on-release
Co-authored-by: Sarah Rainsberger <[email protected]>
ArmandPhilippot
left a comment
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Thanks Florian, this looks great! (hope you'll translate all that 😄 )
I left a few suggestions, mostly nitpickings. For the fonts guide, we might want code snippets that work "out of the box".
What I mean is: when learning a new feature, you might want to copy/paste the code snippets in a new project and get something working without asking yourself what you're doing wrong. So, we might need to develop a bit the code snippets (minimal layout, imported Layout).
This is based on a feedback shared by HiDeoo for content collections, and we improved that for v6. Since, we're rewriting the fonts guide, we could take this opportunity to do the same!
I've only suggested changes for "Using a local font file", but we could do the same for "Using Fontsource".
My suggestions in the Font Provider reference are only for consistency (backticks for code).
Co-authored-by: Armand Philippot <[email protected]>
Co-authored-by: Armand Philippot <[email protected]>
ArmandPhilippot
left a comment
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Rereading the font guide, I noticed we're still talking about @font-face while this is now something implemented under the hood. Maybe, we should reword those steps because the user don't need that to use the Font API?
Co-authored-by: Armand Philippot <[email protected]>
| Registering custom fonts for your Astro project is done through [the `fonts` option](/en/reference/configuration-reference/#fonts) in your Astro config. | ||
|
|
||
| For each font you want to use, you must specify its name, a CSS variable, and an Astro font provider. | ||
| For each font you want to use, you must specify its [name](/en/reference/configuration-reference/#fontname), a [CSS variable](/en/reference/configuration-reference/#fontcssvariable), and an Astro font provider. |
There was a problem hiding this comment.
Maybe need to also add a link for the provider property?
sarah11918
left a comment
sarah11918
left a comment
There was a problem hiding this comment.
Aprooooooove! Depending on how quickly you want this merged, we could get some final boss reviews from Yan and Armand. But I think this ticks all the boxes. Great job, Florian!
ArmandPhilippot
left a comment
ArmandPhilippot
left a comment
There was a problem hiding this comment.
This looks great, Florian! 🙌🏽
Some of the highlighting in the Font Provider API tabs is wrong, but otherwise LGTM!
Co-authored-by: Armand Philippot <[email protected]>
Description (required)
Some questions left:
TODO:s in the configuration reference. In the experimental page, there were headings for "Granular font configuration" and "Caching" that I feel would belong where I put them. But I don't feel like I can easily make a section for them there. So yeah not sure what to doFontsAPI at the end, matching what we've done forastro:env. However here I'm wondering if it wouldn't make sense to have it first as it's easier and more powerful (vs. the env guide which has a bunch more things that are absolutely mandatory first)Related issues & labels (optional)
For Astro version:
6.0.0. See astro PR #15291.