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

[thelovekesh]

Summary

Implemented in #6042, reader themes provide support for producing color palettes, gradient presets, and font size styles. This feature leverages get theme support to identify the necessary presets for generating the styles.

Although theme.json can be used by any theme, it serves as a foundation for the new block themes. This PR seeks to enable theme support features styles generation using the theme.json presets to address the block styles breaking issues in reader mode.

Fixes #7471

Checklist

• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[thelovekesh]

@westonruter Although the support for theme.json was added in WP 5.8, the required APIs like wp_get_global_settings was added in WP 5.9. Should I look for a workaround to get it worked with WP 5.8 as well or we are good to support it on WP 5.9+?

---

1. Right now our CSS unit measurement doesn't look robust as it only handles all values in px

amp-wp/src/ReaderThemeSupportFeatures.php

Line 421 in 2a3d437

':root .is-%1$s-text, :root .has-%1$s-font-size { font-size: %2$spx; }',

but there might be cases when someone defines them in em or rem.

2. Also, we will need to calculate values for clamp() to give support for fluid typography.

To solve the above scenarios WP 6.1 has two functions:

• wp_get_typography_value_and_unit()
• wp_get_computed_fluid_typography_value()

but the caveat is they are available prior to WP 6.1.

I'm considering defining them in our own class right now. Please let me know your thoughts?

[westonruter]

If someone is using block themes, I'd say it's highly unlikely that they would be holding off on updating WordPress core. Therefore, if you want to require WordPress 6.1+ to fully support fluid typography, that's fine with me.

[thelovekesh]

But I think we should update the way of determining measurement units. Someone can use different measurement unit even when fluid typography is not being used.

[thelovekesh]

PR can be tested for fluid typography by disabling CSS tree-shaking till #7291 get fixed.

[westonruter]

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

[thelovekesh]

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() 🤔

[westonruter]

Better to never assume, yeah

[thelovekesh]

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

[westonruter]

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

[westonruter]

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' );

[westonruter]

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.

[thelovekesh]

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.

[westonruter]

This seems very close. I'm testing with 2023 with the following content:

<!-- wp:paragraph {"fontSize":"xx-large"} -->
<p class="has-xx-large-font-size">Hello!</p>
<!-- /wp:paragraph -->

Primary Theme (2023)	Reader Theme (2021) Before	Reader Theme (2021) After

So that's great. What's not quite there yet is the Legacy theme:

Something's not right with margin or line-height or something.

[thelovekesh]

Seems like on legacy themes line-height is causing issues.

With line-height: 1.75em

With line-height: 1.5

[westonruter]

In TT3, there is this rule applying to the content container:

body .is-layout-flow > * + * {
    margin-block-start: 1.5rem;
    margin-block-end: 0;
}

Applying these styles to .amp-wp-article-content mostly fixes the issue:

Not perfectly, but better.

[westonruter]

OK, let's change this:

amp-wp/templates/style.php

Line 116 in 4337065

line-height: 1.75em;

To just 1.75.

[westonruter]

line-height: 1.75em	line-height: 1.75

[westonruter]

Something else that isn't quite right, even with line-height:1.75. Adding a background color to a paragraph that has XXL font size:

<!-- wp:paragraph {"backgroundColor":"primary","fontSize":"xx-large"} -->
<p class="has-primary-background-color has-background has-xx-large-font-size">Hello!</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Praesent interdum, elit non cursus tincidunt, nibh nibh congue metus, ac gravida nunc ipsum ut augue. Donec tempor pulvinar maximus. Sed et tellus nec sapien vulputate sollicitudin. Nam auctor nulla imperdiet, sodales leo ultricies, cursus metus. Aenean ullamcorper dui vel neque luctus faucibus non vel odio. Aenean convallis mattis nisi, nec fringilla sem sagittis eget. Duis ut quam venenatis, blandit elit id, iaculis ipsum. Curabitur tristique euismod nisl, a finibus massa dignissim ac. Nullam nisl orci, imperdiet at libero sed, pulvinar vehicula lacus. Duis eleifend sollicitudin pharetra. Maecenas sapien nunc, mollis a velit sed, ultricies accumsan tortor. Ut accumsan sapien id turpis eleifend, at eleifend orci volutpat. Etiam massa arcu, congue sit amet sem vitae, gravida efficitur sem. Curabitur ornare velit velit, ut accumsan tellus convallis at. Nunc sagittis nisl mauris, nec hendrerit leo sagittis et.</p>
<!-- /wp:paragraph -->

TT3	TT2 Reader	Legacy Reader

[thelovekesh]

Seems like the xx-large font size requires more width on the screen but we have restricted it to 840px in .amp-wp-article which wraps the whole article in it.

amp-wp/templates/style.php

Line 203 in d03e179

max-width: 840px;

[westonruter]

Oh, if I actually look at Legacy in a mobile viewport it renders fine:

Screen.recording.2023-03-17.13.20.20.webm

We could fix this by adding negative margins:

p.has-background {
    margin-left: -2.375em;
    margin-right: -2.375em;
}

Screen.recording.2023-03-17.13.24.26.webm

[westonruter]

Another case to check for: groups with backgrounds.

<!-- wp:group {"style":{"spacing":{"padding":{"top":"var:preset|spacing|40","right":"var:preset|spacing|40","bottom":"var:preset|spacing|40","left":"var:preset|spacing|40"}}},"backgroundColor":"primary","layout":{"type":"constrained"}} -->
<div class="wp-block-group has-primary-background-color has-background" style="padding-top:var(--wp--preset--spacing--40);padding-right:var(--wp--preset--spacing--40);padding-bottom:var(--wp--preset--spacing--40);padding-left:var(--wp--preset--spacing--40)"><!-- wp:paragraph {"backgroundColor":"luminous-vivid-amber"} -->
<p class="has-luminous-vivid-amber-background-color has-background">First</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph {"backgroundColor":"vivid-red"} -->
<p class="has-vivid-red-background-color has-background">Second</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->

Should look like this:

[westonruter]

Looking good!

Non-AMP	Desktop Legacy AMP	Mobile Legacy AMP