From 0131c7180851a99e622b027a36f0533fdd38e217 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Thu, 30 Aug 2018 17:33:58 +0300 Subject: [PATCH 01/58] Start with listing view design improvements. --- .../class-amp-invalid-url-post-type.php | 62 +++++++++---------- .../class-amp-validation-error-taxonomy.php | 7 +++ 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 65b4597db31..92927743010 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -85,13 +85,13 @@ public static function register() { self::POST_TYPE_SLUG, array( 'labels' => array( - 'name' => _x( 'Invalid AMP Pages (URLs)', 'post type general name', 'amp' ), - 'menu_name' => __( 'Invalid Pages', 'amp' ), - 'singular_name' => __( 'Invalid AMP Page (URL)', 'amp' ), - 'not_found' => __( 'No invalid AMP pages found', 'amp' ), - 'not_found_in_trash' => __( 'No invalid AMP pages in trash', 'amp' ), - 'search_items' => __( 'Search invalid AMP pages', 'amp' ), - 'edit_item' => __( 'Invalid AMP Page (URL)', 'amp' ), + 'name' => _x( 'Errors by URL', 'post type general name', 'amp' ), + 'menu_name' => __( 'Errors by URL', 'amp' ), + 'singular_name' => __( 'Errors by URL', 'amp' ), + 'not_found' => __( 'No errors by URL found', 'amp' ), + 'not_found_in_trash' => __( 'No errors by URL in trash', 'amp' ), + 'search_items' => __( 'Search errors by URL', 'amp' ), + 'edit_item' => __( 'Errors by URL', 'amp' ), ), 'supports' => false, 'public' => false, @@ -281,21 +281,21 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['new'] ) { $result[] = esc_html( sprintf( /* translators: %s is count */ - __( '❓ New: %s', 'amp' ), + __( '❓ Identified: %s', 'amp' ), number_format_i18n( $counts['new'] ) ) ); } if ( $counts['accepted'] ) { $result[] = esc_html( sprintf( /* translators: %s is count */ - __( '✅ Accepted: %s', 'amp' ), + __( '✅ Suppressed: %s', 'amp' ), number_format_i18n( $counts['accepted'] ) ) ); } if ( $counts['rejected'] ) { $result[] = esc_html( sprintf( /* translators: %s is count */ - __( '❌ Rejected: %s', 'amp' ), + __( '❌ To Fix Later: %s', 'amp' ), number_format_i18n( $counts['rejected'] ) ) ); } @@ -509,8 +509,8 @@ public static function filter_views_edit( $views ) { sprintf( /* translators: %s: the post count. */ _nx( - 'With New Errors (%s)', - 'With New Errors (%s)', + 'With Identified Errors (%s)', + 'With Identified Errors (%s)', $with_new_query->found_posts, 'posts', 'amp' @@ -532,8 +532,8 @@ public static function filter_views_edit( $views ) { sprintf( /* translators: %s: the post count. */ _nx( - 'With Rejected Errors (%s)', - 'With Rejected Errors (%s)', + 'With To Fix Later Errors (%s)', + 'With To Fix Later Errors (%s)', $with_rejected_query->found_posts, 'posts', 'amp' @@ -555,8 +555,8 @@ public static function filter_views_edit( $views ) { sprintf( /* translators: %s: the post count. */ _nx( - 'With Accepted Errors (%s)', - 'With Accepted Errors (%s)', + 'With Suppressed Errors (%s)', + 'With Suppressed Errors (%s)', $with_accepted_query->found_posts, 'posts', 'amp' @@ -578,18 +578,20 @@ public static function add_post_columns( $columns ) { $columns = array_merge( $columns, array( - 'error_status' => esc_html__( 'Error Status', 'amp' ), - AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS => esc_html__( 'Removed Elements', 'amp' ), - AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES => esc_html__( 'Removed Attributes', 'amp' ), - AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Incompatible Sources', 'amp' ), + 'error_status' => esc_html__( 'Status ❓', 'amp' ) . wp_kses_post( 'This is tooltip.' ), + AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Found elements and attributes', 'amp' ), + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), ) ); + if ( isset( $columns['title' ] ) ) { + $columns['title'] = esc_html__( 'URL', 'amp' ); + } + // Move date to end. if ( isset( $columns['date'] ) ) { - $date = $columns['date']; unset( $columns['date'] ); - $columns['date'] = $date; + $columns['date'] = esc_html__( 'Last Checked' ); } return $columns; @@ -615,9 +617,9 @@ public static function output_custom_column( $column_name, $post_id ) { case 'error_status': self::display_invalid_url_validation_error_counts_summary( $post_id ); break; - case AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS: + case AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES: + $items = array(); if ( ! empty( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS ] ) ) { - $items = array(); foreach ( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS ] as $name => $count ) { if ( 1 === intval( $count ) ) { $items[] = sprintf( '%s', esc_html( $name ) ); @@ -625,14 +627,8 @@ public static function output_custom_column( $column_name, $post_id ) { $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); } } - echo implode( ', ', $items ); // WPCS: XSS OK. - } else { - esc_html_e( '--', 'amp' ); } - break; - case AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES: if ( ! empty( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES ] ) ) { - $items = array(); foreach ( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES ] as $name => $count ) { if ( 1 === intval( $count ) ) { $items[] = sprintf( '%s', esc_html( $name ) ); @@ -640,10 +636,12 @@ public static function output_custom_column( $column_name, $post_id ) { $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); } } + } + if ( ! empty( $items ) ) { echo implode( ', ', $items ); // WPCS: XSS OK. - } else { + } else { esc_html_e( '--', 'amp' ); - } + } break; case AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT: if ( isset( $error_summary[ AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT ] ) ) { diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 8505e0f5669..62b71aa7524 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -82,6 +82,13 @@ class AMP_Validation_Error_Taxonomy { */ const REMOVED_ELEMENTS = 'removed_elements'; + /** + * The key for found elements and attributes. + * + * @var string + */ + const FOUND_ELEMENTS_AND_ATTRIBUTES = 'found_elements_and_attributes'; + /** * The key for removed attributes. * From fd51be51d1d03f35491d3456160629b80b03f853 Mon Sep 17 00:00:00 2001 From: Miina Sikk Date: Tue, 4 Sep 2018 16:13:15 +0300 Subject: [PATCH 02/58] Add icons. Add tooltip skeleton. Fix sources. --- assets/images/amp-logo-icon.svg | 10 ++ assets/images/baseline-error-blue.svg | 12 +++ assets/images/baseline-error.svg | 12 +++ assets/images/editor-help.svg | 1 + .../class-amp-invalid-url-post-type.php | 94 +++++++++++++------ .../class-amp-validation-error-taxonomy.php | 8 +- 6 files changed, 106 insertions(+), 31 deletions(-) create mode 100644 assets/images/amp-logo-icon.svg create mode 100644 assets/images/baseline-error-blue.svg create mode 100644 assets/images/baseline-error.svg create mode 100755 assets/images/editor-help.svg diff --git a/assets/images/amp-logo-icon.svg b/assets/images/amp-logo-icon.svg new file mode 100644 index 00000000000..f6f70c9ba55 --- /dev/null +++ b/assets/images/amp-logo-icon.svg @@ -0,0 +1,10 @@ + + + + + + + diff --git a/assets/images/baseline-error-blue.svg b/assets/images/baseline-error-blue.svg new file mode 100644 index 00000000000..fa6d7953888 --- /dev/null +++ b/assets/images/baseline-error-blue.svg @@ -0,0 +1,12 @@ + + + + + + + + + diff --git a/assets/images/baseline-error.svg b/assets/images/baseline-error.svg new file mode 100644 index 00000000000..e6d4620d34d --- /dev/null +++ b/assets/images/baseline-error.svg @@ -0,0 +1,12 @@ + + + + + + + + + diff --git a/assets/images/editor-help.svg b/assets/images/editor-help.svg new file mode 100755 index 00000000000..e5cdf524755 --- /dev/null +++ b/assets/images/editor-help.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 35db3e05062..2bec248aac4 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -126,6 +126,8 @@ public static function should_show_in_menu() { * Add admin hooks. */ public static function add_admin_hooks() { + add_action( 'admin_enqueue_scripts', array( __CLASS__, 'enqueue_admin_assets' ) ); + add_filter( 'dashboard_glance_items', array( __CLASS__, 'filter_dashboard_glance_items' ) ); add_action( 'rightnow_end', array( __CLASS__, 'print_dashboard_glance_styles' ) ); add_action( 'add_meta_boxes', array( __CLASS__, 'add_meta_boxes' ) ); @@ -162,6 +164,19 @@ public static function add_admin_hooks() { } ); } + /** + * Enqueue style. + */ + public static function enqueue_admin_assets() { + // Styles. + wp_enqueue_style( + 'amp-admin-tables', + amp_get_asset_url( 'css/admin-tables.css' ), + false, + AMP__VERSION + ); + } + /** * Add count of how many validation error posts there are to the admin menu. */ @@ -277,27 +292,31 @@ public static function display_invalid_url_validation_error_counts_summary( $pos } } + // @todo Switch icons. $result = array(); if ( $counts['new'] ) { - $result[] = esc_html( sprintf( + $result[] = sprintf( /* translators: %s is count */ - __( '❓ Identified: %s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), + esc_html__( 'Identified', 'amp' ), number_format_i18n( $counts['new'] ) - ) ); + ); } if ( $counts['accepted'] ) { - $result[] = esc_html( sprintf( - /* translators: %s is count */ - __( '✅ Suppressed: %s', 'amp' ), + $result[] = sprintf( + /* translators: 1. Title, 2. %s is count */ + __( '%1$s: %2$s', 'amp' ), + esc_html__( 'Suppressed', 'amp' ), number_format_i18n( $counts['accepted'] ) - ) ); + ); } if ( $counts['rejected'] ) { - $result[] = esc_html( sprintf( + $result[] = sprintf( /* translators: %s is count */ - __( '❌ To Fix Later: %s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), + esc_html__( 'To Fix Later', 'amp' ), number_format_i18n( $counts['rejected'] ) - ) ); + ); } echo implode( '
', $result ); // WPCS: xss ok. } @@ -623,20 +642,20 @@ public static function add_post_columns( $columns ) { $columns = array_merge( $columns, array( - 'error_status' => esc_html__( 'Status ❓', 'amp' ) . wp_kses_post( 'This is tooltip.' ), - AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Found elements and attributes', 'amp' ), + 'error_status' => sprintf( '%s
%s
', esc_html( 'Status', 'amp' ), esc_html( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. + AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Found', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), ) ); - if ( isset( $columns['title' ] ) ) { - $columns['title'] = esc_html__( 'URL', 'amp' ); - } + if ( isset( $columns['title'] ) ) { + $columns['title'] = esc_html__( 'URL', 'amp' ); + } // Move date to end. if ( isset( $columns['date'] ) ) { unset( $columns['date'] ); - $columns['date'] = esc_html__( 'Last Checked' ); + $columns['date'] = esc_html__( 'Last Checked', 'amp' ); } return $columns; @@ -671,36 +690,55 @@ public static function output_custom_column( $column_name, $post_id ) { if ( ! empty( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS ] ) ) { foreach ( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS ] as $name => $count ) { if ( 1 === intval( $count ) ) { - $items[] = sprintf( '%s', esc_html( $name ) ); + $items[] = sprintf( '%s', esc_html( $name ) ); } else { - $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); + $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); } } } if ( ! empty( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES ] ) ) { foreach ( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES ] as $name => $count ) { if ( 1 === intval( $count ) ) { - $items[] = sprintf( '%s', esc_html( $name ) ); + $items[] = sprintf( '%s', esc_html( $name ) ); } else { - $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); + $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); } } } if ( ! empty( $items ) ) { - echo implode( ', ', $items ); // WPCS: XSS OK. - } else { + echo implode( ',
', $items ); // WPCS: XSS OK. + } else { esc_html_e( '--', 'amp' ); - } + } break; case AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT: if ( isset( $error_summary[ AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT ] ) ) { - $sources = array(); - foreach ( $error_summary[ AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT ] as $type => $names ) { - foreach ( array_unique( $names ) as $name ) { - $sources[] = sprintf( '%s: %s', esc_html( $type ), esc_html( $name ) ); + $sources = $error_summary[ AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT ]; + $output = array(); + + // @todo Add hide/show; Add icons; Get plugin names. + if ( isset( $sources['plugin'] ) ) { + $count = count( array_unique( $sources['plugin'] ) ); + if ( 1 === $count ) { + $output[] = sprintf( '%s', esc_html( 'Plug-In', 'amp' ) ); + } else { + $output[] = sprintf( '%s (%d)', esc_html( 'Plug-In', 'amp' ), $count ); } + $output[] = implode( '
', array_unique( $sources['plugin'] ) ); + } + if ( isset( $sources['core'] ) ) { + $count = count( array_unique( $sources['core'] ) ); + if ( 1 === $count ) { + $output[] = sprintf( '%s', esc_html( 'Other', 'amp' ) ); + } else { + $output[] = sprintf( '%s (%d)', esc_html( 'Other', 'amp' ), $count ); + } + $output[] = implode( '
', array_unique( $sources['core'] ) ); + } + if ( isset( $soures['theme'] ) ) { + $output[] = sprintf( '%s', esc_html( $soures['theme']['name'] ) ); } - echo implode( ', ', $sources ); // WPCS: XSS ok. + echo implode( '
', $output ); // WPCS: XSS ok. } break; } diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 62b71aa7524..4a37e976a9f 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -446,10 +446,12 @@ public static function summarize_validation_errors( $validation_errors ) { } if ( ! empty( $validation_error['sources'] ) ) { - $source = array_shift( $validation_error['sources'] ); + $sources = array_shift( $validation_error['sources'] ); - if ( isset( $source['type'], $source['name'] ) ) { - $invalid_sources[ $source['type'] ][] = $source['name']; + foreach ( $sources['sources'] as $source ) { + if ( isset( $source['type'], $source['name'] ) ) { + $invalid_sources[ $source['type'] ][] = $source['name']; + } } } } From 8ab716653eea5dbdb78d1a4fac2c032540b239df Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Thu, 6 Sep 2018 12:35:39 -0500 Subject: [PATCH 03/58] Conform to validation error data model. It look like there aren't double-nested 'source' arrays. So remove the nested foreach loop. --- .../validation/class-amp-validation-error-taxonomy.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 4a37e976a9f..62b71aa7524 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -446,12 +446,10 @@ public static function summarize_validation_errors( $validation_errors ) { } if ( ! empty( $validation_error['sources'] ) ) { - $sources = array_shift( $validation_error['sources'] ); + $source = array_shift( $validation_error['sources'] ); - foreach ( $sources['sources'] as $source ) { - if ( isset( $source['type'], $source['name'] ) ) { - $invalid_sources[ $source['type'] ][] = $source['name']; - } + if ( isset( $source['type'], $source['name'] ) ) { + $invalid_sources[ $source['type'] ][] = $source['name']; } } } From 99a35b8f57572d9f1082230c8b054ed1a21333f1 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Thu, 6 Sep 2018 12:52:56 -0500 Subject: [PATCH 04/58] Update status names to latest design For example, change 'To Fix Later' to 'Rejected'. Also, update the labels. I don't think all of them need to have 'Errors by URL', like 'No errors by URL found'. --- .../validation/class-amp-invalid-url-post-type.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index af78bbe192b..ffbdef69983 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -86,11 +86,11 @@ public static function register() { array( 'labels' => array( 'name' => _x( 'Errors by URL', 'post type general name', 'amp' ), - 'menu_name' => __( 'Errors by URL', 'amp' ), + 'menu_name' => __( 'Validation: URLs', 'amp' ), 'singular_name' => __( 'Errors by URL', 'amp' ), - 'not_found' => __( 'No errors by URL found', 'amp' ), - 'not_found_in_trash' => __( 'No errors by URL in trash', 'amp' ), - 'search_items' => __( 'Search errors by URL', 'amp' ), + 'not_found' => __( 'No errors found', 'amp' ), + 'not_found_in_trash' => __( 'No errors forgotten', 'amp' ), + 'search_items' => __( 'Search', 'amp' ), 'edit_item' => __( 'Errors by URL', 'amp' ), ), 'supports' => false, @@ -305,8 +305,8 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['accepted'] ) { $result[] = sprintf( /* translators: 1. Title, 2. %s is count */ - __( '%1$s: %2$s', 'amp' ), - esc_html__( 'Suppressed', 'amp' ), + __( '%1$s: %2$s', 'amp' ), + esc_html__( 'Accepted', 'amp' ), number_format_i18n( $counts['accepted'] ) ); } @@ -314,7 +314,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos $result[] = sprintf( /* translators: %s is count */ __( '%1$s: %2$s', 'amp' ), - esc_html__( 'To Fix Later', 'amp' ), + esc_html__( 'Rejected', 'amp' ), number_format_i18n( $counts['rejected'] ) ); } From e263b55f96fbce9c1162c51f27ac47571fe5375d Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:07:47 -0300 Subject: [PATCH 05/58] Use dashicons for tooltip icon --- includes/validation/class-amp-invalid-url-post-type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index ffbdef69983..da664e2a15e 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -526,7 +526,7 @@ public static function add_post_columns( $columns ) { $columns = array_merge( $columns, array( - 'error_status' => sprintf( '%s
%s
', esc_html( 'Status', 'amp' ), esc_html( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. + 'error_status' => sprintf( '%s
%s
', esc_html( 'Status', 'amp' ), esc_html( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Found', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), ) From 9e99f1a353ad5f6e7eb3a2ca9b887da22aa4a032 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:08:13 -0300 Subject: [PATCH 06/58] Add admin-tables.css file --- assets/css/admin-tables.css | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 assets/css/admin-tables.css diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css new file mode 100644 index 00000000000..e69de29bb2d From 273cccfa9acdcab3fd2ccdc72308bb7514c822a4 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:10:07 -0300 Subject: [PATCH 07/58] Hide tooltip --- assets/css/admin-tables.css | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index e69de29bb2d..f7efc50552f 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -0,0 +1,3 @@ +#error_status .tooltip { + display: none; +} From df68ed83ff050057fbccbd645a0dc3c6ce4007a4 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:15:32 -0300 Subject: [PATCH 08/58] Target class instead of id and add color to icon --- assets/css/admin-tables.css | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index f7efc50552f..8800db5f996 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -1,3 +1,6 @@ -#error_status .tooltip { +.column-error_status .tooltip { display: none; } +.column-error_status .dashicons-editor-help { + color: #767676; +} From 54ae458eb8a238e670695252c571426ebbef911a Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:19:45 -0300 Subject: [PATCH 09/58] Change Plug-In to Plugin --- includes/validation/class-amp-invalid-url-post-type.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index da664e2a15e..2bb03ec6c93 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -604,9 +604,9 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['plugin'] ) ) { $count = count( array_unique( $sources['plugin'] ) ); if ( 1 === $count ) { - $output[] = sprintf( '%s', esc_html( 'Plug-In', 'amp' ) ); + $output[] = sprintf( '%s', esc_html( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html( 'Plug-In', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html( 'Plugin', 'amp' ), $count ); } $output[] = implode( '
', array_unique( $sources['plugin'] ) ); } From 4075cb0801c4a26273e1d175fb0681bb428bd6b9 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:20:30 -0300 Subject: [PATCH 10/58] Fix variable typo --- includes/validation/class-amp-invalid-url-post-type.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 2bb03ec6c93..00d40e93d91 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -619,8 +619,8 @@ public static function output_custom_column( $column_name, $post_id ) { } $output[] = implode( '
', array_unique( $sources['core'] ) ); } - if ( isset( $soures['theme'] ) ) { - $output[] = sprintf( '%s', esc_html( $soures['theme']['name'] ) ); + if ( isset( $sources['theme'] ) ) { + $output[] = sprintf( '%s', esc_html( $sources['theme']['name'] ) ); } echo implode( '
', $output ); // WPCS: XSS ok. } From 9094366f542e818f403fef7137d05c50e6207237 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:25:14 -0300 Subject: [PATCH 11/58] Add plugin icon with color from design --- assets/css/admin-tables.css | 3 +++ includes/validation/class-amp-invalid-url-post-type.php | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 8800db5f996..bc4d9703920 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -4,3 +4,6 @@ .column-error_status .dashicons-editor-help { color: #767676; } +.column-sources_with_invalid_output .dashicons-admin-plugins { + color: #64a2e9; +} diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 00d40e93d91..9da90321190 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -604,7 +604,7 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['plugin'] ) ) { $count = count( array_unique( $sources['plugin'] ) ); if ( 1 === $count ) { - $output[] = sprintf( '%s', esc_html( 'Plugin', 'amp' ) ); + $output[] = sprintf( '%s', esc_html( 'Plugin', 'amp' ) ); } else { $output[] = sprintf( '%s (%d)', esc_html( 'Plugin', 'amp' ), $count ); } From 604c4b901f7e791dc985e0bef0368527029b494a Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 17:28:06 -0300 Subject: [PATCH 12/58] Add theme icon with color --- assets/css/admin-tables.css | 4 ++++ includes/validation/class-amp-invalid-url-post-type.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index bc4d9703920..b1313d42487 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -7,3 +7,7 @@ .column-sources_with_invalid_output .dashicons-admin-plugins { color: #64a2e9; } +.column-sources_with_invalid_output .dashicons-admin-appearance { + color: #ebb04f; +} + diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 9da90321190..72a71a970cb 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -620,7 +620,7 @@ public static function output_custom_column( $column_name, $post_id ) { $output[] = implode( '
', array_unique( $sources['core'] ) ); } if ( isset( $sources['theme'] ) ) { - $output[] = sprintf( '%s', esc_html( $sources['theme']['name'] ) ); + $output[] = sprintf( '%s', esc_html( $sources['theme']['name'] ) ); } echo implode( '
', $output ); // WPCS: XSS ok. } From 90b492020e03c61ce70be58342f0f08725b2c62b Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 18:37:40 -0300 Subject: [PATCH 13/58] Remove extra linebreaks --- includes/validation/class-amp-invalid-url-post-type.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 72a71a970cb..f33c67fbf39 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -604,7 +604,7 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['plugin'] ) ) { $count = count( array_unique( $sources['plugin'] ) ); if ( 1 === $count ) { - $output[] = sprintf( '%s', esc_html( 'Plugin', 'amp' ) ); + $output[] = sprintf( '%s
', esc_html( 'Plugin', 'amp' ) ); } else { $output[] = sprintf( '%s (%d)', esc_html( 'Plugin', 'amp' ), $count ); } @@ -613,16 +613,16 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['core'] ) ) { $count = count( array_unique( $sources['core'] ) ); if ( 1 === $count ) { - $output[] = sprintf( '%s', esc_html( 'Other', 'amp' ) ); + $output[] = sprintf( '%s
', esc_html( 'Other', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html( 'Other', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)
', esc_html( 'Other', 'amp' ), $count ); } $output[] = implode( '
', array_unique( $sources['core'] ) ); } if ( isset( $sources['theme'] ) ) { $output[] = sprintf( '%s', esc_html( $sources['theme']['name'] ) ); } - echo implode( '
', $output ); // WPCS: XSS ok. + echo implode( '', $output ); // WPCS: XSS ok. } break; } From 0be3a567f6b4d8e7471b726e0beef17e528ff11f Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 18:37:57 -0300 Subject: [PATCH 14/58] Add containers around sources for styling --- includes/validation/class-amp-invalid-url-post-type.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index f33c67fbf39..3a656e46294 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -608,7 +608,9 @@ public static function output_custom_column( $column_name, $post_id ) { } else { $output[] = sprintf( '%s (%d)', esc_html( 'Plugin', 'amp' ), $count ); } + $output[] = '
'; $output[] = implode( '
', array_unique( $sources['plugin'] ) ); + $output[] = '
'; } if ( isset( $sources['core'] ) ) { $count = count( array_unique( $sources['core'] ) ); @@ -617,7 +619,9 @@ public static function output_custom_column( $column_name, $post_id ) { } else { $output[] = sprintf( '%s (%d)
', esc_html( 'Other', 'amp' ), $count ); } + $output[] = '
'; $output[] = implode( '
', array_unique( $sources['core'] ) ); + $output[] = '
'; } if ( isset( $sources['theme'] ) ) { $output[] = sprintf( '%s', esc_html( $sources['theme']['name'] ) ); From 1cb3da7b2aee658c7e6b9357b78fb5e95f760c72 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 18:38:39 -0300 Subject: [PATCH 15/58] Add margins around sources containers --- assets/css/admin-tables.css | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index b1313d42487..b7c62abcf4b 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -4,6 +4,9 @@ .column-error_status .dashicons-editor-help { color: #767676; } +.column-sources_with_invalid_output .dashicons { + margin-right: 5px; +} .column-sources_with_invalid_output .dashicons-admin-plugins { color: #64a2e9; } @@ -11,3 +14,7 @@ color: #ebb04f; } +.sources-plugins, .sources-core { + margin-left: 25px; + margin-bottom: 15px; +} From 49ca0d8bf8a374f73e97561088b303a771d48ebd Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 18:38:49 -0300 Subject: [PATCH 16/58] Add color to WP icon --- assets/css/admin-tables.css | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index b7c62abcf4b..3f17f5ebf94 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -13,7 +13,9 @@ .column-sources_with_invalid_output .dashicons-admin-appearance { color: #ebb04f; } - +.column-sources_with_invalid_output .dashicons-wordpress-alt { + color: #92b371; +} .sources-plugins, .sources-core { margin-left: 25px; margin-bottom: 15px; From 6d7f35bf68a508f18ec22600c5ac85d15548dfb3 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 18:47:02 -0300 Subject: [PATCH 17/58] Change wording - found to removed --- includes/validation/class-amp-invalid-url-post-type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 3a656e46294..c30f07ab1da 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -527,7 +527,7 @@ public static function add_post_columns( $columns ) { $columns, array( 'error_status' => sprintf( '%s
%s
', esc_html( 'Status', 'amp' ), esc_html( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. - AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Found', 'amp' ), + AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Removed', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), ) ); From f6a087829e1f00f37232d9abe1d50f6a53e2ef2c Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 19:20:35 -0300 Subject: [PATCH 18/58] Add icons to status column --- includes/validation/class-amp-invalid-url-post-type.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index c30f07ab1da..559981e219f 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -297,7 +297,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['new'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Identified', 'amp' ), number_format_i18n( $counts['new'] ) ); @@ -305,7 +305,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['accepted'] ) { $result[] = sprintf( /* translators: 1. Title, 2. %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Accepted', 'amp' ), number_format_i18n( $counts['accepted'] ) ); @@ -313,7 +313,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['rejected'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Rejected', 'amp' ), number_format_i18n( $counts['rejected'] ) ); From c71addbdabc4f17623c49ba7e8364aa527b98f34 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 19:20:46 -0300 Subject: [PATCH 19/58] Add amp logo icon class --- assets/css/admin-tables.css | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 3f17f5ebf94..e0463f96da1 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -20,3 +20,11 @@ margin-left: 25px; margin-bottom: 15px; } +.amp-logo-icon { + background-image: url( '../images/amp-logo-icon.svg' ); + background-color: transparent; + background-size: 20px 20px; + height: 20px; + width: 20px; + display: inline-block; +} From ca2e7d237481cbfe48d7cfcf5ee8514f1bd87998 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 20:18:54 -0300 Subject: [PATCH 20/58] Add error-status class --- includes/validation/class-amp-invalid-url-post-type.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 559981e219f..43074c22eaf 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -297,7 +297,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['new'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Identified', 'amp' ), number_format_i18n( $counts['new'] ) ); @@ -305,7 +305,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['accepted'] ) { $result[] = sprintf( /* translators: 1. Title, 2. %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Accepted', 'amp' ), number_format_i18n( $counts['accepted'] ) ); @@ -313,7 +313,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['rejected'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Rejected', 'amp' ), number_format_i18n( $counts['rejected'] ) ); From 36564fd87e23913ffd9e4bfcc2eac92e9fae387b Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 20:20:17 -0300 Subject: [PATCH 21/58] Style error statuses --- assets/css/admin-tables.css | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index e0463f96da1..80ed47d9ea5 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -28,3 +28,10 @@ width: 20px; display: inline-block; } +.column-error_status .error-status { + line-height: 20px; + display: inline-block; + position: relative; + vertical-align: top; + margin-left: 10px; +} From 736b2f292d299bad67f02ebd8a192a0f8c0a1f14 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 20:32:26 -0300 Subject: [PATCH 22/58] Add closing span tag --- includes/validation/class-amp-invalid-url-post-type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 43074c22eaf..017938760e4 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -297,7 +297,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['new'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Identified', 'amp' ), number_format_i18n( $counts['new'] ) ); From 7af56d6fde388a670c84917b54d3e1c697897921 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 20:33:25 -0300 Subject: [PATCH 23/58] Add color to removed elements column text --- assets/css/admin-tables.css | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 80ed47d9ea5..82c87f36a8e 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -35,3 +35,6 @@ vertical-align: top; margin-left: 10px; } +td.column-found_elements_and_attributes { + color: #c06e60; +} From 25ab2b3ede6aa75750a71fc2844b1ba9e420ebfe Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 20:42:34 -0300 Subject: [PATCH 24/58] Add class to warning icons for styling --- includes/validation/class-amp-invalid-url-post-type.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 017938760e4..1f643341bcd 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -292,12 +292,11 @@ public static function display_invalid_url_validation_error_counts_summary( $pos } } - // @todo Switch icons. $result = array(); if ( $counts['new'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Identified', 'amp' ), number_format_i18n( $counts['new'] ) ); @@ -313,7 +312,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['rejected'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%1$s: %2$s', 'amp' ), esc_html__( 'Rejected', 'amp' ), number_format_i18n( $counts['rejected'] ) ); From 139883ebb24d044e37d7c265a405ecc216cc3850 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 20:43:14 -0300 Subject: [PATCH 25/58] Add color to identified and rejected icons --- assets/css/admin-tables.css | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 82c87f36a8e..177fba5227e 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -38,3 +38,9 @@ td.column-found_elements_and_attributes { color: #c06e60; } +.column-error_status .dashicons-warning.identified { + color: #d98501; +} +.column-error_status .dashicons-warning.rejected { + color: #68c6ff; +} From 2583e44e2196307819a05f8faa75c0dc4ef4dd4c Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 6 Sep 2018 20:52:58 -0300 Subject: [PATCH 26/58] Fix translation functions, switch to esc_html__ --- .../validation/class-amp-invalid-url-post-type.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 1f643341bcd..caecfbb88e7 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -525,7 +525,7 @@ public static function add_post_columns( $columns ) { $columns = array_merge( $columns, array( - 'error_status' => sprintf( '%s
%s
', esc_html( 'Status', 'amp' ), esc_html( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. + 'error_status' => sprintf( '%s
%s
', esc_html( 'Status', 'amp' ), esc_html__( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Removed', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), ) @@ -603,9 +603,9 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['plugin'] ) ) { $count = count( array_unique( $sources['plugin'] ) ); if ( 1 === $count ) { - $output[] = sprintf( '%s
', esc_html( 'Plugin', 'amp' ) ); + $output[] = sprintf( '%s
', esc_html__( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html( 'Plugin', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Plugin', 'amp' ), $count ); } $output[] = '
'; $output[] = implode( '
', array_unique( $sources['plugin'] ) ); @@ -614,9 +614,9 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['core'] ) ) { $count = count( array_unique( $sources['core'] ) ); if ( 1 === $count ) { - $output[] = sprintf( '%s
', esc_html( 'Other', 'amp' ) ); + $output[] = sprintf( '%s
', esc_html__( 'Other', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)
', esc_html( 'Other', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)
', esc_html__( 'Other', 'amp' ), $count ); } $output[] = '
'; $output[] = implode( '
', array_unique( $sources['core'] ) ); From 8649b915c26cf458d121a90e6156e090184c1f08 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Fri, 7 Sep 2018 10:06:39 -0300 Subject: [PATCH 27/58] Add plugins icon when there is multiple plugins --- includes/validation/class-amp-invalid-url-post-type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index caecfbb88e7..4fcb2109360 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -605,7 +605,7 @@ public static function output_custom_column( $column_name, $post_id ) { if ( 1 === $count ) { $output[] = sprintf( '%s
', esc_html__( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Plugin', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Plugin', 'amp' ), $count ); } $output[] = '
'; $output[] = implode( '
', array_unique( $sources['plugin'] ) ); From 53021cc6733c6f70fa57add6fe43317028423030 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 10:23:33 -0300 Subject: [PATCH 28/58] Hide invalid sources if more than 1 and add icon --- assets/css/admin-tables.css | 7 +++++++ .../class-amp-invalid-url-post-type.php | 20 ++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 177fba5227e..240e644736f 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -44,3 +44,10 @@ td.column-found_elements_and_attributes { .column-error_status .dashicons-warning.rejected { color: #68c6ff; } +.column-sources_with_invalid_output .sources-plugins.collapsed, .column-sources_with_invalid_output .sources-core.collapsed { + display: none; +} +.column-sources_with_invalid_output .source { + margin-bottom: 10px; + display: block; +} diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 4fcb2109360..cfe1a10316d 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -599,31 +599,41 @@ public static function output_custom_column( $column_name, $post_id ) { $sources = $error_summary[ AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT ]; $output = array(); - // @todo Add hide/show; Add icons; Get plugin names. + // @todo Add hide/show. if ( isset( $sources['plugin'] ) ) { + $output[] = '
'; $count = count( array_unique( $sources['plugin'] ) ); + $sources_container_classes = 'sources-plugins'; if ( 1 === $count ) { $output[] = sprintf( '%s
', esc_html__( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Plugin', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); + $sources_container_classes .= ' collapsed'; } - $output[] = '
'; + $output[] = '
'; $output[] = implode( '
', array_unique( $sources['plugin'] ) ); $output[] = '
'; + $output[] = '
'; } if ( isset( $sources['core'] ) ) { + $output[] = '
'; $count = count( array_unique( $sources['core'] ) ); + $sources_container_classes = 'sources-core'; if ( 1 === $count ) { $output[] = sprintf( '%s
', esc_html__( 'Other', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)
', esc_html__( 'Other', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)
', esc_html__( 'Other', 'amp' ), $count ); + $sources_container_classes .= ' collapsed'; } - $output[] = '
'; + $output[] = '
'; $output[] = implode( '
', array_unique( $sources['core'] ) ); $output[] = '
'; + $output[] = '
'; } if ( isset( $sources['theme'] ) ) { + $output[] = '
'; $output[] = sprintf( '%s', esc_html( $sources['theme']['name'] ) ); + $output[] = '
'; } echo implode( '', $output ); // WPCS: XSS ok. } From e87cca256cb861b79be50fb241349ebbf6e0b92a Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 10:28:30 -0300 Subject: [PATCH 29/58] Only enqueue stylesheet on its post type screens --- includes/validation/class-amp-invalid-url-post-type.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index cfe1a10316d..1a836e61afd 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -169,6 +169,11 @@ public static function add_admin_hooks() { */ public static function enqueue_admin_assets() { // Styles. + $screen = get_current_screen(); + if ( 'amp_invalid_url' !== $screen->post_type ) { + return; + } + wp_enqueue_style( 'amp-admin-tables', amp_get_asset_url( 'css/admin-tables.css' ), From 8aebc0ca92ce5717576bddc471723f54605f57b9 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 11:21:32 -0300 Subject: [PATCH 30/58] Add common container class for sources --- includes/validation/class-amp-invalid-url-post-type.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 1a836e61afd..9b4108b575e 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -608,11 +608,11 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['plugin'] ) ) { $output[] = '
'; $count = count( array_unique( $sources['plugin'] ) ); - $sources_container_classes = 'sources-plugins'; + $sources_container_classes = 'sources-container sources-plugins'; if ( 1 === $count ) { $output[] = sprintf( '%s
', esc_html__( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); $sources_container_classes .= ' collapsed'; } $output[] = '
'; @@ -623,11 +623,11 @@ public static function output_custom_column( $column_name, $post_id ) { if ( isset( $sources['core'] ) ) { $output[] = '
'; $count = count( array_unique( $sources['core'] ) ); - $sources_container_classes = 'sources-core'; + $sources_container_classes = 'sources-container sources-core'; if ( 1 === $count ) { $output[] = sprintf( '%s
', esc_html__( 'Other', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)
', esc_html__( 'Other', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Other', 'amp' ), $count ); $sources_container_classes .= ' collapsed'; } $output[] = '
'; From 9ae7e5891529af5c5cf5a058eb1513b84e352be7 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 11:21:55 -0300 Subject: [PATCH 31/58] Add JS for admin tables expand and collapse --- assets/js/amp-admin-tables.js | 23 +++++++++++++++++++ .../class-amp-invalid-url-post-type.php | 6 +++++ 2 files changed, 29 insertions(+) create mode 100644 assets/js/amp-admin-tables.js diff --git a/assets/js/amp-admin-tables.js b/assets/js/amp-admin-tables.js new file mode 100644 index 00000000000..91fc9b5a660 --- /dev/null +++ b/assets/js/amp-admin-tables.js @@ -0,0 +1,23 @@ +/* global jQuery */ +( function( $ ) { + 'use strict'; + + var adminTables = { + + load: function() { + $( document ).ready( $.proxy( function() { + this.sourcesHideShow(); + }, this ) ); + }, + + sourcesHideShow: function() { + $( 'span.dashicons.toggle-sources' ).on( 'click', function() { + $( this ).next( '.sources-container' ).toggleClass( 'collapsed' ); + $( this ).toggleClass( 'dashicons-arrow-down-alt2 dashicons-arrow-up-alt2' ); + }); + }, + }; + + adminTables.load(); + +}( jQuery ) ); diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 9b4108b575e..677ec9a8948 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -180,6 +180,12 @@ public static function enqueue_admin_assets() { false, AMP__VERSION ); + wp_enqueue_script( + 'amp-admin-tables', + amp_get_asset_url( 'js/amp-admin-tables.js' ), + [ 'jquery' ], + AMP__VERSION + ); } /** From ffb3fdc1a14bf8f87f8d6bdd73836a2702b5733e Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 11:22:13 -0300 Subject: [PATCH 32/58] Add margin after source container --- assets/css/admin-tables.css | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 240e644736f..4f72ead6f76 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -47,6 +47,9 @@ td.column-found_elements_and_attributes { .column-sources_with_invalid_output .sources-plugins.collapsed, .column-sources_with_invalid_output .sources-core.collapsed { display: none; } +.column-sources_with_invalid_output .source { + margin-bottom: 10px; +} .column-sources_with_invalid_output .source { margin-bottom: 10px; display: block; From 4f14433b02442511edaa3ea0186ac4e8fb859ce1 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 15:48:53 -0300 Subject: [PATCH 33/58] Remove todo --- includes/validation/class-amp-invalid-url-post-type.php | 1 - 1 file changed, 1 deletion(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 18f137bced3..b9f5ea6ea4c 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -637,7 +637,6 @@ public static function output_custom_column( $column_name, $post_id ) { $sources = $error_summary[ AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT ]; $output = array(); - // @todo Add hide/show. if ( isset( $sources['plugin'] ) ) { $output[] = '
'; $count = count( array_unique( $sources['plugin'] ) ); From bda41409a8bc73d487efee620812bb8633ba3e8a Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 15:51:16 -0300 Subject: [PATCH 34/58] Change wording: Removed to Invalid --- includes/validation/class-amp-invalid-url-post-type.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index b9f5ea6ea4c..dbf9d1bd125 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -564,7 +564,7 @@ public static function add_post_columns( $columns ) { $columns, array( 'error_status' => sprintf( '%s
%s
', esc_html( 'Status', 'amp' ), esc_html__( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. - AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Removed', 'amp' ), + AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Invalid', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), ) ); @@ -1405,7 +1405,7 @@ public static function print_validation_errors_meta_box( $post ) {
  • - +
  • - + Date: Mon, 10 Sep 2018 15:55:31 -0300 Subject: [PATCH 35/58] Wrap in and add brackets for attributes --- includes/validation/class-amp-invalid-url-post-type.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index dbf9d1bd125..9f0a68d894a 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -611,18 +611,18 @@ public static function output_custom_column( $column_name, $post_id ) { if ( ! empty( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS ] ) ) { foreach ( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS ] as $name => $count ) { if ( 1 === intval( $count ) ) { - $items[] = sprintf( '%s', esc_html( $name ) ); + $items[] = sprintf( '%s', esc_html( $name ) ); } else { - $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); + $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); } } } if ( ! empty( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES ] ) ) { foreach ( $error_summary[ AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES ] as $name => $count ) { if ( 1 === intval( $count ) ) { - $items[] = sprintf( '%s', esc_html( $name ) ); + $items[] = sprintf( '[%s]', esc_html( $name ) ); } else { - $items[] = sprintf( '%s (%d)', esc_html( $name ), $count ); + $items[] = sprintf( '[%s] (%d)', esc_html( $name ), $count ); } } } From 4ea96389761edefbe6c57ccbab489b7b1632c98c Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 16:42:13 -0300 Subject: [PATCH 36/58] Fix translation function reference --- includes/validation/class-amp-invalid-url-post-type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 9f0a68d894a..21ec04b3649 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -563,7 +563,7 @@ public static function add_post_columns( $columns ) { $columns = array_merge( $columns, array( - 'error_status' => sprintf( '%s
    %s
    ', esc_html( 'Status', 'amp' ), esc_html__( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. + 'error_status' => sprintf( '%s
    %s
    ', esc_html__( 'Status', 'amp' ), esc_html__( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Invalid', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), ) From 392ee24fc518ac657d1928369472f5bf9142c7c5 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 17:04:15 -0300 Subject: [PATCH 37/58] Add double arrow to Sources column --- assets/css/admin-tables.css | 19 +++++++++++++++++++ .../class-amp-invalid-url-post-type.php | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 4f72ead6f76..1acdf1f3c5b 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -54,3 +54,22 @@ td.column-found_elements_and_attributes { margin-bottom: 10px; display: block; } +.column-sources_with_invalid_output .double-arrow { + height: 20px; + width: 12px; + display: inline-block; + position: relative; + margin-left: 110px; +} +.column-sources_with_invalid_output .double-arrow .dashicons.dashicons-arrow-down.top-arrow { + font-size: 20px; + line-height: 1; + position: absolute; + bottom: 0; +} +.column-sources_with_invalid_output .double-arrow .dashicons.dashicons-arrow-down.bottom-arrow { + font-size: 20px; + line-height: 1; + position: absolute; + bottom: -6px; +} diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 21ec04b3649..443c7eb93dc 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -565,7 +565,7 @@ public static function add_post_columns( $columns ) { array( 'error_status' => sprintf( '%s
    %s
    ', esc_html__( 'Status', 'amp' ), esc_html__( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Invalid', 'amp' ), - AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Source', 'amp' ), + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => sprintf( '%s
    ', esc_html__( 'Sources', 'amp' ) ), ) ); From 163908e764f082dfd0ffdce066177bb2650913b4 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Mon, 10 Sep 2018 19:56:48 -0300 Subject: [PATCH 38/58] Add hide all sources functionality and styles --- assets/css/admin-tables.css | 13 ++++++++++--- assets/js/amp-admin-tables.js | 11 ++++++++++- .../validation/class-amp-invalid-url-post-type.php | 4 ++-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 1acdf1f3c5b..36e853eea01 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -54,20 +54,27 @@ td.column-found_elements_and_attributes { margin-bottom: 10px; display: block; } +.dashicons.toggle-sources { + cursor: pointer; + float: right; + margin-right: 20%; +} .column-sources_with_invalid_output .double-arrow { height: 20px; width: 12px; display: inline-block; position: relative; - margin-left: 110px; + float: right; + margin-right: 23%; + cursor: pointer; } -.column-sources_with_invalid_output .double-arrow .dashicons.dashicons-arrow-down.top-arrow { +.column-sources_with_invalid_output .double-arrow .dashicons.top-arrow { font-size: 20px; line-height: 1; position: absolute; bottom: 0; } -.column-sources_with_invalid_output .double-arrow .dashicons.dashicons-arrow-down.bottom-arrow { +.column-sources_with_invalid_output .double-arrow .dashicons.bottom-arrow { font-size: 20px; line-height: 1; position: absolute; diff --git a/assets/js/amp-admin-tables.js b/assets/js/amp-admin-tables.js index 91fc9b5a660..151b7803b9d 100644 --- a/assets/js/amp-admin-tables.js +++ b/assets/js/amp-admin-tables.js @@ -7,15 +7,24 @@ load: function() { $( document ).ready( $.proxy( function() { this.sourcesHideShow(); + this.allSourcesHideShow(); }, this ) ); }, sourcesHideShow: function() { $( 'span.dashicons.toggle-sources' ).on( 'click', function() { $( this ).next( '.sources-container' ).toggleClass( 'collapsed' ); - $( this ).toggleClass( 'dashicons-arrow-down-alt2 dashicons-arrow-up-alt2' ); + $( this ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); }); }, + + allSourcesHideShow: function() { + $( '.double-arrow' ).on( 'click', function() { + $( '.double-arrow' ).find( '.dashicons' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); + $( 'span.dashicons.toggle-sources' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); + $( '.sources-container' ).toggleClass( 'collapsed' ); + }); + } }; adminTables.load(); diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 443c7eb93dc..b5c818d7a59 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -644,7 +644,7 @@ public static function output_custom_column( $column_name, $post_id ) { if ( 1 === $count ) { $output[] = sprintf( '%s
    ', esc_html__( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); $sources_container_classes .= ' collapsed'; } $output[] = '
    '; @@ -659,7 +659,7 @@ public static function output_custom_column( $column_name, $post_id ) { if ( 1 === $count ) { $output[] = sprintf( '%s
    ', esc_html__( 'Other', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Other', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Other', 'amp' ), $count ); $sources_container_classes .= ' collapsed'; } $output[] = '
    '; From 0ad0cf66a65f0ac88f8644fdaae2a5725eda3767 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Tue, 11 Sep 2018 17:24:45 -0300 Subject: [PATCH 39/58] Change new status icons based on AMP options --- assets/css/admin-tables.css | 5 ++++- .../class-amp-invalid-url-post-type.php | 21 ++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 36e853eea01..78d50a26d1c 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -38,9 +38,12 @@ td.column-found_elements_and_attributes { color: #c06e60; } -.column-error_status .dashicons-warning.identified { +.column-error_status .dashicons-flag.identified { color: #d98501; } +.column-error_status .dashicons-yes.identified { + color: #ff0000; +} .column-error_status .dashicons-warning.rejected { color: #68c6ff; } diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index b5c818d7a59..1284ca439ce 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -75,6 +75,17 @@ class AMP_Invalid_URL_Post_Type { */ const VALIDATION_ERRORS_META_BOX = 'amp_validation_errors'; + + /** + * AMP options. + * + * AMP options which we'll need to use to make display decisions based on the options chosen. + * + * @since 1.0 + * @var array[] + */ + protected static $amp_options = array(); + /** * Registers the post type to store URLs with validation errors. * @@ -169,6 +180,8 @@ public static function add_admin_hooks() { $query_vars[] = 'amp_validate_error'; return $query_vars; } ); + + self::$amp_options = \AMP_Options_Manager::get_options(); } /** @@ -312,9 +325,15 @@ public static function display_invalid_url_validation_error_counts_summary( $pos $result = array(); if ( $counts['new'] ) { + if ( true === self::$amp_options['force_sanitization'] && ( 'paired' === self::$amp_options['theme_support'] || 'native' === self::$amp_options['theme_support'] ) ) { + $icon = 'flag'; + } else { + $icon = 'yes'; + } $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + __( '%2$s: %3$s', 'amp' ), + $icon, esc_html__( 'Identified', 'amp' ), number_format_i18n( $counts['new'] ) ); From d4b586f1d0d446853435b0b6359211903b63e069 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 11 Sep 2018 14:01:59 -0700 Subject: [PATCH 40/58] Combine duplicated post_row_actions filters; ensure only relevant links included --- .../class-amp-invalid-url-post-type.php | 80 +++++++++---------- .../test-class-amp-invalid-url-post-type.php | 55 ++++++------- 2 files changed, 61 insertions(+), 74 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 1284ca439ce..d8a0007e779 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -152,7 +152,6 @@ public static function add_admin_hooks() { add_action( 'restrict_manage_posts', array( __CLASS__, 'render_post_filters' ), 10, 2 ); add_filter( 'manage_' . self::POST_TYPE_SLUG . '_posts_columns', array( __CLASS__, 'add_post_columns' ) ); add_action( 'manage_posts_custom_column', array( __CLASS__, 'output_custom_column' ), 10, 2 ); - add_filter( 'post_row_actions', array( __CLASS__, 'filter_row_actions' ), 10, 2 ); add_filter( 'bulk_actions-edit-' . self::POST_TYPE_SLUG, array( __CLASS__, 'filter_bulk_actions' ), 10, 2 ); add_filter( 'handle_bulk_actions-edit-' . self::POST_TYPE_SLUG, array( __CLASS__, 'handle_bulk_action' ), 10, 3 ); add_action( 'admin_notices', array( __CLASS__, 'print_admin_notice' ) ); @@ -697,48 +696,6 @@ public static function output_custom_column( $column_name, $post_id ) { } } - /** - * Adds a 'Recheck' link to the edit.php row actions. - * - * The logic to add the new action is mainly copied from WP_Posts_List_Table::handle_row_actions(). - * - * @param array $actions The actions in the edit.php page. - * @param WP_Post $post The post for the actions. - * @return array $actions The filtered actions. - */ - public static function filter_row_actions( $actions, $post ) { - if ( self::POST_TYPE_SLUG !== $post->post_type ) { - return $actions; - } - - $actions['edit'] = sprintf( - '%s', - esc_url( get_edit_post_link( $post ) ), - esc_html__( 'Details', 'amp' ) - ); - unset( $actions['inline hide-if-no-js'] ); - - $url = self::get_url_from_post( $post ); - if ( $url ) { - $actions['view'] = sprintf( - '%s', - esc_url( add_query_arg( AMP_Validation_Manager::VALIDATE_QUERY_VAR, '', $url ) ), - esc_html__( 'View', 'amp' ) - ); - } - - $actions[ self::VALIDATE_ACTION ] = sprintf( - '%s', - esc_url( self::get_recheck_url( $post ) ), - esc_html__( 'Recheck', 'amp' ) - ); - if ( self::get_post_staleness( $post ) ) { - $actions[ self::VALIDATE_ACTION ] = sprintf( '%s', $actions[ self::VALIDATE_ACTION ] ); - } - - return $actions; - } - /** * Adds a 'Recheck' bulk action to the edit.php page and modifies the 'Move to Trash' text. * @@ -1643,11 +1600,48 @@ public static function print_dashboard_glance_styles() { /** * Filters post row actions. * + * Manages links for details, recheck, view, forget, and forget permanently. + * * @param array $actions Row action links. * @param \WP_Post $post Current WP post. * @return array Filtered action links. */ public static function filter_post_row_actions( $actions, $post ) { + if ( self::POST_TYPE_SLUG !== $post->post_type ) { + return $actions; + } + + // Inline edits are not relevant. + unset( $actions['inline hide-if-no-js'] ); + + if ( isset( $actions['edit'] ) ) { + $actions['edit'] = sprintf( + '%s', + esc_url( get_edit_post_link( $post ) ), + esc_html__( 'Details', 'amp' ) + ); + } + + if ( 'trash' !== $post->post_status ) { + $url = self::get_url_from_post( $post ); + if ( $url ) { + $actions['view'] = sprintf( + '%s', + esc_url( add_query_arg( AMP_Validation_Manager::VALIDATE_QUERY_VAR, '', $url ) ), + esc_html__( 'View', 'amp' ) + ); + } + + $actions[ self::VALIDATE_ACTION ] = sprintf( + '%s', + esc_url( self::get_recheck_url( $post ) ), + esc_html__( 'Recheck', 'amp' ) + ); + if ( self::get_post_staleness( $post ) ) { + $actions[ self::VALIDATE_ACTION ] = sprintf( '%s', $actions[ self::VALIDATE_ACTION ] ); + } + } + // Replace 'Trash' text with 'Forget'. if ( isset( $actions['trash'] ) ) { $actions['trash'] = sprintf( diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index da366ffcdcc..c0f8c78f706 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -94,7 +94,7 @@ public function test_add_admin_hooks() { $this->assertEquals( 10, has_filter( 'restrict_manage_posts', array( self::TESTED_CLASS, 'render_post_filters' ) ) ); $this->assertEquals( 10, has_filter( 'manage_' . AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG . '_posts_columns', array( self::TESTED_CLASS, 'add_post_columns' ) ) ); $this->assertEquals( 10, has_action( 'manage_posts_custom_column', array( self::TESTED_CLASS, 'output_custom_column' ) ) ); - $this->assertEquals( 10, has_filter( 'post_row_actions', array( self::TESTED_CLASS, 'filter_row_actions' ) ) ); + $this->assertEquals( 10, has_filter( 'post_row_actions', array( self::TESTED_CLASS, 'filter_post_row_actions' ) ) ); $this->assertEquals( 10, has_filter( 'bulk_actions-edit-' . AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, array( self::TESTED_CLASS, 'filter_bulk_actions' ) ) ); $this->assertEquals( 10, has_filter( 'handle_bulk_actions-edit-' . AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, array( self::TESTED_CLASS, 'handle_bulk_action' ) ) ); $this->assertEquals( 10, has_action( 'admin_notices', array( self::TESTED_CLASS, 'print_admin_notice' ) ) ); @@ -550,36 +550,6 @@ public function test_output_custom_column( $column_name, $expected_value, $error $this->assertContains( $expected_value, ob_get_clean() ); } - /** - * Test for filter_row_actions() - * - * @covers \AMP_Invalid_URL_Post_Type::filter_row_actions() - */ - public function test_filter_row_actions() { - add_theme_support( 'amp' ); - AMP_Validation_Manager::init(); - - $initial_actions = array( - 'trash' => 'Trash', - ); - - $invalid_post_id = AMP_Invalid_URL_Post_Type::store_validation_errors( - array( - array( 'code' => 'foo' ), - ), - home_url( '/' ) - ); - - $this->assertEquals( $initial_actions, AMP_Invalid_URL_Post_Type::filter_row_actions( $initial_actions, $this->factory()->post->create_and_get() ) ); - - $actions = AMP_Invalid_URL_Post_Type::filter_row_actions( $initial_actions, get_post( $invalid_post_id ) ); - $this->assertArrayNotHasKey( 'inline hide-if-no-js', $actions ); - $this->assertArrayHasKey( 'view', $actions ); - $this->assertArrayHasKey( AMP_Invalid_URL_Post_Type::VALIDATE_ACTION, $actions ); - - $this->assertEquals( $initial_actions['trash'], $actions['trash'] ); - } - /** * Test for filter_bulk_actions() * @@ -1230,6 +1200,29 @@ public function test_filter_dashboard_glance_items() { * @covers \AMP_Invalid_URL_Post_Type::filter_post_row_actions() */ public function test_filter_post_row_actions() { + add_theme_support( 'amp' ); + AMP_Validation_Manager::init(); + + $initial_actions = array( + 'trash' => 'Trash', + ); + + $invalid_post_id = AMP_Invalid_URL_Post_Type::store_validation_errors( + array( + array( 'code' => 'foo' ), + ), + home_url( '/' ) + ); + + $this->assertEquals( $initial_actions, AMP_Invalid_URL_Post_Type::filter_post_row_actions( $initial_actions, $this->factory()->post->create_and_get() ) ); + + $actions = AMP_Invalid_URL_Post_Type::filter_post_row_actions( $initial_actions, get_post( $invalid_post_id ) ); + $this->assertArrayNotHasKey( 'inline hide-if-no-js', $actions ); + $this->assertArrayHasKey( 'view', $actions ); + $this->assertArrayHasKey( AMP_Invalid_URL_Post_Type::VALIDATE_ACTION, $actions ); + + $this->assertEquals( $initial_actions['trash'], $actions['trash'] ); + $this->assertEquals( array(), AMP_Invalid_URL_Post_Type::filter_post_row_actions( array(), null ) ); $actions = array( From 1ab6bd3f9d9980d0c9a82f4355ab0f406e371781 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 11 Sep 2018 14:05:55 -0700 Subject: [PATCH 41/58] Re-use AMP_Validation_Manager::is_sanitization_forcibly_accepted() --- .../class-amp-invalid-url-post-type.php | 15 +-------------- .../test-class-amp-invalid-url-post-type.php | 4 ++-- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index d8a0007e779..bec65a08337 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -75,17 +75,6 @@ class AMP_Invalid_URL_Post_Type { */ const VALIDATION_ERRORS_META_BOX = 'amp_validation_errors'; - - /** - * AMP options. - * - * AMP options which we'll need to use to make display decisions based on the options chosen. - * - * @since 1.0 - * @var array[] - */ - protected static $amp_options = array(); - /** * Registers the post type to store URLs with validation errors. * @@ -179,8 +168,6 @@ public static function add_admin_hooks() { $query_vars[] = 'amp_validate_error'; return $query_vars; } ); - - self::$amp_options = \AMP_Options_Manager::get_options(); } /** @@ -324,7 +311,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos $result = array(); if ( $counts['new'] ) { - if ( true === self::$amp_options['force_sanitization'] && ( 'paired' === self::$amp_options['theme_support'] || 'native' === self::$amp_options['theme_support'] ) ) { + if ( current_theme_supports( 'amp' ) && AMP_Validation_Manager::is_sanitization_forcibly_accepted() ) { $icon = 'flag'; } else { $icon = 'yes'; diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index c0f8c78f706..5485cbf5102 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -169,8 +169,8 @@ public function test_add_admin_menu_new_invalid_url_count() { * @covers \AMP_Invalid_URL_Post_Type::store_validation_errors() */ public function test_get_invalid_url_validation_errors() { - AMP_Options_Manager::update_option( 'force_sanitization', false ); add_theme_support( 'amp', array( 'paired' => true ) ); + AMP_Options_Manager::update_option( 'force_sanitization', false ); AMP_Validation_Manager::init(); $post = $this->factory()->post->create(); $this->assertEmpty( AMP_Invalid_URL_Post_Type::get_invalid_url_validation_errors( get_permalink( $post ) ) ); @@ -219,7 +219,7 @@ public function test_get_invalid_url_validation_errors() { ob_start(); AMP_Invalid_URL_Post_Type::display_invalid_url_validation_error_counts_summary( $invalid_url_post_id ); $summary = ob_get_clean(); - $this->assertContains( 'New: 1', $summary ); + $this->assertContains( 'Identified: 1', $summary ); $this->assertContains( 'Accepted: 1', $summary ); $this->assertContains( 'Rejected: 1', $summary ); } From 7eafdc633e132ded4c567ea8d13521868d043d0d Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Tue, 11 Sep 2018 20:19:34 -0300 Subject: [PATCH 42/58] Add view errors by type title link button --- assets/css/admin-tables.css | 3 +++ assets/js/amp-admin-tables.js | 7 ++++++- .../validation/class-amp-invalid-url-post-type.php | 10 ++++++++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 78d50a26d1c..2b23f9c2667 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -83,3 +83,6 @@ td.column-found_elements_and_attributes { position: absolute; bottom: -6px; } +.wrap .wp-heading-inline + .page-title-action { + margin-left: 1rem; +} diff --git a/assets/js/amp-admin-tables.js b/assets/js/amp-admin-tables.js index 151b7803b9d..d31a7918b7c 100644 --- a/assets/js/amp-admin-tables.js +++ b/assets/js/amp-admin-tables.js @@ -1,4 +1,4 @@ -/* global jQuery */ +/* global jQuery, ampAdminTables */ ( function( $ ) { 'use strict'; @@ -8,6 +8,7 @@ $( document ).ready( $.proxy( function() { this.sourcesHideShow(); this.allSourcesHideShow(); + this.addViewErrorsByTypeLinkButton(); }, this ) ); }, @@ -24,6 +25,10 @@ $( 'span.dashicons.toggle-sources' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); $( '.sources-container' ).toggleClass( 'collapsed' ); }); + }, + + addViewErrorsByTypeLinkButton: function() { + $( '.wp-heading-inline' ).after( 'View errors by Type' ); } }; diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index bec65a08337..2ba6125c518 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -176,7 +176,7 @@ public static function add_admin_hooks() { public static function enqueue_admin_assets() { // Styles. $screen = get_current_screen(); - if ( 'amp_invalid_url' !== $screen->post_type ) { + if ( 'edit-amp_invalid_url' !== $screen->id ) { return; } @@ -192,6 +192,13 @@ public static function enqueue_admin_assets() { [ 'jquery' ], AMP__VERSION ); + wp_localize_script( + 'amp-admin-tables', + 'ampAdminTables', + [ + 'errorsByTypeLink' => get_admin_url( null,'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_invalid_url' ) + ] + ); } /** @@ -1710,5 +1717,4 @@ public static function filter_bulk_post_updated_messages( $messages, $bulk_count return $messages; } - } From 5b9b243b7cc89e7aa303fa4ef98db8116cffbe6f Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Tue, 11 Sep 2018 20:34:57 -0300 Subject: [PATCH 43/58] Change title of page and highlight word URL --- assets/js/amp-admin-tables.js | 10 ++++++++++ .../validation/class-amp-invalid-url-post-type.php | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/assets/js/amp-admin-tables.js b/assets/js/amp-admin-tables.js index d31a7918b7c..2857523dfab 100644 --- a/assets/js/amp-admin-tables.js +++ b/assets/js/amp-admin-tables.js @@ -9,6 +9,7 @@ this.sourcesHideShow(); this.allSourcesHideShow(); this.addViewErrorsByTypeLinkButton(); + this.boldURLInPageTitle(); }, this ) ); }, @@ -29,6 +30,15 @@ addViewErrorsByTypeLinkButton: function() { $( '.wp-heading-inline' ).after( 'View errors by Type' ); + }, + + boldURLInPageTitle: function() { + var $heading = $( 'h1.wp-heading-inline' ), + words = $heading.text().split( ' ' ); + words = words.map( function( item ) { + return item === 'URL' ? '' + item + '' : item; + } ); + $heading.html( words.join(' ') ); } }; diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 2ba6125c518..540f86baed5 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -85,8 +85,8 @@ public static function register() { self::POST_TYPE_SLUG, array( 'labels' => array( - 'name' => _x( 'Invalid AMP Pages (URLs)', 'post type general name', 'amp' ), - 'menu_name' => __( 'Invalid Pages', 'amp' ), + 'name' => _x( 'Errors by URL', 'post type general name', 'amp' ), + 'menu_name' => __( 'Errors by URL', 'amp' ), 'singular_name' => __( 'Invalid AMP Page (URL)', 'amp' ), 'not_found' => __( 'No invalid AMP pages found', 'amp' ), 'not_found_in_trash' => __( 'No forgotten invalid AMP pages', 'amp' ), From 52babee2c2ac742831fb238b335d054484b77474 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Tue, 11 Sep 2018 20:44:17 -0300 Subject: [PATCH 44/58] Bolder font weight for page title action link keyword --- assets/css/admin-tables.css | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 2b23f9c2667..6217a59118d 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -86,3 +86,6 @@ td.column-found_elements_and_attributes { .wrap .wp-heading-inline + .page-title-action { margin-left: 1rem; } +.page-title-action strong { + font-weight: 900; +} From 8f88d0cc3408784fac085492233fd319f62e6449 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Tue, 11 Sep 2018 20:53:00 -0300 Subject: [PATCH 45/58] ESLint fixes --- assets/js/amp-admin-tables.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/assets/js/amp-admin-tables.js b/assets/js/amp-admin-tables.js index 2857523dfab..8c24a1d0489 100644 --- a/assets/js/amp-admin-tables.js +++ b/assets/js/amp-admin-tables.js @@ -17,7 +17,7 @@ $( 'span.dashicons.toggle-sources' ).on( 'click', function() { $( this ).next( '.sources-container' ).toggleClass( 'collapsed' ); $( this ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); - }); + } ); }, allSourcesHideShow: function() { @@ -25,7 +25,7 @@ $( '.double-arrow' ).find( '.dashicons' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); $( 'span.dashicons.toggle-sources' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); $( '.sources-container' ).toggleClass( 'collapsed' ); - }); + } ); }, addViewErrorsByTypeLinkButton: function() { @@ -38,10 +38,9 @@ words = words.map( function( item ) { return item === 'URL' ? '' + item + '' : item; } ); - $heading.html( words.join(' ') ); + $heading.html( words.join( ' ' ) ); } }; adminTables.load(); - }( jQuery ) ); From 1d95cae458dd9257b49095a1def851fafee923cb Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Wed, 12 Sep 2018 08:36:14 -0300 Subject: [PATCH 46/58] Fix PHP 5.3 compatibility issues --- .../validation/class-amp-invalid-url-post-type.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 540f86baed5..f98e6919aab 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -189,16 +189,16 @@ public static function enqueue_admin_assets() { wp_enqueue_script( 'amp-admin-tables', amp_get_asset_url( 'js/amp-admin-tables.js' ), - [ 'jquery' ], + array( 'jquery' ), AMP__VERSION ); wp_localize_script( 'amp-admin-tables', 'ampAdminTables', - [ - 'errorsByTypeLink' => get_admin_url( null,'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_invalid_url' ) - ] - ); + array( + 'errorsByTypeLink' => get_admin_url( null, 'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_invalid_url' ), + ) + ); } /** From a4bdee9724613a551ce3bcb0a08eb99c57718a73 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Wed, 12 Sep 2018 09:13:29 -0300 Subject: [PATCH 47/58] Update tests to reflect changes --- .../validation/class-amp-invalid-url-post-type.php | 2 +- .../class-amp-validation-error-taxonomy.php | 9 ++++++++- .../test-class-amp-invalid-url-post-type.php | 12 ++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index f98e6919aab..ddc1ee7fee3 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -575,7 +575,7 @@ public static function add_post_columns( $columns ) { $columns = array_merge( $columns, array( - 'error_status' => sprintf( '%s
    %s
    ', esc_html__( 'Status', 'amp' ), esc_html__( 'This is tooltip', 'amp' ) ), // @todo Create actual tooltip. + 'error_status' => sprintf( '%s', esc_html__( 'Status', 'amp' ) ), // @todo Create actual tooltip. AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Invalid', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => sprintf( '%s
    ', esc_html__( 'Sources', 'amp' ) ), ) diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 1d3524a888d..795f5dd5dd9 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -141,7 +141,7 @@ class AMP_Validation_Error_Taxonomy { * * @var string */ - const REMOVED_ELEMENTS = 'removed_elements'; + const REMOVED_ELEMENTS = 'sources_with_invalid_output'; /** * The key for found elements and attributes. @@ -171,6 +171,13 @@ class AMP_Validation_Error_Taxonomy { */ const REMOVED_SOURCES = 'removed_sources'; + /** + * The key for the error status. + * + * @var string + */ + const ERROR_STATUS = 'error_status'; + /** * Whether the terms_clauses filter should apply to a term query for validation errors to limit to a given status. * diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index 5485cbf5102..cc67a99f63e 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -41,7 +41,7 @@ public function test_register() { $this->assertTrue( in_array( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, get_post_types(), true ) ); $this->assertEquals( array(), get_all_post_type_supports( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) ); $this->assertEquals( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, $amp_post_type->name ); - $this->assertEquals( 'Invalid AMP Pages (URLs)', $amp_post_type->label ); + $this->assertEquals( 'Errors by URL', $amp_post_type->label ); $this->assertEquals( false, $amp_post_type->public ); $this->assertTrue( $amp_post_type->show_ui ); $this->assertEquals( AMP_Options_Manager::OPTION_NAME, $amp_post_type->show_in_menu ); @@ -479,10 +479,10 @@ public function test_add_post_columns() { array_merge( $initial_columns, array( - AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS => 'Removed Elements', - AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES => 'Removed Attributes', + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Sources
    ', + AMP_Validation_Error_Taxonomy::ERROR_STATUS => 'Status', AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Incompatible Sources', - 'error_status' => 'Error Status', + AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => 'Invalid', ) ), AMP_Invalid_URL_Post_Type::add_post_columns( $initial_columns ) @@ -514,7 +514,7 @@ public function get_custom_columns() { return array( 'invalid_element' => array( - AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS, + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT, 'script', $errors, ), @@ -1204,7 +1204,7 @@ public function test_filter_post_row_actions() { AMP_Validation_Manager::init(); $initial_actions = array( - 'trash' => 'Trash', + 'trash' => 'Forget', ); $invalid_post_id = AMP_Invalid_URL_Post_Type::store_validation_errors( From fe7e4de9b979d7165c93d86f542a18daa269f1a4 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Wed, 12 Sep 2018 11:20:56 -0300 Subject: [PATCH 48/58] Update to tests and add an error check --- includes/validation/class-amp-invalid-url-post-type.php | 2 +- includes/validation/class-amp-validation-error-taxonomy.php | 2 +- tests/validation/test-class-amp-invalid-url-post-type.php | 1 - tests/validation/test-class-amp-validation-error-taxonomy.php | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index ddc1ee7fee3..43cd44bc7de 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -1601,7 +1601,7 @@ public static function print_dashboard_glance_styles() { * @return array Filtered action links. */ public static function filter_post_row_actions( $actions, $post ) { - if ( self::POST_TYPE_SLUG !== $post->post_type ) { + if ( ! is_object( $post ) || self::POST_TYPE_SLUG !== $post->post_type ) { return $actions; } diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 795f5dd5dd9..56fc1cf0234 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -141,7 +141,7 @@ class AMP_Validation_Error_Taxonomy { * * @var string */ - const REMOVED_ELEMENTS = 'sources_with_invalid_output'; + const REMOVED_ELEMENTS = 'removed_elements'; /** * The key for found elements and attributes. diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index cc67a99f63e..b898a1eeebd 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -479,7 +479,6 @@ public function test_add_post_columns() { array_merge( $initial_columns, array( - AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Sources
    ', AMP_Validation_Error_Taxonomy::ERROR_STATUS => 'Status', AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Incompatible Sources', AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => 'Invalid', diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php index 70fa2d50992..9a67edbf08a 100644 --- a/tests/validation/test-class-amp-validation-error-taxonomy.php +++ b/tests/validation/test-class-amp-validation-error-taxonomy.php @@ -336,7 +336,7 @@ public function test_summarize_validation_errors() { AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS => array( $element_node_name => 1, ), - 'sources_with_invalid_output' => array( + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => array( 'plugin' => array( 'foo' ), 'theme' => array( 'bar' ), ), From 6936f05a6f79e38be6610955ca2dba85a11d0e06 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Thu, 13 Sep 2018 14:20:41 -0500 Subject: [PATCH 49/58] Address unit test errors Updates the unit tests to the latest changes They now pass locally. --- includes/validation/class-amp-invalid-url-post-type.php | 4 ++-- tests/validation/test-class-amp-invalid-url-post-type.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 43cd44bc7de..b5b1e3f4bf7 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -650,8 +650,8 @@ public static function output_custom_column( $column_name, $post_id ) { $output = array(); if ( isset( $sources['plugin'] ) ) { - $output[] = '
    '; - $count = count( array_unique( $sources['plugin'] ) ); + $output[] = '
    '; + $count = count( array_unique( $sources['plugin'] ) ); $sources_container_classes = 'sources-container sources-plugins'; if ( 1 === $count ) { $output[] = sprintf( '%s
    ', esc_html__( 'Plugin', 'amp' ) ); diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index b898a1eeebd..9dc3b642efe 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -480,7 +480,7 @@ public function test_add_post_columns() { $initial_columns, array( AMP_Validation_Error_Taxonomy::ERROR_STATUS => 'Status', - AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Incompatible Sources', + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Sources
    ', AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => 'Invalid', ) ), @@ -514,11 +514,11 @@ public function get_custom_columns() { return array( 'invalid_element' => array( AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT, - 'script', + '
    Plugin
    amp
    ', $errors, ), 'removed_attributes' => array( - AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES, + AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES, 'onclick', $errors, ), From 2ce2e5e30079062de1d91a081673ba1993b59d1a Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Thu, 13 Sep 2018 14:28:02 -0500 Subject: [PATCH 50/58] Addres PHPCS warning by aligning = The build failed, with: Equals sign not aligned with surrounding assignments. So address this by aligning the = --- includes/validation/class-amp-invalid-url-post-type.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index f5859b7a02b..fceed3726e8 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -664,7 +664,7 @@ public static function output_custom_column( $column_name, $post_id ) { if ( 1 === $count ) { $output[] = sprintf( '%s
    ', esc_html__( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); $sources_container_classes .= ' collapsed'; } $output[] = '
    '; @@ -673,13 +673,13 @@ public static function output_custom_column( $column_name, $post_id ) { $output[] = '
    '; } if ( isset( $sources['core'] ) ) { - $output[] = '
    '; - $count = count( array_unique( $sources['core'] ) ); + $output[] = '
    '; + $count = count( array_unique( $sources['core'] ) ); $sources_container_classes = 'sources-container sources-core'; if ( 1 === $count ) { $output[] = sprintf( '%s
    ', esc_html__( 'Other', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Other', 'amp' ), $count ); + $output[] = sprintf( '%s (%d)', esc_html__( 'Other', 'amp' ), $count ); $sources_container_classes .= ' collapsed'; } $output[] = '
    '; From 74feb8537228c0caf75620df948d5cb72ff17da1 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 13 Sep 2018 19:10:12 -0300 Subject: [PATCH 51/58] Get full plugin names and fix theme source output --- .../class-amp-invalid-url-post-type.php | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index fceed3726e8..bd40063508b 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -658,8 +658,18 @@ public static function output_custom_column( $column_name, $post_id ) { $output = array(); if ( isset( $sources['plugin'] ) ) { - $output[] = '
    '; - $count = count( array_unique( $sources['plugin'] ) ); + $output[] = '
    '; + $plugin_names = array(); + $plugin_slugs = array_unique( $sources['plugin'] ); + foreach ( $plugin_slugs as $plugin_slug ) { + $plugin_data = get_plugin_data( WP_PLUGIN_DIR . '/' . $plugin_slug . '/' . $plugin_slug . '.php' ); + if ( ! empty( $plugin_data ) && ! empty( $plugin_data['Name'] ) ) { + $plugin_names[] = $plugin_data['Name']; + } else { + $plugin_names[] = $plugin_slug; + } + } + $count = count( $plugin_slugs ); $sources_container_classes = 'sources-container sources-plugins'; if ( 1 === $count ) { $output[] = sprintf( '%s
    ', esc_html__( 'Plugin', 'amp' ) ); @@ -668,7 +678,7 @@ public static function output_custom_column( $column_name, $post_id ) { $sources_container_classes .= ' collapsed'; } $output[] = '
    '; - $output[] = implode( '
    ', array_unique( $sources['plugin'] ) ); + $output[] = implode( '
    ', array_unique( $plugin_names ) ); $output[] = '
    '; $output[] = '
    '; } @@ -689,7 +699,11 @@ public static function output_custom_column( $column_name, $post_id ) { } if ( isset( $sources['theme'] ) ) { $output[] = '
    '; - $output[] = sprintf( '%s', esc_html( $sources['theme']['name'] ) ); + $output[] = ''; + $themes = array_unique( $sources['theme'] ); + foreach ( $themes as $theme_slug ) { + $output[] = sprintf( '%s
    ', esc_html( $theme_slug ) ); + } $output[] = '
    '; } echo implode( '', $output ); // WPCS: XSS ok. From 5baae3fef0d3cb7e371851afa210c23764fa1ae7 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Thu, 13 Sep 2018 19:56:16 -0300 Subject: [PATCH 52/58] Code review changes and text changes --- assets/css/admin-tables.css | 4 +-- assets/js/amp-admin-tables.js | 16 +++------ .../class-amp-invalid-url-post-type.php | 35 ++++++++++--------- .../test-class-amp-invalid-url-post-type.php | 2 +- 4 files changed, 26 insertions(+), 31 deletions(-) diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 6217a59118d..385707aeefa 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -38,10 +38,10 @@ td.column-found_elements_and_attributes { color: #c06e60; } -.column-error_status .dashicons-flag.identified { +.column-error_status .dashicons-flag.new { color: #d98501; } -.column-error_status .dashicons-yes.identified { +.column-error_status .dashicons-yes.new { color: #ff0000; } .column-error_status .dashicons-warning.rejected { diff --git a/assets/js/amp-admin-tables.js b/assets/js/amp-admin-tables.js index 8c24a1d0489..3f2655808f9 100644 --- a/assets/js/amp-admin-tables.js +++ b/assets/js/amp-admin-tables.js @@ -9,7 +9,6 @@ this.sourcesHideShow(); this.allSourcesHideShow(); this.addViewErrorsByTypeLinkButton(); - this.boldURLInPageTitle(); }, this ) ); }, @@ -29,16 +28,11 @@ }, addViewErrorsByTypeLinkButton: function() { - $( '.wp-heading-inline' ).after( 'View errors by Type' ); - }, - - boldURLInPageTitle: function() { - var $heading = $( 'h1.wp-heading-inline' ), - words = $heading.text().split( ' ' ); - words = words.map( function( item ) { - return item === 'URL' ? '' + item + '' : item; - } ); - $heading.html( words.join( ' ' ) ); + var link = $( '' ); + link.prop( 'text', ampAdminTables.errorIndexAnchor ); + link.prop( 'href', ampAdminTables.errorIndexLink ); + link.prop( 'class', 'page-title-action' ); + $( '.wp-heading-inline' ).after( link ); } }; diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index bd40063508b..814a36b0ebc 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -85,13 +85,13 @@ public static function register() { self::POST_TYPE_SLUG, array( 'labels' => array( - 'name' => _x( 'Errors by URL', 'post type general name', 'amp' ), - 'menu_name' => __( 'Errors by URL', 'amp' ), - 'singular_name' => __( 'Invalid AMP Page (URL)', 'amp' ), - 'not_found' => __( 'No invalid AMP pages found', 'amp' ), - 'not_found_in_trash' => __( 'No forgotten invalid AMP pages', 'amp' ), - 'search_items' => __( 'Search invalid AMP pages', 'amp' ), - 'edit_item' => __( 'Invalid AMP Page (URL)', 'amp' ), + 'name' => _x( 'Invalid URLs', 'post type general name', 'amp' ), + 'menu_name' => __( 'Invalid URLs', 'amp' ), + 'singular_name' => __( 'Invalid URL', 'amp' ), + 'not_found' => __( 'No invalid URLs found', 'amp' ), + 'not_found_in_trash' => __( 'No forgotten invalid URLs', 'amp' ), + 'search_items' => __( 'Search invalid URLs', 'amp' ), + 'edit_item' => __( 'Invalid URL', 'amp' ), ), 'supports' => false, 'public' => false, @@ -126,7 +126,7 @@ public static function should_show_in_menu() { * Add admin hooks. */ public static function add_admin_hooks() { - add_action( 'admin_enqueue_scripts', array( __CLASS__, 'enqueue_admin_assets' ) ); + add_action( 'admin_enqueue_scripts', array( __CLASS__, 'enqueue_post_list_screen_scripts' ) ); add_filter( 'dashboard_glance_items', array( __CLASS__, 'filter_dashboard_glance_items' ) ); add_action( 'rightnow_end', array( __CLASS__, 'print_dashboard_glance_styles' ) ); @@ -173,7 +173,7 @@ public static function add_admin_hooks() { /** * Enqueue style. */ - public static function enqueue_admin_assets() { + public static function enqueue_post_list_screen_scripts() { // Styles. $screen = get_current_screen(); if ( 'edit-amp_invalid_url' !== $screen->id ) { @@ -196,7 +196,8 @@ public static function enqueue_admin_assets() { 'amp-admin-tables', 'ampAdminTables', array( - 'errorsByTypeLink' => get_admin_url( null, 'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_invalid_url' ), + 'errorIndexLink' => get_admin_url( null, 'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_invalid_url' ), + 'errorIndexAnchor' => esc_html__( 'View Error Index', 'amp' ), ) ); } @@ -318,23 +319,23 @@ public static function display_invalid_url_validation_error_counts_summary( $pos $result = array(); if ( $counts['new'] ) { - if ( current_theme_supports( 'amp' ) && AMP_Validation_Manager::is_sanitization_forcibly_accepted() ) { + if ( AMP_Validation_Manager::is_sanitization_forcibly_accepted() ) { $icon = 'flag'; } else { $icon = 'yes'; } $result[] = sprintf( /* translators: %s is count */ - __( '%2$s: %3$s', 'amp' ), - $icon, - esc_html__( 'Identified', 'amp' ), + '%2$s: %3$s', + esc_attr( $icon ), + esc_html__( 'New', 'amp' ), number_format_i18n( $counts['new'] ) ); } if ( $counts['accepted'] ) { $result[] = sprintf( /* translators: 1. Title, 2. %s is count */ - __( '%1$s: %2$s', 'amp' ), + '%1$s: %2$s', esc_html__( 'Accepted', 'amp' ), number_format_i18n( $counts['accepted'] ) ); @@ -342,7 +343,7 @@ public static function display_invalid_url_validation_error_counts_summary( $pos if ( $counts['rejected'] ) { $result[] = sprintf( /* translators: %s is count */ - __( '%1$s: %2$s', 'amp' ), + '%1$s: %2$s', esc_html__( 'Rejected', 'amp' ), number_format_i18n( $counts['rejected'] ) ); @@ -583,7 +584,7 @@ public static function add_post_columns( $columns ) { $columns = array_merge( $columns, array( - 'error_status' => sprintf( '%s', esc_html__( 'Status', 'amp' ) ), // @todo Create actual tooltip. + AMP_Validation_Error_Taxonomy::ERROR_STATUS => sprintf( '%s', esc_html__( 'Status', 'amp' ) ), // @todo Create actual tooltip. AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Invalid', 'amp' ), AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => sprintf( '%s
    ', esc_html__( 'Sources', 'amp' ) ), ) diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index e193117441f..0c3e37a9c12 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -219,7 +219,7 @@ public function test_get_invalid_url_validation_errors() { ob_start(); AMP_Invalid_URL_Post_Type::display_invalid_url_validation_error_counts_summary( $invalid_url_post_id ); $summary = ob_get_clean(); - $this->assertContains( 'Identified: 1', $summary ); + $this->assertContains( 'New: 1', $summary ); $this->assertContains( 'Accepted: 1', $summary ); $this->assertContains( 'Rejected: 1', $summary ); } From 6911305f06eb5addb0abd84c4fa264153c5fd2e6 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Fri, 14 Sep 2018 20:52:47 -0300 Subject: [PATCH 53/58] Unify JS for text expansion and update styles --- assets/css/admin-tables.css | 39 --------------- assets/css/amp-validation-error-taxonomy.css | 14 +++++- assets/js/amp-admin-tables.js | 40 --------------- .../src/amp-validation-error-detail-toggle.js | 26 ++++++++-- .../class-amp-invalid-url-post-type.php | 50 ++++++++++--------- 5 files changed, 62 insertions(+), 107 deletions(-) delete mode 100644 assets/js/amp-admin-tables.js diff --git a/assets/css/admin-tables.css b/assets/css/admin-tables.css index 385707aeefa..bd30ea785c2 100644 --- a/assets/css/admin-tables.css +++ b/assets/css/admin-tables.css @@ -1,6 +1,3 @@ -.column-error_status .tooltip { - display: none; -} .column-error_status .dashicons-editor-help { color: #767676; } @@ -16,10 +13,6 @@ .column-sources_with_invalid_output .dashicons-wordpress-alt { color: #92b371; } -.sources-plugins, .sources-core { - margin-left: 25px; - margin-bottom: 15px; -} .amp-logo-icon { background-image: url( '../images/amp-logo-icon.svg' ); background-color: transparent; @@ -47,9 +40,6 @@ td.column-found_elements_and_attributes { .column-error_status .dashicons-warning.rejected { color: #68c6ff; } -.column-sources_with_invalid_output .sources-plugins.collapsed, .column-sources_with_invalid_output .sources-core.collapsed { - display: none; -} .column-sources_with_invalid_output .source { margin-bottom: 10px; } @@ -57,35 +47,6 @@ td.column-found_elements_and_attributes { margin-bottom: 10px; display: block; } -.dashicons.toggle-sources { - cursor: pointer; - float: right; - margin-right: 20%; -} -.column-sources_with_invalid_output .double-arrow { - height: 20px; - width: 12px; - display: inline-block; - position: relative; - float: right; - margin-right: 23%; - cursor: pointer; -} -.column-sources_with_invalid_output .double-arrow .dashicons.top-arrow { - font-size: 20px; - line-height: 1; - position: absolute; - bottom: 0; -} -.column-sources_with_invalid_output .double-arrow .dashicons.bottom-arrow { - font-size: 20px; - line-height: 1; - position: absolute; - bottom: -6px; -} .wrap .wp-heading-inline + .page-title-action { margin-left: 1rem; } -.page-title-action strong { - font-weight: 900; -} diff --git a/assets/css/amp-validation-error-taxonomy.css b/assets/css/amp-validation-error-taxonomy.css index 91192865397..74a3937049b 100644 --- a/assets/css/amp-validation-error-taxonomy.css +++ b/assets/css/amp-validation-error-taxonomy.css @@ -74,17 +74,27 @@ details[open] .details-attributes__summary::after { color: #00a0d2; } +.column-sources_with_invalid_output details[open] .details-attributes__summary { + margin-bottom: 5px; +} +.column-sources_with_invalid_output details > div { + padding-left: 25px; +} + /* Error details toggle button */ -.manage-column.column-details { +.manage-column.column-details, .manage-column.column-sources_with_invalid_output { display: flex; justify-content: space-between; align-items: center; } +.manage-column.column-sources_with_invalid_output .error-details-toggle { + margin: 0; +} .error-details-toggle { display: flex; flex-direction: column; - height: 12px; + height: 14px; margin-right: 10px; padding: 0; background: none; diff --git a/assets/js/amp-admin-tables.js b/assets/js/amp-admin-tables.js deleted file mode 100644 index 3f2655808f9..00000000000 --- a/assets/js/amp-admin-tables.js +++ /dev/null @@ -1,40 +0,0 @@ -/* global jQuery, ampAdminTables */ -( function( $ ) { - 'use strict'; - - var adminTables = { - - load: function() { - $( document ).ready( $.proxy( function() { - this.sourcesHideShow(); - this.allSourcesHideShow(); - this.addViewErrorsByTypeLinkButton(); - }, this ) ); - }, - - sourcesHideShow: function() { - $( 'span.dashicons.toggle-sources' ).on( 'click', function() { - $( this ).next( '.sources-container' ).toggleClass( 'collapsed' ); - $( this ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); - } ); - }, - - allSourcesHideShow: function() { - $( '.double-arrow' ).on( 'click', function() { - $( '.double-arrow' ).find( '.dashicons' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); - $( 'span.dashicons.toggle-sources' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); - $( '.sources-container' ).toggleClass( 'collapsed' ); - } ); - }, - - addViewErrorsByTypeLinkButton: function() { - var link = $( '' ); - link.prop( 'text', ampAdminTables.errorIndexAnchor ); - link.prop( 'href', ampAdminTables.errorIndexLink ); - link.prop( 'class', 'page-title-action' ); - $( '.wp-heading-inline' ).after( link ); - } - }; - - adminTables.load(); -}( jQuery ) ); diff --git a/assets/src/amp-validation-error-detail-toggle.js b/assets/src/amp-validation-error-detail-toggle.js index 85275c20b06..0fd98bd690f 100644 --- a/assets/src/amp-validation-error-detail-toggle.js +++ b/assets/src/amp-validation-error-detail-toggle.js @@ -6,7 +6,7 @@ import domReady from '@wordpress/dom-ready'; /** * Localized data */ -import { btnAriaLabel } from 'amp-validation-i18n'; +import { btnAriaLabel, errorIndexLink, errorIndexAnchor } from 'amp-validation-i18n'; const OPEN_CLASS = 'is-open'; @@ -16,12 +16,19 @@ const OPEN_CLASS = 'is-open'; * table column via backend code. */ function addToggleButtons() { - [ ...document.querySelectorAll( 'th.column-details.manage-column' ) ].forEach( th => { + const addButtons = ( th ) => { const button = document.createElement( 'button' ); button.setAttribute( 'aria-label', btnAriaLabel ); button.setAttribute( 'type', 'button' ); button.setAttribute( 'class', 'error-details-toggle' ); th.appendChild( button ); + }; + + [ ...document.querySelectorAll( 'th.column-details.manage-column' ) ].forEach( th => { + addButtons( th ); + } ); + [ ...document.querySelectorAll( 'th.manage-column.column-sources_with_invalid_output' ) ].forEach( th => { + addButtons( th ); } ); } @@ -31,7 +38,7 @@ function addToggleButtons() { function addToggleListener() { let open = false; - const details = [ ...document.querySelectorAll( '.column-details details' ) ]; + const details = [ ...document.querySelectorAll( '.column-details details, .column-sources_with_invalid_output details' ) ]; const toggleButtons = [ ...document.querySelectorAll( 'button.error-details-toggle' ) ]; const onButtonClick = () => { open = ! open; @@ -54,7 +61,20 @@ function addToggleListener() { } ); } +function addViewErrorsByTypeLinkButton() { + if ( 'undefined' === typeof errorIndexAnchor || 'undefined' === typeof errorIndexLink ) { + return; + } + const heading = document.querySelector( '.wp-heading-inline' ); + const link = document.createElement( 'a' ); + link.innerHTML = errorIndexAnchor; + link.setAttribute( 'href', errorIndexLink ); + link.setAttribute( 'class', 'page-title-action' ); + heading.after( link ); +} + domReady( () => { addToggleButtons(); addToggleListener(); + addViewErrorsByTypeLinkButton(); } ); diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 814a36b0ebc..0118608c84e 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -186,16 +186,24 @@ public static function enqueue_post_list_screen_scripts() { false, AMP__VERSION ); - wp_enqueue_script( - 'amp-admin-tables', - amp_get_asset_url( 'js/amp-admin-tables.js' ), - array( 'jquery' ), + wp_enqueue_style( + 'amp-validation-error-taxonomy', + amp_get_asset_url( 'css/amp-validation-error-taxonomy.css' ), + array( 'common' ), AMP__VERSION ); + wp_enqueue_script( + 'amp-validation-error-detail-toggle', + amp_get_asset_url( 'js/amp-validation-error-detail-toggle-compiled.js' ), + array(), + AMP__VERSION, + true + ); wp_localize_script( - 'amp-admin-tables', - 'ampAdminTables', + 'amp-validation-error-detail-toggle', + 'ampValidationI18n', array( + 'btnAriaLabel' => esc_attr__( 'Toggle all sources', 'amp' ), 'errorIndexLink' => get_admin_url( null, 'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_invalid_url' ), 'errorIndexAnchor' => esc_html__( 'View Error Index', 'amp' ), ) @@ -586,7 +594,7 @@ public static function add_post_columns( $columns ) { array( AMP_Validation_Error_Taxonomy::ERROR_STATUS => sprintf( '%s', esc_html__( 'Status', 'amp' ) ), // @todo Create actual tooltip. AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => esc_html__( 'Invalid', 'amp' ), - AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => sprintf( '%s
    ', esc_html__( 'Sources', 'amp' ) ), + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => esc_html__( 'Sources', 'amp' ), ) ); @@ -659,7 +667,7 @@ public static function output_custom_column( $column_name, $post_id ) { $output = array(); if ( isset( $sources['plugin'] ) ) { - $output[] = '
    '; + $output[] = '
    '; $plugin_names = array(); $plugin_slugs = array_unique( $sources['plugin'] ); foreach ( $plugin_slugs as $plugin_slug ) { @@ -670,33 +678,29 @@ public static function output_custom_column( $column_name, $post_id ) { $plugin_names[] = $plugin_slug; } } - $count = count( $plugin_slugs ); - $sources_container_classes = 'sources-container sources-plugins'; + $count = count( $plugin_slugs ); if ( 1 === $count ) { - $output[] = sprintf( '%s
    ', esc_html__( 'Plugin', 'amp' ) ); + $output[] = sprintf( '%s', esc_html__( 'Plugin', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); - $sources_container_classes .= ' collapsed'; + $output[] = sprintf( '%s (%d)', esc_html__( 'Plugins', 'amp' ), $count ); } - $output[] = '
    '; + $output[] = '
    '; $output[] = implode( '
    ', array_unique( $plugin_names ) ); $output[] = '
    '; - $output[] = '
    '; + $output[] = '
    '; } if ( isset( $sources['core'] ) ) { - $output[] = '
    '; - $count = count( array_unique( $sources['core'] ) ); - $sources_container_classes = 'sources-container sources-core'; + $output[] = '
    '; + $count = count( array_unique( $sources['core'] ) ); if ( 1 === $count ) { - $output[] = sprintf( '%s
    ', esc_html__( 'Other', 'amp' ) ); + $output[] = sprintf( '%s', esc_html__( 'Other', 'amp' ) ); } else { - $output[] = sprintf( '%s (%d)', esc_html__( 'Other', 'amp' ), $count ); - $sources_container_classes .= ' collapsed'; + $output[] = sprintf( '%s (%d)', esc_html__( 'Other', 'amp' ), $count ); } - $output[] = '
    '; + $output[] = '
    '; $output[] = implode( '
    ', array_unique( $sources['core'] ) ); $output[] = '
    '; - $output[] = '
    '; + $output[] = '
    '; } if ( isset( $sources['theme'] ) ) { $output[] = '
    '; From 56915dab8efae6b90db9894feb46756071ed1da2 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Fri, 14 Sep 2018 20:53:17 -0300 Subject: [PATCH 54/58] Implement workaround to fix JS not loading issue --- includes/validation/class-amp-validation-error-taxonomy.php | 2 +- webpack.config.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index e4b16028833..255be7a4ed6 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -678,7 +678,7 @@ public static function add_admin_hooks() { wp_enqueue_script( 'amp-validation-error-detail-toggle', amp_get_asset_url( 'js/amp-validation-error-detail-toggle-compiled.js' ), - array( 'wp-dom-ready' ), + array(), AMP__VERSION, true ); diff --git a/webpack.config.js b/webpack.config.js index 72b982ab5bb..85aded8d146 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -13,7 +13,6 @@ module.exports = { filename: '[name].js' }, externals: { - '@wordpress/dom-ready': 'wp.domReady', 'amp-validation-i18n': 'ampValidationI18n' }, devtool: 'cheap-eval-source-map', From 6488c058e744608c0b3adf176fff9808d0a5771e Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Sat, 15 Sep 2018 15:57:04 -0300 Subject: [PATCH 55/58] Update unit tests based on latest changes --- tests/validation/test-class-amp-invalid-url-post-type.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index 0c3e37a9c12..2805a832e3c 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -41,7 +41,7 @@ public function test_register() { $this->assertTrue( in_array( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, get_post_types(), true ) ); $this->assertEquals( array(), get_all_post_type_supports( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) ); $this->assertEquals( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, $amp_post_type->name ); - $this->assertEquals( 'Errors by URL', $amp_post_type->label ); + $this->assertEquals( 'Invalid URLs', $amp_post_type->label ); $this->assertEquals( false, $amp_post_type->public ); $this->assertTrue( $amp_post_type->show_ui ); $this->assertEquals( AMP_Options_Manager::OPTION_NAME, $amp_post_type->show_in_menu ); @@ -495,7 +495,7 @@ public function test_add_post_columns() { $initial_columns, array( AMP_Validation_Error_Taxonomy::ERROR_STATUS => 'Status', - AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Sources
    ', + AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT => 'Sources', AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES => 'Invalid', ) ), @@ -529,7 +529,7 @@ public function get_custom_columns() { return array( 'invalid_element' => array( AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT, - '
    Plugin
    amp
    ', + '
    Plugin
    AMP
    ', $errors, ), 'removed_attributes' => array( From 145a9efe79390fa15067730d0375596ab2797024 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Sat, 15 Sep 2018 16:31:58 -0300 Subject: [PATCH 56/58] Fix test listing amp plugin --- tests/validation/test-class-amp-invalid-url-post-type.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index 1f3dc84c87f..a36d961791e 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -534,7 +534,7 @@ public function get_custom_columns() { return array( 'invalid_element' => array( AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT, - '
    Plugin
    AMP
    ', + '
    Plugin
    amp
    ', $errors, ), 'removed_attributes' => array( From 6624e68e1e8eb230a4f4824b566982cee6287285 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Sat, 15 Sep 2018 16:44:28 -0300 Subject: [PATCH 57/58] Fix unit tests --- tests/validation/test-class-amp-invalid-url-post-type.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php index a36d961791e..406cb720e3a 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -534,7 +534,7 @@ public function get_custom_columns() { return array( 'invalid_element' => array( AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT, - '
    Plugin
    amp
    ', + '
    Plugin
    AMP
    ', $errors, ), 'removed_attributes' => array( @@ -544,7 +544,7 @@ public function get_custom_columns() { ), 'sources_invalid_input' => array( AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT, - 'amp', + 'AMP', $errors, ), ); From 716a869e077ba916131bf9b2b99000e0884131e5 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 15 Sep 2018 22:31:07 -0700 Subject: [PATCH 58/58] Apply remaining changes from code review * Output theme name if available instead of theme slug. * Fix logic for obtaining plugin name when plugin is not in directory or plugin file doesn't use slug. * Use innerText instead of innerHTML for security hardening. * Better encapsulate logic in \AMP_Validation_Error_Taxonomy::render_link_to_errors_by_url(). * Add missing @wordpress/dom-ready dependency to fix workaround 56915da described in https://github.com/Automattic/amp-wp/pull/1397#issuecomment-421513289 * Update button from "View errors by URL" to "View Invalid URLs". --- .../src/amp-validation-error-detail-toggle.js | 3 ++- .../class-amp-invalid-url-post-type.php | 21 ++++++++++++----- .../class-amp-validation-error-taxonomy.php | 14 +++++++---- package-lock.json | 23 +++++++++++++++++++ package.json | 3 +++ ...st-class-amp-validation-error-taxonomy.php | 2 +- 6 files changed, 54 insertions(+), 12 deletions(-) diff --git a/assets/src/amp-validation-error-detail-toggle.js b/assets/src/amp-validation-error-detail-toggle.js index 0fd98bd690f..38fb0282d25 100644 --- a/assets/src/amp-validation-error-detail-toggle.js +++ b/assets/src/amp-validation-error-detail-toggle.js @@ -61,13 +61,14 @@ function addToggleListener() { } ); } +// @todo This should be harmonized with the approach in PHP via AMP_Validation_Error_Taxonomy::render_link_to_errors_by_url(). function addViewErrorsByTypeLinkButton() { if ( 'undefined' === typeof errorIndexAnchor || 'undefined' === typeof errorIndexLink ) { return; } const heading = document.querySelector( '.wp-heading-inline' ); const link = document.createElement( 'a' ); - link.innerHTML = errorIndexAnchor; + link.innerText = errorIndexAnchor; link.setAttribute( 'href', errorIndexLink ); link.setAttribute( 'class', 'page-title-action' ); heading.after( link ); diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 95029b09234..b18d0b9a470 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -672,13 +672,16 @@ public static function output_custom_column( $column_name, $post_id ) { $output[] = '
    '; $plugin_names = array(); $plugin_slugs = array_unique( $sources['plugin'] ); + $plugins = get_plugins(); foreach ( $plugin_slugs as $plugin_slug ) { - $plugin_data = get_plugin_data( WP_PLUGIN_DIR . '/' . $plugin_slug . '/' . $plugin_slug . '.php' ); - if ( ! empty( $plugin_data ) && ! empty( $plugin_data['Name'] ) ) { - $plugin_names[] = $plugin_data['Name']; - } else { - $plugin_names[] = $plugin_slug; + $name = $plugin_slug; + foreach ( $plugins as $plugin_file => $plugin_data ) { + if ( strtok( $plugin_file, '/' ) === $plugin_slug ) { + $name = $plugin_data['Name']; + break; + } } + $plugin_names[] = $name; } $count = count( $plugin_slugs ); if ( 1 === $count ) { @@ -709,7 +712,13 @@ public static function output_custom_column( $column_name, $post_id ) { $output[] = ''; $themes = array_unique( $sources['theme'] ); foreach ( $themes as $theme_slug ) { - $output[] = sprintf( '%s
    ', esc_html( $theme_slug ) ); + $theme_obj = wp_get_theme( $theme_slug ); + if ( ! $theme_obj->errors() ) { + $theme_name = $theme_obj->get( 'Name' ); + } else { + $theme_name = $theme_slug; + } + $output[] = sprintf( '%s
    ', esc_html( $theme_name ) ); } $output[] = '
    '; } diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 255be7a4ed6..8925eaaf6f0 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -901,9 +901,6 @@ public static function render_taxonomy_filters( $taxonomy_name ) { $( function() { // Move the filter UI after the 'Bulk Actions'