Prevent ACK state of non-modified validation errors from being lost; update validation error icons

[pierlon]

Summary

This PR prevents the ACK state of other validation errors from being affected when modifying a single one. Also, it updates the validation error icons.

Fixes #3725
Fixes #3726

Todo

• Remove validation error count summary from "Validation Error" post metabox
• Update validation error icons with ones from Dashicons
  • Update icons in admin bar
  • Update icons on error index page
  • Update icons validated URLs page
  • Update icons on single validation error page
  • Update icons on single error index page
  • Update icons used in validation error stylesheet summary
  • Remove red and green AMP SVG icons
• Toggle 'new' state for validation error row upon approval/disapproval
• Change validation error status <select> color when toggling between error states
• Show a warning message in post metabox if there are invalid or removed validation errors that are not approved
• Add a new row action for Approve/Unapprove on error index page
• Rebrand “New errors” to “Unapproved errors” on error index page

Screenshots

• Icons in admin bar:
  
  
  
  

• Updated error index page:
  

• Updated single error page:
  

• Updated 'Validated URLs' page:
  

• Updated single 'Validation URL' page (Note: The "Approved" column has since been renamed to "Reviewed"):
  

• Toggling markup and reviewed status gives visual feedback:
  

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[westonruter]

As discussed in another channel, I think we would be a bit worse off by implementing this partial solution. To clear the “new” state for all removed validation errors, users would have to flip all validation errors to Kept, click update, and then flip all to Removed. I think we need to defer this until we have a good UI to manage the new/ack state. Let's hold off on this until v1.6 when we have a good design in #3725.

[pierlon]

Markup status being moved below to the error details section, and also unwrapping <details>:

[westonruter]

As opposed to using JS for this, couldn't the new dt/dd be added in PHP by injecting them before the first <dt>?

[pierlon]

Done in f571c17.

[westonruter]

Whew! Great job here. Huge improvement to the UX.

@schlessera @amedina Anything else before merging this?

[westonruter]

@pierlon I found an issue. This is not currently getting run in Reader mode. When Reader mode is selected, then no Link icon appears in the header.

I tried moving this code to amp_init and that fixed the issue, but then it revealed two others:

1. The amp-icons stylesheet here is not only used for the admin bar but also for other parts of the screens.
2. The stylesheet was being added to every screen in the admin (and the frontend) regardless of whether it was being used.

I've implemented a fix in c4cb9ab.

[schlessera]

Code generally looks good, except for the way the window reload warning is handled (see comment).

I'm fine with merging it as-is for now, though, and focus on improving that in a more drastic rewrite.

[schlessera]

This is overly simplistic and leads to cases where the warning appears even though, for the user, no change has happened as whatever the user did he also undid again.

Fixing this might be overly complicated for now, so I'm fine with keeping as-is as long as we're aware of that issue.

[westonruter]

That's a good point, but it's the same behavior as the block editor. You can make a change and then either manually undo the change or click the Undo button, and in both cases clicking reload will show the same AYS dialog. So it's not any worse than what users expect elsewhere in WP.