Global Styles: absorb editor settings transformation in WP_Theme_JSON #30610
Conversation
oandregal
added
[Type] Code Quality
| @@ -242,7 +242,6 @@ public static function get_core_data() { | |||
| return self::$core; | |||
| } | |||
|
|
|||
| $all_blocks = WP_Theme_JSON::ALL_BLOCKS_NAME; | |||
There was a problem hiding this comment.
This was a leftover I found, so I thought I'd remove it here as well.
| @@ -5,107 +5,6 @@ | |||
| * @package gutenberg | |||
| */ | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
This has been moved verbatim to WP_Theme_JSON::get_from_editor_settings, except for the first 4 lines:
$all_blocks = WP_Theme_JSON::ALL_BLOCKS_NAME;
$theme_settings = array();
$theme_settings['settings'] = array();
$theme_settings['settings'][ $all_blocks ] = array();that were collapsed into one:
$theme_settings = array( 'settings' => array() );
| * @package Gutenberg | ||
| */ | ||
|
|
||
| class Theme_JSON_Legacy_Settings_Test extends WP_UnitTestCase { |
There was a problem hiding this comment.
Tests were moved to class-wp-theme-json-test.php verbatim.
There was a problem hiding this comment.
I personally don't like classic with a lot of static functions, why not just regular functions :) but I guess it's a way to gather related logic.
I'd prefer more obvious patterns: factories, loaders, models...
that said this PR doesn't bother me much, I think ultimately, this logic should be hidden entirely inside a "ThemeSettingsLoader" or something like that and the consumer won't need to know if the settings come from theme.json, user, or theme support flags, everything is merged and processed for him.
|
Size Change: 0 B Total Size: 1.42 MB ℹ️ View Unchanged
|
|
I've been thinking about what would be the trade-offs of transforming
I agree there are other areas we need to improve applying what we've learned (higher-level APIs for consumers). I'm curious about your thoughts on these trade-offs for low-level APIs. |
I actually think that's a bad pattern, it leads to dependencies on things that might have changed over time.
I'm not sure I understand how this is related 😬 -- |
Extracted from #30541 so it can land in pieces and make reviews easier/quicker.
This code deals with a theme.json structure and needs to know its internal shape. It's better this is part of the utils that deal with a theme.json array and not leaked to consumers. Further refactorings need to be done to offer higher-level APIs to consumers, but this is a first step in isolating model changes to a single point (WP_Theme_JSON).
How to test
npm run test-unit-php /var/www/html/wp-content/plugins/gutenberg/phpunit/class-wp-theme-json-test.php