Refactor theme fonts class for readability

[vcanales]

What?

This Pull Request refactors the them fonts class to improve code readability and maintainability. The main focus is on cleaning up the code.

I felt inclined to do this because when debugging for #660, I found some parts of the code hard to follow.

Why?

This PR addresses the need to make the code more readable and maintainable by:

1. Adding documentation and comments.
2. Simplifying functions and improving their readability.

How?

Key Changes:

1. Class documentation added: Added descriptions and the purpose of the CBT_Theme_Fonts class.

2. Function improvements:

   • Inline Descriptions/Justifications:
     • Refactored make_theme_font_src_absolute() to use array_map for cleaner array handling.
     • Replaced double newlines and improved spacing for better readability.
   • Changed foreach to array_map: This reduces the complexity and makes the handling of arrays cleaner.

3. Code clean-up:

   • Removed some redundant comments in favor of "self documenting" practices.
   • Used shorthand assignment (?? null) for checking array keys.
   • Introduced variables for commonly used expressions.

4. Function calls:

   • Replaced CBT_Theme_Fonts:: with self:: for static method calls within the class, enhancing readability and context awareness.

5. Added test cases for parts of the code that could break on further refactors.

[matiasbenedetto]

Probably we should use this refactor to improve the tests of this class. They currently relay on setting users, creating new themes dynamically and after removing it without restoring the previous state.

Example:

create-block-theme/tests/test-theme-fonts.php

Lines 26 to 60 in eabd382

	public function test_copy_activated_fonts_to_theme() {
	wp_set_current_user( self::$admin_id );
	$user_data_begin = CBT_Theme_JSON_Resolver::get_user_data()->get_settings();
	$test_theme_slug = $this->create_blank_theme();
	$this->activate_user_font();
	$user_data_before = CBT_Theme_JSON_Resolver::get_user_data()->get_settings();
	$theme_data_before = CBT_Theme_JSON_Resolver::get_theme_data()->get_settings();
	$this->save_theme();
	$user_data_after = CBT_Theme_JSON_Resolver::get_user_data()->get_settings();
	$theme_data_after = CBT_Theme_JSON_Resolver::get_theme_data()->get_settings();
	// ensure that the font was added and then removed from user space
	$this->assertarraynothaskey( 'typography', $user_data_begin );
	$this->assertequals( 'open-sans', $user_data_before['typography']['fontFamilies']['custom'][0]['slug'] );
	$this->assertarraynothaskey( 'typography', $user_data_after );
	// Ensure that the font was added to the theme
	$this->assertCount( 1, $theme_data_before['typography']['fontFamilies']['theme'] );
	$this->assertCount( 2, $theme_data_after['typography']['fontFamilies']['theme'] );
	$this->assertEquals( 'open-sans', $theme_data_after['typography']['fontFamilies']['theme'][1]['slug'] );
	// Ensure that the URL was changed to a local file and that it was copied to where it should be
	$this->assertEquals( 'file:./assets/fonts/open-sans-normal-400.ttf', $theme_data_after['typography']['fontFamilies']['theme'][1]['fontFace'][0]['src'][0] );
	$this->assertTrue( file_exists( get_stylesheet_directory() . '/assets/fonts/open-sans-normal-400.ttf' ) );
	$this->uninstall_theme( $test_theme_slug );
	}

I think this kind of test is the cause of the destructive tests we found some time ago and we tried to workaround it here: #618

Could we follow the approach we are using in the base case for the Readme class ?. It can improve 3 aspects of these tests:

• Fine tune control and always predictable active theme for test.
• No destructive test env.
• Avoid realying on CBT complex functions.

[matiasbenedetto]

This is probably not needed.

[vcanales]

Without it, the tests break:

Time: 00:00.357, Memory: 42.50 MB

There was 1 error:

1) Test_Create_Block_Theme_Fonts::test_copy_activated_fonts_to_theme
Undefined array key "typography"

/var/www/html/wp-content/plugins/create-block-theme/tests/test-theme-fonts.php:43

ERRORS!
Tests: 67, Assertions: 163, Errors: 1.
Script phpunit handling the test event returned with error code 2
✖ Command failed with exit code 2
Command failed with exit code 2

[vcanales]

Probably we should use this refactor to improve the tests of this class. They currently relay on setting users, creating new themes dynamically and after removing it without restoring the previous state.

I agree that we should work on that, but since the tests currently cover the already-refactored code, I'd prefer refactoring them in another PR.

[pbking]

Seems fine to me. Quality improvements.