-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(fonts)!: fontData instead of getFontData() #13047
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
✅ 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
|
sarah11918
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.
First sentence feels like it could be a little better if we wanted, but then, good to go when you're happy!
|
(As I think I mentioned elsewhere, I didn't realize this was a draft when I reviewed it! I saw how tiny it was and thought I could squeeze it in waiting for other update branch/checks. 😅 So, I'll "un-approve" it, but only because it's not done! |
sarah11918
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.
Will this un-approve?
sarah11918
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.
How about now?
sarah11918
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.
How about now?
If I dismiss, will I have unapproved?
sarah11918
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.
Super close! Just made a note to make sure it's the right release number, an d there's one sentence that doesn't make sense and I'm not sure what it's supposed to be, but this will be good to go really soon!
Co-authored-by: Armand Philippot <[email protected]> Co-authored-by: Sarah Rainsberger <[email protected]>
ArmandPhilippot
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.
I left a nit regarding src, because we're using it in the code snippet and maybe it would be better to explicitly specify this is an array in the description?
And, I guess at one point we'll need to document format and tech too. But this is NWTWWHB, so LGTM. 😄
export interface FontData {
src: Array<{ url: string; format?: string; tech?: string }>;
weight?: string;
style?: string;
}Co-authored-by: Armand Philippot <[email protected]>
sarah11918
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.
LGTM! Thanks both!
Description (required)
Related issues & labels (optional)
For Astro version:
5.16.10. See astro PR #15200.