On the single error term page, apply a similar UI to the single URL details #1449

kienstra · 2018-09-21T19:42:51Z

@johnwatkins0 had created a nice display of the details, which expands on clicking the 'Error' row on the single URL page:

Something similar to this should be applied to the single taxonomy term UI:

John had created a nice display of the details, which expands on clicking the 'Error' row on the single URL page. This begins to apply it to the single error term page.

kienstra · 2018-09-21T19:43:53Z

Request To Work On This

Hi @johnwatkins0,
Could you please work on this? Feel free to push to this branch.

westonruter · 2018-09-21T19:53:57Z

@kienstra I think this certainly looks better than the red/blue colors:

More alignment between the two styles is needed of course. Perhaps it should consistently be fixed font for keys and values?

kienstra · 2018-09-21T20:10:00Z

Hi @westonruter,

Perhaps it should consistently be fixed font for keys and values?

Good idea.

Is this what you had in mind?

westonruter · 2018-09-21T20:20:59Z

@kienstra Each value should be have a fixed-width font, and the values should be indented as well. No need for the braces or the quote marks. For the nested children, in that case the key can just be bold with the value not-bold. The top-level keys could have the gray background like you show above. Disclaimer: I'm not a designer.

johnwatkins0 · 2018-09-21T20:50:12Z

@westonruter @kienstra Here's how it looks with the commit just pushed:

johnwatkins0 · 2018-09-21T20:53:01Z

Based on @westonruter comment a little while ago, I'll change "text : javascript" to "text: javascript".

kienstra · 2018-09-21T20:54:13Z

Thanks, @johnwatkins0. This looks really good.

johnwatkins0 · 2018-09-21T21:06:47Z

@westonruter This is ready for your review.

kienstra · 2018-09-21T22:14:11Z

includes/validation/class-amp-validation-error-taxonomy.php

