-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 Library: Font Collection backend #53816
Conversation
Warning: Type of PR label error To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. |
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/experimental/fonts/font-library/class-wp-font-collection.php ❔ phpunit/tests/fonts/font-library/wpFontCollection/__construct.php ❔ phpunit/tests/fonts/font-library/wpFontCollection/getData.php ❔ phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollection.php ❔ phpunit/tests/fonts/font-library/wpFontLibrary/getFontCollections.php ❔ phpunit/tests/fonts/font-library/wpFontLibrary/registerFontCollection.php ❔ phpunit/tests/fonts/font-library/wpRestFontLibraryController/base.php ❔ phpunit/tests/fonts/font-library/wpRestFontLibraryController/getFontCollection.php ❔ phpunit/tests/fonts/font-library/wpRestFontLibraryController/getFontCollections.php ❔ phpunit/tests/fonts/font-library/wpRestFontLibraryController/registerRoutes.php ❔ phpunit/tests/fonts/font-library/wpRestFontLibraryController/uninstallFonts.php ❔ lib/experimental/fonts/font-library/class-wp-font-library.php ❔ lib/experimental/fonts/font-library/class-wp-rest-font-library-controller.php ❔ lib/experimental/fonts/font-library/font-library.php ❔ lib/load.php ❔ phpunit/tests/fonts/font-library/wpFontFamily/base.php ❔ phpunit/tests/fonts/font-library/wpRestFontLibraryController/installFonts.php |
Flaky tests detected in 60997e9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5979891576
|
If we must use static, then follow the pattern in wp_metadata_lazyloader that creates a instance to a static variable. |
@spacedmonkey Thanks for the review! |
Co-authored-by: Tonya Mork <[email protected]>
The source code LTGM and is ready 👍 Next: the tests need updating for the changes and then a code review. And then this PR is ready for merge. Almost over the finish line with this PR. |
I updated the plugin demo code in the PR description and the plugin's zip file based on the latest PR changes. |
I created a follow-up of this PR to load data from URLs. It depends on this one: #53992 |
* Renamed files to remove `-test` suffix (no longer needed). * Removed extra empty line before closing the class. * Used Core's `assertWPError()`.
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.
The source code LGTM 👍
The tests are good enough. I'll follow-up with a test improvement PR.
This PR is ready (assuming the CI jobs are happy).
Thanks, @hellofromtonya, for your work here! |
I'm closing this PR because it's blocked by the changes requested by @spacedmonkey around the static class properties. I asked for more details about why we would need to avoid that pattern in this PR, and @hellofromtonya added some input about why the usage of the static property are intentional. Two weeks later, we didn't receive an answer, so I assume that the reasons presented are valid. This is blocking us from continuing the work on Font Library intended to be included in WordPress 6.4 release. Because of that, I'm closing this PR and opening a new one. Of course, we are free to further discuss this topic on new issues, PRs, the make blog, or slack. |
What?
Add extensibility capabilities to the Font Library.
Provides a way to provide collections of typographic fonts by code.
Why?
Example:
This is all the code required to create a plugin providing a font collection.
Download the plugin as zip ready to test:
my-font-collection.zip
How?
WP_Font_Collection
class.wp_register_font_collection
filter./fonts/collections
andfonts/collections/<id>
endpoints.To do
We would need this: WP_Theme_JSON sanitization is not working below certain level of theme.json #52798
Tracking issue:
Stage 1: #52698
Stage 2: #53307