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.

Sign up for GitHub

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

Adds an endpoints that returns a list of font families used by a theme #648

Merged
merged 5 commits into from
May 30, 2024
Merged

Adds an endpoints that returns a list of font families used by a theme #648

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0405bc0
adds an endpoints that returns a list of font families used by a them…
matiasbenedetto May 22, 2024
6b65679
remove unwanted line
matiasbenedetto May 22, 2024
55cf62e
move some logic into get_all_fonts
matiasbenedetto May 23, 2024
85001e1
Added get_fonts unit test
pbking May 28, 2024
a159487
Merge branch 'trunk' into add/endpoint-to-get-all-theme-fonts
pbking May 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions admin/create-theme/theme-fonts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,66 @@

class CBT_Theme_Fonts {


/**
* Make the font face theme src urls absolute.
*
* It replaces the 'file:./' prefix with the theme directory uri.
*
* Example: 'file:./assets/fonts/my-font.ttf' -> 'http://example.com/wp-content/themes/my-theme/assets/fonts/my-font.ttf'
* Example: [ 'https://example.com/assets/fonts/my-font.ttf' ] -> [ 'https://example.com/assets/fonts/my-font.ttf' ]
*
* @param array|string $src
* @return array|string
*/
public static function make_theme_font_src_absolute( $src ) {
$make_absolute = function ( $url ) {
if ( str_starts_with( $url, 'file:./' ) ) {
return str_replace( 'file:./', get_stylesheet_directory_uri() . '/', $url );
}
return $url;
};

if ( is_array( $src ) ) {
foreach ( $src as &$url ) {
$url = $make_absolute( $url );
}
} else {
$src = $make_absolute( $src );
}

return $src;
}

/**
* Get all fonts from the theme.json data + all the style variations.
*
* @return array
*/
public static function get_all_fonts() {
$font_families = array();
$theme = CBT_Theme_JSON_Resolver::get_merged_data();
$settings = $theme->get_settings();

if ( isset( $settings['typography']['fontFamilies']['theme'] ) ) {
$font_families = array_merge( $font_families, $settings['typography']['fontFamilies']['theme'] );
}

if ( isset( $settings['typography']['fontFamilies']['custom'] ) ) {
$font_families = array_merge( $font_families, $settings['typography']['fontFamilies']['custom'] );
}

$variations = CBT_Theme_JSON_Resolver::get_style_variations();

foreach ( $variations as $variation ) {
if ( isset( $variation['settings']['typography']['fontFamilies']['theme'] ) ) {
$font_families = array_merge( $font_families, $variation['settings']['typography']['fontFamilies']['theme'] );
}
}

return $font_families;
Copy link
Contributor

@pbking pbking May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If we drop something like this into this get_all_fonts function then we can eliminate the 'make theme absolute' function (which is really just string/array checks and a str_replace), eliminate the iteration logic in the api class and always ensure that the src attributes are arrays.

		// ensure that src is an array
		foreach( $font_families as &$font_family ) {
			if ( isset( $font_family['fontFace'] ) ) {
				foreach( $font_family['fontFace'] as &$font_face ) {
					if ( ! is_array( $font_face['src'] ) ) {
						$font_face['src'] = array( $font_face['src'] );
					}
					// make the src urls absolute
					if ( $absolute_src ) {
						foreach( $font_face['src'] as &$font_face_src ) {
							if ( str_starts_with( $font_face_src, 'file:./' ) ) {
								return str_replace( 'file:./', get_stylesheet_directory_uri() . '/', $font_face_src );
							}
						}
					}
				}
			}
		}

Copy link
Contributor Author

@matiasbenedetto matiasbenedetto May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If we drop something like this into this get_all_fonts function, we can eliminate the 'make theme absolute' .

I don't see the need to grow one function by absorbing the same code as another. The separation of concerns is clear. What's the benefit?

ensure that the src attributes are arrays

Why should we ensure that?

Copy link
Contributor

@pbking pbking May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I see that you have already moved half of it in from the API class; that was most of the concern and that's enough that you can resolve this discussion if you want.

But I find the make_theme_font_src_absolute function overly complicated when the same could be completed using fewer, easier to understand lines in the get_all_fonts function. The make_theme_font_src_absolute logic is not re-used.

By ensuring that the response to the API always returns the src in the form of an array the client won't have to execute the "is it a string or an array" check when it works with this data; it will always be an array (and makes the processing of that easier here too).

}

/**
* Copy any ACTIVATED fonts from USER configuration to THEME configuration including any font face assets.
* Remove any DEACTIVATED fronts from the THEME configuration.
Expand Down
39 changes: 39 additions & 0 deletions includes/class-create-block-theme-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,17 @@ public function register_rest_routes() {
},
),
);
register_rest_route(
'create-block-theme/v1',
'/font-families',
array(
'methods' => 'GET',
'callback' => array( $this, 'rest_get_font_families' ),
'permission_callback' => function () {
return current_user_can( 'edit_theme_options' );
},
),
);
}

function rest_get_theme_data( $request ) {
Expand Down Expand Up @@ -481,6 +492,33 @@ function rest_save_theme( $request ) {
);
}

/**
* Get a list of all the font families used in the theme.
*
* It includes the font families from the theme.json data (theme.json file + global styles) and the theme style variations.
* The font families with font faces containing src urls relative to the theme folder are converted to absolute urls.
*/
function rest_get_font_families( $request ) {
$font_families = CBT_Theme_Fonts::get_all_fonts();

// Iterates through the font families and makes the urls absolute to use in the frontend code.
foreach ( $font_families as &$font_family ) {
if ( isset( $font_family['fontFace'] ) ) {
foreach ( $font_family['fontFace'] as &$font_face ) {
$font_face['src'] = CBT_Theme_Fonts::make_theme_font_src_absolute( $font_face['src'] );
}
}
}
vcanales marked this conversation as resolved.
Show resolved Hide resolved

return new WP_REST_Response(
array(
'status' => 'SUCCESS',
'message' => __( 'Font Families retrieved.', 'create-block-theme' ),
'data' => $font_families,
)
);
}

private function sanitize_theme_data( $theme ) {
$sanitized_theme['name'] = sanitize_text_field( $theme['name'] );
$sanitized_theme['description'] = sanitize_text_field( $theme['description'] ?? '' );
Expand All @@ -492,6 +530,7 @@ private function sanitize_theme_data( $theme ) {
$sanitized_theme['version'] = sanitize_text_field( $theme['version'] ?? '' );
$sanitized_theme['screenshot'] = sanitize_text_field( $theme['screenshot'] ?? '' );
$sanitized_theme['recommended_plugins'] = sanitize_textarea_field( $theme['recommended_plugins'] ?? '' );
$sanitized_theme['font_credits'] = sanitize_textarea_field( $theme['font_credits'] ?? '' );
$sanitized_theme['template'] = '';
$sanitized_theme['slug'] = sanitize_title( $theme['name'] );
$sanitized_theme['text_domain'] = $sanitized_theme['slug'];
Expand Down
Loading