From 2d57fcc41541182b11e4d77eed7324dd07d1c94d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 11:14:34 -0700 Subject: [PATCH 01/17] Linkify URLs in attribute table --- .../class-amp-validation-error-taxonomy.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 39147fdaaff..3e6171ec1be 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -2160,8 +2160,19 @@ public static function render_single_url_error_details( $validation_error, $term printf( '%2$s', esc_attr( $attr_name_class ), esc_html( $attr_name ) ); echo ''; if ( ! empty( $attr_value ) ) { - printf( '%s', esc_html( $attr_value ) ); + echo ''; + $is_url = in_array( $attr_name, [ 'href', 'src' ], true ); + if ( $is_url ) { + // @todo There should be a link to the file editor as well, if available. + printf( '', esc_url( $attr_value ) ); + } + echo esc_html( $attr_value ); + if ( $is_url ) { + echo ''; + } + echo ''; } + echo ''; ?> From bee41ad14835923869d4b85e38356c9d958caf83 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 11:21:50 -0700 Subject: [PATCH 02/17] Align styling of rows for validated URLs and validation errors with comments --- .../css/src/amp-validation-error-taxonomy.css | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index 3fbd639e887..c7213c7232f 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -245,28 +245,34 @@ dl.detailed { margin-left: 10px; } -body.taxonomy-amp_validation_error .wp-list-table .new th, -body.taxonomy-amp_validation_error .wp-list-table .new td, +/*body.taxonomy-amp_validation_error .wp-list-table .new th,*/ +/*body.taxonomy-amp_validation_error .wp-list-table .new td,*/ tr.expanded.new + tr > td:first-of-type, -body.post-type-amp_validated_url .wp-list-table .new th, -body.post-type-amp_validated_url .wp-list-table .new td { +.wp-list-table .new td, +.wp-list-table .new th { background-color: #fef7f1; } -body.taxonomy-amp_validation_error .wp-list-table .new.odd th, -body.taxonomy-amp_validation_error .wp-list-table .new.odd td, -tr.expanded.new.odd + tr > td:first-of-type, -body.post-type-amp_validated_url .wp-list-table .new.odd th, -body.post-type-amp_validated_url .wp-list-table .new.odd td { - background-color: #f9eee5; -} - -body.taxonomy-amp_validation_error .wp-list-table .new th.check-column, tr.expanded.new + tr > td:first-of-type, -body.post-type-amp_validated_url .wp-list-table .new th.check-column { +.wp-list-table .new th.check-column { border-left: 4px solid #d54e21; } +.wp-list-table th, +.wp-list-table td { + box-shadow: inset 0 -1px 0 rgba(0, 0, 0, 0.1); +} + +.wp-list-table tr:last-child th, +.wp-list-table tr:last-child td { + box-shadow: none; +} + +.wp-list-table tr.unapproved + tr.approved th, +.wp-list-table tr.unapproved + tr.approved td { + border-top: 1px solid rgba(0, 0, 0, 0.03); +} + body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input { margin-left: 4px; } From 42bc426593bc4b1f4dce1cccdf0a117e379d566f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 11:22:02 -0700 Subject: [PATCH 03/17] Replace "Found URLs" with just "URLs" in validation error list table Also remove unused function arg --- assets/css/src/amp-validation-error-taxonomy.css | 4 ---- includes/validation/class-amp-validation-error-taxonomy.php | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index c7213c7232f..ad17b08881d 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -31,10 +31,6 @@ th.column-status { width: 15%; } -.fixed th.column-posts { - width: 10%; -} - /* Details column */ .column-details .details-attributes__summary { display: flex; diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 3e6171ec1be..04bb08fde11 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -842,7 +842,7 @@ static function( $old_columns ) { ), 'error_type' => esc_html__( 'Type', 'amp' ), 'created_date_gmt' => esc_html__( 'Last Seen', 'amp' ), - 'posts' => esc_html__( 'Found URLs', 'amp' ), + 'posts' => esc_html__( 'URLs', 'amp' ), ]; } ); @@ -2150,7 +2150,7 @@ public static function render_single_url_error_details( $validation_error, $term - + $attr_value ) : ?> From 116fbcd5ba71fe69505d573153296bf01994bf96 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 11:37:13 -0700 Subject: [PATCH 04/17] Improve styling of context details in error index list table --- assets/css/src/amp-validation-error-taxonomy.css | 5 +++-- includes/validation/class-amp-validation-error-taxonomy.php | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index ad17b08881d..f9af92487a7 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -108,7 +108,7 @@ dl.detailed { padding-bottom: 16px; } -.details-attributes__title code, +.details-attributes__title, .notice .detailed summary code { display: inline-block; min-width: 240px; @@ -116,8 +116,9 @@ dl.detailed { font-weight: 600; } -.details-attributes__title code { +.details-attributes__title { margin-left: 0; + font-weight: bold; } .details-attributes__list { diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 04bb08fde11..b3789824dcf 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -1939,11 +1939,11 @@ public static function filter_manage_custom_columns( $content, $column_name, $te $attributes = []; $attributes_heading = ''; if ( ! empty( $validation_error['node_attributes'] ) ) { - $attributes_heading = sprintf( '
%s
', esc_html__( 'Element attributes:', 'amp' ) ); + $attributes_heading = sprintf( '
%s:
', esc_html( self::get_source_key_label( 'node_attributes', $validation_error ) ) ); $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']; + $attributes_heading = sprintf( '
%s:
', esc_html( self::get_source_key_label( 'element_attributes', $validation_error ) ) ); } if ( empty( $attributes ) ) { @@ -1960,7 +1960,7 @@ public static function filter_manage_custom_columns( $content, $column_name, $te foreach ( $attributes as $attr => $value ) { $content .= sprintf( '
  • %s', esc_html( $attr ) ); - if ( isset( $value ) ) { + if ( ! empty( $value ) ) { $content .= sprintf( ': %s', esc_html( $value ) ); } From c493ced407f4b485b89a52de27ddf9b6b1aeea40 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 11:42:24 -0700 Subject: [PATCH 05/17] Fix positoning of expand/collapse all button Fixes #2078 --- assets/css/src/amp-validation-error-taxonomy.css | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index f9af92487a7..ada4e812055 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -155,16 +155,12 @@ dl.detailed { flex-direction: column; height: 14px; padding: 0; - margin-top: 4px; + margin: 0; background: none; border: none; cursor: pointer; } -.error-details-toggle.is-open { - transform: rotate(180deg); -} - .column-details .error-details-toggle::before, .column-details .error-details-toggle::after { width: 12px; @@ -175,6 +171,11 @@ dl.detailed { content: ""; } +.error-details-toggle.is-open::before, +.error-details-toggle.is-open::after { + transform: rotate(180deg); +} + /* Status text icons */ .status-text { display: flex; From adc775232124b23910afb1682284cd5a8f118b75 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 11:52:07 -0700 Subject: [PATCH 06/17] Prevent validation error contents shifting left when approved --- .../css/src/amp-validation-error-taxonomy.css | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index ada4e812055..10f9f3718e4 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -243,31 +243,35 @@ dl.detailed { margin-left: 10px; } -/*body.taxonomy-amp_validation_error .wp-list-table .new th,*/ -/*body.taxonomy-amp_validation_error .wp-list-table .new td,*/ tr.expanded.new + tr > td:first-of-type, -.wp-list-table .new td, -.wp-list-table .new th { +.wp-list-table > tbody > .new > td, +.wp-list-table > tbody > .new > th { background-color: #fef7f1; } +tr.expanded + tr > td:first-of-type { + border-left: solid 4px transparent; +} + tr.expanded.new + tr > td:first-of-type, .wp-list-table .new th.check-column { border-left: 4px solid #d54e21; } -.wp-list-table th, -.wp-list-table td { +.wp-list-table > tbody > tr > th, +.wp-list-table > tbody > tr > td { box-shadow: inset 0 -1px 0 rgba(0, 0, 0, 0.1); } -.wp-list-table tr:last-child th, -.wp-list-table tr:last-child td { +.wp-list-table > tbody > tr:last-child > th, +.wp-list-table > tbody > tr.expanded > th, +.wp-list-table > tbody > tr:last-child > td, +.wp-list-table > tbody > tr.expanded > td { box-shadow: none; } -.wp-list-table tr.unapproved + tr.approved th, -.wp-list-table tr.unapproved + tr.approved td { +.wp-list-table > tbody > tr.unapproved + tr.approved th, +.wp-list-table > tbody > tr.unapproved + tr.approved td { border-top: 1px solid rgba(0, 0, 0, 0.03); } From 16b39ca8b377bb3b23d2b601c6602b4cceef0931 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 11:52:15 -0700 Subject: [PATCH 07/17] Make details summary have cursor:pointer --- assets/css/src/amp-validation-error-taxonomy.css | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index 10f9f3718e4..273c8f87e84 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -108,6 +108,14 @@ dl.detailed { padding-bottom: 16px; } +dl.detailed details > summary { + cursor: pointer; + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; +} + .details-attributes__title, .notice .detailed summary code { display: inline-block; From 6c9f2597686926eabb0e968d957dd887304ff5e9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 12:13:54 -0700 Subject: [PATCH 08/17] Ensure the new state is removed from rows when clicking Accept/Reject buttons --- .../amp-validated-url-post-edit-screen.js | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/assets/src/amp-validation/amp-validated-url-post-edit-screen.js b/assets/src/amp-validation/amp-validated-url-post-edit-screen.js index edb1807655a..139de5734e3 100644 --- a/assets/src/amp-validation/amp-validated-url-post-edit-screen.js +++ b/assets/src/amp-validation/amp-validated-url-post-edit-screen.js @@ -285,6 +285,22 @@ const updateSelectIcon = ( select ) => { } }; +/** + * Set the row status class. + * + * @param {HTMLSelectElement} select Select element. + */ +const setRowStatusClass = ( select ) => { + const acceptedValue = 3; + const rejectedValue = 2; + const status = parseInt( select.options[ select.selectedIndex ].value ); + const row = select.closest( 'tr' ); + + row.classList.toggle( 'new', isNaN( status ) ); + row.classList.toggle( 'accepted', acceptedValue === status ); + row.classList.toggle( 'rejected', rejectedValue === status ); +}; + /** * Handles a change in the error status, like from 'New' to 'Accepted'. * @@ -292,20 +308,10 @@ const updateSelectIcon = ( select ) => { * And sets this as the src of the status icon . */ const handleStatusChange = () => { - const setRowStatusClass = ( { row, select } ) => { - const acceptedValue = 3; - const rejectedValue = 2; - const status = parseInt( select.options[ select.selectedIndex ].value ); - - row.classList.toggle( 'new', isNaN( status ) ); - row.classList.toggle( 'accepted', acceptedValue === status ); - row.classList.toggle( 'rejected', rejectedValue === status ); - }; - - const onChange = ( { event, row, select } ) => { + const onChange = ( { event, select } ) => { if ( event.target.matches( 'select' ) ) { updateSelectIcon( event.target ); - setRowStatusClass( { row, select } ); + setRowStatusClass( select ); } }; @@ -313,7 +319,7 @@ const handleStatusChange = () => { const select = row.querySelector( '.amp-validation-error-status' ); if ( select ) { - setRowStatusClass( { row, select } ); + setRowStatusClass( /** @type {HTMLSelectElement} */ select ); select.addEventListener( 'change', ( event ) => { onChange( { event, row, select } ); } ); @@ -370,6 +376,7 @@ const handleBulkActions = () => { if ( select.closest( 'tr' ).querySelector( '.check-column input[type=checkbox]' ).checked ) { select.value = '3'; updateSelectIcon( select ); + setRowStatusClass( select ); addBeforeUnloadPrompt(); } } ); @@ -381,6 +388,7 @@ const handleBulkActions = () => { if ( select.closest( 'tr' ).querySelector( '.check-column input[type=checkbox]' ).checked ) { select.value = '2'; updateSelectIcon( select ); + setRowStatusClass( select ); addBeforeUnloadPrompt(); } } ); From 45328c43bc53893de65de660385df940b95c9ed1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 13:18:09 -0700 Subject: [PATCH 09/17] Remove underlines from links in validation errors screen --- .../validation/class-amp-validation-error-taxonomy.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index b3789824dcf..bc78a68a091 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -817,7 +817,7 @@ static function( $old_columns ) { return [ 'cb' => $old_columns['cb'], - 'error' => esc_html__( 'Error', 'amp' ), + 'error_code' => esc_html__( 'Error', 'amp' ), 'status' => sprintf( '%s', esc_html__( 'Status', 'amp' ), @@ -853,7 +853,7 @@ static function( $old_columns ) { static 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['error'] = AMP_Validation_Error_Taxonomy::VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR; + $sortable_columns['error_code'] = AMP_Validation_Error_Taxonomy::VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR; return $sortable_columns; } ); @@ -965,7 +965,7 @@ static function( $parent_file ) { 'list_table_primary_column', static function( $primary_column ) { if ( get_current_screen() && AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG === get_current_screen()->taxonomy ) { - $primary_column = 'error'; + $primary_column = 'error_code'; } return $primary_column; } @@ -1783,7 +1783,7 @@ public static function filter_manage_custom_columns( $content, $column_name, $te } switch ( $column_name ) { - case 'error': + case 'error_code': if ( 'post.php' === $pagenow ) { $content .= sprintf( '', + '', esc_attr__( 'Toggle error details', 'amp' ), esc_html__( 'Details', 'amp' ) ); @@ -1793,7 +1793,7 @@ public static function filter_manage_custom_columns( $content, $column_name, $te } else { $content .= '

    '; $content .= sprintf( - '%s', + '%s', admin_url( add_query_arg( [ @@ -1818,6 +1818,7 @@ public static function filter_manage_custom_columns( $content, $column_name, $te if ( 'post.php' === $pagenow ) { $content .= ''; } else { + $content .= ''; $content .= '

    '; } From f88bb27f292c3853e20318b69e3d3f80a61e5b31 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 14:10:29 -0700 Subject: [PATCH 11/17] Improve column widths --- .../src/amp-validation-single-error-url.css | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/assets/css/src/amp-validation-single-error-url.css b/assets/css/src/amp-validation-single-error-url.css index bcf326295b2..4aac6e9912b 100644 --- a/assets/css/src/amp-validation-single-error-url.css +++ b/assets/css/src/amp-validation-single-error-url.css @@ -96,21 +96,25 @@ table.striped > tbody > tr.even { margin-top: 0.5rem; } -/* Give enough width to prevent the widest column status, 'New Accepted,' from forcing the below the icon. */ +.wp-list-table th.column-status { + width: 150px; } /** Add space between list table and the filter and search box above it. */ From 3d6b9c074e9f710e630b42e551419a760fd4ebb0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 14:18:03 -0700 Subject: [PATCH 12/17] Display of Accept/Reject buttons on validation error details screen --- .../css/src/amp-validation-error-taxonomy.css | 20 ------------------- .../class-amp-validated-url-post-type.php | 16 +++++++-------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index 4ba22d9ad31..5d887a78488 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -304,26 +304,6 @@ body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input { color: #a00; } -.notice.accept-reject-error { - display: flex; - margin-bottom: 0; -} - -.notice.accept-reject-error > p { - display: inline-block; - font-size: 14px; - flex-grow: 10; - margin-right: 20px; -} - -.notice.accept-reject-error > .button { - display: inline-block; - margin: 5px 5px 0 5px; - padding: 0 26px 2px; - flex-grow: 1; - text-align: center; -} - .notice.accept-reject-error > .button.accept { /* @todo Add green colors */ diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 1fa2342aa8d..6aa53cf7461 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -1319,26 +1319,26 @@ public static function print_admin_notice() { if ( ! $sanitization['forced'] ) { echo '
    '; - if ( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS === $sanitization['term_status'] || AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS === $sanitization['term_status'] ) { + if ( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS === $sanitization['term_status'] ) { if ( amp_is_canonical() ) { $info = __( 'Rejecting an error means that any URL on which it occurs will not be served as AMP.', 'amp' ); } else { $info = __( 'Rejecting an error means that any URL on which it occurs will redirect to the non-AMP version.', 'amp' ); } printf( - '

    %s

    %s', + '

    %s

    %s

    ', esc_html__( 'Reject this validation error for all instances.', 'amp' ) . ' ' . esc_html( $info ), esc_url( $reject_all_url ), esc_html__( 'Reject', 'amp' ) ); - } elseif ( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS === $sanitization['term_status'] || AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_REJECTED_STATUS === $sanitization['term_status'] ) { + } elseif ( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_REJECTED_STATUS === $sanitization['term_status'] ) { if ( amp_is_canonical() ) { $info = __( 'Accepting all validation errors which occur on a URL will allow it to be served as AMP.', 'amp' ); } else { $info = __( 'Accepting all validation errors which occur on a URL will allow it to be served as AMP.', 'amp' ); } printf( - '

    %s

    %s', + '

    %s

    %s

    ', esc_html__( 'Accept this error for all instances.', 'amp' ) . ' ' . esc_html( $info ), esc_url( $accept_all_url ), esc_html__( 'Accept', 'amp' ) @@ -1350,12 +1350,12 @@ public static function print_admin_notice() { $info = __( 'Rejecting an error means that any URL on which it occurs will redirect to the non-AMP version. If all errors occurring on a URL are accepted, then it will not redirect.', 'amp' ); } printf( - '

    %s

    %s%s', + '

    %s

    %s %s

    ', esc_html__( 'Accept or Reject this error for all instances.', 'amp' ) . ' ' . esc_html( $info ), - esc_url( $reject_all_url ), - esc_html__( 'Reject', 'amp' ), esc_url( $accept_all_url ), - esc_html__( 'Accept', 'amp' ) + esc_html__( 'Accept', 'amp' ), + esc_url( $reject_all_url ), + esc_html__( 'Reject', 'amp' ) ); } echo '
    '; From 5dd98b91a9254b5d677c079694b844ba2ca5a3d8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 14:28:05 -0700 Subject: [PATCH 13/17] Color the 'Showing X of Y validation errors' text as black since not link --- assets/css/src/amp-validation-single-error-url.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/css/src/amp-validation-single-error-url.css b/assets/css/src/amp-validation-single-error-url.css index 4aac6e9912b..74a9512dd36 100644 --- a/assets/css/src/amp-validation-single-error-url.css +++ b/assets/css/src/amp-validation-single-error-url.css @@ -75,7 +75,7 @@ table.striped > tbody > tr.even { #number-errors { text-align: center; background-color: #d3d3d3b8; - color: #1e8cbecc; + color: black; } #url-post-filter { From 831031fd46ade6f0fabebf823434fd463cf9a8a4 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 14:29:44 -0700 Subject: [PATCH 14/17] Rename 'possible sources' to 'source stack' --- includes/validation/class-amp-validation-error-taxonomy.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index cdf7191991d..c1a95a76d7c 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -2407,8 +2407,8 @@ private static function render_sources( $sources ) { sprintf( /* translators: %s: number of sources. */ _n( - '%s possible source', - '%s possible sources', + 'Source stack (%s)', + 'Source stack (%s)', $source_count, 'amp' ), From f61b82e59ae55ee119cf4797cb5a73c2c64f2bbf Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 22 Oct 2019 15:54:43 -0700 Subject: [PATCH 15/17] Refactor sources list from ol to table so columns align --- .../css/src/amp-validation-error-taxonomy.css | 29 +- .../src/amp-validation-single-error-url.css | 2 +- .../class-amp-validation-error-taxonomy.php | 280 +++++++++--------- 3 files changed, 163 insertions(+), 148 deletions(-) diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index 5d887a78488..d92fbd3f7b1 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -329,23 +329,32 @@ body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input { font-size: 1rem; } -.validation-error-sources th { - font-weight: bold; - text-align: right; +.validation-error-sources { + border-collapse: collapse; +} + +.validation-error-sources tbody:not(:first-child) { + border-top: solid 1px #ddd; + margin: 0; } .validation-error-sources td, .validation-error-sources th { vertical-align: top; - padding: 0 4px; + padding: 2px 4px; } -.validation-error-sources > li { - border-top: solid 1px #ddd; - margin: 0; - padding: 6px 0; +.validation-error-sources tbody > tr:first-child > th, +.validation-error-sources tbody > tr:first-child > td { + padding-top: 0.75em; +} + +.validation-error-sources tbody > tr:last-child > th, +.validation-error-sources tbody > tr:last-child > td { + padding-bottom: 0.75em; } -.validation-error-sources > li:first-child { - border-top: 0; +.validation-error-sources th { + font-weight: bold; + text-align: right; } diff --git a/assets/css/src/amp-validation-single-error-url.css b/assets/css/src/amp-validation-single-error-url.css index 74a9512dd36..527f3270c29 100644 --- a/assets/css/src/amp-validation-single-error-url.css +++ b/assets/css/src/amp-validation-single-error-url.css @@ -75,7 +75,7 @@ table.striped > tbody > tr.even { #number-errors { text-align: center; background-color: #d3d3d3b8; - color: black; + color: #000; } #url-post-filter { diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index c1a95a76d7c..d40aad6a55a 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -2417,178 +2417,184 @@ private static function render_sources( $sources ) { ); ?> -
      - +
    • + $source ) : ?> $source['file'] . ':' . $source['line'], + 'link_url' => self::get_file_editor_url( $source ), + ]; } $is_filter = ! empty( $source['filter'] ); - unset( $source['filter'] ); + unset( $source_table_rows['filter'] ); $dependency_type = null; if ( isset( $source['dependency_type'] ) ) { $dependency_type = $source['dependency_type']; - unset( $source['dependency_type'] ); + unset( $source_table_rows['dependency_type'] ); } $priority = null; if ( isset( $source['priority'] ) ) { $priority = $source['priority']; - unset( $source['priority'] ); + unset( $source_table_rows['priority'] ); } + $row_span = count( $source_table_rows ); ?> -
  • -
    • - $value ) : ?> - - + $key ) : ?> + + + + + + + - - - - - - - - -
    +
    + # + + + + + + '; + esc_html_e( 'Theme', 'amp' ); break; - case 'block_name': - esc_html_e( 'Block Name', 'amp' ); + case 'plugin': + echo ' '; + esc_html_e( 'Plugin', 'amp' ); break; - case 'shortcode': - esc_html_e( 'Shortcode', 'amp' ); + case 'mu-plugin': + echo ' '; + esc_html_e( 'Must-Use Plugin', 'amp' ); break; - case 'type': - esc_html_e( 'Type', 'amp' ); - break; - case 'function': - esc_html_e( 'Function', 'amp' ); - break; - case 'sources': - esc_html_e( 'Sources', 'amp' ); - break; - case 'hook': - if ( $is_filter ) { - esc_html_e( 'Filter', 'amp' ); - } else { - esc_html_e( 'Action', 'amp' ); - } + case 'core': + echo ' '; + esc_html_e( 'Core', 'amp' ); break; default: - echo esc_html( $key ); + echo esc_html( (string) $value ); } - echo ':'; ?> - - - - - - '; - esc_html_e( 'Theme', 'amp' ); - break; - case 'plugin': - echo ' '; - esc_html_e( 'Plugin', 'amp' ); - break; - case 'mu-plugin': - echo ' '; - esc_html_e( 'Must-Use Plugin', 'amp' ); - break; - case 'core': - echo ' '; - esc_html_e( 'Core', 'amp' ); - break; - default: - echo esc_html( (string) $value ); - } - ?> - - - - - - - - - - - labels->singular_name ) ) { - echo esc_html( $post_type->labels->singular_name ); - printf( ' (%s)', esc_html( $value ) ); - } else { - printf( '%s', esc_html( $value ) ); - } - ?> - - - -
    - -
    - : - + + + + + + ', // Note that esc_attr() used instead of esc_url() to allow IDE protocols. - esc_attr( $edit_url ), + esc_attr( $value['link_url'] ), // Open link in new window unless the user has filtered the URL to open their system IDE. - in_array( wp_parse_url( $edit_url, PHP_URL_SCHEME ), [ 'http', 'https' ], true ) ? 'target="_blank"' : '' + in_array( wp_parse_url( $value['link_url'], PHP_URL_SCHEME ), [ 'http', 'https' ], true ) ? 'target="_blank"' : '' ); } ?> - - + + -
    - + + + + + + labels->singular_name ) ) { + echo esc_html( $post_type->labels->singular_name ); + printf( ' (%s)', esc_html( $value ) ); + } else { + printf( '%s', esc_html( $value ) ); + } + ?> + + + +
    + + + + + - + + Date: Tue, 22 Oct 2019 16:14:29 -0700 Subject: [PATCH 16/17] Fix tests for changes to validation screens --- .../validation/test-class-amp-validated-url-post-type.php | 2 +- .../test-class-amp-validation-error-taxonomy.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 173465ab36a..98f195b9a16 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -556,7 +556,7 @@ public function test_add_single_post_columns() { $this->assertEquals( [ 'cb' => '', - 'error' => 'Error', + 'error_code' => 'Error', 'status' => 'Status', 'details' => 'Context', 'sources_with_invalid_output' => 'Sources', diff --git a/tests/php/validation/test-class-amp-validation-error-taxonomy.php b/tests/php/validation/test-class-amp-validation-error-taxonomy.php index 1c523fe71d7..0797ba1bcc9 100644 --- a/tests/php/validation/test-class-amp-validation-error-taxonomy.php +++ b/tests/php/validation/test-class-amp-validation-error-taxonomy.php @@ -583,7 +583,7 @@ public function test_add_admin_hooks() { array_keys( [ 'cb' => $cb, - 'error' => 'Error', + 'error_code' => 'Error', 'status' => 'Status

    Statuses tooltip title

    An accepted validation error is one that will not block a URL from being served as AMP; the validation error will be sanitized, normally resulting in the offending markup being stripped from the response to ensure AMP validity.

    ', 'details' => 'Details

    Details tooltip title

    An accepted validation error is one that will not block a URL from being served as AMP; the validation error will be sanitized, normally resulting in the offending markup being stripped from the response to ensure AMP validity.

    ', 'error_type' => 'Type', @@ -1111,7 +1111,7 @@ public function test_filter_manage_custom_columns() { // Test the 'error' block in the switch. $GLOBALS['pagenow'] = 'post.php'; - $filtered_content = AMP_Validation_Error_Taxonomy::filter_manage_custom_columns( $initial_content, 'error', $term_id ); + $filtered_content = AMP_Validation_Error_Taxonomy::filter_manage_custom_columns( $initial_content, 'error_code', $term_id ); $this->assertStringStartsWith( $initial_content . '