Add AMP plugin editor sidebar

assets/src/block-validation/__mocks__/amp-block-validation.js

@@ -33,4 +33,8 @@ module.exports = {
 	},
 	themeName: 'Test theme',
 	themeSlug: 'test-theme',
+	VALIDATION_ERROR_NEW_REJECTED_STATUS: 0,
+	VALIDATION_ERROR_NEW_ACCEPTED_STATUS: 1,
+	VALIDATION_ERROR_ACK_REJECTED_STATUS: 2,
+	VALIDATION_ERROR_ACK_ACCEPTED_STATUS: 3,
 };

assets/src/block-validation/amp-toolbar-button.js

@@ -20,16 +20,21 @@ import { PLUGIN_NAME, SIDEBAR_NAME } from '.';
  * AMP button displaying in the block toolbar.
  *
  * @param {Object} props Component props.
+ * @param {string} props.clientId Block Client ID.
  * @param {number} props.count The number of errors associated with the block.
  */
-export function AMPToolbarButton( { count } ) {
+export function AMPToolbarButton( { clientId, count } ) {
 	const { openGeneralSidebar } = useDispatch( 'core/edit-post' );
 
 	return (
 		<BlockControls>
 			<ToolbarButton
 				onClick={ () => {
 					openGeneralSidebar( `${ PLUGIN_NAME }/${ SIDEBAR_NAME }` );
+					// eslint-disable-next-line @wordpress/react-no-unsafe-timeout
+					setTimeout( () => {
+						document.querySelector( `.error-${ clientId }:not(.is-opened) button` )?.click();
+					} );
 				} }
 			>
 				<ToolbarIcon count={ count } />
@@ -38,5 +43,6 @@ export function AMPToolbarButton( { count } ) {
 	);
 }
 AMPToolbarButton.propTypes = {
+	clientId: PropTypes.string.isRequired,
 	count: PropTypes.number.isRequired,
 };

assets/src/block-validation/constants.js

@@ -1,8 +1,2 @@
-// See \AMP_Validation_Error_Taxonomy class in PHP.
-export const VALIDATION_ERROR_NEW_REJECTED_STATUS = 0;
-export const VALIDATION_ERROR_NEW_ACCEPTED_STATUS = 1;
-export const VALIDATION_ERROR_ACK_REJECTED_STATUS = 2;
-export const VALIDATION_ERROR_ACK_ACCEPTED_STATUS = 3;
-
 // See \AMP_Validation_Manager::VALIDITY_REST_FIELD_NAME in PHP.
 export const AMP_VALIDITY_REST_FIELD_NAME = 'amp_validity';

assets/src/block-validation/error/error-content.js

@@ -2,7 +2,13 @@
  * External dependencies
  */
 import PropTypes from 'prop-types';
-import { blockSources } from 'amp-block-validation';
+import {
+	blockSources,
+	VALIDATION_ERROR_ACK_ACCEPTED_STATUS,
+	VALIDATION_ERROR_ACK_REJECTED_STATUS,
+	VALIDATION_ERROR_NEW_ACCEPTED_STATUS,
+	VALIDATION_ERROR_NEW_REJECTED_STATUS,
+} from 'amp-block-validation';
 
 /**
  * WordPress dependencies
@@ -12,12 +18,6 @@ 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';
@@ -145,10 +145,8 @@ MarkupStatus.propTypes = {
 /**
  * @param {Object} props
  * @param {string} props.blockTypeTitle Title of the block type.
- * @param {string} props.clientId Block ID.
  */
-function BlockType( { blockTypeTitle, clientId } ) {
-	if ( clientId ) {
+function BlockType( { blockTypeTitle } ) {
 	return (
 		<>
 			<dt>
@@ -162,12 +160,8 @@ function BlockType( { blockTypeTitle, clientId } ) {
 		</>
 	);
 }
-
-	return null;
-}
 BlockType.propTypes = {
 	blockTypeTitle: PropTypes.string,
-	clientId: PropTypes.string,
 };
 
 /**
@@ -207,7 +201,7 @@ export function ErrorContent( { blockType, clientId, status, title, error: { sou
 						</>
 					)
 				}
-				<BlockType blockTypeTitle={ blockTypeTitle } clientId={ clientId } />
+				{ clientId && <BlockType blockTypeTitle={ blockTypeTitle } /> }
 				<ErrorSource blockTypeName={ blockTypeName } clientId={ clientId } sources={ sources } />
 				<MarkupStatus status={ status } />
 			</dl>

assets/src/block-validation/error/error-panel-title.js

@@ -2,6 +2,7 @@
  * External dependencies
  */
 import PropTypes from 'prop-types';
+import { VALIDATION_ERROR_ACK_REJECTED_STATUS, VALIDATION_ERROR_NEW_REJECTED_STATUS } from 'amp-block-validation';
 
 /**
  * WordPress dependencies
@@ -13,11 +14,6 @@ import { __ } from '@wordpress/i18n';
  * Internal dependencies
  */
 import AMPAlert from '../../../images/amp-alert.svg';
-
-/**
- * Internal dependencies
- */
-import { VALIDATION_ERROR_ACK_REJECTED_STATUS, VALIDATION_ERROR_NEW_REJECTED_STATUS } from '../constants';
 import { ErrorTypeIcon } from './error-type-icon';
 
 /**
@@ -38,9 +34,11 @@ export function ErrorPanelTitle( { blockType, title, error: { type }, status } )
 	return (
 		<>
 			<div className="amp-error__icons">
-				<div className={ `amp-error__error-type-icon amp-error__error-type-icon--${ type.replace( /_/g, '-' ) }` }>
+				{ type && (
+					<div className={ `amp-error__error-type-icon amp-error__error-type-icon--${ type?.replace( /_/g, '-' ) }` }>
 						<ErrorTypeIcon type={ type } />
 					</div>
+				) }
 				{ blockType?.icon && (
 					<div className="amp-error__block-type-icon">
 						<BlockIcon icon={ blockType.icon } />

assets/src/block-validation/error/index.js

@@ -2,23 +2,22 @@
  * External dependencies
  */
 import PropTypes from 'prop-types';
+import {
+	VALIDATION_ERROR_ACK_ACCEPTED_STATUS,
+	VALIDATION_ERROR_ACK_REJECTED_STATUS,
+} from 'amp-block-validation';
 
 /**
  * WordPress dependencies
  */
 import { useSelect, useDispatch } from '@wordpress/data';
-import { PanelBody, Button } from '@wordpress/components';
+import { PanelBody, Button, ExternalLink } from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
  */
-import {
-	AMP_VALIDITY_REST_FIELD_NAME,
-	VALIDATION_ERROR_ACK_ACCEPTED_STATUS,
-	VALIDATION_ERROR_ACK_REJECTED_STATUS,
-} from '../constants';
-import { NewTabIcon } from '../icon';
+import { AMP_VALIDITY_REST_FIELD_NAME } from '../constants';
 import { ErrorPanelTitle } from './error-panel-title';
 import { ErrorContent } from './error-content';
 
@@ -50,7 +49,7 @@ export function Error( { clientId, status, term_id: termId, ...props } ) {
 	return (
 		<li className="amp-error-container">
 			<PanelBody
-				className={ `amp-error amp-error--${ reviewed ? 'reviewed' : 'new' }` }
+				className={ `amp-error amp-error--${ reviewed ? 'reviewed' : 'new' }${ clientId ? ` error-${ clientId }` : '' }` }
 				title={
 					<ErrorPanelTitle { ...props } blockType={ blockType } status={ status } />
 				}
@@ -70,15 +69,14 @@ export function Error( { clientId, status, term_id: termId, ...props } ) {
 							{ __( 'Select block', 'amp' ) }
 						</Button>
 					) }
-					<a
+					<ExternalLink
 						href={ detailsUrl.href }
 						target="_blank"
 						rel="noreferrer"
 						className="amp-error__details-link"
 					>
 						{ __( 'View details', 'amp' ) }
-						<NewTabIcon />
-					</a>
+					</ExternalLink>
 				</div>
 
 			</PanelBody>

assets/src/block-validation/error/test/error.js

@@ -3,25 +3,24 @@
  */
 import { act } from 'react-dom/test-utils';
 import { noop } from 'lodash';
+import {
+	VALIDATION_ERROR_ACK_ACCEPTED_STATUS,
+	VALIDATION_ERROR_ACK_REJECTED_STATUS,
+	VALIDATION_ERROR_NEW_ACCEPTED_STATUS,
+	VALIDATION_ERROR_NEW_REJECTED_STATUS,
+} from 'amp-block-validation';
 
 /**
  * WordPress dependencies
  */
 import { render } from '@wordpress/element';
 import { dispatch, select } from '@wordpress/data';
 import { registerBlockType, createBlock } from '@wordpress/blocks';
-import '@wordpress/editor';
 
 /**
  * Internal dependencies
  */
 import { Error } from '..';
-import {
-	VALIDATION_ERROR_ACK_ACCEPTED_STATUS,
-	VALIDATION_ERROR_ACK_REJECTED_STATUS,
-	VALIDATION_ERROR_NEW_ACCEPTED_STATUS,
-	VALIDATION_ERROR_NEW_REJECTED_STATUS,
-} from '../../constants';
 import { createStore } from '../../store';
 
 let container, pluginBlock, themeBlock, coreBlock, unknownBlock;

assets/src/block-validation/icon.js

@@ -8,7 +8,6 @@ import PropTypes from 'prop-types';
  */
 import AMPToolbarIcon from '../../images/amp-icon-toolbar.svg';
 import AMPToolbarIconBroken from '../../images/amp-toolbar-icon-broken.svg';
-import AMPNewTabIcon from '../../images/amp-new-tab-icon.svg';
 
 /**
  * Plugin icon.
@@ -33,7 +32,7 @@ IconSVG.propTypes = {
  * @param {Object} props
  * @param {boolean} props.hasBadge Whether the icon is showing a number.
  */
-export function BrokenIconSVG( { hasBadge } ) {
+function BrokenIconSVG( { hasBadge } ) {
 	return (
 		<span className={ `amp-toolbar-broken-icon${ hasBadge ? ' amp-toolbar-broken-icon--has-badge' : '' }` } >
 			<AMPToolbarIconBroken />
@@ -76,10 +75,3 @@ ToolbarIcon.propTypes = {
 export function MoreMenuIcon() {
 	return <IconSVG hasBadge={ false } />;
 }
-
-/**
- * An icon indicating that a link opens in a new tab.
- */
-export function NewTabIcon() {
-	return <AMPNewTabIcon />;
-}

assets/src/block-validation/sidebar.js

@@ -1,7 +1,7 @@
 /**
  * WordPress dependencies
  */
-import { ToggleControl, PanelBody } from '@wordpress/components';
+import { ToggleControl, PanelBody, ExternalLink } from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
 import { useDispatch, useSelect } from '@wordpress/data';
 import { useEffect } from '@wordpress/element';
@@ -14,7 +14,6 @@ import AMPValidationErrorsIcon from '../../images/amp-validation-errors.svg';
 import AMPValidationErrorsKeptIcon from '../../images/amp-validation-errors-kept.svg';
 import { Error } from './error';
 import { BLOCK_VALIDATION_STORE_KEY } from './store';
-import { NewTabIcon } from './icon';
 import { AMP_VALIDITY_REST_FIELD_NAME } from './constants';
 
 /**
@@ -100,10 +99,9 @@ export function Sidebar() {
 
 						<p>
 							{ reviewLink && (
-								<a href={ reviewLink } className="amp-sidebar__review-link" target="_blank" rel="noreferrer">
+								<ExternalLink href={ reviewLink } className="amp-sidebar__review-link">
 									{ __( 'View technical details', 'amp' ) }
-									<NewTabIcon />
-								</a>
+								</ExternalLink>
 							) }
 						</p>
 					</div>
@@ -122,7 +120,7 @@ export function Sidebar() {
 			) }
 
 			{
-				! saved && (
+				! saved && 0 === validationErrors.length && (
 					<PanelBody opened={ true }>
 						<p>
 							{ __( 'The permalink will be checked for validation issues when the post is saved.', 'amp' ) }
@@ -138,7 +136,7 @@ export function Sidebar() {
 				</PanelBody>
 			) }
 
-			{ saved && 0 < validationErrors.length && (
+			{ 0 < validationErrors.length && (
 				0 < displayedErrors.length ? (
 					<ul>
 						{ displayedErrors.map( ( validationError, index ) => (

assets/src/block-validation/store.js

@@ -4,14 +4,14 @@
 import { registerStore } from '@wordpress/data';
 
 /**
- * Internal dependencies
+ * External dependencies
  */
 import {
 	VALIDATION_ERROR_ACK_ACCEPTED_STATUS,
 	VALIDATION_ERROR_ACK_REJECTED_STATUS,
 	VALIDATION_ERROR_NEW_ACCEPTED_STATUS,
 	VALIDATION_ERROR_NEW_REJECTED_STATUS,
-} from './constants';
+} from 'amp-block-validation';
 
 export const BLOCK_VALIDATION_STORE_KEY = 'amp/block-validation';
 

assets/src/block-validation/test/icon.js

@@ -1,11 +1,13 @@
 /**
  * External dependencies
  */
+import { act } from 'react-dom/test-utils';
+
 /**
  * WordPress dependencies
  */
 import { render } from '@wordpress/element';
-import { act } from 'react-dom/test-utils';
+
 /**
  * Internal dependencies
  */

assets/src/block-validation/test/store.js

@@ -1,10 +1,11 @@
-/**
- * Internal dependencies
- */
 /**
  * WordPress dependencies
  */
 import { dispatch, select } from '@wordpress/data';
+
+/**
+ * Internal dependencies
+ */
 import { BLOCK_VALIDATION_STORE_KEY, createStore, INITIAL_STATE } from '../store';
 import { rawValidationErrors } from './__data__/raw-validation-errors';
 

assets/src/block-validation/use-validation-error-state-updates.js

@@ -87,7 +87,7 @@ export function useValidationErrorStateUpdates() {
 		validationErrorsFromPost: select( 'core/editor' ).getEditedPostAttribute( AMP_VALIDITY_REST_FIELD_NAME )?.results || [],
 	} ), [] );
 
-	/**
+	/*
 	 * Runs an equality check when validation errors are received before running the heavier effect.
 	 */
 	useEffect( () => {
@@ -96,7 +96,7 @@ export function useValidationErrorStateUpdates() {
 		}
 	}, [ trackedValidationErrorsFromPost, validationErrorsFromPost ] );
 
-	/**
+	/*
 	 * Adds clientIds to the validation errors that are associated with blocks.
 	 */
 	useEffect( () => {