Plugin sidebar style updates

ampproject · Dec 7, 2020 · eeaa4c7 · eeaa4c7
1 parent d5631a1
commit eeaa4c7
Show file tree

Hide file tree

Showing 8 changed files with 123 additions and 28 deletions.
diff --git a/assets/images/amp-alert.svg b/assets/images/amp-alert.svg
diff --git a/assets/images/amp-delete.svg b/assets/images/amp-delete.svg
diff --git a/assets/images/amp-toolbar-icon-broken.svg b/assets/images/amp-toolbar-icon-broken.svg
diff --git a/assets/src/block-validation/error/error-content.js b/assets/src/block-validation/error/error-content.js
@@ -12,6 +12,8 @@ import { sprintf, __ } from '@wordpress/i18n';
  * Internal dependencies
  */
 import { VALIDATION_ERROR_ACK_ACCEPTED_STATUS, VALIDATION_ERROR_ACK_REJECTED_STATUS, VALIDATION_ERROR_NEW_ACCEPTED_STATUS, VALIDATION_ERROR_NEW_REJECTED_STATUS } from '../constants';
+import AMPAlert from '../../../images/amp-alert.svg';
+import AMPDelete from '../../../images/amp-delete.svg';
 import { getErrorSourceTitle } from './get-error-source-title';
 
 /**
@@ -87,9 +89,30 @@ ErrorSource.propTypes = {
 function MarkupStatus( { status } ) {
 	let keptRemoved;
 	if ( [ VALIDATION_ERROR_NEW_ACCEPTED_STATUS, VALIDATION_ERROR_ACK_ACCEPTED_STATUS ].includes( status ) ) {
-		keptRemoved = __( 'Removed', 'amp' );
+		keptRemoved = (
+			<span className="amp-error__kept-removed amp-error__kept-removed--removed">
+				{ __( 'Removed', 'amp' ) }
+				<span>
+					<AMPDelete />
+				</span>
+			</span>
+		);
+	} else {
+		keptRemoved = (
+			<span className="amp-error__kept-removed amp-error__kept-removed--kept">
+				{ __( 'Kept', 'amp' ) }
+				<span>
+					<AMPAlert />
+				</span>
+			</span>
+		);
+	}
+
+	let reviewed;
+	if ( [ VALIDATION_ERROR_ACK_ACCEPTED_STATUS, VALIDATION_ERROR_ACK_REJECTED_STATUS ].includes( status ) ) {
+		reviewed = __( 'Yes', 'amp' );
 	} else {
-		keptRemoved = __( 'Kept', 'amp' );
+		reviewed = __( 'No', 'amp' );
 	}
 
 	return (
@@ -100,6 +123,12 @@ function MarkupStatus( { status } ) {
 			<dd>
 				{ keptRemoved }
 			</dd>
+			<dt>
+				{ __( 'Reviewed', 'amp' ) }
+			</dt>
+			<dd>
+				{ reviewed }
+			</dd>
 		</>
 	);
 }
@@ -142,13 +171,16 @@ BlockType.propTypes = {
  * @param {Object} props.blockType Block type details.
  * @param {string} props.clientId Block client ID
  * @param {number} props.status Number indicating the error status.
+ * @param {string} props.title Error title.
  * @param {Object} props.error Error details.
  * @param {Object[]} props.error.sources Sources from the PHP backtrace for the error.
  */
