Skip to content

Add initial AMP support for Twenty Nineteen#1587

Merged
westonruter merged 8 commits into1.0from
add/twentynineteen-compat
Nov 15, 2018
Merged

Add initial AMP support for Twenty Nineteen#1587
westonruter merged 8 commits into1.0from
add/twentynineteen-compat

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Nov 3, 2018

There are outstanding things todo here, including:

Fixes #1515.

@westonruter westonruter requested a review from kienstra November 4, 2018 19:51
Copy link
Copy Markdown
Contributor

@kienstra kienstra left a comment

Choose a reason for hiding this comment

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

Approved

Hi @westonruter,
This looks good, and validation of the site with Twenty Nineteen shows no validation error.

Before

before-core-sanitizer-update

After

after-core-sanitizer-update

$native_description = __( 'Reuses active theme\'s templates to display AMP responses but does not use separate URLs for AMP. Your canonical URLs are AMP. AMP-specific blocks are available for inserting into content. Any AMP validation errors are automatically sanitized.', 'amp' );
$builtin_support = in_array( get_template(), array( 'twentyfifteen', 'twentysixteen', 'twentyseventeen' ), true );

$builtin_support = in_array( get_template(), AMP_Core_Theme_Sanitizer::get_supported_themes(), true );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea to abstract this into a method.

@westonruter westonruter changed the title [WIP] Add initial AMP support for Twenty Nineteen Add initial AMP support for Twenty Nineteen Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants