-
Notifications
You must be signed in to change notification settings - Fork 384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply new error taxonomy page design #1394
Conversation
Mainly using: https://sketch.cloud/s/PoWy8/all/page-1/amp-validation-errors Change the heading names of these, and change the date to be the time ago.
The conflict was in: test-class-amp-validation-error-taxonomy.php Retain both edits, just on different lines.
I've added several commits to this PR continuing @kienstra 's work for #1361. The main changes are:
Still to do:
|
public static function get_reader_friendly_error_type_text( $error_type ) { | ||
switch ( $error_type ) { | ||
case 'js_error': | ||
return esc_html__( 'Javascript', 'amp' ); |
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.
This spelling matches the design, but could it be JavaScript
instead?
When Nodes Have No Attribute Hi @johnwatkins0, This probably isn't a common case. But maybe if an To reproduce this, you could create a post with a 'Custom HTML' block with: <script>doThis();</script> |
Thanks, @kienstra!. I made those changes in my last round of commits. I'm sending this to code review now. All the AC in #1361 are covered with the exception of the tooltips on the "Status" and "Details" columns (which are potentially part of #1400 now). Note: in the CSS I make use of icons added in #1397, which hasn't been merged yet. I did add the |
* Prevent duplicating parent. * Use translated heading for attributes. * Prevent showing details if there are no attributes. * Prevent PHP notice when validation error lacks type.
@@ -620,32 +635,47 @@ public static function add_admin_hooks() { | |||
add_filter( 'manage_edit-' . self::TAXONOMY_SLUG . '_columns', function( $old_columns ) { | |||
return array( | |||
'cb' => $old_columns['cb'], | |||
'error' => __( 'Error', 'amp' ), | |||
'created_date_gmt' => __( 'Created Date', 'amp' ), | |||
'error' => __( 'Error Inventory', 'amp' ), |
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.
Actually I don't think this column name makes sense here. If we are going to use “Error Inventory” then this would be for the entire screen, not just for the one column. The column makes more sense as just “Error” I believe. I'll change it.
* Restore "Error" as column name instead of "Error Inventory". * Use just "Type" as opposed to "Error Type".
|
||
case AMP_Validation_Error_Taxonomy::VALIDATION_DETAILS_NODE_NAME_QUERY_VAR: | ||
$clauses['orderby'] = $wpdb->prepare( | ||
'ORDER BY SUBSTR(tt.description, LOCATE(%s, tt.description))', |
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.
@johnwatkins0 This is very clever!
Some other changes are applied in PR #1373, like adding the filters, and changing the page
<h1>
from 'AMP Validation Errors' to 'Errors by Type.'Closes #1361