-
Notifications
You must be signed in to change notification settings - Fork 385
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.
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
#849 Add paired mode custom templates #856
Changes from 17 commits
57926fb
309af3d
ed5594b
63dae3e
2bb0bb4
6bbe965
ca4536c
1c5878d
95af9e7
184e89a
13bbfab
a378cd5
624cdb6
2af460e
e5b5490
6285f9f
b10c0fe
9b1828d
34c18cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,9 +131,9 @@ function amp_maybe_add_actions() { | |
// Add hooks for when a themes that support AMP. | ||
if ( current_theme_supports( 'amp' ) ) { | ||
if ( amp_is_canonical() || is_amp_endpoint() ) { | ||
AMP_Theme_Support::register_hooks(); | ||
AMP_Theme_Support::init(); | ||
} else { | ||
AMP_Frontend_Actions::register_hooks(); | ||
amp_add_frontend_actions(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why calling this here again since it is already a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ThierryA I don't understand. This function is only being called here in the |
||
} | ||
return; | ||
} | ||
|
@@ -152,7 +152,7 @@ function amp_maybe_add_actions() { | |
$supports = post_supports_amp( $post ); | ||
|
||
if ( ! $supports ) { | ||
if ( $is_amp_endpoint ) { | ||
if ( $is_amp_endpoint && isset( $post->ID ) ) { | ||
wp_safe_redirect( get_permalink( $post->ID ), 301 ); | ||
exit; | ||
} | ||
|
@@ -172,7 +172,7 @@ function amp_maybe_add_actions() { | |
* Themes can register support for this with `add_theme_support( 'amp' )`. | ||
* Then, this will change the plugin from 'paired mode,' and it won't use its own templates. | ||
* Nor output frontend markup like the 'rel' link. If the theme registers support for AMP with: | ||
* `add_theme_support( 'amp', array( 'template_path' => get_template_directory() . 'my-amp-templates/' ) )` | ||
* `add_theme_support( 'amp', array( 'template_dir' => 'my-amp-templates' ) )` | ||
* it will retain 'paired mode. | ||
* | ||
* @return boolean Whether this is in AMP 'canonical mode'. | ||
|
@@ -184,7 +184,7 @@ function amp_is_canonical() { | |
} | ||
if ( is_array( $support ) ) { | ||
$args = array_shift( $support ); | ||
if ( empty( $args['template_path'] ) ) { | ||
if ( empty( $args['template_dir'] ) ) { | ||
return true; | ||
} | ||
} | ||
|
@@ -196,12 +196,13 @@ function amp_load_classes() { | |
} | ||
|
||
function amp_add_frontend_actions() { | ||
AMP_Frontend_Actions::register_hooks(); | ||
require_once AMP__DIR__ . '/includes/amp-frontend-actions.php'; | ||
} | ||
|
||
function amp_add_post_template_actions() { | ||
AMP_Paired_Post_Actions::register_hooks(); | ||
require_once( AMP__DIR__ . '/includes/amp-post-template-functions.php' ); | ||
require_once AMP__DIR__ . '/includes/amp-post-template-actions.php'; | ||
require_once AMP__DIR__ . '/includes/amp-post-template-functions.php'; | ||
amp_post_template_init_hooks(); | ||
} | ||
|
||
function amp_prepare_render() { | ||
|
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
<?php | ||
/** | ||
* Callbacks for adding AMP-related things to the main theme. | ||
* | ||
* @package AMP | ||
*/ | ||
|
||
add_action( 'wp_head', 'amp_frontend_add_canonical' ); | ||
|
||
/** | ||
* Add amphtml link to frontend. | ||
* | ||
* @since 0.2 | ||
*/ | ||
function amp_frontend_add_canonical() { | ||
|
||
// Prevent showing amphtml link if theme supports AMP but paired mode is not available. | ||
if ( current_theme_supports( 'amp' ) && ! AMP_Theme_Support::is_paired_available() ) { | ||
return; | ||
} | ||
|
||
/** | ||
* Filters whether to show the amphtml link on the frontend. | ||
* | ||
* @since 0.2 | ||
*/ | ||
if ( false === apply_filters( 'amp_frontend_show_canonical', true ) ) { | ||
return; | ||
} | ||
|
||
$amp_url = amp_get_permalink( get_queried_object_id() ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really a need to store this in a variable, the following would be readable too: printf(
'<link rel="amphtml" href="%s">',
esc_url( amp_get_permalink( get_queried_object_id() ) )
); |
||
printf( '<link rel="amphtml" href="%s">', esc_url( $amp_url ) ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_amp_endpoint()
returnstrue
ifamp_is_canonical()
so this if statement could simply beif ( is_amp_endpoint() )
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
$is_amp_endpoint = is_amp_endpoint();
is declared further down, I would suggest to move it up to the top of this function and replaceis_amp_endpoint()
with the variable.