Improve URL Listing view #1397
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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' => __( '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, | ||
|
|
@@ -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() { | ||
|
There was a problem hiding this comment. See also #1414 where an There was a problem hiding this comment. Can this name be made more specific? There is |
||
| // Styles. | ||
| wp_enqueue_style( | ||
|
There was a problem hiding this comment. This should only be enqueued for the post list screen for this specific post type |
||
| 'amp-admin-tables', | ||
| amp_get_asset_url( 'css/admin-tables.css' ), | ||
|
There was a problem hiding this comment. Was this CSS file not committed? |
||
| 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[] = sprintf( | ||
| /* translators: %s is count */ | ||
| __( '<span class="identified">%1$s: %2$s', 'amp' ), | ||
| esc_html__( 'Identified', 'amp' ), | ||
|
There was a problem hiding this comment. This can move back to “New” |
||
| number_format_i18n( $counts['new'] ) | ||
| ); | ||
| } | ||
| if ( $counts['accepted'] ) { | ||
| $result[] = sprintf( | ||
| /* translators: 1. Title, 2. %s is count */ | ||
| __( '<span class="suppressed">%1$s: %2$s</span>', 'amp' ), | ||
| esc_html__( 'Suppressed', 'amp' ), | ||
| number_format_i18n( $counts['accepted'] ) | ||
| ); | ||
| } | ||
| if ( $counts['rejected'] ) { | ||
| $result[] = sprintf( | ||
| /* translators: %s is count */ | ||
| __( '<span class="rejected">%1$s: %2$s</span>', 'amp' ), | ||
| esc_html__( 'To Fix Later', 'amp' ), | ||
| number_format_i18n( $counts['rejected'] ) | ||
| ); | ||
| } | ||
| echo implode( '<br>', $result ); // WPCS: xss ok. | ||
| } | ||
|
|
@@ -554,8 +573,8 @@ public static function filter_views_edit( $views ) { | |
| sprintf( | ||
| /* translators: %s: the post count. */ | ||
| _nx( | ||
| 'With Identified Errors <span class="count">(%s)</span>', | ||
| 'With Identified Errors <span class="count">(%s)</span>', | ||
| $with_new_query->found_posts, | ||
| 'posts', | ||
| 'amp' | ||
|
|
@@ -577,8 +596,8 @@ public static function filter_views_edit( $views ) { | |
| sprintf( | ||
| /* translators: %s: the post count. */ | ||
| _nx( | ||
| 'With To Fix Later Errors <span class="count">(%s)</span>', | ||
| 'With To Fix Later Errors <span class="count">(%s)</span>', | ||
| $with_rejected_query->found_posts, | ||
| 'posts', | ||
| 'amp' | ||
|
|
@@ -600,8 +619,8 @@ public static function filter_views_edit( $views ) { | |
| sprintf( | ||
| /* translators: %s: the post count. */ | ||
| _nx( | ||
| 'With Suppressed Errors <span class="count">(%s)</span>', | ||
| 'With Suppressed Errors <span class="count">(%s)</span>', | ||
| $with_accepted_query->found_posts, | ||
| 'posts', | ||
| 'amp' | ||
|
|
@@ -623,18 +642,20 @@ public static function add_post_columns( $columns ) { | |
| $columns = array_merge( | ||
| $columns, | ||
| array( | ||
| 'error_status' => sprintf( '%s<span class="icon-help"></span><div class="tooltip">%s</div>', 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' ); | ||
| } | ||
|
|
||
| // Move date to end. | ||
| if ( isset( $columns['date'] ) ) { | ||
| unset( $columns['date'] ); | ||
| $columns['date'] = esc_html__( 'Last Checked', 'amp' ); | ||
| } | ||
|
|
||
| return $columns; | ||
|
|
@@ -664,45 +685,60 @@ public static function output_custom_column( $column_name, $post_id ) { | |
| } | ||
| self::display_invalid_url_validation_error_counts_summary( $post_id ); | ||
| break; | ||
| case AMP_Validation_Error_Taxonomy::FOUND_ELEMENTS_AND_ATTRIBUTES: | ||
| $items = array(); | ||
| 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( '<span>%s</span>', esc_html( $name ) ); | ||
| } else { | ||
| $items[] = sprintf( '<span>%s</span> (%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( '<span>%s</span>', esc_html( $name ) ); | ||
| } else { | ||
| $items[] = sprintf( '<span>%s</span> (%d)', esc_html( $name ), $count ); | ||
| } | ||
| } | ||
| } | ||
| if ( ! empty( $items ) ) { | ||
| echo implode( ',<br/>', $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 = $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( '<strong>%s</strong>', esc_html( 'Plug-In', 'amp' ) ); | ||
| } else { | ||
| $output[] = sprintf( '<strong>%s (%d)</strong>', esc_html( 'Plug-In', 'amp' ), $count ); | ||
| } | ||
| $output[] = implode( '<br/>', array_unique( $sources['plugin'] ) ); | ||
|
There was a problem hiding this comment. This should obtain the actual names of the plugins using |
||
| } | ||
| if ( isset( $sources['core'] ) ) { | ||
| $count = count( array_unique( $sources['core'] ) ); | ||
| if ( 1 === $count ) { | ||
| $output[] = sprintf( '<strong>%s</strong>', esc_html( 'Other', 'amp' ) ); | ||
| } else { | ||
| $output[] = sprintf( '<strong>%s (%d)</strong>', esc_html( 'Other', 'amp' ), $count ); | ||
| } | ||
| $output[] = implode( '<br/>', array_unique( $sources['core'] ) ); | ||
| } | ||
| if ( isset( $soures['theme'] ) ) { | ||
| $output[] = sprintf( '<strong>%s</strong>', esc_html( $soures['theme']['name'] ) ); | ||
| } | ||
| echo implode( '<br/>', $output ); // WPCS: XSS ok. | ||
| } | ||
| break; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
| * | ||
|
|
@@ -439,10 +446,12 @@ public static function summarize_validation_errors( $validation_errors ) { | |
| } | ||
|
|
||
| if ( ! empty( $validation_error['sources'] ) ) { | ||
| $sources = array_shift( $validation_error['sources'] ); | ||
|
|
||
| foreach ( $sources['sources'] as $source ) { | ||
|
There was a problem hiding this comment. ^ Changed this since the sources were not properly displaying in the table -- the Meaning that the However, this change broke the tests since the input data of test is assuming that a validation error looks like this instead: Is the structure of validation error incorrect for some reason and it should be as assumed in the tests? Maybe the fix should be removing the second nested There was a problem hiding this comment. Hi @miina, It looks like there aren't double-nested
Maybe the validation error data model changed. It might help to delete all validation error posts and terms, and regenerate them (thanks to Weston, though I might not have copied it right): wp post delete $(wp post list --post_type='amp_invalid_url' --format=ids)
wp term list amp_validation_error --field=term_id | xargs wp term delete amp_validation_errorYou might already have some logic to output validation errors, but here's a simple plugin I've used (also partially copied from Weston) And the WP-CLI script can then regenerate errors: wp amp validate-site --limit=5 |
||
| if ( isset( $source['type'], $source['name'] ) ) { | ||
| $invalid_sources[ $source['type'] ][] = $source['name']; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change these to “Invalid URLs” throughout. As part of this, we can remove code related to bolding the “URLs” string.