@@ -1903,8 +1903,15 @@ public static function render_single_url_error_details( $validation_error, $term
 						<div class="detailed">
 							<?php if ( is_string( $value ) ) : ?>
 								<?php echo esc_html( $value ); ?>
-							<?php else : ?>
-								<pre><?php echo esc_html( wp_json_encode( $value, 128 /* JSON_PRETTY_PRINT */ | 64 /* JSON_UNESCAPED_SLASHES */ ) ); ?></pre>
+							<?php elseif ( is_array( $value ) ) : ?>


This looked to have changed the display of the sources in the single error URL details:

Addressed with 5ebc340:

Thanks for catching that, @kienstra

All good, thanks a lot for applying this UI.

…single term page Before, sources were output with wp_json_encode(), using a pretty-print constant. Because they don't appear on the single term page, this commit shouldn't affect that. But restore the old means of outputting the sources for the single error URL page.

amedina · 2018-09-22T00:56:46Z

In the single error term page, the top box with the error details should be open by default

amedina · 2018-09-22T00:58:09Z

And perhaps the inner components should be open as well; the reason being that the user will go there specifically to look at the details of the errors and the URLs that have it

…pdate/single-error-details

westonruter · 2018-09-22T01:48:09Z

For the details on the removed, why are the angle brackets removed? It looks truncated. Also, shouldn't it be monospace?

And the node attributes is mostly just a duplicate of this. There should be a vertical list of attributes instead.

And all except sources should probably be expanded by default.

westonruter · 2018-09-22T01:50:16Z

Need to figure out why the sources are being duplicated for each row. I'll fix this.

westonruter · 2018-09-22T01:58:37Z

The order of the errors no longer corresponds to the document order. I'll look into that.

…mattic/amp-wp into update/single-error-details

johnwatkins0 · 2018-09-22T03:44:04Z

Just pushed changes for the following:

In the single error term page, the top box with the error details should be open by default
And perhaps the inner components should be open as well; the reason being that the user will go there specifically to look at the details of the errors and the URLs that have it
Also, shouldn't it be monospace?
And all except sources should probably be expanded by default.

@westonruter, on "For the details on the removed, why are the angle brackets removed?", that field is the attribute key and values concatenated to look like the removed attributes. I suppose the element could be prepended (with angle brackets) to make it look like an actual element, but I'm not comfortable enough with all the different conditions to make that happen.

westonruter · 2018-09-22T04:15:28Z

I'm changing the plugin/other sources to eliminate the details when there is only one item.

So going from:

To:

westonruter · 2018-09-22T04:18:58Z

The column checkboxes are erroneously including the tooltips:

… post type

… from screen options

westonruter · 2018-09-22T04:56:04Z

on "For the details on the removed, why are the angle brackets removed?", that field is the attribute key and values concatenated to look like the removed attributes. I suppose the element could be prepended (with angle brackets) to make it look like an actual element, but I'm not comfortable enough with all the different conditions to make that happen.

Here's what I'll push that uses a mark element. For an invalid element:

For an invalid attribute:

Begin to apply John's work on details to the single error taxonomy

67aaa93

John had created a nice display of the details, which expands on clicking the 'Error' row on the single URL page. This begins to apply it to the single error term page.

kienstra assigned johnwatkins0 Sep 21, 2018

Fix resetting All Error Types dropdown to first option

5d6a181

westonruter added this to the v1.0 milestone Sep 21, 2018

westonruter added the Compatability Tool label Sep 21, 2018

Single error term page - styling for details notice

4199f54

johnwatkins0 added 2 commits September 21, 2018 15:54

Single error term page - style fix for node_attributes field

723bb2d

Single error term page - PHPCS fixes

f23a736

johnwatkins0 changed the title ~~[WIP] On the single error term page, apply a similar UI to the single URL details~~ On the single error term page, apply a similar UI to the single URL details Sep 21, 2018

johnwatkins0 requested a review from westonruter September 21, 2018 21:07

kienstra commented Sep 21, 2018

View reviewed changes

Merge branch 'develop' of https://github.com/Automattic/amp-wp into u…

7ee322b

…pdate/single-error-details

westonruter and others added 4 commits September 21, 2018 20:12

Ensure terms on invalid URL screen are listed in order of occurrence

a9f5797

Error details style changes

aa291fd

Merge branch 'update/single-error-details' of https://github.com/Auto…

730e13d

…mattic/amp-wp into update/single-error-details

Fix sources being repeated for each validation error

ebae9a4

westonruter added 2 commits September 21, 2018 21:12

Eliminate showing source details when only one item

620a8aa

Show active theme as source when no sources found

3be5767

westonruter and others added 2 commits September 21, 2018 21:40

Prevent excerpts from being shown instead of list for amp_invalid_url…

7e54ccb

… post type

Validation tooltips - add content as data attribute so it is stripped…

ac3207e

… from screen options

amedina approved these changes Sep 22, 2018

View reviewed changes

westonruter and others added 2 commits September 21, 2018 21:57

Display invalid markup with marks to highlight problem

8c57fae

Details column style changes

552b72a

westonruter approved these changes Sep 22, 2018

View reviewed changes

westonruter merged commit 42dfafd into develop Sep 22, 2018

westonruter deleted the update/single-error-details branch September 22, 2018 05:23

westonruter mentioned this pull request Sep 25, 2018

Fix incorrect attribution of theme as source for content validation errors #1467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

On the single error term page, apply a similar UI to the single URL details #1449

On the single error term page, apply a similar UI to the single URL details #1449

kienstra commented Sep 21, 2018 •

edited

Loading

kienstra commented Sep 21, 2018 •

edited

Loading

westonruter commented Sep 21, 2018

kienstra commented Sep 21, 2018 •

edited

Loading

westonruter commented Sep 21, 2018

johnwatkins0 commented Sep 21, 2018

johnwatkins0 commented Sep 21, 2018

kienstra commented Sep 21, 2018

johnwatkins0 commented Sep 21, 2018

kienstra Sep 21, 2018 •

edited

Loading

kienstra Sep 21, 2018 •

edited

Loading

johnwatkins0 Sep 21, 2018

kienstra Sep 21, 2018

amedina commented Sep 22, 2018 •

edited by johnwatkins0

Loading

amedina commented Sep 22, 2018 •

edited by johnwatkins0

Loading

westonruter commented Sep 22, 2018 •

edited by johnwatkins0

Loading

westonruter commented Sep 22, 2018 •

edited

Loading

westonruter commented Sep 22, 2018 •

edited

Loading

johnwatkins0 commented Sep 22, 2018 •

edited

Loading

westonruter commented Sep 22, 2018

westonruter commented Sep 22, 2018 •

edited

Loading

westonruter commented Sep 22, 2018

On the single error term page, apply a similar UI to the single URL details #1449

On the single error term page, apply a similar UI to the single URL details #1449

Conversation

kienstra commented Sep 21, 2018 • edited Loading

kienstra commented Sep 21, 2018 • edited Loading

westonruter commented Sep 21, 2018

kienstra commented Sep 21, 2018 • edited Loading

westonruter commented Sep 21, 2018

johnwatkins0 commented Sep 21, 2018

johnwatkins0 commented Sep 21, 2018

kienstra commented Sep 21, 2018

johnwatkins0 commented Sep 21, 2018

kienstra Sep 21, 2018 • edited Loading

Choose a reason for hiding this comment

kienstra Sep 21, 2018 • edited Loading

Choose a reason for hiding this comment

johnwatkins0 Sep 21, 2018

Choose a reason for hiding this comment

kienstra Sep 21, 2018

Choose a reason for hiding this comment

amedina commented Sep 22, 2018 • edited by johnwatkins0 Loading

amedina commented Sep 22, 2018 • edited by johnwatkins0 Loading

westonruter commented Sep 22, 2018 • edited by johnwatkins0 Loading

westonruter commented Sep 22, 2018 • edited Loading

westonruter commented Sep 22, 2018 • edited Loading

johnwatkins0 commented Sep 22, 2018 • edited Loading

westonruter commented Sep 22, 2018

westonruter commented Sep 22, 2018 • edited Loading

westonruter commented Sep 22, 2018

kienstra commented Sep 21, 2018 •

edited

Loading

kienstra commented Sep 21, 2018 •

edited

Loading

kienstra commented Sep 21, 2018 •

edited

Loading

kienstra Sep 21, 2018 •

edited

Loading

kienstra Sep 21, 2018 •

edited

Loading

amedina commented Sep 22, 2018 •

edited by johnwatkins0

Loading

amedina commented Sep 22, 2018 •

edited by johnwatkins0

Loading

westonruter commented Sep 22, 2018 •

edited by johnwatkins0

Loading

westonruter commented Sep 22, 2018 •

edited

Loading

westonruter commented Sep 22, 2018 •

edited

Loading

johnwatkins0 commented Sep 22, 2018 •

edited

Loading

westonruter commented Sep 22, 2018 •

edited

Loading