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.

Sign up for GitHub

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

Add page support #825

Merged
merged 13 commits into from
Dec 8, 2017
Merged

Add page support #825

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6b4de15
176: add page support
ThierryA Dec 7, 2017
c5231b0
Fix amp_get_permalink() for permalinks containing query params (e.g. …
westonruter Dec 6, 2017
dcf7eb1
Always use amp query var in permalinks for hierarchical post types
westonruter Dec 8, 2017
3a76316
Restore Schema.org properties for WebPage entity
westonruter Dec 8, 2017
2ae58d2
Show AMP as disabled without UI to make enabled if post is password p…
westonruter Dec 7, 2017
e7221f4
Return false from post_supports_amp() for homepage and page for posts…
westonruter Dec 8, 2017
51c5b17
Update readme
westonruter Dec 8, 2017
5dd2bc6
Refactor header/footer template parts to extract html-start/html-end …
westonruter Dec 8, 2017
57f5849
Always display AMP status + notice where applicable
ThierryA Dec 8, 2017
269b937
Improve post status notice
ThierryA Dec 8, 2017
cff791e
Include link to AMP post type settings screen in notice; use alt style
westonruter Dec 8, 2017
9630709
Refactor amp_post_supports_error() into AMP_Post_Type_Support::get_su…
westonruter Dec 8, 2017
34a0e9e
Include fallback error message for when all known reasons for unavail…
westonruter Dec 8, 2017
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
32 changes: 30 additions & 2 deletions includes/amp-helper-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,47 @@
<?php

/**
* Get AMP permalink.
*
* @since 0.1
*
* @param int $post_id Post ID.
* @return string AMP permalink.
*/
function amp_get_permalink( $post_id ) {

/**
* Filters the AMP permalink to short-circuit normal generation.
*
* Returning a non-false value in this filter will cause the `get_permalink()` to get called and the `amp_get_permalink` filter to not apply.
*
* @since 0.4
*
* @param false $url Short-circuited URL.
* @param int $post_id Post ID.
*/
$pre_url = apply_filters( 'amp_pre_get_permalink', false, $post_id );

if ( false !== $pre_url ) {
return $pre_url;
}

$parsed_url = wp_parse_url( get_permalink( $post_id ) );
$structure = get_option( 'permalink_structure' );
if ( empty( $structure ) ) {
$amp_url = add_query_arg( AMP_QUERY_VAR, 1, get_permalink( $post_id ) );
if ( empty( $structure ) || ! empty( $parsed_url['query'] ) || is_post_type_hierarchical( get_post_type( $post_id ) ) ) {
$amp_url = add_query_arg( AMP_QUERY_VAR, '', get_permalink( $post_id ) );
} else {
$amp_url = trailingslashit( get_permalink( $post_id ) ) . user_trailingslashit( AMP_QUERY_VAR, 'single_amp' );
}

/**
* Filters AMP permalink.
*
* @since 0.2
*
* @param false $amp_url AMP URL.
* @param int $post_id Post ID.
*/
return apply_filters( 'amp_get_permalink', $amp_url, $post_id );
}

Expand Down
30 changes: 18 additions & 12 deletions includes/class-amp-post-template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ public function get_customizer_setting( $name, $default = null ) {
}

public function load() {
$this->load_parts( array( 'single' ) );
$template = is_page() ? 'page' : 'single';
$this->load_parts( array( $template ) );
}

public function load_parts( $templates ) {
Expand Down Expand Up @@ -254,22 +255,27 @@ private function build_post_data() {
) );

$metadata = array(
'@context' => 'http://schema.org',
'@type' => 'BlogPosting',
'@context' => 'http://schema.org',
'mainEntityOfPage' => $this->get( 'canonical_url' ),
'publisher' => array(
'headline' => $post_title,
'publisher' => array(
'@type' => 'Organization',
'name' => $this->get( 'blog_name' ),
),
'headline' => $post_title,
'datePublished' => date( 'c', $post_publish_timestamp ),
Copy link
Member

Choose a reason for hiding this comment

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

These Schema.org properties are also relevant to WebPage entities: http://schema.org/WebPage

So I don't see why we need to limit them just to posts.

Copy link
Collaborator Author

@ThierryA ThierryA Dec 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@westonruter I would argue that. This really depends on the use cases but in most cases pages published/modified date are not relevant, or even inconvenient. It is a very common use case not to include the datePublished and datePublished. If we take Apple for example, we can see that datePublished is used on news article but not on pages.

Copy link
Member

Choose a reason for hiding this comment

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

True, but this is just in regards to the template rendering of the data. We're talking about metadata here. It should be up to the application to decide whether or not to show the datePublished if the type is WebPage. In other words, the WP REST API for pages includes the published date, and that I think is a better parallel to the Schema.org metadata here.

Copy link
Collaborator Author

@ThierryA ThierryA Dec 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Sounds good, I am on board 😄

'dateModified' => date( 'c', $post_modified_timestamp ),
'author' => array(
'@type' => 'Person',
'name' => $post_author->display_name,
'name' => $this->get( 'blog_name' ),
),
);

if ( is_page() ) {
$metadata['@type'] = 'WebPage';
} else {
$metadata['@type'] = 'BlogPosting';
$metadata['datePublished'] = date( 'c', $post_publish_timestamp );
$metadata['dateModified'] = date( 'c', $post_modified_timestamp );
$metadata['author'] = array(
'@type' => 'Person',
'name' => $post_author->display_name,
);
}

$site_icon_url = $this->get( 'site_icon_url' );
if ( $site_icon_url ) {
$metadata['publisher']['logo'] = array(
Expand Down
2 changes: 1 addition & 1 deletion includes/class-amp-post-type-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static function init() {
* @return string[] Post types.
*/
public static function get_builtin_supported_post_types() {
return array_filter( array( 'post' ), 'post_type_exists' );
return array_filter( array( 'post', 'page' ), 'post_type_exists' );
}

/**
Expand Down
25 changes: 24 additions & 1 deletion templates/footer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,32 @@
<?php
/**
* Footer template.
*
* @package AMP
*/

/**
* Context.
*
* @var AMP_Post_Template $this
*/
?>
<footer class="amp-wp-footer">
<div>
<h2><?php echo esc_html( $this->get( 'blog_name' ) ); ?></h2>
<p>
<a href="<?php echo esc_url( esc_html__( 'https://wordpress.org/', 'amp' ) ); ?>"><?php echo esc_html( sprintf( __( 'Powered by %s', 'amp' ), 'WordPress' ) ); ?></a>
<a href="<?php echo esc_url( esc_html__( 'https://wordpress.org/', 'amp' ) ); ?>">
<?php
// translators: %1$s is WordPress.
echo esc_html( sprintf( __( 'Powered by %s', 'amp' ), 'WordPress' ) );
?>
</a>
</p>
<a href="#top" class="back-to-top"><?php esc_html_e( 'Back to top', 'amp' ); ?></a>
</div>
</footer>

<?php do_action( 'amp_post_template_footer', $this ); ?>
Copy link
Member

Choose a reason for hiding this comment

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

Putting this inside of footer will be problematic for any themes that have their own existing footer.php override.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great thoughts!


</body>
</html>
28 changes: 28 additions & 0 deletions templates/header.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/**
* Header template.
*
* @package AMP
*/

/**
* Context.
*
* @var AMP_Post_Template $this
*/
?>
<!doctype html>
<html amp <?php echo AMP_HTML_Utils::build_attributes_string( $this->get( 'html_tag_attributes' ) ); // WPCS: XSS ok. ?>>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1,minimum-scale=1,maximum-scale=1,user-scalable=no">
<?php do_action( 'amp_post_template_head', $this ); ?>
<style amp-custom>
<?php $this->load_parts( array( 'style' ) ); ?>
<?php do_action( 'amp_post_template_css', $this ); ?>
</style>
</head>

<body class="<?php echo esc_attr( $this->get( 'body_class' ) ); ?>">

<?php $this->load_parts( array( 'header-bar' ) ); ?>
28 changes: 28 additions & 0 deletions templates/page.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/**
* Page view template.
*
* @package AMP
*/

/**
* Context.
*
* @var AMP_Post_Template $this
*/
?>
<?php $this->load_parts( array( 'header' ) ); ?>

<article class="amp-wp-article">
<header class="amp-wp-article-header">
<h1 class="amp-wp-title"><?php echo esc_html( $this->get( 'post_title' ) ); ?></h1>
</header>

<?php $this->load_parts( array( 'featured-image' ) ); ?>

<div class="amp-wp-article-content">
<?php echo $this->get( 'post_amp_content' ); // WPCS: XSS ok. Handled in AMP_Content::transform(). ?>
</div>
</article>

<?php $this->load_parts( array( 'footer' ) ); ?>
40 changes: 16 additions & 24 deletions templates/single.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,33 @@
<!doctype html>
<html amp <?php echo AMP_HTML_Utils::build_attributes_string( $this->get( 'html_tag_attributes' ) ); ?>>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1,minimum-scale=1,maximum-scale=1,user-scalable=no">
<?php do_action( 'amp_post_template_head', $this ); ?>
<style amp-custom>
<?php $this->load_parts( array( 'style' ) ); ?>
<?php do_action( 'amp_post_template_css', $this ); ?>
</style>
</head>

<body class="<?php echo esc_attr( $this->get( 'body_class' ) ); ?>">

<?php $this->load_parts( array( 'header-bar' ) ); ?>
<?php
/**
* Single view template.
*
* @package AMP
*/

/**
* Context.
*
* @var AMP_Post_Template $this
*/
?>
<?php $this->load_parts( array( 'header' ) ); ?>

<article class="amp-wp-article">

<header class="amp-wp-article-header">
<h1 class="amp-wp-title"><?php echo wp_kses_data( $this->get( 'post_title' ) ); ?></h1>
<h1 class="amp-wp-title"><?php echo esc_html( $this->get( 'post_title' ) ); ?></h1>
<?php $this->load_parts( apply_filters( 'amp_post_article_header_meta', array( 'meta-author', 'meta-time' ) ) ); ?>
</header>

<?php $this->load_parts( array( 'featured-image' ) ); ?>

<div class="amp-wp-article-content">
<?php echo $this->get( 'post_amp_content' ); // amphtml content; no kses ?>
<?php echo $this->get( 'post_amp_content' ); // WPCS: XSS ok. Handled in AMP_Content::transform(). ?>
</div>

<footer class="amp-wp-article-footer">
<?php $this->load_parts( apply_filters( 'amp_post_article_footer_meta', array( 'meta-taxonomy', 'meta-comments-link' ) ) ); ?>
</footer>

</article>

<?php $this->load_parts( array( 'footer' ) ); ?>

<?php do_action( 'amp_post_template_footer', $this ); ?>

</body>
</html>
4 changes: 2 additions & 2 deletions templates/style.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
display: flex;
flex-wrap: wrap;
justify-content: space-between;
margin: 1.5em 16px 1.5em;
margin: 1.5em 16px 0;
}

.amp-wp-title {
Expand All @@ -180,7 +180,7 @@
flex: 2 1 50%;
font-size: .875em;
line-height: 1.5em;
margin: 0;
margin: 0 0 1.5em;
padding: 0;
}

Expand Down
105 changes: 105 additions & 0 deletions tests/test-amp-helper-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<?php
/**
* Test AMP helper functions.
*
* @package AMP
*/

/**
* Class Test_AMP_Helper_Functions
*/
class Test_AMP_Helper_Functions extends WP_UnitTestCase {

/**
* Filter for amp_pre_get_permalink and amp_get_permalink.
*
* @param string $url URL.
* @param int $post_id Post ID.
* @return string URL.
*/
public function return_example_url( $url, $post_id ) {
$current_filter = current_filter();
return 'http://overridden.example.com/?' . build_query( compact( 'url', 'post_id', 'current_filter' ) );
}

/**
* Test amp_get_permalink() without pretty permalinks.
*
* @covers amp_get_permalink()
*/
public function test_amp_get_permalink_without_pretty_permalinks() {
delete_option( 'permalink_structure' );
flush_rewrite_rules();

$drafted_post = $this->factory()->post->create( array(
'post_name' => 'draft',
'post_status' => 'draft',
'post_type' => 'post',
) );
$published_post = $this->factory()->post->create( array(
'post_name' => 'publish',
'post_status' => 'publish',
'post_type' => 'post',
) );
$published_page = $this->factory()->post->create( array(
'post_name' => 'publish',
'post_status' => 'publish',
'post_type' => 'page',
) );

$this->assertStringEndsWith( '&amp', amp_get_permalink( $published_post ) );
$this->assertStringEndsWith( '&amp', amp_get_permalink( $drafted_post ) );
$this->assertStringEndsWith( '&amp', amp_get_permalink( $published_page ) );

add_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
add_filter( 'amp_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
$url = amp_get_permalink( $published_post );
$this->assertContains( 'current_filter=amp_pre_get_permalink', $url );
$this->assertContains( 'url=0', $url );

remove_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10 );
$url = amp_get_permalink( $published_post );
$this->assertContains( 'current_filter=amp_get_permalink', $url );
}

/**
* Test amp_get_permalink() with pretty permalinks.
*
* @covers amp_get_permalink()
*/
public function test_amp_get_permalink_with_pretty_permalinks() {
global $wp_rewrite;
update_option( 'permalink_structure', '/%year%/%monthnum%/%day%/%postname%/' );
$wp_rewrite->use_trailing_slashes = true;
$wp_rewrite->init();
$wp_rewrite->flush_rules();

$drafted_post = $this->factory()->post->create( array(
'post_name' => 'draft',
'post_status' => 'draft',
) );
$published_post = $this->factory()->post->create( array(
'post_name' => 'publish',
'post_status' => 'publish',
) );
$published_page = $this->factory()->post->create( array(
'post_name' => 'publish',
'post_status' => 'publish',
'post_type' => 'page',
) );

$this->assertStringEndsWith( '&amp', amp_get_permalink( $drafted_post ) );
$this->assertStringEndsWith( '/amp/', amp_get_permalink( $published_post ) );
$this->assertStringEndsWith( '?amp', amp_get_permalink( $published_page ) );

add_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
add_filter( 'amp_get_permalink', array( $this, 'return_example_url' ), 10, 2 );
$url = amp_get_permalink( $published_post );
$this->assertContains( 'current_filter=amp_pre_get_permalink', $url );
$this->assertContains( 'url=0', $url );

remove_filter( 'amp_pre_get_permalink', array( $this, 'return_example_url' ), 10 );
$url = amp_get_permalink( $published_post );
$this->assertContains( 'current_filter=amp_get_permalink', $url );
}
}
3 changes: 2 additions & 1 deletion tests/test-class-amp-post-type-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function test_init() {
* @covers AMP_Post_Type_Support::get_builtin_supported_post_types()
*/
public function test_get_builtin_supported_post_types() {
$this->assertEquals( array( 'post' ), AMP_Post_Type_Support::get_builtin_supported_post_types() );
$this->assertEquals( array( 'post', 'page' ), AMP_Post_Type_Support::get_builtin_supported_post_types() );
}

/**
Expand All @@ -59,6 +59,7 @@ public function test_get_eligible_post_types() {
$this->assertEquals(
array(
'post',
'page',
'book',
),
AMP_Post_Type_Support::get_eligible_post_types()
Expand Down