From 4f589491e89627fe1a3b2bf0063a08641bd0a9ab Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 6 Dec 2017 13:36:28 -0800 Subject: [PATCH] Load JS for AMP post meta box if post type supports not if post is not skipped. * Restore focus on edit link for AMP status edit for accessibility. * Do not add AMP preview button if AMP has been disabled. * Change postmeta to be flag indicating whether AMP is disabled. * Fix spelling and clean up phpdoc. --- assets/css/amp-post-meta-box.css | 2 +- assets/js/amp-post-meta-box.js | 33 +++++++++----- includes/admin/class-amp-post-meta-box.php | 52 ++++++++++++---------- includes/amp-helper-functions.php | 24 ++++++++-- templates/admin/amp-status.php | 8 ++-- tests/test-class-amp-meta-box.php | 25 +++++++---- 6 files changed, 95 insertions(+), 49 deletions(-) diff --git a/assets/css/amp-post-meta-box.css b/assets/css/amp-post-meta-box.css index d49ccae2929..d11cc755ba1 100644 --- a/assets/css/amp-post-meta-box.css +++ b/assets/css/amp-post-meta-box.css @@ -5,7 +5,7 @@ */ /* Core preview button */ -#post-preview { +#post-preview.without-amp { border-top-right-radius: 0; border-bottom-right-radius: 0; float: none; diff --git a/assets/js/amp-post-meta-box.js b/assets/js/amp-post-meta-box.js index 1305d2396cb..67e41255e24 100644 --- a/assets/js/amp-post-meta-box.js +++ b/assets/js/amp-post-meta-box.js @@ -15,7 +15,11 @@ var ampPostMetaBox = ( function( $ ) { * * @since 0.6 */ - data: {}, + data: { + previewLink: '', + disabled: false, + statusInputName: '' + }, /** * Toggle animation speed. @@ -48,7 +52,9 @@ var ampPostMetaBox = ( function( $ ) { boot: function( data ) { this.data = data; $( document ).ready( function() { - this.addPreviewButton(); + if ( ! this.data.disabled ) { + this.addPreviewButton(); + } this.listen(); }.bind( this ) ); }, @@ -82,9 +88,11 @@ var ampPostMetaBox = ( function( $ ) { * @return {void} */ addPreviewButton: function() { - $( this.previewBtn ) + var previewBtn = $( this.previewBtn ); + previewBtn.addClass( 'without-amp' ); + previewBtn .clone() - .insertAfter( this.previewBtn ) + .insertAfter( previewBtn ) .prop( { 'href': this.data.previewLink, 'id': this.ampPreviewBtn.replace( '#', '' ) @@ -119,7 +127,7 @@ var ampPostMetaBox = ( function( $ ) { }, /** - * Add AMP Preview button. + * Add AMP status toggle. * * @since 0.6 * @param {Object} $target Event target. @@ -128,22 +136,27 @@ var ampPostMetaBox = ( function( $ ) { toggleAmpStatus: function( $target ) { var $container = $( '#amp-status-select' ), status = $container.data( 'amp-status' ), - $checked; + $checked, + editAmpStatus = $( '.edit-amp-status' ); // Don't modify status on cancel button click. if ( ! $target.hasClass( 'button-cancel' ) ) { - status = $( '[name="amp_status"]:checked' ).val(); + status = $( '[name="' + this.data.statusInputName + '"]:checked' ).val(); } - $checked = $( '#amp-satus-' + status ); + $checked = $( '#amp-status-' + status ); // Toggle elements. - $( '.edit-amp-status' ).fadeToggle( this.toggleSpeed ); + editAmpStatus.fadeToggle( this.toggleSpeed, function() { + if ( editAmpStatus.is( ':visible' ) ) { + editAmpStatus.focus(); + } + } ); $container.slideToggle( this.toggleSpeed ); // Update status. $container.data( 'amp-status', status ); - $checked.prop( 'checked', 'checked' ); + $checked.prop( 'checked', true ); $( '.amp-status-text' ).text( $checked.next().text() ); } }; diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 9aa8634c8b3..4101df023b3 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -22,12 +22,20 @@ class AMP_Post_Meta_Box { const ASSETS_HANDLE = 'amp-post-meta-box'; /** - * The post meta key. + * The post meta key for whether the post is skipped. * * @since 0.6 * @var string */ - const POST_META_KEY = 'amp_status'; + const DISABLED_POST_META_KEY = 'amp_disabled'; + + /** + * The field name for the enabled/disabled radio buttons. + * + * @since 0.6 + * @var string + */ + const STATUS_INPUT_NAME = 'amp_status'; /** * The nonce name. @@ -35,7 +43,7 @@ class AMP_Post_Meta_Box { * @since 0.6 * @var string */ - const NONCE_NAME = 'amp-status'; + const NONCE_NAME = 'amp-status-nonce'; /** * The nonce action. @@ -61,7 +69,6 @@ public function init() { * Enqueue admin assets. * * @since 0.6 - * @return Void Void on failure. */ public function enqueue_admin_assets() { $post = get_post(); @@ -71,10 +78,9 @@ public function enqueue_admin_assets() { && 'post' === $screen->base && - true === post_supports_amp( $post ) + post_type_supports( $post->post_type, AMP_QUERY_VAR ) ); - - if ( true !== $validate ) { + if ( ! $validate ) { return; } @@ -95,7 +101,9 @@ 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, true, get_preview_post_link( $post ) ) ), + 'previewLink' => esc_url_raw( add_query_arg( AMP_QUERY_VAR, '', get_preview_post_link( $post ) ) ), + 'disabled' => (bool) get_post_meta( $post->ID, self::DISABLED_POST_META_KEY, true ), + 'statusInputName' => self::STATUS_INPUT_NAME, ) ) ) ); } @@ -104,7 +112,7 @@ public function enqueue_admin_assets() { * Render AMP status. * * @since 0.6 - * @param object $post WP_POST object. + * @param WP_Post $post Post. */ public function render_status( $post ) { $verify = ( @@ -121,17 +129,13 @@ public function render_status( $post ) { return; } - $status = get_post_meta( $post->ID, self::POST_META_KEY, true ); - $labels = array( + $disabled = (bool) get_post_meta( $post->ID, self::DISABLED_POST_META_KEY, true ); + $status = $disabled ? 'disabled' : 'enabled'; + $labels = array( 'enabled' => __( 'Enabled', 'amp' ), 'disabled' => __( 'Disabled', 'amp' ), ); - // Set default. - if ( empty( $status ) ) { - $status = 'enabled'; - } - include_once AMP__DIR__ . '/templates/admin/amp-status.php'; } @@ -145,7 +149,7 @@ public function save_amp_status( $post_id ) { $verify = ( isset( $_POST[ self::NONCE_NAME ] ) && - isset( $_POST[ self::POST_META_KEY ] ) + isset( $_POST[ self::STATUS_INPUT_NAME ] ) && wp_verify_nonce( sanitize_key( wp_unslash( $_POST[ self::NONCE_NAME ] ) ), self::NONCE_ACTION ) && @@ -157,11 +161,11 @@ public function save_amp_status( $post_id ) { ); if ( true === $verify ) { - update_post_meta( - $post_id, - self::POST_META_KEY, - sanitize_key( wp_unslash( $_POST[ self::POST_META_KEY ] ) ) - ); + if ( 'disabled' === $_POST[ self::STATUS_INPUT_NAME ] ) { + update_post_meta( $post_id, self::DISABLED_POST_META_KEY, true ); + } else { + delete_post_meta( $post_id, self::DISABLED_POST_META_KEY ); + } } } @@ -170,8 +174,10 @@ public function save_amp_status( $post_id ) { * * Add the AMP query var is the amp-preview flag is set. * - * @param string $link The post preview link. * @since 0.6 + * + * @param string $link The post preview link. + * @return string Preview URL. */ public function preview_post_link( $link ) { $is_amp = ( diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 159a446186f..040418d440f 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -17,14 +17,23 @@ function amp_get_permalink( $post_id ) { return apply_filters( 'amp_get_permalink', $amp_url, $post_id ); } +/** + * Determine whether a given post supports AMP. + * + * @since 0.1 + * + * @param WP_Post $post Post. + * @return bool Whether the post supports AMP. + */ function post_supports_amp( $post ) { - // Because `add_rewrite_endpoint` doesn't let us target specific post_types :( + + // Because `add_rewrite_endpoint` doesn't let us target specific post_types. if ( ! post_type_supports( $post->post_type, AMP_QUERY_VAR ) ) { return false; } - // Listen to post meta. - if ( ! isset( $post->ID ) || 'disabled' === get_post_meta( $post->ID, AMP_Post_Meta_Box::POST_META_KEY, true ) ) { + // Skip based on postmeta. + if ( ! isset( $post->ID ) || (bool) get_post_meta( $post->ID, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ) { return false; } @@ -32,6 +41,15 @@ function post_supports_amp( $post ) { return false; } + /** + * Filters whether to skip the post from AMP. + * + * @since 0.3 + * + * @param bool $skipped Skipped. + * @param int $post_id Post ID. + * @param WP_Post $post Post. + */ if ( true === apply_filters( 'amp_skip_post', false, $post->ID, $post ) ) { return false; } diff --git a/templates/admin/amp-status.php b/templates/admin/amp-status.php index 7a6d03d7f48..170b4f13c7b 100644 --- a/templates/admin/amp-status.php +++ b/templates/admin/amp-status.php @@ -19,11 +19,11 @@
- > - + > +
- > - + > +
diff --git a/tests/test-class-amp-meta-box.php b/tests/test-class-amp-meta-box.php index bddbb510808..e9b7d7eaa71 100644 --- a/tests/test-class-amp-meta-box.php +++ b/tests/test-class-amp-meta-box.php @@ -108,29 +108,38 @@ public function test_render_status() { public function test_save_amp_status() { // Test failure. $post_id = $this->factory->post->create(); - $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::POST_META_KEY, true ) ); + $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); // Setup for success. wp_set_current_user( $this->factory->user->create( array( 'role' => 'administrator', ) ) ); - $_POST[ AMP_Post_Meta_Box::NONCE_NAME ] = wp_create_nonce( AMP_Post_Meta_Box::NONCE_ACTION ); - $_POST[ AMP_Post_Meta_Box::POST_META_KEY ] = 'foo'; + $_POST[ AMP_Post_Meta_Box::NONCE_NAME ] = wp_create_nonce( AMP_Post_Meta_Box::NONCE_ACTION ); + $_POST[ AMP_Post_Meta_Box::STATUS_INPUT_NAME ] = 'disabled'; // Test revision bail. $post_id = $this->factory->post->create(); - delete_post_meta( $post_id, AMP_Post_Meta_Box::POST_META_KEY ); + delete_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY ); wp_save_post_revision( $post_id ); - $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::POST_META_KEY, true ) ); + $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); - // Test post update success. + // Test post update success to disable. $post_id = $this->factory->post->create(); - delete_post_meta( $post_id, AMP_Post_Meta_Box::POST_META_KEY ); + delete_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY ); wp_update_post( array( 'ID' => $post_id, 'post_title' => 'updated', ) ); - $this->assertEquals( 'foo', get_post_meta( $post_id, AMP_Post_Meta_Box::POST_META_KEY, true ) ); + $this->assertTrue( (bool) get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); + + // Test post update success to enable. + $_POST[ AMP_Post_Meta_Box::STATUS_INPUT_NAME ] = 'enabled'; + delete_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY ); + wp_update_post( array( + 'ID' => $post_id, + 'post_title' => 'updated', + ) ); + $this->assertFalse( (bool) get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); } /**