Skip to content
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: Adds a GB specific resolver to capture changes not yet in Core #54990

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

hellofromtonya
Copy link
Contributor

What?

Adds and uses Gutenberg_Font_Face_Resolver_6_4.

Why?

Font Face is already merged into WordPress Core. Since it loads before the plugin, the plugin's Font Face files do not get loaded into memory. Thus any changes made or needed will not get loaded when WP 6.4 or Core's trunk is used (such as in the test suite).

By having a duplicate of the resolver that is for the plugin only, this new resolver does get loaded into memory and is used when not running WP 6.4 or Core's trunk. Thus all changes get tested.

How?

  • Adds a new resolver that is prefixed for only Gutenberg.
  • When running WP 6.4 or Core's trunk, it unhooks Core's wp_print_font_faces callback, adds a GB specific callback, invokes the new resolver, and then passes the results to wp_print_font_faces().

Testing Instructions

All CI jobs should pass.

And the fonts tests should pass when running:

npm run test:unit:php:base -- --group fonts

@hellofromtonya hellofromtonya removed the request for review from spacedmonkey October 3, 2023 00:00
@hellofromtonya hellofromtonya self-assigned this Oct 3, 2023
@hellofromtonya hellofromtonya added Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended labels Oct 3, 2023
@github-actions
Copy link

github-actions bot commented Oct 3, 2023

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/compat/wordpress-6.4/fonts/font-face/class-gutenberg-font-face-resolver-6-4.php
❔ lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php
❔ lib/compat/wordpress-6.4/fonts/fonts.php
❔ lib/load.php
❔ phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php

@hellofromtonya
Copy link
Contributor Author

This PR should resolve why the tests are failing in #53273.

Copy link
Contributor

@matiasbenedetto matiasbenedetto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this and the Gutenberg specific font face resolver (Gutenberg_Font_Face_Resolver_6_4) it is loading fine. The code of it is the same as in 6.4 core and all the PHP unit tests are running as expected so I think this is good to go.

I think this PR was initially planned to fix some problems on #53273, probably that PR will be replaced by #56447. I tried #56447 with and without the changes in this PR and it's working the same. Apart from that if we anticipate changes in the resolver that should be in Gutenberg this PR is a good addition so I'm approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Typography Font and typography-related issues and PRs Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants