Enable AMP Compatibility

[amedina]

Feature Request
Build the theme with opt-in AMP compatibility. This way, users could start with a baseline AMP-compatible 2019 theme and build AMP-compatible sites on top of it. Would love to help with this.

[westonruter]

For reference…

Example for how to add support to _s: Automattic/_s#1286
And WP Rig has opt-in support as well: https://github.com/wprig/wprig#amp-ready

[aaronjorbin]

I don't think default themes should contain code designed for a 3rd parties plugin.

[westonruter]

@aaronjorbin Normally I'd agree, but it's a plugin that I work on so I make an exception 😉But Matt did suggest once that the next default theme could be canonical AMP.

Nevertheless, the main problem I see is the hard-coded use of the is_amp_endpoint() function in #155. What if instead of that there was a theme-specific filter which any plugin could then hook into to indicate that AMP is the target? For example:

/**
 * Check if AMP plugin is enabled and if AMP endpoint.
 */
function twentynineteen_is_amp() {
	return apply_filters( 'twentynineteen_is_amp', false );
}

Then the AMP plugin or any other plugin that adds AMP compatibility (or child theme) would just have to do the following for Twenty Nineteen:

add_theme_support( 'amp' );
add_filter( 'twentynineteen_is_amp', 'is_amp_endpoint' );

In this way the theme would not be including code that is specific to a 3rd-party plugin. But at the same time, it would facilitate the theme to opt-in to be used in AMPHTML, similar to how WordPress started adding support for HTML5 in addition to XHTML.

[aaronjorbin]

I do think the theme should do everything in its power to make it easy for plugins to work. I don't see where twentynineteen_is_amp is being used in #155 right now. Is it not possible for the amp plugin to detect that twentynineteen is the theme and to handle everything it needs in the plugin?

[westonruter]

Yeah, #155 is somewhat premature at the moment. See the pull request for _s which has examples for how this function would be used: https://github.com/Automattic/_s/pull/1286/files?w=1

Most of it boils down to not enqueueing JS when when in AMP and to instead output AMP alternatives to do things like toggling the nav menu.

It's true that the AMP plugin can add AMP compatibility on top of Twenty Nineteen as it is currently doing for for the other themes currently bundled in core. The logic for doing so is hard-coded into a special core theme sanitizer: https://github.com/Automattic/amp-wp/blob/develop/includes/sanitizers/class-amp-core-theme-sanitizer.php

In order to some things like adding the nav menu toggle, this requires the plugin to use the output-buffered HTML response passed into a DOMDocument to manipulate in order make the necessary changes: https://github.com/Automattic/amp-wp/blob/6bc204df9fd3bb8f1983da29eec9b49300c134fb/includes/sanitizers/class-amp-core-theme-sanitizer.php#L869-L925

It would be ideal if the theme could output the AMP-specific markup in the first place without having to go through hoops to add it after the fact.

[joyously]

Does AMP need the wp_body_open action?

[westonruter]

It doesn't require it. At least one component should it be added right after the body, but it isn't mandatory.

[aaronjorbin]

I'm wondering if we could do all the things needed for the amp plugin to work without DOMDocument as general enhancements to the theme to make it easier to extend for everyone. This could benefit AMP in that it would demonstrate ways for other people to make a theme that would Just Work™, would make the theme more extendable, and wouldn't put third-party specific code in a default theme.

[westonruter]

I think that's what this issue is suggesting, to avoid having to resort to output buffering into a DOMDocument by having the theme output the required markup when appropriate in the first place, right?

[aaronjorbin]

The theme should make it easy for plugins to output any markup they need.

[allancole]

We ended up not adding explicit support for AMP to this theme. To do this properly there needs to be a method to enqueue scripts with an async or defer method which is an old core problem that still needs to be sorted out. As an alternative, I suggested installing the latest version of the AMP plugin, by XWP and @westonruter :-) : https://wordpress.org/plugins/amp/.

Since we're closing out this repo to move the theme development fully to the Core Trac, I'm going to close this issue. Thanks!