-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Font Face: Prepare for merge into Core. #53858
Conversation
* Moves the Font Face files into the compat directory. * Adds `@since 6.4.0`. * Adds Core merge instructions. Note: Leaves the BC Layer files in experimental, as these are plugin only.
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/load.php ❔ lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php ❔ lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face.php ❔ lib/compat/wordpress-6.4/fonts/fonts.php |
Here's the Trac ticket for merging Font Face into Core https://core.trac.wordpress.org/ticket/59165. |
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've reviewed this PR.
I can't say that I fully understand all the nuances, but the explanation makes sense, and the code seems to be good.
This PR does what it says: prepares Font Face for merging into Core.
I'm not able to find any issues with it.
I do see some changes in how the fonts-related functions/classes are loaded, but these changes seem to improve what's already in place.
Therefore, I approve this PR.
@hellofromtonya I see that this and other font-face related PRs have the "Backport to WP Beta/RC" label. That label is only used during Beta/RC period for bug fixes that need cherry-picking. I think the correct label you're after here is "Needs PHP backport", for PRs with PHP changes that need to be manually added to core. Can you confirm if that's the case? Otherwise, during Alpha period no labels are needed as it's assumed all changes will go into the next release (unless they're behind an experiment flag, of course!). |
Aha my bad. Thank you @tellthemachines! |
Removed backport label as Font Face was merged into Core via changeset https://core.trac.wordpress.org/changeset/56500 ✅ |
What?
Prepares Font Face for merge into Core.
Why?
When ready, I'll create a Core PR to merge it into Core's
trunk
for WP 6.4, i.e. to replace the temporary stopgap code.Font Face is a direct replacement for Core's
_wp_theme_json_webfonts_handler()
.'file:./'
placeholder within a theme'stheme.json
file.@font-face
properties.@font-face
CSS in the same way.Thus, Font Face can go into Core with or without Font Library.
How?
@since 6.4.0
.Note: Leaves the BC Layer files in experimental, as these are plugin only.
Testing Instructions
All CI jobs should pass ✅
As functionality is not touched and remains the same as before, no further testing is required.