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.

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

Issue 848: Allow theme support of canonical AMP #852

Merged
merged 6 commits into from
Jan 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 44 additions & 4 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Plugin URI: https://github.com/automattic/amp-wp
* Author: Automattic
* Author URI: https://automattic.com
* Version: 0.6-alpha
* Version: 0.7-alpha
* Text Domain: amp
* Domain Path: /languages/
* License: GPLv2 or later
Expand All @@ -15,7 +15,7 @@

define( 'AMP__FILE__', __FILE__ );
define( 'AMP__DIR__', dirname( __FILE__ ) );
define( 'AMP__VERSION', '0.6-alpha' );
define( 'AMP__VERSION', '0.7-alpha' );

require_once AMP__DIR__ . '/includes/class-amp-autoloader.php';
AMP_Autoloader::register();
Expand Down Expand Up @@ -117,8 +117,17 @@ function amp_force_query_var_value( $query_vars ) {
return $query_vars;
}

/**
* Conditionally add AMP actions or render the 'paired mode' template(s).
*
* If the request is for an AMP page and this is in 'canonical mode,' redirect to the non-AMP page.
* It won't need this plugin's template system, nor the frontend actions like the 'rel' link.
*
* @since 0.2
* @return void
*/
function amp_maybe_add_actions() {
if ( ! is_singular() || is_feed() ) {
if ( amp_is_canonical() || ! is_singular() || is_feed() ) {
return;
}

Expand All @@ -145,6 +154,31 @@ function amp_maybe_add_actions() {
}
}

/**
* Whether this is in 'canonical mode.'
*
* 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/' ) )`
* it will retain 'paired mode.
*
* @return boolean Whether this is in AMP 'canonical mode'.
*/
function amp_is_canonical() {
$support = get_theme_support( 'amp' );
if ( true === $support ) {
return true;
}
if ( is_array( $support ) ) {
$args = array_shift( $support );
if ( empty( $args['template_path'] ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

The use of template_path will probably need to be iterated upon. For example, theme_root per \WP_Theme::get_theme_root() or template_dir per \WP_Theme::get_template_directory(). This will need some more discovery in #849 as to what makes the most sense. To me it seems the theme_root and template_directory are the same. /cc @kaitnyl

return true;
}
}
return false;
}

function amp_load_classes() {
_deprecated_function( __FUNCTION__, '0.6' );
}
Expand Down Expand Up @@ -225,10 +259,16 @@ function amp_render_post( $post ) {
/**
* Bootstraps the AMP customizer.
*
* Uses the priority of 12 for the 'after_setup_theme' action.
* Many themes run `add_theme_support()` on the 'after_setup_theme' hook, at the default priority of 10.
* And that function's documentation suggests adding it to that action.
* So this enables themes to `add_theme_support( 'amp' )`.
* And `amp_init_customizer()` will be able to recognize theme support by calling `amp_is_canonical()`.
*
* @since 0.4
*/
function _amp_bootstrap_customizer() {
add_action( 'after_setup_theme', 'amp_init_customizer' );
add_action( 'after_setup_theme', 'amp_init_customizer', 12 );
}
add_action( 'plugins_loaded', '_amp_bootstrap_customizer', 9 ); // Should be hooked before priority 10 on 'plugins_loaded' to properly unhook core panels.

Expand Down
3 changes: 2 additions & 1 deletion assets/js/amp-post-meta-box.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var ampPostMetaBox = ( function( $ ) {
* @since 0.6
*/
data: {
canonical: false,
previewLink: '',
disabled: false,
statusInputName: '',
Expand Down Expand Up @@ -58,7 +59,7 @@ var ampPostMetaBox = ( function( $ ) {
component.boot = function boot( data ) {
component.data = data;
$( document ).ready( function() {
if ( ! component.data.disabled ) {
if ( ! component.data.disabled && ! component.data.canonical ) {
component.addPreviewButton();
}
component.listen();
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
"homepage": "https://github.com/Automattic/amp-wp",
"type": "wordpress-plugin",
"license": "GPL-2.0",
"version": "0.6.0"
"version": "0.7.0"
}
3 changes: 3 additions & 0 deletions includes/admin/class-amp-post-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ public function enqueue_admin_assets() {
wp_add_inline_script( self::ASSETS_HANDLE, sprintf( 'ampPostMetaBox.boot( %s );',
wp_json_encode( array(
'previewLink' => esc_url_raw( add_query_arg( AMP_QUERY_VAR, '', get_preview_post_link( $post ) ) ),
'canonical' => amp_is_canonical(),
'disabled' => (bool) get_post_meta( $post->ID, self::DISABLED_POST_META_KEY, true ) || ! $this->is_amp_available( $post ),
'statusInputName' => self::STATUS_INPUT_NAME,
'l10n' => array(
Expand All @@ -142,6 +143,8 @@ public function render_status( $post ) {
isset( $post->ID )
&&
current_user_can( 'edit_post', $post->ID )
&&
! amp_is_canonical()
);

if ( true !== $verify ) {
Expand Down
8 changes: 8 additions & 0 deletions includes/admin/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,16 @@

/**
* Sets up the AMP template editor for the Customizer.
*
* If this is in AMP canonical mode, exit.
* There's no need for the 'AMP' Customizer panel,
* And this does not need to toggle between the AMP and normal display.
*/
function amp_init_customizer() {
if ( amp_is_canonical() ) {
return;
}

// Fire up the AMP Customizer.
add_action( 'customize_register', array( 'AMP_Template_Customizer', 'init' ), 500 );

Expand Down
18 changes: 14 additions & 4 deletions includes/options/class-amp-options-menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public function render_post_types_support() {
<?php foreach ( array_map( 'get_post_type_object', AMP_Post_Type_Support::get_eligible_post_types() ) as $post_type ) : ?>
<?php
$element_id = AMP_Options_Manager::OPTION_NAME . "-supported_post_types-{$post_type->name}";
$is_builtin = in_array( $post_type->name, $builtin_support, true );
$is_builtin = amp_is_canonical() || in_array( $post_type->name, $builtin_support, true );
?>
<?php if ( $is_builtin ) : ?>
<input type="hidden" name="<?php echo esc_attr( $element_name ); ?>" value="<?php echo esc_attr( $post_type->name ); ?>">
Expand All @@ -94,15 +94,23 @@ public function render_post_types_support() {
id="<?php echo esc_attr( $element_id ); ?>"
name="<?php echo esc_attr( $element_name ); ?>"
value="<?php echo esc_attr( $post_type->name ); ?>"
<?php checked( true, post_type_supports( $post_type->name, AMP_QUERY_VAR ) ); ?>
<?php checked( true, amp_is_canonical() || post_type_supports( $post_type->name, AMP_QUERY_VAR ) ); ?>
<?php disabled( $is_builtin ); ?>
>
<label for="<?php echo esc_attr( $element_id ); ?>">
<?php echo esc_html( $post_type->label ); ?>
</label>
<br>
<?php endforeach; ?>
<p class="description"><?php esc_html_e( 'Enable/disable AMP post type(s) support', 'amp' ); ?></p>
<p class="description">
<?php
if ( ! amp_is_canonical() ) :
esc_html_e( 'Enable/disable AMP post type(s) support', 'amp' );
else :
esc_html_e( 'Canonical AMP is enabled in your theme, so all post types will render.', 'amp' );
endif;
?>
</p>
</fieldset>
<?php
}
Expand All @@ -124,7 +132,9 @@ public function render_screen() {
<?php
settings_fields( AMP_Options_Manager::OPTION_NAME );
do_settings_sections( AMP_Options_Manager::OPTION_NAME );
submit_button();
if ( ! amp_is_canonical() ) {
submit_button();
}
?>
</form>
</div>
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"type": "git",
"url": "https://github.com/Automattic/amp-wp.git"
},
"version": "0.6.0",
"version": "0.7.0",
"license": "GPL-2.0+",
"private": true,
"devDependencies": {
Expand Down
3 changes: 3 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ Follow along with or [contribute](https://github.com/Automattic/amp-wp/blob/deve

## Changelog ##

### 0.7 (unreleased) ###
- Add support for canonical AMP.

### 0.6 (unreleased) ###
- Add: support for the "page" post type, except when used as homepage or page for posts. A new `page.php` is introduced with template parts factored out (`html-start.php`, `header.php`, `footer.php`, `html-end.php`) and re-used from `single.php`. Note that AMP URLs will end in `?amp` instead of `/amp/`. See [#825](https://github.com/Automattic/amp-wp/pull/825). Props technosailor, ThierryA, westonruter.
- Add: AMP post preview button alongside non-AMP preview button. See [#813](https://github.com/Automattic/amp-wp/pull/813). Props ThierryA, westonruter.
Expand Down
4 changes: 4 additions & 0 deletions readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ Follow along with or [contribute](https://github.com/Automattic/amp-wp/blob/deve

== Changelog ==

= 0.7 (unreleased) =

- Add support for canonical AMP.

= 0.6 (unreleased) =

- Add: support for the "page" post type, except when used as homepage or page for posts. A new `page.php` is introduced with template parts factored out (`html-start.php`, `header.php`, `footer.php`, `html-end.php`) and re-used from `single.php`. Note that AMP URLs will end in `?amp` instead of `/amp/`. See [#825](https://github.com/Automattic/amp-wp/pull/825). Props technosailor, ThierryA, westonruter.
Expand Down
45 changes: 45 additions & 0 deletions tests/test-amp.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php
/**
* Tests for amp.php.
*
* @package AMP
*/

/**
* Tests for amp.php.
*/
class Test_AMP extends WP_UnitTestCase {

/**
* Tear down and clean up.
*/
public function tearDown() {
parent::tearDown();
remove_theme_support( 'amp' );
}

/**
* Test amp_is_canonical().
*
* @covers amp_is_canonical()
*/
public function test_amp_is_canonical() {
remove_theme_support( 'amp' );
$this->assertFalse( amp_is_canonical() );

add_theme_support( 'amp' );
$this->assertTrue( amp_is_canonical() );

remove_theme_support( 'amp' );
add_theme_support( 'amp', array(
'template_path' => get_template_directory() . 'amp-templates/',
) );
$this->assertFalse( amp_is_canonical() );

remove_theme_support( 'amp' );
add_theme_support( 'amp', array(
'custom_prop' => 'something',
) );
$this->assertTrue( amp_is_canonical() );
}
}
11 changes: 10 additions & 1 deletion tests/test-class-amp-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,19 @@ public function test_render_status() {
wp_set_current_user( $this->factory->user->create( array(
'role' => 'administrator',
) ) );
$amp_status_markup = '<div class="misc-pub-section misc-amp-status"';

// This is in AMP 'canonical mode,' so it shouldn't have the AMP status.
add_theme_support( 'amp' );
ob_start();
$this->instance->render_status( $post );
$this->assertContains( '<div class="misc-pub-section misc-amp-status"', ob_get_clean() );
$this->assertNotContains( $amp_status_markup, ob_get_clean() );

// This is not in AMP 'canonical mode'.
remove_theme_support( 'amp' );
ob_start();
$this->instance->render_status( $post );
$this->assertContains( $amp_status_markup, ob_get_clean() );

remove_post_type_support( 'post', AMP_QUERY_VAR );

Expand Down