-
Notifications
You must be signed in to change notification settings - Fork 37
Fonts: Add Noto Serif SC, KR, and subsetting tool for CJK #684
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
zh_CN locale| if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { | ||
| trigger_error( sprintf( 'Requested font file %s does not exist.', esc_html( $filepath ) ), E_USER_WARNING ); | ||
| } |
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 a helpful error message. Ideally it would be flagged to the translators, because it seems to trigger most often when they've translated the subset into their language, but instead we can silently ignore it. The only effect of ignoring it is that the font is not _pre_loaded, but it is still loaded.
afc90a6 to
b2ffd3d
Compare
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 font file is large. Can use a tool like https://chinese-font.netlify.app/en/online-split/ to slice it to increase loading speed if possible.
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 for pointing out that package — I've tried it out a little and it creates a lot of smaller font files, which would not be great for managing the fonts. I'll do some more testing and see if we can find a balance.
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 for pointing out that package — I've tried it out a little and it creates a lot of smaller font files, which would not be great for managing the fonts. I'll do some more testing and see if we can find a balance.
Documentation is at https://github.com/KonghaYao/cn-font-split/tree/release/packages/ffi-js#%E5%AE%8C%E6%95%B4%E7%9A%84%E5%8F%AF%E4%BC%A0%E5%85%A5%E5%8F%82%E6%95%B0
chunkSize and maxAllowSubsetsCount parameters seem to control the subpacket size
|
@ryelle Would you be able to double check the combined PR is correct? Visually it seems to have merged correctly.. but would like another set of eyes before I go to trunk |
|
@dd32 Yep, this looks good to merge — it will need to be deployed along with parent & main theme updates: WordPress/wporg-parent-2021#173, WordPress/wporg-main-2022#560. |
See WordPress/wporg-parent-2021#149 — This PR adds Noto Serif SC (Simplified Chinese), following the other additions of Noto for Japanese and Central Kurdish. I've also added a filter when calling to preload fonts, this will avoid excess locale conditionals in the blocks file.
On its own, this PR should not have any noticeable effect, except that Noto Kufi will not be preloaded. See WordPress/wporg-parent-2021#173 for actually loading Noto Serif SC, and test instructions.