diff --git a/assets/css/src/amp-validation-error-taxonomy.css b/assets/css/src/amp-validation-error-taxonomy.css index 3fbd639e887..3ff23a90a60 100644 --- a/assets/css/src/amp-validation-error-taxonomy.css +++ b/assets/css/src/amp-validation-error-taxonomy.css @@ -23,7 +23,7 @@ th.column-error_type { width: 15%; } -td.column-error .error-code { +td.column-error_code .error-code { font-family: Consolas, Monaco, monospace; } @@ -31,10 +31,6 @@ th.column-status { width: 15%; } -.fixed th.column-posts { - width: 10%; -} - /* Details column */ .column-details .details-attributes__summary { display: flex; @@ -47,6 +43,14 @@ details[open] .details-attributes__summary { margin-bottom: 15px; } +.details-attributes__summary { + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; + cursor: pointer; +} + .column-details .details-attributes__summary::-webkit-details-marker, .column-details .notice details > summary::-webkit-details-marker { display: none; @@ -64,7 +68,8 @@ details[open] .details-attributes__summary { } tr.expanded .details-attributes__summary::after, -details.single-error-detail[open] .single-error-detail-summary::after { +details[open] > .details-attributes__summary::after, +details[open] > .single-error-detail-summary::after { transform: rotate(180deg); } @@ -112,7 +117,15 @@ dl.detailed { padding-bottom: 16px; } -.details-attributes__title code, +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; min-width: 240px; @@ -120,8 +133,9 @@ dl.detailed { font-weight: 600; } -.details-attributes__title code { +.details-attributes__title { margin-left: 0; + font-weight: bold; } .details-attributes__list { @@ -158,26 +172,27 @@ 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; - height: 12px; + height: 10px; background-image: url("../images/down-triangle.svg"); background-size: cover; background-position: center; content: ""; } +.error-details-toggle.is-open::before, +.error-details-toggle.is-open::after { + transform: rotate(180deg); +} + /* Status text icons */ .status-text { display: flex; @@ -245,58 +260,48 @@ 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, -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 > tbody > .new > td, +.wp-list-table > tbody > .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; +tr.expanded + tr > td:first-of-type { + border-left: solid 4px transparent; } -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; } -body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input { - margin-left: 4px; +.wp-list-table > tbody > tr > th, +.wp-list-table > tbody > tr > td { + box-shadow: inset 0 -1px 0 rgba(0, 0, 0, 0.1); } -.row-actions .amp_validation_error_accept > a { - color: #006505; +.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; } -.row-actions .amp_validation_error_reject > a { - color: #a00; +.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); } -.notice.accept-reject-error { - display: flex; - margin-bottom: 0; +body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input { + margin-left: 4px; } -.notice.accept-reject-error > p { - display: inline-block; - font-size: 14px; - flex-grow: 10; - margin-right: 20px; +.row-actions .amp_validation_error_accept > a { + color: #006505; } -.notice.accept-reject-error > .button { - display: inline-block; - margin: 5px 5px 0 5px; - padding: 0 26px 2px; - flex-grow: 1; - text-align: center; +.row-actions .amp_validation_error_reject > a { + color: #a00; } .notice.accept-reject-error > .button.accept { @@ -324,35 +329,32 @@ body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input { font-size: 1rem; } -/** Details post action. */ -.details button { - display: inline-block; - background: none; - border: none; - padding: 0; - text-align: left; - color: #0073aa; - cursor: pointer; - text-decoration: underline; +.validation-error-sources { + border-collapse: collapse; } -.validation-error-sources th { - font-weight: bold; - text-align: right; +.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 f97f4472b41..527f3270c29 100644 --- a/assets/css/src/amp-validation-single-error-url.css +++ b/assets/css/src/amp-validation-single-error-url.css @@ -1,5 +1,5 @@ /** Arrow icon on title in error column. */ -.column-error > .single-url-detail-toggle { +.column-error_code > .single-url-detail-toggle { position: relative; width: 100%; padding: 5px 36px 5px 0; @@ -11,7 +11,7 @@ cursor: pointer; } -.column-error > .single-url-detail-toggle::after { +.column-error_code > .single-url-detail-toggle::after { position: absolute; top: 0; right: 0; @@ -46,18 +46,6 @@ table.striped > tbody > tr.even { display: none; } -/** Details post action. */ -.details button { - display: inline-block; - background: none; - border: none; - padding: 0; - text-align: left; - color: #0073aa; - cursor: pointer; - text-decoration: underline; -} - /** Details row styles. */ .details details.details-attributes:hover { cursor: pointer; @@ -87,7 +75,7 @@ table.striped > tbody > tr.even { #number-errors { text-align: center; background-color: #d3d3d3b8; - color: #1e8cbecc; + color: #000; } #url-post-filter { @@ -108,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. */ 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(); } } ); diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 3fe5d384bef..6aa53cf7461 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -933,7 +933,7 @@ public static function add_post_columns( $columns ) { public static function add_single_post_columns() { return [ 'cb' => '', - 'error' => __( 'Error', 'amp' ), + 'error_code' => __( 'Error', 'amp' ), 'error_type' => __( 'Type', 'amp' ), 'details' => sprintf( '%s', @@ -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 '
'; diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 39147fdaaff..d40aad6a55a 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' ), @@ -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' ), ]; } ); @@ -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; } @@ -1627,7 +1627,7 @@ public static function filter_tag_row_actions( $actions, WP_Term $tag ) { if ( 'post.php' === $pagenow ) { $actions['details'] = sprintf( - '', + '', esc_attr__( 'Toggle error details', 'amp' ), esc_html__( 'Details', 'amp' ) ); @@ -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( ''; } else { + $content .= ''; $content .= '

'; } @@ -1939,11 +1940,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 +1961,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 ) ); } @@ -2017,7 +2018,7 @@ public static function add_single_post_sortable_columns( $sortable_columns ) { return array_merge( $sortable_columns, [ - 'error' => self::VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR, + 'error_code' => self::VALIDATION_DETAILS_ERROR_CODE_QUERY_VAR, 'error_type' => self::VALIDATION_ERROR_TYPE_QUERY_VAR, ] ); @@ -2150,7 +2151,7 @@ public static function render_single_url_error_details( $validation_error, $term - + $attr_value ) : ?> @@ -2160,8 +2161,19 @@ public static function render_single_url_error_details( $validation_error, $term printf( '', esc_attr( $attr_name_class ), esc_html( $attr_name ) ); echo ''; ?> @@ -2395,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' ), @@ -2405,178 +2417,184 @@ private static function render_sources( $sources ) { ); ?> -
      - +
    %2$s'; 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 '
    + $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 'function': - esc_html_e( 'Function', 'amp' ); + case 'plugin': + echo ' '; + esc_html_e( 'Plugin', 'amp' ); break; - case 'sources': - esc_html_e( 'Sources', 'amp' ); + case 'mu-plugin': + echo ' '; + esc_html_e( 'Must-Use Plugin', '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 ) ); + } + ?> + + + +
    + + + + + - + + 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 . '