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

assets/css/src/amp-icons.css

@@ -0,0 +1,50 @@
+#wpadminbar span.amp-icon {
+	top: 2px;
+}
+
+#wpadminbar .ab-sub-wrapper span.amp-icon {
+	position: absolute !important;
+	margin-left: 2px;
+	padding: initial !important;
+}
+
+body .amp-icon {
+	/* stylelint-disable font-family-no-missing-generic-family-keyword */
+	font: normal 20px/1 dashicons;
+}
+
+/*
+ * Colors are taken from the official WordPress color palette.
+ * See https://make.wordpress.org/design/handbook/design-guide/foundations/colors/#auxiliary-hues
+ */
+.amp-icon.amp-valid::before {
+	content: "\f12a";
+
+	/* Accent Green */
+	color: #46b450 !important;
+}
+
+.amp-icon.amp-invalid::before {
+	content: "\f153";
+
+	/* Accent Red */
+	color: #dc3232 !important;
+}
+
+.amp-icon.amp-warning::before {
+	content: "\f534";
+
+	/* Accent Orange */
+	color: #ffc733 !important;
+}
+
+.ab-icon.amp-icon.amp-link::before {
+	content: "\f103";
+}
+
+.amp-icon.amp-link:not(.ab-icon)::before {
+	content: "\f103";
+
+	/* Medium Blue */
+	color: #00a0d2 !important;
+}

assets/css/src/amp-validation-error-taxonomy.css

@@ -202,23 +202,26 @@ dl.detailed details > summary {
 	padding-bottom: 0.6rem;
 }
 
-.status-text::before {
-	margin-right: 6px;
-	background-size: 16px 16px;
-	height: 16px;
-	width: 16px;
+#amp-enabled-icon.status-text::before {
+	margin-right: 4px;
+	background-size: 20px 20px;
+	height: 20px;
+	width: 20px;
 	content: "";
-	min-width: 16px;
+	min-width: 20px;
 }
 
