-
Notifications
You must be signed in to change notification settings - Fork 62
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
fix: display theme fonts in font picker #3227
Conversation
🤖 Pull request artifacts
|
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 theme font should not be loaded the same way Stackable loads our Google fonts in the backend & frontend (they are not necessarily Google Fonts). Since they are theme fonts, then they should be loaded from the theme
src/fonts.php
Outdated
|
||
function __construct() { | ||
if ( is_frontend() ) { | ||
add_action( 'init', array( $this, 'gather_theme_fonts' ) ); |
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.
We can optimize this area. Checking the code, looks like this init
action is only needed to load all the fonts once so we can have a local list of what all the fonts are.. then this list is only needed by the is_theme_font
function.
I recommend deleting this add_action
since it's just an initializer, and instead call gather_theme_fonts
directly inside the function is_theme_font
when it's needed. We can also only do the gather once.
Suggestion:
// Change the default value to `false` so we know when it's not yet initialized
public static $theme_fonts = false;
public static function gather_theme_fonts() {
if ( self::$theme_fonts !== false ) {
return self::$theme_fonts;
}
self::$theme_fonts = [];
// Proceed with the original function...
return self::$theme_fonts;
}
public static function is_theme_font( $font_name ) {
return in_array( strtolower( $font_name ), self::gather_theme_fonts() );
}
src/fonts.php
Outdated
if ( ! method_exists( 'WP_Font_Face_Resolver', 'get_fonts_from_theme_json' ) ) { | ||
return false; | ||
} | ||
|
||
$theme_fonts = WP_Font_Face_Resolver::get_fonts_from_theme_json(); | ||
|
||
$response = array(); | ||
foreach ( $theme_fonts as $key => $value ) { | ||
$response[] = $value[ 0 ][ 'font-family' ]; | ||
} |
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.
Looks like this is doing the same thing as gather_theme_fonts
except returning it as a REST response. Maybe we can combine for DRY code
…kable into fix/3195-get-block-theme-fonts # Conflicts: # src/fonts.php
fixes #3195