-export function ErrorContent( { blockType, clientId, status, error: { sources } } ) {
+export function ErrorContent( { blockType, clientId, status, title, error: { sources } } ) {
 	const blockTypeTitle = blockType?.title;
 	const blockTypeName = blockType?.name;
 
+	const [ titleText, nodeName ] = title.split( ':' ).map( ( item ) => item.trim() );
+
 	return (
 		<>
 			{ ! clientId && (
@@ -157,6 +189,17 @@ export function ErrorContent( { blockType, clientId, status, error: { sources }
 				</p>
 			) }
 			<dl className="amp-error__details">
+				{
+					// If node name is empty, the title text displayed in the panel header is enough.
+					nodeName && (
+						<>
+							<dt>
+								{ titleText }
+							</dt>
+							<dd dangerouslySetInnerHTML={ { __html: nodeName } } />
+						</>
+					)
+				}
 				<BlockType blockTypeTitle={ blockTypeTitle } clientId={ clientId } />
 				<ErrorSource blockTypeName={ blockTypeName } clientId={ clientId } sources={ sources } />
 				<MarkupStatus status={ status } />
@@ -176,6 +219,7 @@ ErrorContent.propTypes = {
 		VALIDATION_ERROR_NEW_REJECTED_STATUS,
 		VALIDATION_ERROR_NEW_ACCEPTED_STATUS,
 	] ),
+	title: PropTypes.string,
 	error: PropTypes.shape( {
 		sources: PropTypes.arrayOf( PropTypes.object ),
 	} ),

diff --git a/assets/src/block-validation/error/error-panel-title.js b/assets/src/block-validation/error/error-panel-title.js
@@ -8,6 +8,10 @@ import PropTypes from 'prop-types';
  */
 import { BlockIcon } from '@wordpress/block-editor';
 import { __ } from '@wordpress/i18n';
+/**
+ * Internal dependencies
+ */
+import AMPAlert from '../../../images/amp-alert.svg';
 
 /**
  * Internal dependencies
@@ -28,6 +32,8 @@ import { ErrorTypeIcon } from './error-type-icon';
 export function ErrorPanelTitle( { blockType, title, error: { type }, status } ) {
 	const kept = status === VALIDATION_ERROR_ACK_REJECTED_STATUS || status === VALIDATION_ERROR_NEW_REJECTED_STATUS;
 
+	const [ titleText ] = title.split( ':' );
+
 	return (
 		<>
 			<div className="amp-error__icons">
@@ -40,14 +46,17 @@ export function ErrorPanelTitle( { blockType, title, error: { type }, status } )
 					</div>
 				) }
 			</div>
-			<span className="amp-error__title-text">
-				<span dangerouslySetInnerHTML={ { __html: title } } />
+			<div className="amp-error__title">
+				<div className="amp-error__title-text">
+					{ titleText }
+				</div>
 				{ kept && (
-					<span className="amp-error-alert" title={ __( 'This error has been kept, making this URL not AMP-compatible.', 'amp' ) }>
-						{ '!' }
-					</span>
+					<div className="amp-error-alert" title={ __( 'This error has been kept, making this URL not AMP-compatible.', 'amp' ) }>
+						<AMPAlert />
+						{ __( 'Kept', 'amp' ) }
+					</div>
 				) }
-			</span>
+			</div>
 		</>
 	);
 }

diff --git a/assets/src/block-validation/error/get-error-source-title.js b/assets/src/block-validation/error/get-error-source-title.js
@@ -30,11 +30,11 @@ export function getErrorSourceTitle( sources ) {
 		const muPluginCount = muPluginNames.length;
 
 		if ( 0 < pluginCount ) {
-			output.push( sprintf( '%1$s (%2$d)', __( 'Plugins', 'amp' ) ) );
+			output.push( sprintf( '%1$s (%2$d)', __( 'Plugins', 'amp' ), pluginCount ) );
 		}
 
 		if ( 0 < muPluginCount ) {
-			output.push( sprintf( '%1$s (%2$d)', __( 'Must-use plugins', 'amp' ) ) );
+			output.push( sprintf( '%1$s (%2$d)', __( 'Must-use plugins', 'amp' ), muPluginCount ) );
 		}
 	}
 

diff --git a/assets/src/block-validation/sidebar.js b/assets/src/block-validation/sidebar.js
@@ -72,7 +72,7 @@ export function Sidebar() {
 					<PanelBody opened={ true } className="amp-sidebar__broken">
 						<BrokenIconSVG />
 						<p>
-							{ __( 'AMP is broken at this URL because there are validation errors that have not been removed.', 'amp' ) }
+							{ __( 'This URL is not fully AMP-compatible because there are validation errors that have not been removed.', 'amp' ) }
 						</p>
 					</PanelBody>
 				)
@@ -119,7 +119,7 @@ export function Sidebar() {
 							<PanelBody opened={ true }>
 								<CheckboxControl
 									checked={ isShowingReviewed }
-									label={ __( 'Include previously reviewed errors', 'amp' ) }
+									label={ __( 'Include reviewed errors', 'amp' ) }
 									onChange={ ( newIsShowingReviewed ) => {
 										setIsShowingReviewed( newIsShowingReviewed );
 									} }

diff --git a/assets/src/block-validation/style.css b/assets/src/block-validation/style.css
@@ -29,7 +29,6 @@
 	fill: none;
 }
 
-.amp-error-alert,
 .amp-error-count-badge {
 	align-items: center;
 	background: #bb522e;
@@ -125,13 +124,10 @@
 	width: 15px;
 }
 
-.amp-error {
-	border-left: 4px solid #e0e0e0;
-}
-
-.amp-error--new {
+.amp-error--new .components-panel__body-title,
+.amp-error--new .components-panel__body-title:hover {
 	background: #fef7f1;
-	border-left-color: #d54e21;
+	border-left: 4px solid #d54e21;
 }
 
 .amp-error .components-panel__body-title,
@@ -146,6 +142,10 @@
 	padding-right: 14px;
 }
 
+.components-panel__body.is-opened > .components-panel__body-title {
+	margin-bottom: 10px;
+}
+
 .amp-error__icons {
 	align-items: center;
 	display: flex;
@@ -168,20 +168,34 @@
 	padding: 0.375rem;
 }
 
-.amp-error__title-text {
+.amp-error__title {
+	align-items: flex-end;
+	display: flex;
+	flex-wrap: wrap;
 	font-size: 13px;
+	justify-content: flex-start;
 	line-height: 1.4;
-	padding-right: 1.5rem;
+	padding-right: 1.782rem;
 }
 
-.amp-error__title-text span:first-child {
+.amp-error__title-text {
 	margin-right: 5px;
 }
 
 .amp-error__title-text code {
 	white-space: nowrap;
 }
 
+.amp-error-alert {
+	color: #be2c23;
+	white-space: nowrap;
+}
+
+.amp-error-alert svg {
+	margin-bottom: -2px;
+	margin-right: 3px;
+}
+
 .amp-error__block-type-description {
 	margin-right: 2px;
 }
@@ -225,6 +239,28 @@
 }
 
 .amp-error__details dd {
+	margin-bottom: 8px;
 	margin-left: 0;
+
+}
+
+.amp-error__kept-removed {
+	display: flex;
+}
+
+.amp-error__kept-removed--removed > span,
+.amp-error__kept-removed--kept > span {
+	align-items: center;
+	background: #fcddd3;
+	border-radius: 5px;
+	display: flex;
+	height: 24px;
+	justify-content: center;
+	margin-left: 5px;
+	margin-top: -5px;
+	width: 24px;
 }
 
+.amp-error__kept-removed--removed > span {
+	background: #d2e5e5;
+}