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

[Reader Mode] Add theme support features from theme.json #7481

Merged
merged 38 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
2a3d437
Add theme support features from theme.json
thelovekesh Mar 13, 2023
5f5d914
Add `ReaderThemeSupportFeatures::get_typography_value_and_unit()`
thelovekesh Mar 14, 2023
0083e8c
Add support for fluid typography in reader mode
thelovekesh Mar 14, 2023
5e141a9
Avoid reducing theme support features for fontSizes while using theme…
thelovekesh Mar 14, 2023
7b4560c
Add check to return correct font size format
thelovekesh Mar 14, 2023
1fc0531
Add test cases for `ReaderThemeSupportFeatures::maybe_use_theme_json()`
thelovekesh Mar 14, 2023
4ce8f09
Update `ReaderThemeSupportFeatures::maybe_use_theme_json()` to be ski…
thelovekesh Mar 14, 2023
8702afe
Merge branch 'develop' into fix/reader-themes/block-theme-styles
thelovekesh Mar 15, 2023
1608869
Update checks for theme.json with `theme_has_theme_json`
thelovekesh Mar 15, 2023
2451c92
Revert test cases for `ReaderThemeSupportFeatures::maybe_use_theme_js…
thelovekesh Mar 15, 2023
e6ff86e
Update `::theme_has_theme_json()` to bail if `wp_get_global_settings(…
thelovekesh Mar 15, 2023
ce11732
Ignore code coverage for functions taken from WP Core
thelovekesh Mar 15, 2023
7487201
Update escaping to use `wp_strip_all_tags()` for css values
thelovekesh Mar 15, 2023
0c604ba
Add tests to verify when fontSize is null in ::get_typography_value_a…
thelovekesh Mar 16, 2023
3cc3845
Add tests for getting theme support features from theme.json
thelovekesh Mar 16, 2023
f1d16af
Skip tests theme.json theme features on WP < 5.9
thelovekesh Mar 16, 2023
d319244
Merge branch 'develop' into fix/reader-themes/block-theme-styles
thelovekesh Mar 16, 2023
722476c
Fix test skip conditiona for `::test_get_theme_support_features_with_…
thelovekesh Mar 16, 2023
1f86856
Merge branch 'develop' into fix/reader-themes/block-theme-styles
thelovekesh Mar 16, 2023
ca4d199
Use `strip_tags()` to escape css
thelovekesh Mar 17, 2023
bcb0081
Use `wp_theme_has_theme_json()` if exists
thelovekesh Mar 17, 2023
6de102d
Use `wp_get_typography_value_and_unit` if exists
thelovekesh Mar 17, 2023
910ee2f
Remove React 17 requirement for `AfterActivationSiteScan` service
thelovekesh Mar 16, 2023
72af542
Fix phpcs errors due to `strip_tags()`
thelovekesh Mar 17, 2023
5723c78
Fix usage of `::get_typography_value_and_unit()`
thelovekesh Mar 17, 2023
7696b82
Merge branch 'develop' into fix/reader-themes/block-theme-styles
thelovekesh Mar 17, 2023
9f690e4
Add test case to verify fluid typography in reader mode
thelovekesh Mar 17, 2023
bf09356
Fix test skip message
thelovekesh Mar 17, 2023
7fa8769
Remove resetting active theme state in test method
thelovekesh Mar 17, 2023
d03e179
Remove `wp_get_global_settings` function exists check from `::theme_h…
thelovekesh Mar 17, 2023
1b2e100
Update line height to `1.75` in legacy reader theme
thelovekesh Mar 20, 2023
7996b3a
Fix background margin; block group margin in reader mode
thelovekesh Mar 20, 2023
dda1105
Add `::print_spacing_sizes_custom_properties()`
thelovekesh Mar 20, 2023
9e5783d
Update global settings keys with constants
thelovekesh Mar 20, 2023
2ac115a
Add test cases to account for spacing size features in `theme.json`
thelovekesh Mar 20, 2023
4affefd
Update `::print_spacing_sizes_custom_properties()` to determine corre…
thelovekesh Mar 20, 2023
8ed814d
Update PHPUnit multisite workflow to continue on error
thelovekesh Mar 20, 2023
21f12f2
Merge branch 'develop' into fix/reader-themes/block-theme-styles
thelovekesh Mar 20, 2023
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
197 changes: 194 additions & 3 deletions src/ReaderThemeSupportFeatures.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,48 @@ final class ReaderThemeSupportFeatures implements Service, Registerable {
*/
const KEY_GRADIENT = 'gradient';

/**
* Key gradients.
*
* @var string
*/
const KEY_GRADIENTS = 'gradients';

/**
* Key palette.
*
* @var string
*/
const KEY_PALETTE = 'palette';

/**
* Key fontSizes.
*
* @var string
*/
const KEY_FONT_SIZES = 'fontSizes';

/**
* Key typography.
*
* @var string
*/
const KEY_TYPOGRAPHY = 'typography';

/**
* Key for `theme` presets in block editor.
*
* @var string
*/
const KEY_THEME = 'theme';

/**
* Key for `default` presets in block editor.
*
* @var string
*/
const KEY_DEFAULT = 'default';

/**
* Action fired when the cached primary_theme_support should be updated.
*
Expand All @@ -91,6 +133,17 @@ final class ReaderThemeSupportFeatures implements Service, Registerable {
self::FEATURE_EDITOR_FONT_SIZES => [ self::KEY_SLUG, self::KEY_SIZE ],
];

/**
* The theme.json paths mapping to be fetched using `wp_get_global_settings()`.
*
* @var array[]
*/
const SUPPORTED_THEME_JSON_FEATURES = [
self::FEATURE_EDITOR_COLOR_PALETTE => [ self::KEY_COLOR, self::KEY_PALETTE ],
self::FEATURE_EDITOR_GRADIENT_PRESETS => [ self::KEY_COLOR, self::KEY_GRADIENTS ],
self::FEATURE_EDITOR_FONT_SIZES => [ self::KEY_TYPOGRAPHY, self::KEY_FONT_SIZES ],
];

/**
* Reader theme loader.
*
Expand Down Expand Up @@ -203,13 +256,35 @@ public function update_cached_theme_support() {
*/
public function get_theme_support_features( $reduced = false ) {
$features = [];

foreach ( array_keys( self::SUPPORTED_FEATURES ) as $feature_key ) {
$feature_value = current( (array) get_theme_support( $feature_key ) );
if ( $this->maybe_use_theme_json() ) {
$feature_value = [];
$global_settings = wp_get_global_settings( self::SUPPORTED_THEME_JSON_FEATURES[ $feature_key ], 'theme' );

if ( isset( $global_settings[ self::KEY_THEME ] ) ) {
$feature_value = array_merge( $feature_value, $global_settings[ self::KEY_THEME ] );
}

if ( isset( $global_settings[ self::KEY_DEFAULT ] ) ) {
$feature_value = array_merge( $feature_value, $global_settings[ self::KEY_DEFAULT ] );
}
} else {
$feature_value = current( (array) get_theme_support( $feature_key ) );
}

if ( ! is_array( $feature_value ) || empty( $feature_value ) ) {
continue;
}

// Avoid reducing font sizes if theme.json is used for the sake of fluid typography.
if ( $this->maybe_use_theme_json() && self::FEATURE_EDITOR_FONT_SIZES === $feature_key ) {
$reduced = false;
}

if ( $reduced ) {
$features[ $feature_key ] = [];

foreach ( $feature_value as $item ) {
if ( $this->has_required_feature_props( $feature_key, $item ) ) {
$features[ $feature_key ][] = wp_array_slice_assoc( $item, self::SUPPORTED_FEATURES[ $feature_key ] );
Expand All @@ -219,9 +294,29 @@ public function get_theme_support_features( $reduced = false ) {
$features[ $feature_key ] = $feature_value;
}
}

return $features;
}

/**
* Check if theme.json can be used to determine theme support.
* Due to the lack of `wp_get_global_settings()`, this will always return false for WP < 5.9.
*
* @since 2.4.1
*
* @return bool Whether theme.json can be used to determine theme support.
*/
private function maybe_use_theme_json() {
// wp_get_global_settings() is only available in WP 5.9+.
if ( ! function_exists( 'wp_get_global_settings' ) ) {
return false;
}

// Do not rely on `wp_is_block_theme()` as theme.json can be used in non-block themes.
// See: https://developer.wordpress.org/themes/advanced-topics/theme-json/#a-theme-json-can-be-added-to-any-theme.
return ( is_readable( get_stylesheet_directory() . '/theme.json' ) ? true : is_readable( get_template_directory() . '/theme.json' ) );
Copy link
Member

Choose a reason for hiding this comment

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

This would seem simpler as:

Suggested change
return ( is_readable( get_stylesheet_directory() . '/theme.json' ) ? true : is_readable( get_template_directory() . '/theme.json' ) );
return is_readable( get_stylesheet_directory() . '/theme.json' ) || is_readable( get_template_directory() . '/theme.json' );

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I suggest using wp_theme_has_theme_json() if it exists, or else copy the function as a method into this class:

https://github.com/WordPress/wordpress-develop/blob/200868214a1ae0a108dac491677ba82e7541fc8d/src/wp-includes/global-styles-and-settings.php#L384-L414

Note how it caches the file exists check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note how it caches the file exists check.

Oh yes. Bette to copy it since we will need to check for theme.json on WP 5.9+ and this function is added in WP 6.2.

}

/**
* Determines whether the request is for an AMP page in Reader mode.
*
Expand Down Expand Up @@ -327,10 +422,20 @@ private function print_editor_font_sizes_styles( array $font_sizes ) {
if ( ! $this->has_required_feature_props( self::FEATURE_EDITOR_FONT_SIZES, $font_size ) ) {
continue;
}

// Just in case the font size is not in the expected format.
$font_size[ self::KEY_SIZE ] = $this->get_typography_value_and_unit( $font_size[ self::KEY_SIZE ] );

if ( empty( $font_size[ self::KEY_SIZE ] ) ) {
continue;
}

printf(
':root .is-%1$s-text, :root .has-%1$s-font-size { font-size: %2$spx; }',
':root .is-%1$s-text, :root .has-%1$s-font-size { font-size: %2$s; }',
sanitize_key( $font_size[ self::KEY_SLUG ] ),
(float) $font_size[ self::KEY_SIZE ]
function_exists( 'wp_get_typography_font_size_value' )
? esc_attr( wp_get_typography_font_size_value( $font_size ) )
: esc_attr( $font_size[ self::KEY_SIZE ] )
Copy link
Member

Choose a reason for hiding this comment

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

Minor point, but I don't think esc_attr() is right here. I think strip_tags() is the right choice. See https://github.com/WordPress/wordpress-develop/blob/200868214a1ae0a108dac491677ba82e7541fc8d/src/wp-includes/theme.php#L1891-L1896

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does make sense, but I am not sure if we will ever get tags here. Or better to never assume and update it to use strip_tags() 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Better to never assume, yeah

Copy link
Collaborator Author

@thelovekesh thelovekesh Mar 15, 2023

Choose a reason for hiding this comment

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

Just realized, we will still need the escaping, as strip_tags is not an escaping utility.

Copy link
Member

Choose a reason for hiding this comment

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

Well, it is somewhat. It's what core uses in wp_custom_css_cb().

);
}
echo '</style>';
Expand Down Expand Up @@ -385,4 +490,90 @@ public function get_relative_luminance_from_hex( $hex ) {
$lum = ( 0.2126 * $red ) + ( 0.7152 * $green ) + ( 0.0722 * $blue );
return (int) round( $lum );
}

/**
* Checks a string for a unit and value and returns an array
* consisting of `'value'` and `'unit'`, e.g. array( '42', 'rem' ).
*
* Copied from `wp_get_typography_value_and_unit()`
*
* @see https://github.com/WordPress/WordPress/blob/9caf1c4adeddff2577c24d622ebbbf278a671271/wp-includes/block-supports/typography.php#L297
*
* @since 2.4.1
*
* @param string|int|float $raw_value Raw size value from theme.json.
* @param array $options {
* Optional. An associative array of options. Default is empty array.
*
* @type string $coerce_to Coerce the value to rem or px. Default `'rem'`.
* @type int $root_size_value Value of root font size for rem|em <-> px conversion. Default `16`.
* @type string[] $acceptable_units An array of font size units. Default `array( 'rem', 'px', 'em' )`;
* }
* @return string|null The value and unit, or null if the value is empty.
*/
private function get_typography_value_and_unit( $raw_value, $options = [] ) {
if ( ! is_string( $raw_value ) && ! is_int( $raw_value ) && ! is_float( $raw_value ) ) {
_doing_it_wrong(
__METHOD__,
esc_html__( 'Raw size value must be a string, integer, or float.', 'default' ),
'2.4.1'
);
return null;
}

if ( empty( $raw_value ) ) {
return null;
}

// Converts numbers to pixel values by default.
if ( is_numeric( $raw_value ) ) {
$raw_value = $raw_value . 'px';
}

$defaults = [
'coerce_to' => '',
'root_size_value' => 16,
'acceptable_units' => [ 'rem', 'px', 'em' ],
];

$options = wp_parse_args( $options, $defaults );

$acceptable_units_group = implode( '|', $options['acceptable_units'] );
$pattern = '/^(\d*\.?\d+)(' . $acceptable_units_group . '){1,1}$/';

preg_match( $pattern, $raw_value, $matches );

// Bails out if not a number value and a px or rem unit.
if ( ! isset( $matches[1] ) || ! isset( $matches[2] ) ) {
return null;
}

$value = $matches[1];
$unit = $matches[2];

/*
* Default browser font size. Later, possibly could inject some JS to
* compute this `getComputedStyle( document.querySelector( "html" ) ).fontSize`.
*/
if ( 'px' === $options['coerce_to'] && ( 'em' === $unit || 'rem' === $unit ) ) {
$value = $value * $options['root_size_value'];
$unit = $options['coerce_to'];
}

if ( 'px' === $unit && ( 'em' === $options['coerce_to'] || 'rem' === $options['coerce_to'] ) ) {
$value = $value / $options['root_size_value'];
$unit = $options['coerce_to'];
}

/*
* No calculation is required if swapping between em and rem yet,
* since we assume a root size value. Later we might like to differentiate between
* :root font size (rem) and parent element font size (em) relativity.
*/
if ( ( 'em' === $options['coerce_to'] || 'rem' === $options['coerce_to'] ) && ( 'em' === $unit || 'rem' === $unit ) ) {
$unit = $options['coerce_to'];
}

return round( $value, 3 ) . $unit;
}
}
49 changes: 49 additions & 0 deletions tests/php/src/ReaderThemeSupportFeaturesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use AmpProject\AmpWP\ReaderThemeSupportFeatures;
use AmpProject\AmpWP\Tests\Helpers\LoadsCoreThemes;
use AmpProject\AmpWp\Tests\Helpers\PrivateAccess;
use org\bovigo\vfs;

/** @coversDefaultClass \AmpProject\AmpWP\ReaderThemeSupportFeatures */
final class ReaderThemeSupportFeaturesTest extends DependencyInjectedTestCase {
Expand Down Expand Up @@ -428,6 +429,54 @@ public function test_get_relative_luminance_from_hex( $hex, $luminance ) {
);
}

/**
* @covers ::maybe_use_theme_json()
*/
public function test_maybe_use_theme_json() {
if ( function_exists( 'wp_get_global_settings' ) ) {

$current_theme = wp_get_theme()->get_stylesheet();

switch_theme( 'twentytwentythree' );

if ( 'twentytwentythree' !== wp_get_theme()->get_stylesheet() ) {
$this->markTestSkipped( 'Twenty Twenty Three theme is not activated.' );
}

$this->assertTrue( file_exists( get_stylesheet_directory() . '/theme.json' ) );
$this->assertTrue( $this->call_private_method( $this->instance, 'maybe_use_theme_json' ) );

$themes_directory = 'themes';
$mock_theme = 'example-theme';
$mock_directory = vfs\vfsStream::setup( $themes_directory, null, [ $mock_theme => [] ] );

// Add filters so that get_template_directory() the theme in the mock filesystem.
add_filter(
'theme_root',
function() use ( $mock_directory ) {
return $mock_directory->url();
}
);

add_filter(
'template',
function() use ( $mock_theme ) {
return $mock_theme;
}
);

$mock_directory->getChild( $mock_theme )->addChild( vfs\vfsStream::newFile( 'theme.json' ) );

$this->assertTrue( file_exists( get_template_directory() . '/theme.json' ) );
$this->assertTrue( $this->call_private_method( $this->instance, 'maybe_use_theme_json' ) );

// Switch back to the current theme.
switch_theme( $current_theme );
} else {
$this->markTestSkipped( 'This test is only relevant for WordPress 5.9 and above.' );
}
}

/** @param array $features */
private function add_theme_supports( $features ) {
foreach ( $features as $feature => $supports ) {
Expand Down