Amp-wp: Add theme support for Twenty Twenty (and WordPress 5.3)

[westonruter]

WordPress 5.3 will have a new default core theme: Twenty Twenty. We need to make sure that it works as well as possible in AMP.

Note that Twenty Nineteen support (#1587) did not have 100% feature parity in AMP specifically due to the navigation+ menu on desktop and the sidebar menu on mobile. Ideally we'd be able to achieve complete parity. Note that this may involve submitting patches to Twenty Twenty itself to reduce dependence on JavaScript for implementing the desired functionality.

[schlessera]

An initial validation run on a site with the new theme in its latest master and the Theme Unit Test data set gives me this:

Crawling the site for AMP validity.
Validating 239 URLs...  100% [======================================] 0:41 / 0:40
Success: 0 crawled URLs have unaccepted issue(s) out of 239 total with AMP validation
issue(s); 239 URLs were crawled.
+--------------------------+-----------+---------------+
| Template or content type | URL Count | Validity Rate |
+--------------------------+-----------+---------------+
| home                     | 1         | 100%          |
| post                     | 49        | 100%          |
| page                     | 18        | 100%          |
| attachment               | 39        | 100%          |
| category                 | 66        | 100%          |
| post_tag                 | 61        | 100%          |
| author                   | 3         | 100%          |
| date                     | 1         | 100%          |
| search                   | 1         | 100%          |
+--------------------------+-----------+---------------+

Here's the "Hello World" post:

It's pretty much the same validation errors on all pages, except for those that contain the VideoPress block. They have additional JS validation issues due to the embed.

I'll dive deeper know to understand what that means and how to fix it.

[schlessera]

Here's the manifest from the frontpage, which has visible defects in terms of styling some of the blocks:

The style[amp-custom] element is populated with:
--
  | 0 B: style[type=text/css]
  | 81 B: style
  | 196 B (49%): link#amp-default-css[rel=stylesheet][href=https://amp-wp.localhost/wp-content/plugins/amp/assets/css/amp-default.css?ver=1.3.1-alpha][type=text/css][media=all]
  | 46595 B (76%): link#twentytwenty-style-css[rel=stylesheet][href=https://amp-wp.localhost/wp-content/themes/twentytwenty/style.css?ver=1.0][type=text/css][media=all]
  | 39 B: style[type=text/css][media=print]
  | 78 B: p.amp-wp-200a92a
  | 164 B: div.wp-block-cover has-background-dim alignleft amp-wp-758017c
  | 150 B: div.wp-block-cover has-pale-pink-background-color has-background-dim has-left-content aligncenter amp-wp-9418096
  | 151 B: div.wp-block-cover has-background-dim alignwide amp-wp-16a1d9a
  | 87 B: div.wp-block-cover has-background-dim-60 has-background-dim amp-wp-661b5da
  | 74 B: div.wp-block-spacer amp-wp-833a708[aria-hidden=true]
  | 83 B: figure.wp-block-pullquote amp-wp-d51f7b5
  | 79 B: p.amp-wp-f6e3d7f
  | 80 B: p.amp-wp-13076d9
  | 77 B: p.amp-wp-f14de1e
  | 87 B: p.has-text-color has-background has-very-light-gray-color amp-wp-3f44715
  | 76 B: p.has-text-color amp-wp-2800f28
  | 151 B: div.wp-block-cover has-background-dim amp-wp-8a2632a
  | Total included size: 48,248 bytes (76% of 62,963 total after tree shaking)
  |  
  | The following stylesheets are too large to be included in style[amp-custom]:
  | 13404 B (78%): link#wp-block-library-css[rel=stylesheet][href=https://amp-wp.localhost/wp-includes/css/dist/block-library/style.min.css?ver=5.2.3][type=text/css][media=all]
  | 2099 B (95%): style#twentytwenty-style-inline-css[type=text/css]
  | Total excluded size: 15,503 bytes (80% of 19,235 total after tree shaking)
  |  
  | Total combined size: 63,751 bytes (77% of 82,198 total after tree shaking)

These defects are gone once you open these posts in singular view.

[westonruter]

What I see from looking at the Gutenberg demo post:

• ~The theme is using meta[http-equiv=Content-Type] when it should be using meta[charset].~ This conversion could be handled by the post-processor instead. Update: See https://github.com/WordPress/twentytwenty/pull/640.
• The theme is enqueueing a construct.js which handles all of the interactive features. This needs to be evaluated.
• The theme includes some standard no-js => js class name replacement. This we handle in the core theme sanitizer by just unhooking wp_head actions like twentyseventeen_javascript_detection.

Interestingly I wasn't seeing excessive_css. My CSS manifest included:

The style[amp-custom] element is populated with:
   220 B: style[type=text/css]
    81 B: style
   583 B (91%): link#amp-default-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/plugins/amp/assets/css/amp-default.css?ver=1.3.1-alpha][type=text/css][media=all]
  5149 B (33%): link#wp-block-library-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/style.css?ver=1569601791][type=text/css][media=all]
     0 B: link#contact-form-7-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/plugins/contact-form-7/includes/css/styles.css?ver=5.1.4][type=text/css][media=all]
 24216 B (65%): link#twentytwenty-style-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/themes/twentytwenty/style.css?ver=0.1][type=text/css][media=all]
    39 B: style[type=text/css][media=print]
   119 B: div.wp-block-cover has-background-dim alignwide amp-wp-d28a8c9
    88 B: img.wp-smiley amp-wp-843f19c[src=https://s.w.org/images/core/emoji/12.0.0-1/72x72/1f44b.png][alt=👋][width=72][height=72]
Total included size: 30,495 bytes (56% of 54,032 total after tree shaking)

I suppose it's just because I don't have enough content on the page. The excessive CSS issue would be addressed by #2326 on Standard mode sites. We should evaluate whether the amount of CSS is going to be too much in a normal case. I wasn't able to see excessive_css when using the Block Unit Test data either:

The style[amp-custom] element is populated with:
    81 B: style
   139 B (41%): link#amp-default-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/plugins/amp/assets/css/amp-default.css?ver=1.3.1-alpha][type=text/css][media=all]
  8764 B (52%): link#wp-block-library-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/plugins/gutenberg/build/block-library/style.css?ver=1569601791][type=text/css][media=all]
     0 B: link#contact-form-7-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/plugins/contact-form-7/includes/css/styles.css?ver=5.1.4][type=text/css][media=all]
 24058 B (65%): link#twentytwenty-style-css[rel=stylesheet][href=https://wordpressdev.lndo.site/content/themes/twentytwenty/style.css?ver=0.1][type=text/css][media=all]
    39 B: style[type=text/css][media=print]
   179 B: div.wp-block-cover has-background-dim amp-wp-6730970
Total included size: 33,260 bytes (60% of 55,004 total after tree shaking)

@schlessera How many posts do you have set to display on the front page? And I assume full content is being displayed?

[schlessera]

How many posts do you have set to display on the front page? And I assume full content is being displayed?

It is set to display 10 posts, however it is displaying 11 because of the sticky post.

[markbiernat]

I can confirm that using AMP+"Twenty Twenty" + 3.5 without any special plugins.
The mobile menu and search tool at the top does not work.
Jetpack mobile is not used.
I do not know if this is the right place to post that. However, I am trying to test as much as AMP+Twenty Twenty as I can and thought I would post it as I know you want to clean it up as much as possible before release.

[schlessera]

The CSS issue on my site is produced because my front page shows the full post for testing the core blocks. So, if a site uses many core blocks, the block stylesheet provided by core will blow up (almost hitting the 50kb on its own here). The more blocks that are used, the less the tree shaking can remove from that big stylesheet, of course.

[westonruter]

@markbiernat Thanks! This is the exact place to submit your testing feedback. The mobile menu not working is due to this point above:

• The theme is enqueueing a construct.js which handles all of the interactive features. This needs to be evaluated.

This file has since been named to index.js. From looking at the source, at the moment we need to account for:

• Cover Modals
• Intrinsic Ratio Videos: This logic shouldn't be relevant in AMP since intrinsic layout is built-in.
• Smooth Scroll: This is actually built-in to CSS via scroll-behavior but AMP also has a AMP.scrollTo() action which has smooth scroll support. It was used in Twenty Seventeen AMP-compat.
• Modal Menu
• Primary Menu
• Toggles?

[schlessera]

the block stylesheet provided by core will blow up

Correction: It's the stylesheet provided by twentytwenty that is blowing up, and therefore the block stylesheet from Core is pushed out of the <style amp-custom> block.

Looking at the generated styles, one big part of that deals with the custom font, "Inter".

Also, some styling is overly verbose because the Gutenberg markup is missing vital information for targeting things like "any block" or "no alignment set", like this example:

.wp-block-columns:not(.alignwide):not(.alignfull),.wp-block-cover:not(.alignwide):not(.alignfull):not(.alignleft):not(.alignright):not(.aligncenter),.wp-block-embed:not(.alignwide):not(.alignfull):not(.alignleft):not(.alignright):not(.aligncenter),.wp-block-gallery:not(.alignwide):not(.alignfull):not(.alignleft):not(.alignright):not(.aligncenter),.wp-block-image:not(.alignwide):not(.alignfull):not(.alignleft):not(.alignright):not(.aligncenter),.wp-block-media-text:not(.alignwide):not(.alignfull),.wp-block-pullquote:not(.alignwide):not(.alignfull):not(.alignleft):not(.alignright):not(.aligncenter),.wp-block-quote,.wp-block-quote.is-style-large,.wp-block-video:not(.alignwide):not(.alignfull):not(.alignleft):not(.alignright):not(.aligncenter){margin-bottom:5rem;margin-top:5rem}

[csossi]

"Search" isn't working on mobile. I can call up the Search field by tapping the magnifying glass, but if I tap within the field to enter a search term, the field disappears

[schlessera]

I can confirm the issue and will create a new PR to fix it.

[schlessera]

The culprit seems to be this change: https://github.com/ampproject/amp-wp/commit/c9b9afbfeae0f72cab4bf97e0431a986abfe89c6#diff-5d05e505f93786061c2078486784c46dR1744-R1748

I'll need to think of a way to fix this without reintroducing the initial problem it was meant to fix.

[westonruter]

"Search" isn't working on mobile. I can call up the Search field by tapping the magnifying glass, but if I tap within the field to enter a search term, the field disappears

@csossi Fix is ready for QA.

[csossi]

verified in qa

[markbiernat]

Is there any update to when the AMP menu and Search will work with the Twenty Twenty theme?

[westonruter]

They work just fine.

Given Twenty Twenty with a header looking like so:

The Search modal opens:

And the mobile menu opens:

[markbiernat]

@westonruter thank you for the reply.
https://political-economy.com/
is an example of a site of mine which it does not work on. I tried various setups and I know it works on the twenty nineteen theme, but as soon as I switch it to Twenty Twenty it does not.

[westonruter]

Humm. For some reason the button elements for the nav menu and search do not have the expected on attribute added.

On your site:

<button class="toggle search-toggle" data-toggle-target=".search-modal" data-toggle-screen-lock="true" data-toggle-body-class="showing-search-modal" data-set-focus=".search-modal .search-field" aria-expanded="false">...</button>

On my machine:

<button class="toggle search-toggle desktop-search-toggle" data-toggle-target=".search-modal" data-toggle-body-class="showing-search-modal" data-set-focus=".search-modal .search-field" aria-expanded="false" on="tap:amp-wp-id.open,amp-wp-id.toggleClass(class=show-modal,force=true),body.toggleClass(class=showing-modal,force=true),amp-wp-id.toggleClass(class='active'),AMP.setState({amp_wp_id: !amp_wp_id}),amp-wp-id-3.toggleClass(class='active'),amp-wp-id-4.toggleClass(class='active'),amp-wp-id-5.toggleClass(class='active'),body.toggleClass(class='showing-search-modal'),search-form-1.focus" id="amp-wp-id-4" data-amp-bind-aria-expanded="amp_wp_id ? 'true' : 'false'">...</button>

I think it's because your theme slug is twentytwenty-master instead of the expected twentytwenty. Rename the theme directory in wp-content/themes to twentytwenty and that should fix the issue. (Switch back to Twenty Nineteen while doing this.)

[markbiernat]

Thank you very much. It is appreciated. I am surprised I did not see that.