diff --git a/assets/css/amp-validation-error-taxonomy.css b/assets/css/amp-validation-error-taxonomy.css new file mode 100644 index 00000000000..91192865397 --- /dev/null +++ b/assets/css/amp-validation-error-taxonomy.css @@ -0,0 +1,138 @@ +#col-left { + display: none; +} +#col-right { + float: none; + width: auto; +} + +/* Improve column widths */ +td.column-details pre, +td.column-sources pre { + overflow: auto; +} + +th.column-created_date_gmt, +th.column-error_type { + width: 15%; +} + +th.column-status { + width: 10%; +} + +.fixed th.column-posts { + width: 10%; +} + +/* Details column */ +.details-attributes__summary { + display: flex; + justify-content: space-between; + align-items: center; +} + +details[open] .details-attributes__summary { + font-weight: 600; + margin-bottom: 15px; +} + +.details-attributes__summary::-webkit-details-marker { + display: none; +} + +.details-attributes__summary::after { + order: 99; + width: 12px; + height: 12px; + background-image: url("../images/down-triangle.svg"); + background-size: cover; + background-position: center; + content: ""; +} + +details[open] .details-attributes__summary::after { + transform: rotate(180deg); +} + +.details-attributes__title, +.details-attributes__attr { + color: #dc3232; +} + +.details-attributes__list { + margin-top: 0; + padding-left: 45px; + list-style-type: disc; +} + +.details-attributes__list li { + word-break: break-all; +} + +.details-attributes__value { + color: #00a0d2; +} + +/* Error details toggle button */ +.manage-column.column-details { + display: flex; + justify-content: space-between; + align-items: center; +} + +.error-details-toggle { + display: flex; + flex-direction: column; + height: 12px; + margin-right: 10px; + padding: 0; + background: none; + border: none; + cursor: pointer; +} + +.error-details-toggle.is-open { + transform: rotate(180deg); +} + +.error-details-toggle::before, +.error-details-toggle::after { + width: 12px; + height: 12px; + background-image: url("../images/down-triangle.svg"); + background-size: cover; + background-position: center; + content: ""; +} + +/* Status text icons */ +.status-text { + display: flex; + align-items: center; +} + +.status-text::before { + margin-right: 10px; + background-size: 20px 20px; + height: 20px; + width: 20px; + content: ""; +} + +.status-text.sanitized::before { + background-image: url( '../images/amp-logo-icon.svg' ); +} + +.status-text.new::before { + background-image: url( '../images/baseline-error.svg' ); +} + +.status-text.accepted::before { + background-image: url( '../images/baseline-check-circle-green.svg' ); +} + +.status-text.rejected::before { + background-image: url( '../images/baseline-error-blue.svg' ); +} + diff --git a/assets/images/baseline-check-circle-green.svg b/assets/images/baseline-check-circle-green.svg new file mode 100644 index 00000000000..7e015acf334 --- /dev/null +++ b/assets/images/baseline-check-circle-green.svg @@ -0,0 +1,4 @@ + + + + diff --git a/assets/images/down-triangle.svg b/assets/images/down-triangle.svg new file mode 100644 index 00000000000..0baeab7fc11 --- /dev/null +++ b/assets/images/down-triangle.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/src/amp-validation-error-detail-toggle.js b/assets/src/amp-validation-error-detail-toggle.js new file mode 100644 index 00000000000..85275c20b06 --- /dev/null +++ b/assets/src/amp-validation-error-detail-toggle.js @@ -0,0 +1,60 @@ +/** + * WordPress dependencies + */ +import domReady from '@wordpress/dom-ready'; + +/** + * Localized data + */ +import { btnAriaLabel } from 'amp-validation-i18n'; + +const OPEN_CLASS = 'is-open'; + +/** + * Adds detail toggle buttons to the header and footer rows of the validation error "details" column. + * The buttons are added via JS because there's no easy way to append them to the heading of a sortable + * table column via backend code. + */ +function addToggleButtons() { + [ ...document.querySelectorAll( 'th.column-details.manage-column' ) ].forEach( th => { + const button = document.createElement( 'button' ); + button.setAttribute( 'aria-label', btnAriaLabel ); + button.setAttribute( 'type', 'button' ); + button.setAttribute( 'class', 'error-details-toggle' ); + th.appendChild( button ); + } ); +} + +/** + * Adds a listener toggling all details in the error type taxonomy details column. + */ +function addToggleListener() { + let open = false; + + const details = [ ...document.querySelectorAll( '.column-details details' ) ]; + const toggleButtons = [ ...document.querySelectorAll( 'button.error-details-toggle' ) ]; + const onButtonClick = () => { + open = ! open; + toggleButtons.forEach( btn => { + btn.classList.toggle( OPEN_CLASS ); + } ); + details.forEach( detail => { + if ( open ) { + detail.setAttribute( 'open', true ); + } else { + detail.removeAttribute( 'open' ); + } + } ); + }; + + window.addEventListener( 'click', event => { + if ( toggleButtons.includes( event.target ) ) { + onButtonClick(); + } + } ); +} + +domReady( () => { + addToggleButtons(); + addToggleListener(); +} ); diff --git a/dev-lib b/dev-lib index 213ab6905ac..32430f45c03 160000 --- a/dev-lib +++ b/dev-lib @@ -1 +1 @@ -Subproject commit 213ab6905acef0d7655390f1487315c184f130af +Subproject commit 32430f45c03ce40b3755f7c2c0b03c8857154d59 diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index f6d2329aef7..9e617ecd745 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -32,6 +32,13 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer { */ const TREE_SHAKING_ERROR_CODE = 'removed_unused_css_rules'; + /** + * Error code for illegal at-rule. + * + * @var string + */ + const ILLEGAL_AT_RULE_ERROR_CODE = 'illegal_css_at_rule'; + /** * Inline style selector's specificity multiplier, i.e. used to generate the number of ':not(#_)' placeholders. * @@ -233,7 +240,7 @@ public static function get_css_parser_validation_error_codes() { return array( 'css_parse_error', 'excessive_css', - 'illegal_css_at_rule', + self::ILLEGAL_AT_RULE_ERROR_CODE, 'illegal_css_important', 'illegal_css_property', self::TREE_SHAKING_ERROR_CODE, @@ -1243,7 +1250,7 @@ private function process_css_list( CSSList $css_list, $options ) { } elseif ( $css_item instanceof AtRuleBlockList ) { if ( ! in_array( $css_item->atRuleName(), $options['allowed_at_rules'], true ) ) { $error = array( - 'code' => 'illegal_css_at_rule', + 'code' => self::ILLEGAL_AT_RULE_ERROR_CODE, 'at_rule' => $css_item->atRuleName(), 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); @@ -1264,7 +1271,7 @@ private function process_css_list( CSSList $css_list, $options ) { } elseif ( $css_item instanceof AtRuleSet ) { if ( ! in_array( $css_item->atRuleName(), $options['allowed_at_rules'], true ) ) { $error = array( - 'code' => 'illegal_css_at_rule', + 'code' => self::ILLEGAL_AT_RULE_ERROR_CODE, 'at_rule' => $css_item->atRuleName(), 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); @@ -1281,7 +1288,7 @@ private function process_css_list( CSSList $css_list, $options ) { } elseif ( $css_item instanceof KeyFrame ) { if ( ! in_array( 'keyframes', $options['allowed_at_rules'], true ) ) { $error = array( - 'code' => 'illegal_css_at_rule', + 'code' => self::ILLEGAL_AT_RULE_ERROR_CODE, 'at_rule' => $css_item->atRuleName(), 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); @@ -1297,7 +1304,7 @@ private function process_css_list( CSSList $css_list, $options ) { } } elseif ( $css_item instanceof AtRule ) { $error = array( - 'code' => 'illegal_css_at_rule', + 'code' => self::ILLEGAL_AT_RULE_ERROR_CODE, 'at_rule' => $css_item->atRuleName(), 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index f02c25ea56e..c46ce83c5b4 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -68,6 +68,20 @@ class AMP_Validation_Error_Taxonomy { */ const VALIDATION_ERROR_TYPE_QUERY_VAR = 'amp_validation_error_type'; + /** + * Query var used for ordering list by node name. + * + * @var string + */ + const VALIDATION_DETAILS_NODE_NAME_QUERY_VAR = 'amp_validation_node_name'; + + /** + * Query var used for ordering list by error code. + * + * @var string + */ + const VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR = 'amp_validation_code'; + /** * The value to not filter at all, like for 'All Statuses'. * @@ -183,13 +197,13 @@ public static function register() { register_taxonomy( self::TAXONOMY_SLUG, AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, array( 'labels' => array( - 'name' => _x( 'Errors by Type', 'taxonomy general name', 'amp' ), + 'name' => _x( 'AMP Validation Error Index', 'taxonomy general name', 'amp' ), 'singular_name' => _x( 'AMP Validation Error', 'taxonomy singular name', 'amp' ), 'search_items' => __( 'Search AMP Validation Errors', 'amp' ), 'all_items' => __( 'All AMP Validation Errors', 'amp' ), 'edit_item' => __( 'Edit AMP Validation Error', 'amp' ), 'update_item' => __( 'Update AMP Validation Error', 'amp' ), - 'menu_name' => __( 'Errors by Type', 'amp' ), + 'menu_name' => __( 'Error Index', 'amp' ), 'back_to_items' => __( 'Back to AMP Validation Errors', 'amp' ), 'popular_items' => __( 'Frequent Validation Errors', 'amp' ), 'view_item' => __( 'View Validation Error', 'amp' ), @@ -560,6 +574,7 @@ public static function add_admin_hooks() { add_filter( 'redirect_term_location', array( __CLASS__, 'add_term_filter_query_var' ), 10, 2 ); add_action( 'load-edit-tags.php', array( __CLASS__, 'add_group_terms_clauses_filter' ) ); add_action( 'load-edit-tags.php', array( __CLASS__, 'add_error_type_clauses_filter' ) ); + add_action( 'load-edit-tags.php', array( __CLASS__, 'add_order_clauses_from_description_json' ) ); add_action( sprintf( 'after-%s-table', self::TAXONOMY_SLUG ), array( __CLASS__, 'render_taxonomy_filters' ) ); add_action( sprintf( 'after-%s-table', self::TAXONOMY_SLUG ), array( __CLASS__, 'render_link_to_errors_by_url' ) ); add_action( 'load-edit-tags.php', function() { @@ -621,31 +636,46 @@ public static function add_admin_hooks() { return array( 'cb' => $old_columns['cb'], 'error' => __( 'Error', 'amp' ), - 'created_date_gmt' => __( 'Created Date', 'amp' ), 'status' => __( 'Status', 'amp' ), 'details' => __( 'Details', 'amp' ), - 'posts' => __( 'URLs', 'amp' ), + 'error_type' => __( 'Type', 'amp' ), + 'created_date_gmt' => __( 'Last Seen', 'amp' ), + 'posts' => __( 'Found URLs', 'amp' ), ); } ); // Let the created date column sort by term ID. add_filter( 'manage_edit-' . self::TAXONOMY_SLUG . '_sortable_columns', function( $sortable_columns ) { $sortable_columns['created_date_gmt'] = 'term_id'; + $sortable_columns['error_type'] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR; + $sortable_columns['details'] = AMP_Validation_Error_Taxonomy::VALIDATION_DETAILS_NODE_NAME_QUERY_VAR; + $sortable_columns['error'] = AMP_Validation_Error_Taxonomy::VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR; return $sortable_columns; } ); // Hide empty term addition form. add_action( 'admin_enqueue_scripts', function() { if ( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG === get_current_screen()->taxonomy ) { - wp_add_inline_style( 'common', ' - #col-left { display: none; } - #col-right { float:none; width: auto; } - - /* Improve column widths */ - td.column-details pre, td.column-sources pre { overflow:auto; } - th.column-created_date_gmt { width:15%; } - th.column-status { width:10%; } - ' ); + 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( 'wp-dom-ready' ), + AMP__VERSION, + true + ); + + wp_localize_script( + 'amp-validation-error-detail-toggle', + 'ampValidationI18n', + array( 'btnAriaLabel' => esc_attr__( 'Toggle all details', 'amp' ) ) + ); } } ); @@ -764,6 +794,7 @@ public static function add_error_type_clauses_filter() { if ( ! in_array( $type, self::get_error_types(), true ) ) { return; } + add_filter( 'terms_clauses', function( $clauses, $taxonomies ) use ( $type ) { global $wpdb; if ( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG === $taxonomies[0] ) { @@ -773,6 +804,62 @@ public static function add_error_type_clauses_filter() { }, 10, 2 ); } + /** + * If ordering the list by a field in the description JSON, locate the best spot in the JSON string by which to sort + * alphabetically. + */ + public static function add_order_clauses_from_description_json() { + if ( self::TAXONOMY_SLUG !== get_current_screen()->taxonomy ) { + return; + } + + $sortable_column_vars = array( + self::VALIDATION_ERROR_TYPE_QUERY_VAR, + self::VALIDATION_DETAILS_NODE_NAME_QUERY_VAR, + self::VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR, + ); + + if ( ! isset( $_GET['orderby'] ) || ! in_array( $_GET['orderby'], $sortable_column_vars, true ) ) { // WPCS: CSRF ok. + return; + } + + add_filter( 'terms_clauses', function( $clauses ) { + global $wpdb; + + if ( isset( $_GET['order'] ) && 'desc' === $_GET['order'] ) { // WPCS: CSRF ok. + $clauses['order'] = 'DESC'; + } else { + $clauses['order'] = 'ASC'; + } + + switch ( $_GET['orderby'] ) { // WPCS: CSRF ok. + case AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR: + $clauses['orderby'] = $wpdb->prepare( + 'ORDER BY SUBSTR(tt.description, LOCATE(%s, tt.description, LOCATE(%s, tt.description)))', + '"type":"', + '}' // Start substr search after the first closing bracket to skip the "type" nested in the element_attributes object. + ); + break; + + case AMP_Validation_Error_Taxonomy::VALIDATION_DETAILS_NODE_NAME_QUERY_VAR: + $clauses['orderby'] = $wpdb->prepare( + 'ORDER BY SUBSTR(tt.description, LOCATE(%s, tt.description))', + '"node_name":"' + ); + break; + + case AMP_Validation_Error_Taxonomy::VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR: + $clauses['orderby'] = $wpdb->prepare( + 'ORDER BY SUBSTR(tt.description, LOCATE(%s, tt.description))', + '"code":"' + ); + break; + } + + return $clauses; + }, 10, 2 ); + } + /** * Outputs the taxonomy filter UI for this taxonomy type. * @@ -812,7 +899,7 @@ public static function render_taxonomy_filters( $taxonomy_name ) { } /** - * On the 'Errors by Type' taxonomy page, renders a link to the 'Errors by URL' page. + * On the 'Error Index' taxonomy page, renders a link to the 'Errors by URL' page. * * @param string $taxonomy_name The name of the taxonomy. */ @@ -839,7 +926,7 @@ public static function render_link_to_errors_by_url( $taxonomy_name ) { * There is a difference how the errors are counted, depending on which screen this is on. * For example: Accepted Errors (10). * This status filter element is rendered on the validation error post page (Errors by URL), - * and the validation error taxonomy page (Errors by Type). + * and the validation error taxonomy page (Error Index). * On the taxonomy page, this simply needs to count the number of terms with a given type. * On the post page, this needs to count the number of posts that have at least one error of a given type. */ @@ -993,7 +1080,7 @@ public static function get_error_types() { * Renders the filter for error type. * * This type filter element is rendered on the validation error post page (Errors by URL), - * and the validation error taxonomy page (Errors by Type). + * and the validation error taxonomy page (Error Index). */ public static function render_error_type_filter() { $error_type_filter_value = isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_key( wp_unslash( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK. @@ -1143,6 +1230,15 @@ public static function filter_tag_row_actions( $actions, WP_Term $tag ) { */ unset( $actions['delete'] ); + $actions['details'] = sprintf( + '%s', + admin_url( add_query_arg( array( + self::TAXONOMY_SLUG => $term->name, + 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, + ), 'edit.php' ) ), + esc_html__( 'Details', 'amp' ) + ); + $sanitization = self::get_validation_error_sanitization( json_decode( $term->description, true ) ); if ( self::VALIDATION_ERROR_REJECTED_STATUS !== $sanitization['term_status'] ) { $actions[ self::VALIDATION_ERROR_REJECT_ACTION ] = sprintf( @@ -1174,7 +1270,7 @@ public static function filter_tag_row_actions( $actions, WP_Term $tag ) { * Show AMP validation errors under AMP admin menu. */ public static function add_admin_menu_validation_error_item() { - $menu_item_label = esc_html__( 'Errors by Type', 'amp' ); + $menu_item_label = esc_html__( 'Error Index', 'amp' ); $new_error_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_NEW_STATUS, ) ); @@ -1193,6 +1289,32 @@ public static function add_admin_menu_validation_error_item() { ); } + /** + * Provides a reader-friendly string for a term's error type. + * + * @param string $error_type The error type from the term's validation error JSON. + * @return string Reader-friendly string. + */ + public static function get_reader_friendly_error_type_text( $error_type ) { + switch ( $error_type ) { + case 'js_error': + return esc_html__( 'JavaScript', 'amp' ); + + case 'html_element_error': + return esc_html__( 'HTML (Element)', 'amp' ); + + case 'html_attribute_error': + return esc_html__( 'HTML (Attribute)', 'amp' ); + + case 'css_error': + return esc_html__( 'CSS', 'amp' ); + + default: + return $error_type; + } + + } + /** * Supply the content for the custom columns. * @@ -1212,9 +1334,20 @@ public static function filter_manage_custom_columns( $content, $column_name, $te switch ( $column_name ) { case 'error': $content .= ''; - $content .= sprintf( '%s', esc_html( $validation_error['code'] ) ); - if ( 'invalid_element' === $validation_error['code'] || 'invalid_attribute' === $validation_error['code'] ) { - $content .= sprintf( ': %s', esc_html( $validation_error['node_name'] ) ); + $content .= sprintf( + '%s', + admin_url( add_query_arg( array( + self::TAXONOMY_SLUG => $term->name, + 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, + ), 'edit.php' ) ), + esc_html( $validation_error['code'] ) + ); + if ( self::INVALID_ELEMENT_CODE === $validation_error['code'] ) { + $content .= sprintf( ': <%s>', esc_html( $validation_error['node_name'] ) ); + } elseif ( self::INVALID_ATTRIBUTE_CODE === $validation_error['code'] ) { + $content .= sprintf( ': [%s]', esc_html( $validation_error['node_name'] ) ); + } elseif ( 'illegal_css_at_rule' === $validation_error['code'] ) { + $content .= sprintf( ': @%s', esc_html( $validation_error['at_rule'] ) ); } $content .= ''; @@ -1226,21 +1359,23 @@ public static function filter_manage_custom_columns( $content, $column_name, $te $sanitization = self::get_validation_error_sanitization( $validation_error ); if ( self::VALIDATION_ERROR_ACCEPTED_STATUS === $sanitization['term_status'] ) { if ( $sanitization['forced'] && $sanitization['term_status'] !== $sanitization['status'] ) { - $content .= '🚩'; + $class = 'sanitized'; } else { - $content .= '✅'; + $class = 'accepted'; } - $content .= ' ' . esc_html__( 'Accepted', 'amp' ); + $text = __( 'Accepted', 'amp' ); } elseif ( self::VALIDATION_ERROR_REJECTED_STATUS === $sanitization['term_status'] ) { if ( $sanitization['forced'] && $sanitization['term_status'] !== $sanitization['status'] ) { - $content .= '🚩'; + $class = 'sanitized'; } else { - $content .= '❌'; + $class = 'rejected'; } - $content .= ' ' . esc_html__( 'Rejected', 'amp' ); + $text = __( 'Rejected', 'amp' ); } else { - $content = '❓ ' . esc_html__( 'New', 'amp' ); + $class = 'new'; + $text = __( 'New', 'amp' ); } + $content .= sprintf( '%s', esc_attr( $class ), esc_html( $text ) ); break; case 'created_date_gmt': $created_datetime = null; @@ -1261,15 +1396,13 @@ public static function filter_manage_custom_columns( $content, $column_name, $te } if ( ! $created_datetime ) { $time_ago = __( 'n/a', 'amp' ); - } elseif ( time() - $created_datetime->getTimestamp() < DAY_IN_SECONDS ) { + } else { $time_ago = sprintf( '%s', esc_attr( $created_datetime->format( __( 'Y/m/d g:i:s a', 'default' ) ) ), /* translators: %s is relative time */ esc_html( sprintf( __( '%s ago', 'default' ), human_time_diff( $created_datetime->getTimestamp() ) ) ) ); - } else { - $time_ago = mysql2date( __( 'Y/m/d g:i:s a', 'default' ), $created_date_gmt ); } if ( $created_datetime ) { @@ -1283,9 +1416,62 @@ public static function filter_manage_custom_columns( $content, $column_name, $te break; case 'details': - unset( $validation_error['code'] ); - unset( $validation_error['message'] ); - $content = sprintf( '%s', esc_html( wp_json_encode( $validation_error, 128 /* JSON_PRETTY_PRINT */ | 64 /* JSON_UNESCAPED_SLASHES */ ) ) ); + if ( isset( $validation_error['parent_name'] ) ) { + if ( self::INVALID_ATTRIBUTE_CODE === $validation_error['code'] || self::INVALID_ELEMENT_CODE === $validation_error['code'] ) { + $summary_label = sprintf( '<%s>', $validation_error['parent_name'] ); + } elseif ( isset( $validation_error['node_name'] ) ) { + $summary_label = sprintf( '<%s>', $validation_error['node_name'] ); + } elseif ( isset( $validation_error['node_name'] ) ) { + $summary_label = $validation_error['node_name']; + } else { + $summary_label = '…'; + } + + $summary = '' . esc_html( $summary_label ) . ''; + unset( $validation_error['error_type'] ); + unset( $validation_error['parent_name'] ); + + $attributes = array(); + $attributes_heading = ''; + if ( ! empty( $validation_error['node_attributes'] ) ) { + $attributes_heading = sprintf( '%s', esc_html__( 'Element attributes:', 'amp' ) ); + $attributes = $validation_error['node_attributes']; + } elseif ( ! empty( $validation_error['element_attributes'] ) ) { + $attributes_heading = sprintf( '%s', esc_html__( 'Other attributes:', 'amp' ) ); + $attributes = $validation_error['element_attributes']; + } + + if ( empty( $attributes ) ) { + $content .= $summary; + } else { + $content = ''; + $content .= ''; + $content .= $summary; + $content .= ''; + + $content .= $attributes_heading; + $content .= ''; + + foreach ( $attributes as $attr => $value ) { + $content .= sprintf( '%s', esc_html( $attr ) ); + + if ( isset( $value ) ) { + $content .= sprintf( ': %s', esc_html( $value ) ); + } + + $content .= ''; + } + + $content .= ''; + $content .= ''; + } + } + + break; + case 'error_type': + if ( isset( $validation_error['type'] ) ) { + $content = self::get_reader_friendly_error_type_text( $validation_error['type'] ); + } break; } return $content; 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 c66df64b74b..9f9e50644bd 100644 --- a/tests/validation/test-class-amp-invalid-url-post-type.php +++ b/tests/validation/test-class-amp-invalid-url-post-type.php @@ -1103,7 +1103,7 @@ public function test_render_post_filters() { } $new_error_count = sprintf( - 'New Errors (%d)', + 'With New Errors (%d)', $number_of_errors ); $correct_post_type = AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG; diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php index 70fa2d50992..c7e6774efc9 100644 --- a/tests/validation/test-class-amp-validation-error-taxonomy.php +++ b/tests/validation/test-class-amp-validation-error-taxonomy.php @@ -56,18 +56,18 @@ public function test_register() { $this->assertFalse( $taxonomy_object->hierarchical ); $this->assertTrue( $taxonomy_object->show_in_menu ); $this->assertFalse( $taxonomy_object->meta_box_cb ); - $this->assertEquals( 'Errors by Type', $taxonomy_object->label ); + $this->assertEquals( 'AMP Validation Error Index', $taxonomy_object->label ); $this->assertEquals( 'do_not_allow', $taxonomy_object->cap->assign_terms ); $this->assertEquals( array( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ), $taxonomy_object->object_type ); $labels = $taxonomy_object->labels; - $this->assertEquals( 'Errors by Type', $labels->name ); + $this->assertEquals( 'AMP Validation Error Index', $labels->name ); $this->assertEquals( 'AMP Validation Error', $labels->singular_name ); $this->assertEquals( 'Search AMP Validation Errors', $labels->search_items ); $this->assertEquals( 'All AMP Validation Errors', $labels->all_items ); $this->assertEquals( 'Edit AMP Validation Error', $labels->edit_item ); $this->assertEquals( 'Update AMP Validation Error', $labels->update_item ); - $this->assertEquals( 'Errors by Type', $labels->menu_name ); + $this->assertEquals( 'Error Index', $labels->menu_name ); $this->assertEquals( 'Back to AMP Validation Errors', $labels->back_to_items ); $this->assertEquals( 'Frequent Validation Errors', $labels->popular_items ); $this->assertEquals( 'View Validation Error', $labels->view_item ); @@ -373,6 +373,21 @@ public function test_add_admin_hooks() { $this->assertEquals( 10, has_filter( 'handle_bulk_actions-edit-' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'handle_validation_error_update' ) ) ); $this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'handle_inline_edit_request' ) ) ); + $cb = ''; + $initial_columns = array( 'cb' => $cb ); + $this->assertEquals( + array( + 'cb' => $cb, + 'error' => 'Error', + 'status' => 'Status', + 'details' => 'Details', + 'created_date_gmt' => 'Last Seen', + 'posts' => 'Found URLs', + 'error_type' => 'Type', + ), + apply_filters( 'manage_edit-' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_columns', $initial_columns ) // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores + ); + // Assert that the 'query_vars' callback adds these query vars. $this->assertEmpty( array_diff( array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR ), @@ -789,15 +804,28 @@ public function test_add_admin_menu_validation_error_item() { wp_set_current_user( $this->factory()->user->create( array( 'role' => 'administrator' ) ) ); AMP_Validation_Error_Taxonomy::add_admin_menu_validation_error_item(); $expected_submenu = array( - 'Errors by Type', + 'Error Index', 'manage_categories', 'edit-tags.php?taxonomy=amp_validation_error&post_type=amp_invalid_url', - 'Errors by Type', + 'Error Index', ); $amp_options = $submenu[ AMP_Options_Manager::OPTION_NAME ]; $this->assertEquals( $expected_submenu, end( $amp_options ) ); } + /** + * Test get_reader_friendly_error_type_text. + * + * @covers \AMP_Validation_Error_Taxonomy::get_reader_friendly_error_type_text() + */ + public function test_get_reader_friendly_error_type_text() { + $this->assertEquals( 'JavaScript', AMP_Validation_Error_Taxonomy::get_reader_friendly_error_type_text( 'js_error' ) ); + $this->assertEquals( 'HTML (Element)', AMP_Validation_Error_Taxonomy::get_reader_friendly_error_type_text( 'html_element_error' ) ); + $this->assertEquals( 'HTML (Attribute)', AMP_Validation_Error_Taxonomy::get_reader_friendly_error_type_text( 'html_attribute_error' ) ); + $this->assertEquals( 'CSS', AMP_Validation_Error_Taxonomy::get_reader_friendly_error_type_text( 'css_error' ) ); + $this->assertEquals( 'some_other_error', AMP_Validation_Error_Taxonomy::get_reader_friendly_error_type_text( 'some_other_error' ) ); + } + /** * Test filter_manage_custom_columns. * @@ -812,13 +840,19 @@ public function test_filter_manage_custom_columns() { 'description' => wp_json_encode( $validation_error ), ) ); + $term = get_term( $term_id, AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ); + + $url = admin_url( add_query_arg( array( + AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG => $term->name, + 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, + ), 'edit.php' ) ); // Test the 'error' block in the switch. $filtered_content = AMP_Validation_Error_Taxonomy::filter_manage_custom_columns( $initial_content, 'error', $term_id ); - $this->assertEquals( $initial_content . 'illegal_css_at_rule', $filtered_content ); + $this->assertEquals( $initial_content . 'illegal_css_at_rule: @-ms-viewport', $filtered_content ); // Test the 'status' block in the switch. $filtered_content = AMP_Validation_Error_Taxonomy::filter_manage_custom_columns( $initial_content, 'status', $term_id ); - $this->assertEquals( '❓ New', $filtered_content ); + $this->assertEquals( $initial_content . 'New', $filtered_content ); // Test the 'created_date_gmt' block in the switch. $date = current_time( 'mysql', true ); diff --git a/webpack.config.js b/webpack.config.js index dbb7200ac6d..72b982ab5bb 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -5,12 +5,17 @@ const path = require( 'path' ); module.exports = { entry: { './assets/js/amp-blocks-compiled': './blocks/index.js', - './assets/js/amp-block-editor-toggle-compiled': './assets/src/amp-block-editor-toggle.js' + './assets/js/amp-block-editor-toggle-compiled': './assets/src/amp-block-editor-toggle.js', + './assets/js/amp-validation-error-detail-toggle-compiled': './assets/src/amp-validation-error-detail-toggle.js' }, output: { path: path.resolve( __dirname ), filename: '[name].js' }, + externals: { + '@wordpress/dom-ready': 'wp.domReady', + 'amp-validation-i18n': 'ampValidationI18n' + }, devtool: 'cheap-eval-source-map', module: { rules: [
'; - $content .= sprintf( '%s', esc_html( $validation_error['code'] ) ); - if ( 'invalid_element' === $validation_error['code'] || 'invalid_attribute' === $validation_error['code'] ) { - $content .= sprintf( ': %s', esc_html( $validation_error['node_name'] ) ); + $content .= sprintf( + '%s', + admin_url( add_query_arg( array( + self::TAXONOMY_SLUG => $term->name, + 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, + ), 'edit.php' ) ), + esc_html( $validation_error['code'] ) + ); + if ( self::INVALID_ELEMENT_CODE === $validation_error['code'] ) { + $content .= sprintf( ': <%s>', esc_html( $validation_error['node_name'] ) ); + } elseif ( self::INVALID_ATTRIBUTE_CODE === $validation_error['code'] ) { + $content .= sprintf( ': [%s]', esc_html( $validation_error['node_name'] ) ); + } elseif ( 'illegal_css_at_rule' === $validation_error['code'] ) { + $content .= sprintf( ': @%s', esc_html( $validation_error['at_rule'] ) ); } $content .= '
%s
<%s>
[%s]
@%s
' . esc_html( $summary_label ) . '
illegal_css_at_rule
illegal_css_at_rule: @-ms-viewport
@-ms-viewport