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 API] Do not print in admin using 'admin_print_styles' for themes with theme.json #50259

Merged
merged 2 commits into from
May 9, 2023

Conversation

hellofromtonya
Copy link
Contributor

@hellofromtonya hellofromtonya commented May 2, 2023

Fixes #50064

What?

Adds decision making around hooking into 'admin_print_styles': If a theme does not have a theme.json file, then hook into the action; else, do not.

Why?

Prior to this PR, hooking into this action caused the fonts to be printed twice for theme's with a theme.json file.

Why?

Fonts are printed via the iframed editor assets functionality which directly invokes wp_print_fonts(). So instead of being in the main web page's <head>, the fonts are printed into the iframe.

For theme's without a theme.json file, there is no iframed editor. Thus, the hook registration is needed to print the fonts into the admin.

How?

  1. Moves both hook registrations into the wp_fonts() initialization code. No need to hook if wp_fonts() is never called.
  2. Checks if a theme without a theme.json is being used. If yes, then hooks into 'admin_print_styles'.

Testing Instructions

Set up on your test site:

  1. Activate the Gutenberg plugin.
  2. Activate TT3 theme.

Test in the admin:

  1. Go to Appearance > Themes (or any admin screen that does not have a block editor in it).
  2. Using your browser's dev tools, search the HMTL for wp-fonts-local <style> element.
    Expected: The element should not exist.
  3. Go to the Site Editor.
  4. Using your browser's dev tools, search the HMTL for wp-fonts-local <style> element.
    Expected: The element should not be in the main document's <head>. Rather, it should only be in the iframe.
  5. Go to Posts > open an existing post.
  6. Using your browser's dev tools, search the HMTL for wp-fonts-local <style> element.
    Expected: The element should not be in the main document's <head>. Rather, it should only be in the iframe.

Test in the front-end:

  1. Go to the front-end.
  2. Using your browser's dev tools, search the HMTL for wp-fonts-local <style> element.
    Expected: The element should be in the main document's <head>.

Screenshots or screencast

Non-block editor admin area
pr50259-1

Site Editor:
pr50259-2

In a Post:
pr50259-3

In the front-end:
pr50259-4

@hellofromtonya hellofromtonya added [Type] Bug An existing feature does not function as intended [Feature] Fonts API labels May 2, 2023
@hellofromtonya hellofromtonya self-assigned this May 2, 2023
@hellofromtonya hellofromtonya marked this pull request as ready for review May 2, 2023 17:47
@hellofromtonya hellofromtonya requested review from aristath, ironprogrammer and anton-vlasenko and removed request for spacedmonkey May 2, 2023 17:47
Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fonts API] Do not print fonts with 'admin_print_styles' for block themes
2 participants