-.wp-heading-inline .status-text::before {
-	margin-right: 10px;
-	background-size: 1em 1em;
-	height: 1em;
-	width: 1em;
-	content: "";
-	min-width: 1em;
+.status-text > .amp-icon::before {
+	margin-right: 6px;
+	height: 20px;
+	width: 20px;
+	min-width: 20px;
+}
+
+.wp-heading-inline .status-text > .amp-icon::before {
+	font-size: 23px;
 	vertical-align: middle;
+	margin-right: inherit;
 }
 
 .status-text.amp-enabled::before {
@@ -229,14 +232,6 @@ dl.detailed details > summary {
 	background-image: url("../images/amp-logo-gray.svg");
 }
 
-.status-text.accepted::before {
-	background-image: url("../images/amp-logo-green.svg");
-}
-
-.status-text.rejected::before {
-	background-image: url("../images/amp-logo-red.svg");
-}
-
 .single-error-detail {
 	margin: 5px 0 5px 0;
 }
@@ -322,7 +317,6 @@ body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input {
 
 .wp-heading-inline .status-text {
 	display: inline-block;
-	margin-left: 10px;
 	vertical-align: middle;
 	padding-bottom: 0;
 }
@@ -370,6 +364,11 @@ body.taxonomy-amp_validation_error .wp-list-table .new th.check-column input {
 	margin-bottom: 1em;
 }
 
+.amp-stylesheet-summary span.amp-icon {
+	font-size: 16px;
+	vertical-align: middle;
+}
+
 .amp-stylesheet-summary th {
 	text-align: right;
 	padding-right: 1ex;

assets/css/src/amp-validation-single-error-url.css

@@ -71,10 +71,6 @@ table.striped > tbody > tr.even {
 	vertical-align: top;
 }
 
-.column-status img {
-	width: 1.2rem;
-}
-
 #number-errors {
 	text-align: center;
 	background-color: #d3d3d3b8;
@@ -120,6 +116,10 @@ table.striped > tbody > tr.even {
 	width: 150px;
 }
 
+.wp-list-table th.column-reviewed {
+	width: 100px;
+}
+
 /** Add space between list table and the filter and search box above it. */
 #post-body-content button.action,
 #post-body-content #url-post-filter,
@@ -131,7 +131,7 @@ table.striped > tbody > tr.even {
 	margin-left: 4px;
 }
 
-#accept-reject-buttons:not(.hidden) {
+#remove-keep-buttons:not(.hidden) {
 	display: inline-block;
 }
 
@@ -152,4 +152,10 @@ table.striped > tbody > tr.even {
 	margin-left: 5px;
 	width: auto;
 	float: none;
+	border-width: 2px;
+}
+
+#amp-invalid-markup > .amp-icon::before {
+	margin-right: 2px;
+	margin-left: -2px;
 }

assets/src/amp-validation/amp-validated-url-post-edit-screen.js

@@ -28,7 +28,7 @@ domReady( () => {
 	handleFiltering();
 	handleSearching();
 	setValidationErrorRowsSeenClass();
-	handleStatusChange();
+	handleRowEvents();
 	handleBulkActions();
 	watchForUnsavedChanges();
 	setupStylesheetsMetabox();
@@ -56,12 +56,14 @@ const addBeforeUnloadPrompt = () => {
 /**
  * Watch for unsaved changes.
  *
- * Add an beforeunload warning when attempting to leave the page when there are unsaved changes,
- * unless the user is pressing the trash link or update button.
+ * Add an beforeunload warning when there are unsaved changes for the markup or review status.
  */
 const watchForUnsavedChanges = () => {
 	const onChange = ( event ) => {
-		if ( event.target.matches( 'select' ) && event.target.getAttribute( 'id' ) !== 'amp_validation_error_type' ) {
+		if (
+			event.target.matches( '.amp-validation-error-status' ) ||
+			event.target.matches( '.amp-validation-error-status-review' )
+		) {
 			document.getElementById( 'post' ).removeEventListener( 'change', onChange );
 			addBeforeUnloadPrompt();
 		}
@@ -283,32 +285,35 @@ const handleSearching = () => {
 const updateSelectIcon = ( select ) => {
 	const newOption = select.options[ select.selectedIndex ];
 	if ( newOption ) {
-		const iconSrc = newOption.getAttribute( 'data-status-icon' );
-		select.parentNode.querySelector( 'img' ).setAttribute( 'src', iconSrc );
+		const color = newOption.getAttribute( 'data-color' );
+		select.style.borderColor = color;
 	}
 };
 
 /**
- * Handles a change in the error status, like from 'Removed' to 'Kept'.
- *
- * Gets the data-status-icon value from the newly-selected <option>.
- * And sets this as the src of the status icon <img>.
+ * Handles events that may occur for a row.
  */
-const handleStatusChange = () => {
-	const onChange = ( { event } ) => {
-		if ( event.target.matches( 'select' ) ) {
-			updateSelectIcon( event.target );
-		}
-	};
-
+const handleRowEvents = () => {
 	document.querySelectorAll( 'tr[id^="tag-"]' ).forEach( ( row ) => {
-		const select = row.querySelector( '.amp-validation-error-status' );
+		const statusSelect = row.querySelector( '.amp-validation-error-status' );
+		const reviewCheckbox = row.querySelector( '.amp-validation-error-status-review' );
 
-		if ( select ) {
-			select.addEventListener( 'change', ( event ) => {
-				onChange( { event, row, select } );
+		if ( statusSelect ) {
+			/*
+			 * Handle a change in the error status, like from 'Removed' to 'Kept'. It gets the data-color value
+			 * from the newly-selected <option> and sets this as the border color of the <select>.
+			 */
+			statusSelect.addEventListener( 'change', ( event ) => {
+				if ( event.target.matches( 'select' ) ) {
+					updateSelectIcon( event.target );
+				}
 			} );
 		}
+
+		if ( reviewCheckbox ) {
+			// Toggle the 'new' state for the row depending on the state of approval for the validation error.
+			reviewCheckbox.addEventListener( 'change', () => row.classList.toggle( 'new' ) );
+		}
 	} );
 };
 
@@ -319,9 +324,9 @@ const handleStatusChange = () => {
  * Also, on unchecking the last checked box, this hides these buttons.
  */
 const handleBulkActions = () => {
-	const acceptButton = document.querySelector( 'button.action.accept' );
-	const rejectButton = document.querySelector( 'button.action.reject' );
-	const acceptAndRejectContainer = document.getElementById( 'accept-reject-buttons' );
+	const removeButton = document.querySelector( 'button.action.remove' );
+	const keepButton = document.querySelector( 'button.action.keep' );
+	const acceptAndRejectContainer = document.getElementById( 'remove-keep-buttons' );
 
 	const onChange = ( event ) => {
 		let areThereCheckedBoxes;
@@ -356,7 +361,7 @@ const handleBulkActions = () => {
 	} );
 
 	// Handle click on bulk "Remove" button.
-	acceptButton.addEventListener( 'click', () => {
+	removeButton.addEventListener( 'click', () => {
 		Array.prototype.forEach.call( document.querySelectorAll( 'select.amp-validation-error-status' ), ( select ) => {
 			if ( select.closest( 'tr' ).querySelector( '.check-column input[type=checkbox]' ).checked ) {
 				select.value = '3'; // See AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS.
@@ -367,7 +372,7 @@ const handleBulkActions = () => {
 	} );
 
 	// Handle click on bulk "Keep" button.
-	rejectButton.addEventListener( 'click', () => {
+	keepButton.addEventListener( 'click', () => {
 		Array.prototype.forEach.call( document.querySelectorAll( 'select.amp-validation-error-status' ), ( select ) => {
 			if ( select.closest( 'tr' ).querySelector( '.check-column input[type=checkbox]' ).checked ) {
 				select.value = '2'; // See AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_REJECTED_STATUS.

assets/src/amp-validation/amp-validation-error-edit-screen.js

@@ -0,0 +1,34 @@
+/**
+ * WordPress dependencies
+ */
+import domReady from '@wordpress/dom-ready';
+import { __ } from '@wordpress/i18n';
+
+domReady( () => {
+	const errorDetailList = document.querySelector( 'details > dl.detailed' );
+
+	addAdditionalErrorDetails( errorDetailList );
+	unwrapErrorDetails( errorDetailList );
+} );
+
+const addAdditionalErrorDetails = ( errorDetailList ) => {
+	addStatusDetail( errorDetailList );
+};
+
+const addStatusDetail = ( errorDetails ) => {
+	const validationStatus = document.querySelector( 'span.status-text' );
+
+	const term = document.createElement( 'dt' );
+	term.textContent = __( 'Status', 'amp' );
+
+	const detail = document.createElement( 'dd' );
+	detail.append( validationStatus );
+
+	errorDetails.prepend( term, detail );
+};
+
+const unwrapErrorDetails = ( errorDetailList ) => {
+	const detailsElement = errorDetailList.parentNode;
+	detailsElement.parentNode.append( errorDetailList );
+	detailsElement.style.display = 'none';
+};

includes/amp-helper-functions.php

@@ -5,6 +5,7 @@
  * @package AMP
  */
 
+use AmpProject\AmpWP\Icon;
 use AmpProject\AmpWP\Option;
 use AmpProject\AmpWP\Services;
 
@@ -1624,13 +1625,16 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) {
 		$href = add_query_arg( amp_get_slug(), '', amp_get_current_url() );
 	}
 
-	$icon = '🔗'; // LINK SYMBOL.
-
 	$parent = [
 		'id'    => 'amp',
 		'title' => sprintf(
-			'<span id="amp-admin-bar-item-status-icon">%s</span> %s',
-			$icon,
+			'%s %s',
+			Icon::link()->to_html(
+				[
+					'id'    => 'amp-admin-bar-item-status-icon',
+					'class' => 'ab-icon',
+				]
+			),
 			esc_html( is_amp_endpoint() ? __( 'Non-AMP', 'amp' ) : __( 'AMP', 'amp' ) )
 		),
 		'href'  => esc_url( $href ),

includes/class-amp-theme-support.php

@@ -1439,8 +1439,6 @@ public static function init_admin_bar() {
 				$data .= 'html:not(#_) > body { position:unset !important; }';
 			}
 
-			$data .= sprintf( '#amp-admin-bar-item-status-icon { font-family: %s; }', Fonts::getEmojiFontFamilyValue() );
-
 			wp_add_inline_style( 'admin-bar', $data );
 		}