diff --git a/.eslintrc b/.eslintrc index 9ca57237473..2ec892c61af 100644 --- a/.eslintrc +++ b/.eslintrc @@ -57,7 +57,7 @@ "react/no-unused-prop-types": "error", "react/self-closing-comp": "error", "import/no-unresolved": [ "error", { - "ignore": [ "jquery", "amp-block-editor-data", "amp-settings" ] + "ignore": [ "jquery", "amp-block-editor-data", "amp-settings", "amp-block-validation" ] } ], "import/order": [ "error", { "groups": [ "builtin", [ "external", "unknown" ], "internal", "parent", "sibling", "index" ] } ], "jsdoc/check-indentation": "error", diff --git a/.gitattributes b/.gitattributes index 75e82ba8417..289761dafa2 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,3 +6,4 @@ includes/sanitizers/class-amp-allowed-tags-generated.php linguist-generated=true docs/**/*.md linguist-generated=true docs/docs.json linguist-generated=true +**/__data__/*.js linguist-generated=true diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php index 804117ba0e1..ebbca0b6c9f 100644 --- a/.phpstorm.meta.php +++ b/.phpstorm.meta.php @@ -16,11 +16,13 @@ 'amp_slug_customization_watcher' => \AmpProject\AmpWP\AmpSlugCustomizationWatcher::class, 'css_transient_cache.ajax_handler' => \AmpProject\AmpWP\Admin\ReenableCssTransientCachingAjaxAction::class, 'css_transient_cache.monitor' => \AmpProject\AmpWP\BackgroundTask\MonitorCssTransientCaching::class, + 'dev_tools.block_sources' => \AmpProject\AmpWP\DevTools\BlockSources::class, 'dev_tools.callback_reflection' => \AmpProject\AmpWP\DevTools\CallbackReflection::class, 'dev_tools.error_page' => \AmpProject\AmpWP\DevTools\ErrorPage::class, 'dev_tools.file_reflection' => \AmpProject\AmpWP\DevTools\FileReflection::class, 'dev_tools.likely_culprit_detector' => \AmpProject\AmpWP\DevTools\LikelyCulpritDetector::class, 'dev_tools.user_access' => \AmpProject\AmpWP\DevTools\UserAccess::class, + 'editor.editor_support' => \AmpProject\AmpWP\Editor\EditorSupport::class, 'extra_theme_and_plugin_headers' => \AmpProject\AmpWP\ExtraThemeAndPluginHeaders::class, 'injector' => \AmpProject\AmpWP\Infrastructure\Injector::class, 'mobile_redirection' => \AmpProject\AmpWP\MobileRedirection::class, diff --git a/assets/css/src/amp-validation-single-error-url.css b/assets/css/src/amp-validation-single-error-url.css index 3e3d7a4e8b2..5f029facfa7 100644 --- a/assets/css/src/amp-validation-single-error-url.css +++ b/assets/css/src/amp-validation-single-error-url.css @@ -1,3 +1,23 @@ +#the-list tr { + scroll-margin-top: 32px; +} + +@media screen and (max-width: 782px) { + + #the-list tr { + scroll-margin-top: 46px; + } +} + +@media screen and (max-width: 600px) { + + #the-list tr { + + /* Since the admin bar is not sticky */ + scroll-margin-top: 0; + } +} + /** Arrow icon on title in error column. */ .column-error_code > .single-url-detail-toggle { position: relative; diff --git a/assets/images/amp-alert.svg b/assets/images/amp-alert.svg new file mode 100644 index 00000000000..24acfb27d90 --- /dev/null +++ b/assets/images/amp-alert.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/images/amp-css-error-icon.svg b/assets/images/amp-css-error-icon.svg new file mode 100644 index 00000000000..f2a17179b0f --- /dev/null +++ b/assets/images/amp-css-error-icon.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/images/amp-delete.svg b/assets/images/amp-delete.svg new file mode 100644 index 00000000000..444cf74ec1d --- /dev/null +++ b/assets/images/amp-delete.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/images/amp-html-error-icon.svg b/assets/images/amp-html-error-icon.svg new file mode 100644 index 00000000000..3f78c6aa80c --- /dev/null +++ b/assets/images/amp-html-error-icon.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/images/amp-icon-toolbar.svg b/assets/images/amp-icon-toolbar.svg new file mode 100644 index 00000000000..c3681743c55 --- /dev/null +++ b/assets/images/amp-icon-toolbar.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/images/amp-js-error-icon.svg b/assets/images/amp-js-error-icon.svg new file mode 100644 index 00000000000..db6b8c5807f --- /dev/null +++ b/assets/images/amp-js-error-icon.svg @@ -0,0 +1,4 @@ + + + + diff --git a/assets/images/amp-toolbar-icon-broken.svg b/assets/images/amp-toolbar-icon-broken.svg new file mode 100644 index 00000000000..c60ae4a3c72 --- /dev/null +++ b/assets/images/amp-toolbar-icon-broken.svg @@ -0,0 +1,7 @@ + + + + + + + diff --git a/assets/images/amp-validation-errors-kept.svg b/assets/images/amp-validation-errors-kept.svg new file mode 100644 index 00000000000..9b13d0f17ee --- /dev/null +++ b/assets/images/amp-validation-errors-kept.svg @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + diff --git a/assets/images/amp-validation-errors.svg b/assets/images/amp-validation-errors.svg new file mode 100644 index 00000000000..a374fe793a3 --- /dev/null +++ b/assets/images/amp-validation-errors.svg @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/assets/src/amp-validation/amp-validation-single-error-url-details.js b/assets/src/amp-validation/amp-validation-single-error-url-details.js index 110b5195605..32e9d0d6994 100644 --- a/assets/src/amp-validation/amp-validation-single-error-url-details.js +++ b/assets/src/amp-validation/amp-validation-single-error-url-details.js @@ -7,9 +7,10 @@ import domReady from '@wordpress/dom-ready'; * Toggles the contents of a details element as an additional table tr. */ class RowToggler { - constructor( tr, index ) { + constructor( tr, index, activeTermId ) { this.tr = tr; this.index = index; + this.activeTermId = activeTermId; // Since we're adding additional rows, we need to override default .striped tables styles. this.tr.classList.add( this.index % 2 ? 'odd' : 'even' ); @@ -33,6 +34,19 @@ class RowToggler { } ); } ); } + + this.maybeInitiallyOpenRow(); + } + + /** + * If the term ID retrieved from the URL query param matches this row's term ID, expand the row on load. + */ + maybeInitiallyOpenRow() { + if ( ! this.activeTermId || this.tr.id !== `tag-${ this.activeTermId }` ) { + return; + } + + this.toggle( this.tr.querySelector( '.single-url-detail-toggle' ) ); } /** @@ -71,7 +85,7 @@ class RowToggler { * * @param {Object} target The click event target. */ - toggle = ( target ) => { + toggle( target ) { if ( this.tr.classList.contains( 'expanded' ) ) { this.onClose( target ); } else { @@ -112,10 +126,10 @@ class RowToggler { * Sets up expandable details for errors when viewing a single URL error list. */ class ErrorRows { - constructor() { + constructor( activeTermId ) { this.rows = [ ...document.querySelectorAll( '.wp-list-table tr[id^="tag-"]' ) ] .map( ( tr, index ) => { - const rowHandler = new RowToggler( tr, index ); + const rowHandler = new RowToggler( tr, index, activeTermId ); rowHandler.init(); return rowHandler; } ) @@ -153,5 +167,12 @@ class ErrorRows { } domReady( () => { - new ErrorRows().init(); + let activeTermId = null; + + const matches = window.location.hash.match( /^#tag-(\d+)/ ); + if ( matches ) { + activeTermId = parseInt( matches[ 1 ] ); + } + + new ErrorRows( activeTermId ).init(); } ); diff --git a/assets/src/block-validation/__mocks__/amp-block-validation.js b/assets/src/block-validation/__mocks__/amp-block-validation.js new file mode 100644 index 00000000000..90dc2081427 --- /dev/null +++ b/assets/src/block-validation/__mocks__/amp-block-validation.js @@ -0,0 +1,40 @@ +module.exports = { + blockSources: { + 'my-plugin/test-block': { + source: 'plugin', + title: 'My plugin', + }, + 'my-mu-plugin/test-block': { + source: 'mu-plugin', + title: 'My MU plugin', + }, + 'my-theme/test-block': { + source: 'theme', + title: 'My theme', + }, + 'core/test-block': { + source: '', + title: 'WordPress core', + }, + 'unknown/test-block': { + source: '', + name: '', + }, + }, + CSS_ERROR_TYPE: 'css_error', + HTML_ATTRIBUTE_ERROR_TYPE: 'html_attribute_error', + HTML_ELEMENT_ERROR_TYPE: 'html_element_error', + JS_ERROR_TYPE: 'js_error', + pluginNames: { + 'test-plugin': 'Test plugin', + 'test-mu-plugin': 'Test MU plugin', + 'test-plugin-2': 'Test plugin 2', + 'test-mu-plugin-2': 'Test MU plugin 2', + }, + 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, +}; diff --git a/assets/src/block-validation/amp-toolbar-button.js b/assets/src/block-validation/amp-toolbar-button.js new file mode 100644 index 00000000000..1ff6495ff37 --- /dev/null +++ b/assets/src/block-validation/amp-toolbar-button.js @@ -0,0 +1,63 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * WordPress dependencies + */ +import { BlockControls } from '@wordpress/block-editor'; +import { ToolbarButton } from '@wordpress/components'; +import { useDispatch } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { ToolbarIcon } from './icon'; +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( { clientId, count } ) { + const { openGeneralSidebar } = useDispatch( 'core/edit-post' ); + + return ( + + { + openGeneralSidebar( `${ PLUGIN_NAME }/${ SIDEBAR_NAME }` ); + // eslint-disable-next-line @wordpress/react-no-unsafe-timeout + setTimeout( () => { + const buttons = Array.from( document.querySelectorAll( `.error-${ clientId } button` ) ); + const firstButton = buttons[ 0 ]; + + // Ensure all errors are expanded. + // @todo This would be more elegant if this state were captured in the store? + buttons.reverse(); // Reverse so that the first one is focused first. + for ( const button of buttons ) { + if ( 'false' === button.getAttribute( 'aria-expanded' ) ) { + button.click(); + } + } + + // Make sure the first is scrolled into view. + if ( firstButton ) { + firstButton.scrollIntoView( { block: 'start', inline: 'nearest', behavior: 'smooth' } ); + } + } ); + } } + > + + + + ); +} +AMPToolbarButton.propTypes = { + clientId: PropTypes.string.isRequired, + count: PropTypes.number.isRequired, +}; diff --git a/assets/src/block-validation/components/higher-order/with-validation-error-notice/edit.css b/assets/src/block-validation/components/higher-order/with-validation-error-notice/edit.css deleted file mode 100644 index 798264e4bd6..00000000000 --- a/assets/src/block-validation/components/higher-order/with-validation-error-notice/edit.css +++ /dev/null @@ -1,15 +0,0 @@ -.amp-block-validation-errors, -.amp-block-validation-errors * { - font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif; - font-size: 13px; - line-height: 1.5; -} - -.amp-block-validation-errors .amp-block-validation-errors__summary { - margin: 0.5em 0; - padding: 2px; -} - -.amp-block-validation-errors .amp-block-validation-errors__list { - padding-left: 2.5em; -} diff --git a/assets/src/block-validation/components/higher-order/with-validation-error-notice/index.js b/assets/src/block-validation/components/higher-order/with-validation-error-notice/index.js deleted file mode 100644 index 7fe7737b6e0..00000000000 --- a/assets/src/block-validation/components/higher-order/with-validation-error-notice/index.js +++ /dev/null @@ -1,86 +0,0 @@ -/** - * WordPress dependencies - */ -import { Notice } from '@wordpress/components'; -import { __, _n, sprintf } from '@wordpress/i18n'; -import { createHigherOrderComponent } from '@wordpress/compose'; -import { withSelect } from '@wordpress/data'; - -/** - * Internal dependencies - */ -import { ValidationErrorMessage } from '../../'; -import './edit.css'; - -const applyWithSelect = withSelect( ( select, { clientId } ) => { - const { getBlockValidationErrors } = select( 'amp/block-validation' ); - - const blockValidationErrors = getBlockValidationErrors( clientId ); - - return { - blockValidationErrors: blockValidationErrors.length ? blockValidationErrors : undefined, - }; -} ); - -/** - * Wraps the edit() method of a block, and conditionally adds a Notice. - * - * @param {Function} BlockEdit - The original edit() method of the block. - * @return {Function} The edit() method, conditionally wrapped in a notice for AMP validation error(s). - */ -export default createHigherOrderComponent( - ( BlockEdit ) => { - return applyWithSelect( ( props ) => { - const { blockValidationErrors, onReplace } = props; - - if ( ! blockValidationErrors ) { - return ; - } - - const errorCount = blockValidationErrors.length; - - const actions = [ - { - label: __( 'Remove Block', 'amp' ), - onClick: () => onReplace( [] ), - }, - ]; - - return ( - <> - -
- - { sprintf( - /* translators: %s is the number of issues */ - _n( - 'There is %s issue from AMP validation.', - 'There are %s issues from AMP validation.', - errorCount, - 'amp', - ), - errorCount, - ) } - -
    - { blockValidationErrors.map( ( error, key ) => { - return ( -
  • - -
  • - ); - } ) } -
-
-
- - - ); - } ); - }, - 'withValidationErrorNotice', -); diff --git a/assets/src/block-validation/components/index.js b/assets/src/block-validation/components/index.js deleted file mode 100644 index 4b76466fa68..00000000000 --- a/assets/src/block-validation/components/index.js +++ /dev/null @@ -1,2 +0,0 @@ -export { default as ValidationErrorMessage } from './validation-error-message'; -export { default as withValidationErrorNotice } from './higher-order/with-validation-error-notice'; diff --git a/assets/src/block-validation/components/validation-error-message/index.js b/assets/src/block-validation/components/validation-error-message/index.js deleted file mode 100644 index 826b4a26239..00000000000 --- a/assets/src/block-validation/components/validation-error-message/index.js +++ /dev/null @@ -1,47 +0,0 @@ -/** - * External dependencies - */ -import PropTypes from 'prop-types'; -import { ReactElement } from 'react'; - -/** - * WordPress dependencies - */ -import { __ } from '@wordpress/i18n'; - -/** - * Get message for validation error. - * - * @param {Object} props Component props. - * @param {?string} props.title Title for error (with HTML) as provided by \AMP_Validation_Error_Taxonomy::get_error_title_from_code(). - * @param {?string} props.code Error code. - * @param {?string|ReactElement} props.message Error message. - * - * @return {ReactElement} Validation error message. - */ -const ValidationErrorMessage = ( { title, message, code } ) => { - if ( message ) { - return message; // @todo It doesn't appear this is ever set? - } - - if ( title ) { - return ; - } - - return ( - <> - { __( 'Error code: ', 'amp' ) } - - { code || __( 'unknown', 'amp' ) } - - - ); -}; - -ValidationErrorMessage.propTypes = { - message: PropTypes.string, - title: PropTypes.string, - code: PropTypes.string, -}; - -export default ValidationErrorMessage; diff --git a/assets/src/block-validation/components/validation-error-message/test/__snapshots__/index.js.snap b/assets/src/block-validation/components/validation-error-message/test/__snapshots__/index.js.snap deleted file mode 100644 index b863e11570b..00000000000 --- a/assets/src/block-validation/components/validation-error-message/test/__snapshots__/index.js.snap +++ /dev/null @@ -1,30 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ValidationErrorMessage renders an error for a custom error code 1`] = ` -Array [ - "Error code: ", - - some_other_error - , -] -`; - -exports[`ValidationErrorMessage renders an error with a title 1`] = ` - - Invalid attribute - - onclick - - -`; - -exports[`ValidationErrorMessage renders an error for an unknown error code 1`] = ` -Array [ - "Error code: ", - - unknown - , -] -`; - -exports[`ValidationErrorMessage renders an error with a custom message 1`] = `null`; diff --git a/assets/src/block-validation/components/validation-error-message/test/index.js b/assets/src/block-validation/components/validation-error-message/test/index.js deleted file mode 100644 index db90e62fbba..00000000000 --- a/assets/src/block-validation/components/validation-error-message/test/index.js +++ /dev/null @@ -1,31 +0,0 @@ -/** - * External dependencies - */ -import { render } from 'enzyme'; - -/** - * Internal dependencies - */ -import ValidationErrorMessage from '../'; - -describe( 'ValidationErrorMessage', () => { - it( 'renders an error with a custom message', () => { - const errorMessage = render( ); - expect( errorMessage ).toMatchSnapshot(); - } ); - - it( 'renders an error with a title', () => { - const errorMessage = render( ); - expect( errorMessage ).toMatchSnapshot(); - } ); - - it( 'renders an error for a custom error code', () => { - const errorMessage = render( ); - expect( errorMessage ).toMatchSnapshot(); - } ); - - it( 'renders an error for an unknown error code', () => { - const errorMessage = render( ); - expect( errorMessage ).toMatchSnapshot(); - } ); -} ); diff --git a/assets/src/block-validation/constants.js b/assets/src/block-validation/constants.js index a9e94e6e8e4..891587b2097 100644 --- a/assets/src/block-validation/constants.js +++ b/assets/src/block-validation/constants.js @@ -1,10 +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; - -export const AMP_VALIDATION_ERROR_NOTICE_ID = 'amp-errors-notice'; - // See \AMP_Validation_Manager::VALIDITY_REST_FIELD_NAME in PHP. export const AMP_VALIDITY_REST_FIELD_NAME = 'amp_validity'; diff --git a/assets/src/block-validation/error/error-content.js b/assets/src/block-validation/error/error-content.js new file mode 100644 index 00000000000..5e9694191ff --- /dev/null +++ b/assets/src/block-validation/error/error-content.js @@ -0,0 +1,227 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +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 + */ +import { sprintf, __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import AMPAlert from '../../../images/amp-alert.svg'; +import AMPDelete from '../../../images/amp-delete.svg'; +import { getErrorSourceTitle } from './get-error-source-title'; + +/** + * @param {Object} props + * @param {string} props.clientId Error client ID. + * @param {string} props.blockTypeName Block type name. + * @param {Object[]} props.sources List of source objects from the PHP backtrace. + */ +function ErrorSource( { clientId, blockTypeName, sources } ) { + let source; + + const blockSource = blockSources?.[ blockTypeName ]; + + if ( clientId ) { + switch ( blockSource?.source ) { + case 'plugin': + source = sprintf( + /* translators: %s: plugin name. */ + __( `%s (plugin)`, 'amp' ), + blockSource.title, + ); + break; + + case 'mu-plugin': + source = sprintf( + /* translators: %s: plugin name. */ + __( `%s (must-use plugin)`, 'amp' ), + blockSource.title, + ); + break; + + case 'theme': + source = sprintf( + /* translators: %s: theme name. */ + __( `%s (theme)`, 'amp' ), + blockSource.title, + ); + break; + + default: + source = blockSource?.title || getErrorSourceTitle( sources ); + break; + } + } else { + source = getErrorSourceTitle( sources ); + } + + if ( ! source ) { + source = __( 'Unknown', 'amp' ); + } + + return ( + <> +
+ { __( 'Source', 'amp' ) } +
+
+ { source } +
+ + ); +} +ErrorSource.propTypes = { + blockTypeName: PropTypes.string, + clientId: PropTypes.string, + sources: PropTypes.arrayOf( PropTypes.object ), +}; + +/** + * @param {Object} props + * @param {number} props.status Error status. + */ +function MarkupStatus( { status } ) { + let keptRemoved; + if ( [ VALIDATION_ERROR_NEW_ACCEPTED_STATUS, VALIDATION_ERROR_ACK_ACCEPTED_STATUS ].includes( status ) ) { + keptRemoved = ( + + { __( 'Removed', 'amp' ) } + + + + + ); + } else { + keptRemoved = ( + + { __( 'Kept', 'amp' ) } + + + + + ); + } + + let reviewed; + if ( [ VALIDATION_ERROR_ACK_ACCEPTED_STATUS, VALIDATION_ERROR_ACK_REJECTED_STATUS ].includes( status ) ) { + reviewed = __( 'Yes', 'amp' ); + } else { + reviewed = __( 'No', 'amp' ); + } + + return ( + <> +
+ { __( 'Markup status', 'amp' ) } +
+
+ { keptRemoved } +
+
+ { __( 'Reviewed', 'amp' ) } +
+
+ { reviewed } +
+ + ); +} +MarkupStatus.propTypes = { + status: PropTypes.number.isRequired, +}; + +/** + * @param {Object} props + * @param {string} props.blockTypeTitle Title of the block type. + */ +function BlockType( { blockTypeTitle } ) { + return ( + <> +
+ { __( 'Block type', 'amp' ) } +
+
+ + { blockTypeTitle || __( 'unknown', 'amp' ) } + +
+ + ); +} +BlockType.propTypes = { + blockTypeTitle: PropTypes.string, +}; + +/** + * Content inside an error panel. + * + * @param {Object} props Component props. + * @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, title, error: { sources } } ) { + const blockTypeTitle = blockType?.title; + const blockTypeName = blockType?.name; + + // @todo Refactor AMP_Validation_Error_Taxonomy::get_error_title_from_code() to return structured data. + const [ titleText, nodeName ] = title.split( ':' ).map( ( item ) => item.trim() ); + + return ( + <> + { ! clientId && ( +

+ { __( 'This error comes from outside the post content.', 'amp' ) } +

+ ) } +
+ { + // If node name is empty, the title text displayed in the panel header is enough. + nodeName && ( + <> +
+ { titleText } +
+
+ + ) + } + { clientId && } + + +
+ + ); +} +ErrorContent.propTypes = { + blockType: PropTypes.shape( { + name: PropTypes.string, + title: PropTypes.string, + } ), + clientId: PropTypes.string, + status: PropTypes.oneOf( [ + VALIDATION_ERROR_ACK_ACCEPTED_STATUS, + VALIDATION_ERROR_ACK_REJECTED_STATUS, + VALIDATION_ERROR_NEW_REJECTED_STATUS, + VALIDATION_ERROR_NEW_ACCEPTED_STATUS, + ] ).isRequired, + title: PropTypes.string.isRequired, + error: PropTypes.shape( { + sources: PropTypes.arrayOf( PropTypes.object ), + } ).isRequired, +}; diff --git a/assets/src/block-validation/error/error-panel-title.js b/assets/src/block-validation/error/error-panel-title.js new file mode 100644 index 00000000000..08322a00159 --- /dev/null +++ b/assets/src/block-validation/error/error-panel-title.js @@ -0,0 +1,69 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +import { VALIDATION_ERROR_ACK_REJECTED_STATUS, VALIDATION_ERROR_NEW_REJECTED_STATUS } from 'amp-block-validation'; + +/** + * WordPress dependencies + */ +import { BlockIcon } from '@wordpress/block-editor'; +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import AMPAlert from '../../../images/amp-alert.svg'; +import { ErrorTypeIcon } from './error-type-icon'; + +/** + * Panel title component for an individual error. + * + * @param {Object} props Component props. + * @param {Object} props.blockType + * @param {string} props.title Title string from error data. + * @param {Object} props.error Error details. + * @param {string} props.error.type Error type. + * @param {number} props.status Error status. + */ +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 ( + <> +
+ { type && ( +
+ +
+ ) } + { blockType?.icon && ( +
+ +
+ ) } +
+
+
+ { titleText } +
+ { kept && ( +
+ + { __( 'Kept', 'amp' ) } +
+ ) } +
+ + ); +} +ErrorPanelTitle.propTypes = { + blockType: PropTypes.object, + title: PropTypes.string.isRequired, + error: PropTypes.shape( { + type: PropTypes.string, + } ).isRequired, + status: PropTypes.number.isRequired, +}; diff --git a/assets/src/block-validation/error/error-type-icon.js b/assets/src/block-validation/error/error-type-icon.js new file mode 100644 index 00000000000..53a7ce76744 --- /dev/null +++ b/assets/src/block-validation/error/error-type-icon.js @@ -0,0 +1,43 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +import { + CSS_ERROR_TYPE, + HTML_ATTRIBUTE_ERROR_TYPE, + HTML_ELEMENT_ERROR_TYPE, + JS_ERROR_TYPE, +} from 'amp-block-validation'; + +/** + * Internal dependencies + */ +import HTMLErrorIcon from '../../../images/amp-html-error-icon.svg'; +import JSErrorIcon from '../../../images/amp-js-error-icon.svg'; +import CSSErrorIcon from '../../../images/amp-css-error-icon.svg'; + +/** + * Component rendering an icon representing JS, CSS, or HTML. + * + * @param {Object} props + * @param {string} props.type The error type. + */ +export function ErrorTypeIcon( { type } ) { + switch ( type ) { + case HTML_ATTRIBUTE_ERROR_TYPE: + case HTML_ELEMENT_ERROR_TYPE: + return ; + + case JS_ERROR_TYPE: + return ; + + case CSS_ERROR_TYPE: + return ; + + default: + return null; + } +} +ErrorTypeIcon.propTypes = { + type: PropTypes.string.isRequired, +}; diff --git a/assets/src/block-validation/error/get-error-source-title.js b/assets/src/block-validation/error/get-error-source-title.js new file mode 100644 index 00000000000..6a7692425fd --- /dev/null +++ b/assets/src/block-validation/error/get-error-source-title.js @@ -0,0 +1,74 @@ +/** + * External dependencies + */ +import { pluginNames, themeName, themeSlug } from 'amp-block-validation'; + +/** + * WordPress dependencies + */ +import { __, sprintf } from '@wordpress/i18n'; + +/** + * Attempts to get the title of the plugin or theme responsible for an error. + * + * Adapted from AMP_Validated_URL_Post_Type::render_sources_column PHP method. + * + * @param {Object[]} sources Error source details from the PHP backtrace. + */ +export function getErrorSourceTitle( sources ) { + const keyedSources = { theme: [], plugin: [], 'mu-plugin': [], embed: [], core: [], blocks: [] }; + for ( const source of sources ) { + if ( source.type && source.type in keyedSources ) { + keyedSources[ source.type ].push( source ); + } else if ( 'block_name' in source ) { + keyedSources.blocks.push( source ); + } + } + + const output = []; + const uniquePluginNames = [ ...new Set( keyedSources.plugin.map( ( { name } ) => name ) ) ]; + const muPluginNames = [ ...new Set( keyedSources[ 'mu-plugin' ].map( ( { name } ) => name ) ) ]; + const combinedPluginNames = [ ...uniquePluginNames, ...muPluginNames ]; + + if ( 1 === combinedPluginNames.length ) { + output.push( pluginNames[ combinedPluginNames[ 0 ] ] || combinedPluginNames[ 0 ] ); + } else { + const pluginCount = uniquePluginNames.length; + const muPluginCount = muPluginNames.length; + + if ( 0 < pluginCount ) { + output.push( sprintf( '%1$s (%2$d)', __( 'Plugins', 'amp' ), pluginCount ) ); + } + + if ( 0 < muPluginCount ) { + output.push( sprintf( '%1$s (%2$d)', __( 'Must-use plugins', 'amp' ), muPluginCount ) ); + } + } + + if ( 0 === keyedSources.embed.length ) { + const activeThemeSources = keyedSources.theme.filter( ( { name } ) => themeSlug === name ); + const inactiveThemeSources = keyedSources.theme.filter( ( { name } ) => themeSlug !== name ); + if ( 0 < activeThemeSources.length ) { + output.push( themeName ); + } + + if ( 0 < inactiveThemeSources.length ) { + /* translators: placeholder is the slug of an inactive WordPress theme. */ + output.push( __( 'Inactive theme(s)', 'amp' ) ); + } + } + + if ( 0 === output.length && 0 < keyedSources.blocks.length ) { + output.push( keyedSources.blocks[ 0 ].block_name ); + } + + if ( 0 === output.length && 0 < keyedSources.embed.length ) { + output.push( __( 'Embed', 'amp' ) ); + } + + if ( 0 === output.length && 0 < keyedSources.core.length ) { + output.push( __( 'Core', 'amp' ) ); + } + + return output.join( ', ' ); +} diff --git a/assets/src/block-validation/error/index.js b/assets/src/block-validation/error/index.js new file mode 100644 index 00000000000..520453e4cc8 --- /dev/null +++ b/assets/src/block-validation/error/index.js @@ -0,0 +1,90 @@ +/** + * 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, ExternalLink } from '@wordpress/components'; +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import { AMP_VALIDITY_REST_FIELD_NAME } from '../constants'; +import { ErrorPanelTitle } from './error-panel-title'; +import { ErrorContent } from './error-content'; + +/** + * Component rendering an individual error. Parent component is a
    . + * + * @param {Object} args Component props. + * @param {string} args.clientId + * @param {number} args.status + * @param {number} args.term_id + */ +export function Error( { clientId, status, term_id: termId, ...props } ) { + const { selectBlock } = useDispatch( 'core/block-editor' ); + const { review_link: reviewLink } = useSelect( ( select ) => select( 'core/editor' )?.getEditedPostAttribute( AMP_VALIDITY_REST_FIELD_NAME ), [] ) || {}; + const reviewed = status === VALIDATION_ERROR_ACK_ACCEPTED_STATUS || status === VALIDATION_ERROR_ACK_REJECTED_STATUS; + + const { blockType } = useSelect( ( select ) => { + const blockDetails = clientId ? select( 'core/block-editor' ).getBlock( clientId ) : null; + const blockTypeDetails = blockDetails ? select( 'core/blocks' ).getBlockType( blockDetails.name ) : null; + + return { + blockType: blockTypeDetails, + }; + }, [ clientId ] ); + + const detailsUrl = new URL( reviewLink ); + detailsUrl.hash = `#tag-${ termId }`; + + return ( +
  • + + } + initialOpen={ false } + > + + +
    + { clientId && ( + + ) } + + { __( 'View details', 'amp' ) } + +
    + +
    +
  • + ); +} +Error.propTypes = { + clientId: PropTypes.string, + status: PropTypes.number.isRequired, + term_id: PropTypes.number.isRequired, +}; diff --git a/assets/src/block-validation/error/test/error.js b/assets/src/block-validation/error/test/error.js new file mode 100644 index 00000000000..a2993fabf8e --- /dev/null +++ b/assets/src/block-validation/error/test/error.js @@ -0,0 +1,394 @@ +/** + * External dependencies + */ +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'; + +/** + * Internal dependencies + */ +import { Error } from '..'; +import { createStore } from '../../store'; + +let container, pluginBlock, themeBlock, coreBlock, unknownBlock; + +const TEST_PLUGIN_BLOCK = 'my-plugin/test-block'; +const TEST_MU_PLUGIN_BLOCK = 'my-mu-plugin/test-block'; +const TEST_THEME_BLOCK = 'my-theme/test-block'; +const TEST_CORE_BLOCK = 'core/test-block'; +const TEST_UNKNOWN_BLOCK = 'unknown/test-block'; + +global.URL = class {}; + +registerBlockType( TEST_PLUGIN_BLOCK, { + attributes: {}, + save: noop, + category: 'widgets', + title: 'test plugin block', +} ); + +registerBlockType( TEST_MU_PLUGIN_BLOCK, { + attributes: {}, + save: noop, + category: 'widgets', + title: 'test mu-plugin block', +} ); + +registerBlockType( TEST_THEME_BLOCK, { + attributes: {}, + save: noop, + category: 'widgets', + title: 'test theme block', +} ); + +registerBlockType( TEST_CORE_BLOCK, { + attributes: {}, + save: noop, + category: 'widgets', + title: 'test core block', +} ); + +registerBlockType( TEST_UNKNOWN_BLOCK, { + attributes: {}, + save: noop, + category: 'widgets', + title: 'test unknown block', +} ); + +function createTestStoreAndBlocks() { + pluginBlock = createBlock( TEST_PLUGIN_BLOCK, {} ); + themeBlock = createBlock( TEST_THEME_BLOCK, {} ); + coreBlock = createBlock( TEST_CORE_BLOCK, {} ); + unknownBlock = createBlock( TEST_UNKNOWN_BLOCK, {} ); + + dispatch( 'core/block-editor' ).insertBlocks( [ pluginBlock, themeBlock, coreBlock, unknownBlock ] ); + + createStore( { + validationErrors: [ + { + clientId: pluginBlock.clientId, + code: 'DISALLOWED_TAG', + status: 3, + term_id: 12, + title: 'Invalid script: jquery.js', + error: { + type: 'js_error', + sources: [], + }, + }, + { + clientId: themeBlock.clientId, + code: 'DISALLOWED_TAG', + status: 3, + term_id: 12, + title: 'Invalid script: jquery.js', + error: { + type: 'js_error', + }, + }, + { + clientId: coreBlock.clientId, + code: 'DISALLOWED_TAG', + status: 3, + term_id: 12, + title: 'Invalid script: jquery.js', + error: { + type: 'js_error', + sources: [], + }, + }, + { + clientId: unknownBlock.clientId, + code: 'DISALLOWED_TAG', + status: 3, + term_id: 12, + title: 'Invalid script: jquery.js', + error: { + type: 'js_error', + sources: [], + }, + }, + ], + } ); +} + +function getTestBlock( type ) { + switch ( type ) { + case 'plugin': + return pluginBlock; + + case 'theme': + return themeBlock; + + case 'core': + return coreBlock; + + case 'unknown': + return unknownBlock; + + default: + return null; + } +} + +describe( 'Error', () => { + beforeAll( () => { + createTestStoreAndBlocks(); + } ); + + beforeEach( () => { + container = document.createElement( 'ul' ); + document.body.appendChild( container ); + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it.each( [ + VALIDATION_ERROR_ACK_ACCEPTED_STATUS, + VALIDATION_ERROR_ACK_REJECTED_STATUS, + VALIDATION_ERROR_NEW_ACCEPTED_STATUS, + VALIDATION_ERROR_NEW_REJECTED_STATUS, + ].map( ( status ) => [ + status, + () => ( + + ), + ] ) )( 'errors with no associated blocks work correctly', ( status, ErrorComponent ) => { + act( () => { + render( + , + container, + ); + } ); + + const newReviewed = [ VALIDATION_ERROR_NEW_REJECTED_STATUS, VALIDATION_ERROR_NEW_ACCEPTED_STATUS ].includes( status ) ? 'new' : 'reviewed'; + + expect( container.querySelector( 'li' ).getAttribute( 'class' ) ).toBe( 'amp-error-container' ); + expect( container.querySelectorAll( `.amp-error--${ newReviewed }` ) ).toHaveLength( 1 ); + expect( container.querySelector( '.amp-error__details-link' ) ).toBeNull(); + expect( container.querySelector( `.amp-error--${ newReviewed } button` ) ).not.toBeNull(); + expect( container.querySelector( '.amp-error__block-type-icon' ) ).toBeNull(); + + container.querySelector( `.amp-error--${ newReviewed } button` ).click(); + expect( container.querySelector( '.amp-error__details-link' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-error__select-block' ) ).toBeNull(); + } ); + + it.each( [ + VALIDATION_ERROR_ACK_ACCEPTED_STATUS, + VALIDATION_ERROR_ACK_REJECTED_STATUS, + VALIDATION_ERROR_NEW_ACCEPTED_STATUS, + VALIDATION_ERROR_NEW_REJECTED_STATUS, + ].map( ( status ) => [ + status, + () => ( + + ), + ] ) )( 'errors with associated blocks work correctly', ( status, ErrorComponent ) => { + act( () => { + render( + , + container, + ); + } ); + + const newReviewed = [ VALIDATION_ERROR_NEW_REJECTED_STATUS, VALIDATION_ERROR_NEW_ACCEPTED_STATUS ].includes( status ) ? 'new' : 'reviewed'; + + expect( container.querySelector( 'li' ).getAttribute( 'class' ) ).toBe( 'amp-error-container' ); + expect( container.querySelectorAll( `.amp-error--${ newReviewed }` ) ).toHaveLength( 1 ); + expect( container.querySelector( '.amp-error__details-link' ) ).toBeNull(); + expect( container.querySelector( `.amp-error--${ newReviewed } button` ) ).not.toBeNull(); + expect( container.querySelector( '.amp-error__block-type-icon' ) ).not.toBeNull(); + + container.querySelector( `.amp-error--${ newReviewed } button` ).click(); + expect( container.querySelector( '.amp-error__details-link' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-error__select-block' ) ).not.toBeNull(); + } ); +} ); + +describe( 'ErrorTypeIcon', () => { + beforeEach( () => { + container = document.createElement( 'ul' ); + document.body.appendChild( container ); + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it.each( + [ + 'js_error', + 'html_attribute_error', + 'html_element_error', + 'css_error', + 'unknown_error', + ], + )( 'shows the correct error icon', ( errorType ) => { + act( () => { + render( + , + container, + ); + } ); + + let expectedClass; + switch ( errorType ) { + case 'html_attribute_error': + expectedClass = '.amp-error__error-type-icon--html-attribute-error'; + break; + + case 'html_element_error': + expectedClass = '.amp-error__error-type-icon--html-element-error'; + break; + + case 'js_error': + expectedClass = '.amp-error__error-type-icon--js-error'; + break; + + case 'css_error': + expectedClass = '.amp-error__error-type-icon--css-error'; + break; + + default: + expectedClass = null; + } + + if ( ! expectedClass ) { + expect( container.querySelector( 'svg[class^=amp-error__error-type-icon]' ) ).toBeNull(); + } else { + expect( container.querySelector( expectedClass ) ).not.toBeNull(); + } + } ); +} ); + +describe( 'ErrorContent', () => { + beforeAll( () => { + createTestStoreAndBlocks(); + } ); + + beforeEach( () => { + container = document.createElement( 'ul' ); + document.body.appendChild( container ); + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it.each( [ + null, + 'plugin', + 'mu-plugin', + 'theme', + 'core', + ].reduce( + ( collection, testBlockSource ) => [ + ...collection, + ...[ + VALIDATION_ERROR_ACK_ACCEPTED_STATUS, + VALIDATION_ERROR_ACK_REJECTED_STATUS, + VALIDATION_ERROR_NEW_ACCEPTED_STATUS, + VALIDATION_ERROR_NEW_REJECTED_STATUS, + ].map( + ( status ) => [ testBlockSource, status ], + ), + ], + [], + ) )( 'shows expected content based on whether or not the error has an associated block', ( testBlockSource, status ) => { + const clientId = getTestBlock( testBlockSource )?.clientId || null; + + render( + , + container, + ); + + container.querySelector( `.components-button` ).click(); + + expect( container.innerHTML ).toContain( 'Markup status' ); + + if ( null === clientId ) { + expect( container.innerHTML ).toContain( 'outside the post content' ); + return; + } + + expect( container.innerHTML ).toContain( '
    Source' ); + expect( container.innerHTML ).not.toContain( 'outside the post content' ); + + switch ( testBlockSource ) { + case 'plugin': + expect( container.innerHTML ).toContain( 'test plugin block' ); + expect( container.innerHTML ).toContain( 'My plugin (plugin)' ); + break; + + case 'mu-plugin': + expect( container.innerHTML ).toContain( 'test mu-plugin block' ); + expect( container.innerHTML ).toContain( 'My MU plugin (must-use plugin)' ); + break; + + case 'theme': + expect( container.innerHTML ).toContain( 'test theme block' ); + expect( container.innerHTML ).toContain( 'My theme (theme)' ); + break; + + case 'core': + expect( container.innerHTML ).toContain( 'test core block' ); + expect( container.innerHTML ).toContain( '
    WordPress core' ); + break; + + default: + break; + } + + expect( container.innerHTML ).toContain( + [ VALIDATION_ERROR_NEW_ACCEPTED_STATUS, VALIDATION_ERROR_ACK_ACCEPTED_STATUS ].includes( status ) ? 'Removed' : 'Kept', + ); + + expect( container.innerHTML ).not.toContain( + [ VALIDATION_ERROR_ACK_REJECTED_STATUS, VALIDATION_ERROR_NEW_REJECTED_STATUS ].includes( status ) ? 'Removed' : 'Kept', + ); + + container.querySelector( '.amp-error__select-block' ).click(); + expect( select( 'core/block-editor' ).getSelectedBlock().clientId ).toBe( clientId ); + } ); +} ); diff --git a/assets/src/block-validation/error/test/get-error-source-title.js b/assets/src/block-validation/error/test/get-error-source-title.js new file mode 100644 index 00000000000..a40b625e162 --- /dev/null +++ b/assets/src/block-validation/error/test/get-error-source-title.js @@ -0,0 +1,91 @@ +/** + * Internal dependencies + */ +import { getErrorSourceTitle } from '../get-error-source-title'; + +describe( 'getErrorSorceTitle', () => { + it( 'returns an empty string if nothing is passed', () => { + expect( getErrorSourceTitle( [] ) ).toBe( '' ); + } ); + + it( 'returns a plugin name if one plugin matches', () => { + expect( getErrorSourceTitle( [ + { + type: 'plugin', + name: 'test-plugin', + }, + ] ) ).toBe( 'Test plugin' ); + + expect( getErrorSourceTitle( [ + { + type: 'mu-plugin', + name: 'test-mu-plugin', + }, + ] ) ).toBe( 'Test MU plugin' ); + } ); + + it( 'returns generic text with count if multiple plugins', () => { + expect( getErrorSourceTitle( [ + { + type: 'plugin', + name: 'test-plugin', + }, + { + type: 'plugin', + name: 'test-plugin-2', + }, + { + type: 'mu-plugin', + name: 'test-mu-plugin', + }, + { + type: 'mu-plugin', + name: 'test-mu-plugin-2', + }, + ] ) ).toBe( 'Plugins (2), Must-use plugins (2)' ); + } ); + + it( 'returns theme name if theme is source', () => { + expect( getErrorSourceTitle( + [ + { + type: 'theme', + name: 'test-theme', + }, + ], + ) ).toBe( 'Test theme' ); + } ); + + it( 'returns inactive theme if inactive theme is source', () => { + expect( getErrorSourceTitle( + [ + { + type: 'theme', + name: 'test-other-theme', + }, + ], + ) ).toBe( 'Inactive theme(s)' ); + } ); + + it( 'returns Embed for embed', () => { + expect( getErrorSourceTitle( [ + { + type: 'embed', + name: 'test-theme', + }, + ] ) ).toBe( 'Embed' ); + } ); + + it( 'returns Core for core', () => { + expect( getErrorSourceTitle( [ + { + type: 'unknown-type', + name: 'core source', + }, + { + type: 'core', + name: 'test-theme', + }, + ] ) ).toBe( 'Core' ); + } ); +} ); diff --git a/assets/src/block-validation/helpers/index.js b/assets/src/block-validation/helpers/index.js deleted file mode 100644 index 0da0e2de445..00000000000 --- a/assets/src/block-validation/helpers/index.js +++ /dev/null @@ -1,243 +0,0 @@ -/** - * External dependencies - */ -import { isEqual } from 'lodash'; - -/** - * WordPress dependencies - */ -import { dispatch, select } from '@wordpress/data'; -import { __, _n, sprintf } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import { - VALIDATION_ERROR_ACK_ACCEPTED_STATUS, - VALIDATION_ERROR_ACK_REJECTED_STATUS, - VALIDATION_ERROR_NEW_REJECTED_STATUS, - AMP_VALIDATION_ERROR_NOTICE_ID, - AMP_VALIDITY_REST_FIELD_NAME, -} from '../constants'; - -export const removeValidationErrorNotice = () => { - const { getNotices } = select( 'core/notices' ); - const { removeNotice } = dispatch( 'core/notices' ); - - if ( getNotices().filter( ( { id } ) => id === AMP_VALIDATION_ERROR_NOTICE_ID ) ) { - removeNotice( AMP_VALIDATION_ERROR_NOTICE_ID ); - } -}; - -let previousValidationErrors = []; - -export const maybeResetValidationErrors = () => { - const { getValidationErrors } = select( 'amp/block-validation' ); - const { resetValidationErrors } = dispatch( 'amp/block-validation' ); - - if ( getValidationErrors().length > 0 ) { - resetValidationErrors(); - removeValidationErrorNotice(); - previousValidationErrors = []; - } -}; - -/** - * Update blocks' validation errors in the store. - */ -export const updateValidationErrors = () => { - const { getBlockCount, getClientIdsWithDescendants, getBlock } = select( 'core/block-editor' ); - const { resetValidationErrors, addValidationError, updateReviewLink } = dispatch( 'amp/block-validation' ); - - if ( 0 === getBlockCount() ) { - return; - } - - const { getCurrentPost } = select( 'core/editor' ); - - const currentPost = getCurrentPost(); - - /** - * @param {Object} ampValidity AMP validation result object. - * @param {Object[]} ampValidity.results AMP validation results. - * @param {string} ampValidity.review_link URL for reviewing validation error details. - */ - const ampValidity = currentPost[ AMP_VALIDITY_REST_FIELD_NAME ] || {}; - - if ( ! ampValidity.results || ! ampValidity.review_link ) { - return; - } - - /** - * @param {Object} result Validation error result. - * @param {Object} result.error Error object. - * @param {string} result.title Error title. - * @param {boolean} result.forced Whether sanitization was forced. - * @param {boolean} result.sanitized Whether the error has been sanitized or not. - * @param {number} result.status Validation error status. - * @param {number} result.term_status Error status. - */ - const validationErrors = ampValidity.results.filter( ( result ) => { - return result.term_status !== VALIDATION_ERROR_ACK_ACCEPTED_STATUS; // If not accepted by the user. - } ).map( ( { error, status, title } ) => ( { ...error, status, title } ) ); // Merge status into error since needed in maybeDisplayNotice. - - if ( isEqual( validationErrors, previousValidationErrors ) ) { - return; - } - - previousValidationErrors = validationErrors; - resetValidationErrors(); - - if ( 0 === validationErrors.length ) { - removeValidationErrorNotice(); - - return; - } - - updateReviewLink( ampValidity.review_link ); - - const blockOrder = getClientIdsWithDescendants(); - - for ( const validationError of validationErrors ) { - if ( ! validationError.sources ) { - addValidationError( validationError ); - - break; - } - - let clientId; - - /** - * @param {Object} source Error source information. - * @param {string} source.block_name Name of the block associated with the error. - * @param {number} source.block_content_index The block's index in the list of blocks. - * @param {number} source.post_id ID of the post associated with the error. - */ - for ( const source of validationError.sources ) { - // Skip sources that are not for blocks. - if ( ! source.block_name || undefined === source.block_content_index || currentPost.id !== source.post_id ) { - continue; - } - - // Look up the block ID by index, assuming the blocks of content in the editor are the same as blocks rendered on frontend. - const newClientId = blockOrder[ source.block_content_index ]; - - if ( ! newClientId ) { - continue; - } - - // Sanity check that block exists for clientId. - const block = getBlock( newClientId ); - if ( ! block ) { - continue; - } - - // Check the block type in case a block is dynamically added/removed via the_content filter to cause alignment error. - if ( block.name !== source.block_name ) { - continue; - } - - clientId = newClientId; - } - - addValidationError( validationError, clientId ); - } - - maybeDisplayNotice(); -}; - -/** - * Handle state change regarding validation errors. - * - * This is essentially a JS implementation of \AMP_Validation_Manager::print_edit_form_validation_status() in PHP. - * - * @return {void} - */ -export const maybeDisplayNotice = () => { - const { getValidationErrors, getReviewLink } = select( 'amp/block-validation' ); - const { createWarningNotice } = dispatch( 'core/notices' ); - - const validationErrors = getValidationErrors(); - const validationErrorCount = validationErrors.length; - - let noticeMessage; - - noticeMessage = sprintf( - /* translators: %s: number of issues */ - _n( - 'There is %s issue from AMP validation which needs review.', - 'There are %s issues from AMP validation which need review.', - validationErrorCount, - 'amp', - ), - validationErrorCount, - ); - - const blockValidationErrors = validationErrors.filter( ( { clientId } ) => clientId ); - const blockValidationErrorCount = blockValidationErrors.length; - - if ( blockValidationErrorCount > 0 ) { - noticeMessage += ' ' + sprintf( - /* translators: %s: number of block errors. */ - _n( - '%s issue is directly due to content here.', - '%s issues are directly due to content here.', - blockValidationErrorCount, - 'amp', - ), - blockValidationErrorCount, - ); - } else if ( validationErrors.length === 1 ) { - noticeMessage += ' ' + __( 'The issue is not directly due to content here.', 'amp' ); - } else { - noticeMessage += ' ' + __( 'The issues are not directly due to content here.', 'amp' ); - } - - noticeMessage += ' '; - - const rejectedBlockValidationErrors = blockValidationErrors.filter( ( error ) => { - return ( - VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status || - VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status - ); - } ); - - const rejectedValidationErrors = validationErrors.filter( ( error ) => { - return ( - VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status || - VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status - ); - } ); - - const totalRejectedErrorsCount = rejectedBlockValidationErrors.length + rejectedValidationErrors.length; - if ( totalRejectedErrorsCount === 0 ) { - noticeMessage += __( 'The invalid markup has been automatically removed.', 'amp' ); - } else { - noticeMessage += _n( - 'You will have to remove the invalid markup (or allow the plugin to remove it) to serve AMP.', - 'You will have to remove the invalid markup (or allow the plugin to remove it) to serve AMP.', - validationErrors.length, - 'amp', - ); - } - - const options = { - id: AMP_VALIDATION_ERROR_NOTICE_ID, - }; - - const reviewLink = getReviewLink(); - - if ( reviewLink ) { - options.actions = [ - { - label: __( 'Review issues', 'amp' ), - className: 'is-link', - onClick: () => { - window.open( reviewLink, '_blank' ); - }, - }, - ]; - } - - createWarningNotice( noticeMessage, options ); -}; diff --git a/assets/src/block-validation/icon.js b/assets/src/block-validation/icon.js new file mode 100644 index 00000000000..3bfdcb4900c --- /dev/null +++ b/assets/src/block-validation/icon.js @@ -0,0 +1,77 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * Internal dependencies + */ +import AMPToolbarIcon from '../../images/amp-icon-toolbar.svg'; +import AMPToolbarIconBroken from '../../images/amp-toolbar-icon-broken.svg'; + +/** + * Plugin icon. + * + * @param {Object} props + * @param {boolean} props.hasBadge Whether the icon is showing a number. + */ +function IconSVG( { hasBadge } ) { + return ( + + + + ); +} +IconSVG.propTypes = { + hasBadge: PropTypes.bool.isRequired, +}; + +/** + * Plugin icon when AMP is broken at the URL. + * + * @param {Object} props + * @param {boolean} props.hasBadge Whether the icon is showing a number. + */ +function BrokenIconSVG( { hasBadge } ) { + return ( + + + + ); +} +BrokenIconSVG.propTypes = { + hasBadge: PropTypes.bool.isRequired, +}; + +/** + * The icon to display in the editor toolbar to toggle the editor sidebar. + * + * @param {Object} props + * @param {boolean} props.broken Whether AMP is broken at the URL. + * @param {number} props.count The number of new errors at the URL. + */ +export function ToolbarIcon( { broken = false, count } ) { + return ( +
    + { + broken ? : + } + { 0 < count && ( +
    + { count } +
    + ) } +
    + ); +} +ToolbarIcon.propTypes = { + broken: PropTypes.bool, + count: PropTypes.number.isRequired, +}; + +/** + * The icon to display in the editor more menu. + */ +export function MoreMenuIcon() { + return ; +} diff --git a/assets/src/block-validation/index.js b/assets/src/block-validation/index.js index 97efe682b79..9dbbd36bdcc 100644 --- a/assets/src/block-validation/index.js +++ b/assets/src/block-validation/index.js @@ -1,37 +1,69 @@ -/** - * Validates blocks for AMP compatibility. - * - * This uses the REST API response from saving a page to find validation errors. - * If one exists for a block, it display it inline with a Notice component. - */ - /** * WordPress dependencies */ +import { registerPlugin } from '@wordpress/plugins'; +import { __ } from '@wordpress/i18n'; import { addFilter } from '@wordpress/hooks'; -import { select, subscribe } from '@wordpress/data'; +import { PluginSidebar, PluginSidebarMoreMenuItem } from '@wordpress/edit-post'; +import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ -import { isAMPEnabled } from '../block-editor/helpers'; -import { updateValidationErrors, maybeResetValidationErrors } from './helpers'; -import { withValidationErrorNotice } from './components'; -import './store'; -import '../block-editor/store'; - -const { isEditedPostDirty } = select( 'core/editor' ); - -subscribe( () => { - try { - if ( ! isEditedPostDirty() ) { - if ( ! isAMPEnabled() ) { - maybeResetValidationErrors(); - } else { - updateValidationErrors(); - } - } - } catch ( err ) {} -} ); - -addFilter( 'editor.BlockEdit', 'amp/add-notice', withValidationErrorNotice, 99 ); +import { INITIAL_STATE, createStore, BLOCK_VALIDATION_STORE_KEY } from './store'; +import { MoreMenuIcon, ToolbarIcon } from './icon'; +import { withAMPToolbarButton } from './with-amp-toolbar-button'; +import { Sidebar } from './sidebar'; +import { InvalidBlockOutline } from './invalid-block-outline'; +import { useValidationErrorStateUpdates } from './use-validation-error-state-updates'; + +export const PLUGIN_NAME = 'amp-block-validation'; +export const SIDEBAR_NAME = 'amp-editor-sidebar'; +export const PLUGIN_TITLE = __( 'AMP for WordPress', 'amp' ); + +createStore( INITIAL_STATE ); + +/** + * Provides a dedicated sidebar for the plugin, with toggle buttons in the editor toolbar and more menu. + */ +function AMPBlockValidation() { + const { broken, errorCount } = useSelect( ( select ) => ( { + broken: select( BLOCK_VALIDATION_STORE_KEY ).getAMPCompatibilityBroken(), + errorCount: select( BLOCK_VALIDATION_STORE_KEY ).getUnreviewedValidationErrors()?.length || 0, + } ), [] ); + + useValidationErrorStateUpdates(); + + return ( + <> + } + target={ SIDEBAR_NAME } + > + { PLUGIN_TITLE } + + + ) } + name={ SIDEBAR_NAME } + title={ PLUGIN_TITLE } + > + + + + + + ); +} + +registerPlugin( + PLUGIN_NAME, + { + icon: MoreMenuIcon, + render: AMPBlockValidation, + }, +); + +addFilter( 'editor.BlockEdit', 'ampBlockValidation/filterEdit', withAMPToolbarButton, -99 ); diff --git a/assets/src/block-validation/invalid-block-outline.js b/assets/src/block-validation/invalid-block-outline.js new file mode 100644 index 00000000000..778b30ba81b --- /dev/null +++ b/assets/src/block-validation/invalid-block-outline.js @@ -0,0 +1,42 @@ +/** + * WordPress dependencies + */ +import { useSelect } from '@wordpress/data'; +import { useMemo } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { BLOCK_VALIDATION_STORE_KEY } from './store'; + +/** + * Adds a style rule for all blocks with validation errors. + */ +export function InvalidBlockOutline() { + const validationErrors = useSelect( ( select ) => select( BLOCK_VALIDATION_STORE_KEY ).getUnreviewedValidationErrors(), [] ); + + const selectors = useMemo( () => { + const clientIds = validationErrors.map( ( { clientId } ) => clientId ) + .filter( ( clientId ) => clientId ); + + return clientIds.map( ( clientId ) => `#block-${ clientId }::before` ); + }, [ validationErrors ] ); + + return ( + + ); +} diff --git a/assets/src/block-validation/sidebar.js b/assets/src/block-validation/sidebar.js new file mode 100644 index 00000000000..4ef8a284e94 --- /dev/null +++ b/assets/src/block-validation/sidebar.js @@ -0,0 +1,158 @@ +/** + * WordPress dependencies + */ +import { ToggleControl, PanelBody, ExternalLink } from '@wordpress/components'; +import { __ } from '@wordpress/i18n'; +import { useDispatch, useSelect } from '@wordpress/data'; +import { useEffect } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import './style.css'; +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 { AMP_VALIDITY_REST_FIELD_NAME } from './constants'; + +/** + * Editor sidebar. + */ +export function Sidebar() { + const { setIsShowingReviewed } = useDispatch( BLOCK_VALIDATION_STORE_KEY ); + + const { ampCompatibilityBroken, isShowingReviewed, status, reviewLink } = useSelect( ( select ) => ( { + ampCompatibilityBroken: select( BLOCK_VALIDATION_STORE_KEY ).getAMPCompatibilityBroken(), + isShowingReviewed: select( BLOCK_VALIDATION_STORE_KEY ).getIsShowingReviewed(), + status: select( 'core/editor' )?.getEditedPostAttribute( 'status' ), + // eslint-disable-next-line camelcase + reviewLink: select( 'core/editor' ).getEditedPostAttribute( AMP_VALIDITY_REST_FIELD_NAME )?.review_link || null, + } ), [] ); + + const { displayedErrors, reviewedValidationErrors, unreviewedValidationErrors, validationErrors } = useSelect( ( select ) => { + let updatedDisplayedErrors; + + const updatedValidationErrors = select( BLOCK_VALIDATION_STORE_KEY ).getValidationErrors(); + const updatedReviewedValidationErrors = select( BLOCK_VALIDATION_STORE_KEY ).getReviewedValidationErrors(); + const updatedUnreviewedValidationErrors = select( BLOCK_VALIDATION_STORE_KEY ).getUnreviewedValidationErrors(); + + if ( isShowingReviewed ) { + updatedDisplayedErrors = updatedValidationErrors; + } else { + updatedDisplayedErrors = updatedUnreviewedValidationErrors; + + // If there are no unreviewed errors, we show the reviewed errors. + if ( 0 === updatedDisplayedErrors.length ) { + updatedDisplayedErrors = updatedReviewedValidationErrors; + } + } + + return { + displayedErrors: updatedDisplayedErrors, + reviewedValidationErrors: updatedReviewedValidationErrors, + unreviewedValidationErrors: updatedUnreviewedValidationErrors, + validationErrors: updatedValidationErrors, + }; + }, [ isShowingReviewed ] ); + + /** + * Focus the first focusable element when the sidebar opens. + */ + useEffect( () => { + const element = document.querySelector( '.amp-sidebar a, .amp-sidebar button, .amp-sidebar input' ); + if ( element ) { + element.focus(); + } + }, [] ); + + const saved = 'auto-draft' !== status; + + return ( +
    + { + ampCompatibilityBroken && ( +
    +
    +
    + +
    +
    +

    + { __( 'Invalid markup kept', 'amp' ) } +

    + { __( 'The permalink will not be served as valid AMP.', 'amp' ) } +
    +
    +
    + ) + } + { 0 < validationErrors.length && ( + +
    + +
    +
    +

    + { __( 'Validation Issues', 'amp' ) } +

    + +

    + { reviewLink && ( + + { __( 'View technical details', 'amp' ) } + + ) } +

    +
    + { ( 0 < reviewedValidationErrors.length && 0 < unreviewedValidationErrors.length ) && ( +
    + { + setIsShowingReviewed( newIsShowingReviewed ); + } } + /> +
    + ) } +
    + ) } + + { + ! saved && 0 === validationErrors.length && ( + +

    + { __( 'Validation issues will be checked for when the post is saved.', 'amp' ) } +

    +
    + ) + } + { saved && validationErrors.length === 0 && ( + +

    + { __( 'There are no AMP validation issues.', 'amp' ) } +

    +
    + ) } + + { 0 < validationErrors.length && ( + 0 < displayedErrors.length ? ( +
      + { displayedErrors.map( ( validationError, index ) => ( + + ) ) } +
    + ) + : saved && ( + +

    + { __( 'All AMP validation issues have been reviewed.', 'amp' ) } +

    +
    + ) + ) } + +
    + ); +} diff --git a/assets/src/block-validation/store.js b/assets/src/block-validation/store.js new file mode 100644 index 00000000000..db85324f147 --- /dev/null +++ b/assets/src/block-validation/store.js @@ -0,0 +1,85 @@ +/** + * WordPress dependencies + */ +import { registerStore } from '@wordpress/data'; + +/** + * External dependencies + */ +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'; + +export const BLOCK_VALIDATION_STORE_KEY = 'amp/block-validation'; + +const SET_IS_SHOWING_REVIEWED = 'SET_IS_SHOWING_REVIEWED'; +const SET_VALIDATION_ERRORS = 'SET_VALIDATION_ERRORS'; + +export const INITIAL_STATE = { + ampCompatibilityBroken: false, + isShowingReviewed: false, + rawValidationErrors: [], + reviewLink: null, + reviewedValidationErrors: [], + unreviewedValidationErrors: [], + validationErrors: [], +}; + +/** + * Register the store for block validation. + * + * @param {Object} initialState Initial store state. + */ +export function createStore( initialState ) { + registerStore( + BLOCK_VALIDATION_STORE_KEY, + { + reducer: ( state = initialState, action ) => { + switch ( action.type ) { + case SET_IS_SHOWING_REVIEWED: + return { ...state, isShowingReviewed: action.isShowingReviewed }; + + case SET_VALIDATION_ERRORS: + return { + ...state, + ampCompatibilityBroken: Boolean( + action.validationErrors.filter( ( { status } ) => + status === VALIDATION_ERROR_NEW_REJECTED_STATUS || status === VALIDATION_ERROR_ACK_REJECTED_STATUS, + )?.length, + ), + + reviewedValidationErrors: action.validationErrors + .filter( ( { status } ) => + status === VALIDATION_ERROR_ACK_ACCEPTED_STATUS || status === VALIDATION_ERROR_ACK_REJECTED_STATUS, + ), + + unreviewedValidationErrors: action.validationErrors + .filter( ( { status } ) => + status === VALIDATION_ERROR_NEW_ACCEPTED_STATUS || status === VALIDATION_ERROR_NEW_REJECTED_STATUS, + ), + + validationErrors: action.validationErrors, + }; + + default: + return state; + } + }, + actions: { + setIsShowingReviewed: ( isShowingReviewed ) => ( { type: SET_IS_SHOWING_REVIEWED, isShowingReviewed } ), + setValidationErrors: ( validationErrors ) => ( { type: SET_VALIDATION_ERRORS, validationErrors } ), + }, + selectors: { + getAMPCompatibilityBroken: ( { ampCompatibilityBroken } ) => ampCompatibilityBroken, + getIsShowingReviewed: ( { isShowingReviewed } ) => isShowingReviewed, + getValidationErrors: ( { validationErrors } ) => validationErrors, + getReviewedValidationErrors: ( { reviewedValidationErrors } ) => reviewedValidationErrors, + getUnreviewedValidationErrors: ( { unreviewedValidationErrors } ) => unreviewedValidationErrors, + }, + initialState, + }, + ); +} diff --git a/assets/src/block-validation/store/actions.js b/assets/src/block-validation/store/actions.js deleted file mode 100644 index e038cd5cda8..00000000000 --- a/assets/src/block-validation/store/actions.js +++ /dev/null @@ -1,40 +0,0 @@ -/** - * Returns an action object in signalling that a validation error should be added. - * - * @param {Object} error Validation error. - * @param {?string} clientId Optional. Block client ID. Used when the validation error is specific to a block. - * - * @return {Object} Action object. - */ -export function addValidationError( error, clientId ) { - return { - type: 'ADD_VALIDATION_ERROR', - error, - clientId, - }; -} - -/** - * Returns an action object in signalling that validation errors should be reset. - * - * @return {Object} Action object. - */ -export function resetValidationErrors() { - return { - type: 'RESET_VALIDATION_ERRORS', - }; -} - -/** - * Returns an action object in signalling that the review URL should be updated. - * - * @param {string} url Issue review URL. - * - * @return {Object} Action object. - */ -export function updateReviewLink( url ) { - return { - type: 'UPDATE_REVIEW_LINK', - url, - }; -} diff --git a/assets/src/block-validation/store/index.js b/assets/src/block-validation/store/index.js deleted file mode 100644 index f8e5c6bd49c..00000000000 --- a/assets/src/block-validation/store/index.js +++ /dev/null @@ -1,30 +0,0 @@ -/** - * WordPress dependencies - */ -import { registerStore } from '@wordpress/data'; - -/** - * Internal dependencies - */ -import reducer from './reducer'; -import * as actions from './actions'; -import * as selectors from './selectors'; - -/** - * Module Constants - */ -const MODULE_KEY = 'amp/block-validation'; - -export default registerStore( - MODULE_KEY, - { - reducer, - selectors, - actions, - initialState: { - ...window.ampBlockValidation, - errors: [], - reviewLink: undefined, - }, - }, -); diff --git a/assets/src/block-validation/store/reducer.js b/assets/src/block-validation/store/reducer.js deleted file mode 100644 index c1360096b8e..00000000000 --- a/assets/src/block-validation/store/reducer.js +++ /dev/null @@ -1,39 +0,0 @@ -/** - * Reducer handling changes related to block validation. - * - * @param {Object} state Current state. - * @param {Object} action Dispatched action. - * - * @return {Object} Updated state. - */ -export default ( state = undefined, action ) => { - const { type, url, error, clientId } = action; - - switch ( type ) { - case 'ADD_VALIDATION_ERROR': - const errors = state ? state.errors : []; - const enhancedError = { - ...error, - clientId, - }; - - return { - ...state, - errors: [ ...errors, enhancedError ], - }; - case 'RESET_VALIDATION_ERRORS': - return { - ...state, - errors: [], - }; - - case 'UPDATE_REVIEW_LINK': - return { - ...state, - reviewLink: url, - }; - - default: - return state; - } -}; diff --git a/assets/src/block-validation/store/selectors.js b/assets/src/block-validation/store/selectors.js deleted file mode 100644 index eeb7da837e7..00000000000 --- a/assets/src/block-validation/store/selectors.js +++ /dev/null @@ -1,46 +0,0 @@ -/** - * Returns general validation errors. - * - * @param {Object} state Editor state. - * - * @return {Array} Validation errors. - */ -export function getValidationErrors( state ) { - return state.errors; -} - -/** - * Returns the block validation errors for a given clientId. - * - * @param {Object} state Editor state. - * @param {string} clientId Block client ID. - * - * @return {Array} Block validation errors. - */ -export function getBlockValidationErrors( state, clientId ) { - return state.errors.filter( ( error ) => error.clientId === clientId ); -} - -/** - * Returns the URL for reviewing validation issues. - * - * @param {Object} state Editor state. - * - * @return {string} Validation errors review link. - */ -export function getReviewLink( state ) { - return state.reviewLink; -} - -/** - * Returns whether sanitization errors are auto-accepted. - * - * Auto-acceptance is from either checking 'Automatically accept sanitization...' or from being in Standard mode. - * - * @param {Object} state Editor state. - * - * @return {boolean} Whether sanitization errors are auto-accepted. - */ -export function isSanitizationAutoAccepted( state ) { - return Boolean( state.isSanitizationAutoAccepted ); -} diff --git a/assets/src/block-validation/store/test/actions.js b/assets/src/block-validation/store/test/actions.js deleted file mode 100644 index 11d79a4cda7..00000000000 --- a/assets/src/block-validation/store/test/actions.js +++ /dev/null @@ -1,57 +0,0 @@ -/** - * Internal dependencies - */ -import { - addValidationError, - resetValidationErrors, - updateReviewLink, -} from '../actions'; - -describe( 'actions', () => { - describe( 'addValidationError', () => { - it( 'should return the ADD_VALIDATION_ERROR action', () => { - const error = { foo: 'bar' }; - - const result = addValidationError( error ); - expect( result ).toStrictEqual( { - type: 'ADD_VALIDATION_ERROR', - error, - clientId: undefined, - } ); - } ); - - it( 'should return the ADD_VALIDATION_ERROR action with a clientId', () => { - const clientId = 'foo'; - const error = { foo: 'bar' }; - - const result = addValidationError( error, clientId ); - expect( result ).toStrictEqual( { - type: 'ADD_VALIDATION_ERROR', - error, - clientId, - } ); - } ); - } ); - - describe( 'resetValidationErrors', () => { - it( 'should return the RESET_VALIDATION_ERRORS action', () => { - const result = resetValidationErrors(); - - expect( result ).toStrictEqual( { - type: 'RESET_VALIDATION_ERRORS', - } ); - } ); - } ); - - describe( 'updateReviewLink', () => { - it( 'should return the UPDATE_REVIEW_LINK action', () => { - const url = 'https://example.com/'; - const result = updateReviewLink( url ); - - expect( result ).toStrictEqual( { - type: 'UPDATE_REVIEW_LINK', - url, - } ); - } ); - } ); -} ); diff --git a/assets/src/block-validation/store/test/reducer.js b/assets/src/block-validation/store/test/reducer.js deleted file mode 100644 index c3a2dbda30b..00000000000 --- a/assets/src/block-validation/store/test/reducer.js +++ /dev/null @@ -1,51 +0,0 @@ -/** - * WordPress dependencies - */ -import '@wordpress/block-editor'; - -/** - * Internal dependencies - */ -import reducer from '../reducer'; - -describe( 'reducer', () => { - it( 'should add new validation error', () => { - const clientId = 'foo'; - const error = { bar: 'baz' }; - - const state = reducer( undefined, { - type: 'ADD_VALIDATION_ERROR', - error, - clientId, - } ); - - expect( state ).toStrictEqual( { - errors: [ - { ...error, clientId }, - ], - } ); - } ); - - it( 'should reset validation errors', () => { - const initialState = { errors: [ { foo: 'bar' }, { bar: 'baz' } ] }; - - const state = reducer( initialState, { - type: 'RESET_VALIDATION_ERRORS', - } ); - - expect( state ).toStrictEqual( { - errors: [], - } ); - } ); - - it( 'should update the review link', () => { - const state = reducer( undefined, { - type: 'UPDATE_REVIEW_LINK', - url: 'https://example.com', - } ); - - expect( state ).toStrictEqual( { - reviewLink: 'https://example.com', - } ); - } ); -} ); diff --git a/assets/src/block-validation/store/test/selectors.js b/assets/src/block-validation/store/test/selectors.js deleted file mode 100644 index 422eb98752c..00000000000 --- a/assets/src/block-validation/store/test/selectors.js +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Internal dependencies - */ -import { - getValidationErrors, - getBlockValidationErrors, - getReviewLink, - isSanitizationAutoAccepted, -} from '../selectors'; - -describe( 'selectors', () => { - describe( 'getValidationErrors', () => { - it( 'should return a list of validation errors', () => { - const errors = [ { foo: 'bar' }, { bar: 'baz' } ]; - - const state = { - errors, - }; - - expect( getValidationErrors( state ) ).toStrictEqual( errors ); - } ); - } ); - - describe( 'getBlockValidationErrors', () => { - it( 'should return a list of block validation errors', () => { - const errors = [ { foo: 'bar' }, { bar: 'baz' }, { baz: 'boo', clientId: 'foo' } ]; - - const state = { - errors, - }; - - expect( getBlockValidationErrors( state, 'foo' ) ).toStrictEqual( [ { baz: 'boo', clientId: 'foo' } ] ); - } ); - } ); - - describe( 'getReviewLink', () => { - it( 'should return the validation errors review link', () => { - const state = { reviewLink: 'https://example.com' }; - - expect( getReviewLink( state ) ).toStrictEqual( 'https://example.com' ); - } ); - } ); - - describe( 'isSanitizationAutoAccepted', () => { - it( 'should return a boolean', () => { - const state = { isSanitizationAutoAccepted: '1' }; - - expect( isSanitizationAutoAccepted( state ) ).toStrictEqual( true ); - } ); - } ); -} ); diff --git a/assets/src/block-validation/style.css b/assets/src/block-validation/style.css new file mode 100644 index 00000000000..c1d3c7373fa --- /dev/null +++ b/assets/src/block-validation/style.css @@ -0,0 +1,305 @@ +.components-button[aria-label="AMP for WordPress"], +.components-button[aria-label="AMP for WordPress"]:hover, +.components-button[aria-label="AMP for WordPress"]:focus { + color: transparent; +} + +.interface-pinned-items .components-button.has-icon[aria-label="AMP for WordPress"] { + padding: 0; + position: relative; +} + +.amp-plugin-icon { + align-items: center; + display: flex; + height: 100%; + justify-content: center; + width: 100%; +} + +.amp-toolbar-icon svg { + fill: #000; +} + +.amp-plugin-icon--has-badge svg { + margin-left: -5px; +} + +.amp-toolbar-broken-icon svg { + fill: none; +} + +.amp-error-count-badge { + align-items: center; + background: #bb522e; + border: 2px solid transparent; + border-radius: 8px; + color: #fff; + display: inline-flex; + flex-shrink: 0; + font-size: 10px; + height: 16px; + justify-content: center; + width: 16px; +} + +.amp-error-count-badge { + display: flex; + position: absolute; + right: 1px; + top: 2px; +} + +.is-pressed .amp-error-count-badge { + border-color: #1e1e1e; +} + +.is-pressed .amp-plugin-icon:not(.amp-plugin-icon--broken) svg { + fill: #fff; +} + +.amp-block-validation-sidebar { + border-bottom-width: 0; +} + +.amp-sidebar__validation-errors-icon { + float: left; + margin-right: 1rem; +} + +.amp-sidebar > .components-panel__body { + border-bottom-width: 0; + border-top-width: 0; + padding-bottom: 1rem; + padding-top: 1rem; +} + +.amp-sidebar__description-panel { + border-bottom: 2px solid #e3e4e7; +} + +.amp-sidebar__broken-container { + background: #f4f4f4; + padding: 18px 13px; +} + +.amp-sidebar__broken { + background: #fff; + border-radius: 15px; + display: flex; + padding: 10px; +} + +.amp-sidebar__broken h3 { + margin-bottom: 0.25rem; + margin-top: 0; +} + +.amp-sidebar__validation-errors-kept-icon { + align-items: center; + background-color: #fff3ef; + border-radius: 50%; + display: flex; + flex: 0 0 auto; + height: 40px; + justify-content: center; + margin: 10px 15px 15px 0; + width: 40px; +} + +.amp-sidebar__validation-errors-kept-icon svg { + height: 25px; + width: 25px; +} + +.amp-sidebar > ul > li { + margin-bottom: 0; +} + + +.amp-sidebar > .components-panel__body h2 { + margin-top: 0; + margin-bottom: 0.375rem; +} + +.amp-sidebar > .components-panel__body p { + margin-bottom: 0; +} + +.amp-sidebar .components-panel__body-title .components-button { + border-radius: 0; +} + +.amp-sidebar > ul, +.amp-sidebar > li { + margin-bottom: 0; + margin-top: 0; +} + +.amp-sidebar > ul > li:first-of-type .components-panel__body { + border-top-width: 0; +} + +.amp-sidebar > ul > li:not(:last-of-type) .components-panel__body { + border-bottom-width: 0; +} + +.amp-sidebar__review-link { + align-items: center; + color: #111; + display: inline-flex; + font-size: 13px; +} + +.amp-sidebar__review-link svg, +.amp-error__details-link svg { + height: 15px; + margin-left: 6px; + width: 15px; +} + +.amp-sidebar__options { + margin-top: 2rem; +} + +.amp-error--new .components-panel__body-title, +.amp-error--new .components-panel__body-title:hover { + background: #fef7f1; + border-left: 4px solid #d54e21; +} + +.amp-error .components-panel__body-title, +.amp-error .components-panel__body-toggle.components-button { + font-size: 13px; + font-weight: 400; + line-height: 1.5; +} + +.amp-error > h2 > .components-button { + padding-left: 14px; + padding-right: 14px; +} + +.components-panel__body.is-opened > .components-panel__body-title { + margin-bottom: 10px; +} + +.amp-error__icons { + align-items: center; + display: flex; + padding-right: 0.5rem; +} + +.amp-error__error-type-icon { + align-items: center; + background: #707070; + border: 1px solid #707070; + border-radius: 12px; + display: flex; + height: 24px; + justify-content: center; + margin-left: -5px; + width: 24px; +} + +.amp-error__block-type-icon { + padding: 0.375rem; +} + +.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.782rem; +} + +.amp-error__title-text { + margin-right: 5px; +} + +.amp-error 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; +} + +.components-button.amp-error__select-block { + align-items: center; + display: inline-flex; + margin-right: 3px; +} + +.amp-error__actions { + display: flex; +} + +.amp-error__actions .components-button { + margin-right: 7px; +} + +.amp-error__select-block svg { + fill: none; + margin-right: 9px; +} + +.amp-error__details-link { + align-items: center; + display: flex; +} + +.amp-error__details-link svg path { + fill: currentColor; +} + +.amp-error__details dt { + float: left; + font-weight: 600; +} + +.amp-error__details dt::after { + content: ":"; + margin-right: 7px; +} + +.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; + transform: translateY(2px); + width: 24px; +} + +.amp-error__kept-removed--removed > span { + background: #d2e5e5; +} diff --git a/assets/src/block-validation/test/__data__/raw-validation-errors.js b/assets/src/block-validation/test/__data__/raw-validation-errors.js new file mode 100644 index 00000000000..faa24567db2 --- /dev/null +++ b/assets/src/block-validation/test/__data__/raw-validation-errors.js @@ -0,0 +1,236 @@ +export const rawValidationErrors = [ + { + sanitized: true, + title: 'Invalid script: jquery.js', + error: { + node_name: 'script', + parent_name: 'head', + code: 'DISALLOWED_TAG', + type: 'js_error', + node_attributes: { + src: 'https://test.site/wp-includes/js/jquery/jquery.js?ver=__normalized__', + id: 'jquery-core-js', + }, + node_type: 1, + sources: [], + }, + status: 3, + term_status: 3, + forced: false, + term_id: 12, + }, + { + sanitized: false, + title: 'Invalid attribute: bad-attr', + error: { + code: 'DISALLOWED_ATTR', + element_attributes: { + 'bad-attr': 'bad-attr', + }, + node_name: 'bad-attr', + parent_name: 'div', + type: 'html_attribute_error', + node_type: 2, + sources: [ + { + hook: 'the_content', + filter: true, + post_id: 1, + post_type: 'post', + sources: [], + }, + { + block_name: 'core/html', + post_id: 1, + block_content_index: 1, + }, + ], + }, + status: 1, + term_status: 1, + forced: false, + term_id: 27, + }, + { + sanitized: false, + title: 'Invalid element: <bad-element>', + error: { + node_name: 'bad-element', + parent_name: 'div', + code: 'DISALLOWED_TAG', + type: 'html_element_error', + node_attributes: [], + node_type: 1, + sources: [ + { + hook: 'the_content', + filter: true, + post_id: 1, + post_type: 'post', + sources: [], + }, + { + block_name: 'core/html', + post_id: 1, + block_content_index: 2, + }, + ], + }, + status: 1, + term_status: 1, + forced: false, + term_id: 28, + }, + { + sanitized: false, + title: 'Invalid inline script', + error: { + node_name: 'script', + parent_name: 'p', + code: 'DISALLOWED_TAG', + type: 'js_error', + node_attributes: [], + text: "alert('danger');", + node_type: 1, + sources: [ + { + hook: 'the_content', + filter: true, + post_id: 1, + post_type: 'post', + sources: [], + }, + { + block_name: 'core/html', + post_id: 1, + block_content_index: 3, + }, + ], + }, + status: 1, + term_status: 1, + forced: false, + term_id: 29, + }, + { + sanitized: false, + title: 'Invalid inline script', + error: { + node_name: 'script', + parent_name: 'div', + code: 'DISALLOWED_TAG', + type: 'js_error', + node_attributes: { + class: 'wp-block-bad-blocks-script', + }, + text: 'alert("hello")', + node_type: 1, + sources: [ + { + hook: 'the_content', + filter: true, + post_id: 1, + post_type: 'post', + sources: [], + }, + { + block_name: 'bad-blocks/script', + post_id: 1, + block_content_index: 10, + }, + ], + }, + status: 1, + term_status: 1, + forced: false, + term_id: 33, + }, + { + sanitized: false, + title: 'Invalid element: <unknown-element>', + error: { + node_name: 'unknown-element', + parent_name: 'div', + code: 'DISALLOWED_TAG', + type: 'html_element_error', + node_attributes: { + class: 'wp-block-bad-blocks-unknown-element', + }, + node_type: 1, + sources: [ + { + hook: 'the_content', + filter: true, + post_id: 1, + post_type: 'post', + sources: [], + }, + { + block_name: 'bad-blocks/unknown-element', + post_id: 1, + block_content_index: 11, + }, + ], + }, + status: 1, + term_status: 1, + forced: false, + term_id: 34, + }, + { + sanitized: false, + title: 'Invalid inline script', + error: { + node_name: 'script', + parent_name: 'body', + code: 'DISALLOWED_TAG', + type: 'js_error', + node_attributes: [], + text: "\n\t\t( function ( body ) {\n\t\t\t'use strict';\n\t\t\tbody.className = body.className.replace( /\\bTest-no-js\\b/, 'Test-js' );\n\t\t} )( document.body );\n\t\t", + node_type: 1, + sources: [ + { + type: 'plugin', + name: 'the-test-calendar', + file: 'common/src/Test/Main.php', + line: 401, + function: 'Test__Main::toggle_js_class', + hook: 'wp_footer', + priority: 10, + }, + ], + }, + status: 2, + term_status: 2, + forced: false, + term_id: 10, + }, + { + sanitized: true, + title: 'Invalid inline script', + error: { + node_name: 'script', + parent_name: 'body', + code: 'DISALLOWED_TAG', + type: 'js_error', + node_attributes: [], + text: ' /* */ ', + node_type: 1, + sources: [ + { + type: 'plugin', + name: 'the-test-calendar', + file: 'common/src/Test/Asset/Data.php', + line: 54, + function: 'Test__Asset__Data::render_json', + hook: 'wp_footer', + priority: 10, + }, + ], + }, + status: 3, + term_status: 3, + forced: false, + term_id: 11, + }, +]; diff --git a/assets/src/block-validation/test/add-toolbar-button-to-block.js b/assets/src/block-validation/test/add-toolbar-button-to-block.js new file mode 100644 index 00000000000..769ee2c1c7e --- /dev/null +++ b/assets/src/block-validation/test/add-toolbar-button-to-block.js @@ -0,0 +1,195 @@ +/** + * External dependencies + */ +import { act } from 'react-dom/test-utils'; +import { noop } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component, render } from '@wordpress/element'; +import { dispatch } from '@wordpress/data'; +import { registerBlockType, createBlock } from '@wordpress/blocks'; +import '@wordpress/block-editor'; // Block editor data store needed. + +/** + * Internal dependencies + */ +import { createStore } from '../store'; +import { withAMPToolbarButton } from '../with-amp-toolbar-button'; + +let container, block; +let toolbarButtonWasRendered = false; + +const TEST_BLOCK = 'my-plugin/test-block'; + +jest.mock( '../amp-toolbar-button', () => ( { + AMPToolbarButton() { + toolbarButtonWasRendered = true; + + return null; + } } ) ); + +registerBlockType( TEST_BLOCK, { + attributes: {}, + save: noop, + category: 'widgets', + title: 'test block', +} ); + +describe( 'withAMPToolbarButton: filtering with errors', () => { + beforeAll( () => { + block = createBlock( TEST_BLOCK, {} ); + dispatch( 'core/block-editor' ).insertBlock( block ); + + createStore( { + reviewLink: 'http://review-link.test', + unreviewedValidationErrors: [ + { + clientId: block.clientId, + code: 'DISALLOWED_TAG', + status: 3, + term_id: 12, + title: 'Invalid script: jquery.js', + type: 'js_error', + }, + ], + validationErrors: [ + { + clientId: block.clientId, + code: 'DISALLOWED_TAG', + status: 3, + term_id: 12, + title: 'Invalid script: jquery.js', + type: 'js_error', + }, + ], + } ); + } ); + + beforeEach( () => { + container = document.createElement( 'ul' ); + document.body.appendChild( container ); + toolbarButtonWasRendered = false; + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it( 'is filtered correctly with a class component', () => { + class UnfilteredComponent extends Component { + render() { + return ( +
    + { '' } +
    + ); + } + } + + const FilteredComponent = withAMPToolbarButton( UnfilteredComponent ); + + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '#default-component-element' ) ).not.toBeNull(); + expect( toolbarButtonWasRendered ).toBe( true ); + } ); + + it( 'is filtered correctly with a function component', () => { + function UnfilteredComponent() { + return ( +
    + { '' } +
    + ); + } + + const FilteredComponent = withAMPToolbarButton( UnfilteredComponent ); + + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '#default-component-element' ) ).not.toBeNull(); + expect( toolbarButtonWasRendered ).toBe( true ); + } ); +} ); + +describe( 'withAMPToolbarButton: filtering without errors', () => { + beforeAll( () => { + block = createBlock( TEST_BLOCK, {} ); + dispatch( 'core/block-editor' ).insertBlock( block ); + + createStore( { + reviewLink: 'http://review-link.test', + validationErrors: [], + } ); + } ); + + beforeEach( () => { + container = document.createElement( 'ul' ); + document.body.appendChild( container ); + toolbarButtonWasRendered = false; + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it( 'is not filtered with a class component and no errors', () => { + class UnfilteredComponent extends Component { + render() { + return ( +
    + { '' } +
    + ); + } + } + + const FilteredComponent = withAMPToolbarButton( UnfilteredComponent ); + + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '#default-component-element' ) ).not.toBeNull(); + expect( toolbarButtonWasRendered ).toBe( false ); + } ); + + it( 'is not filtered with a function component and no errors', () => { + function UnfilteredComponent() { + return ( +
    + { '' } +
    + ); + } + + const FilteredComponent = withAMPToolbarButton( UnfilteredComponent ); + + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '#default-component-element' ) ).not.toBeNull(); + expect( toolbarButtonWasRendered ).toBe( false ); + } ); +} ); diff --git a/assets/src/block-validation/test/icon.js b/assets/src/block-validation/test/icon.js new file mode 100644 index 00000000000..b56ff91723e --- /dev/null +++ b/assets/src/block-validation/test/icon.js @@ -0,0 +1,96 @@ +/** + * External dependencies + */ +import { act } from 'react-dom/test-utils'; + +/** + * WordPress dependencies + */ +import { render } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { MoreMenuIcon, ToolbarIcon } from '../icon'; + +let container; + +describe( 'Icons', () => { + beforeEach( () => { + container = document.createElement( 'ul' ); + document.body.appendChild( container ); + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it( 'renders a toolbar icon without AMP broken and no badge', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-toolbar-icon' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-toolbar-icon--has-badge' ) ).toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon' ) ).toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon--has-badge' ) ).toBeNull(); + } ); + + it( 'renders a toolbar icon without AMP broken and with a badge', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-toolbar-icon' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-toolbar-icon--has-badge' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon' ) ).toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon--has-badge' ) ).toBeNull(); + } ); + + it( 'renders a toolbar icon with AMP broken and with no badge', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-toolbar-icon' ) ).toBeNull(); + expect( container.querySelector( '.amp-toolbar-icon--has-badge' ) ).toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon--has-badge' ) ).toBeNull(); + } ); + + it( 'renders a toolbar icon with AMP broken and with a badge', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-toolbar-icon' ) ).toBeNull(); + expect( container.querySelector( '.amp-toolbar-icon--has-badge' ) ).toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-toolbar-broken-icon--has-badge' ) ).not.toBeNull(); + } ); + + it( 'renders the MoreMenuIcon', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-toolbar-icon' ) ).not.toBeNull(); + expect( container.querySelector( '.amp-toolbar-icon--has-badge' ) ).toBeNull(); + } ); +} ); diff --git a/assets/src/block-validation/test/store.js b/assets/src/block-validation/test/store.js new file mode 100644 index 00000000000..11516021cec --- /dev/null +++ b/assets/src/block-validation/test/store.js @@ -0,0 +1,33 @@ +/** + * 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'; + +describe( 'Block validation data store', () => { + beforeEach( () => { + createStore( INITIAL_STATE ); + } ); + + it( 'sets and selects state correctly', () => { + dispatch( BLOCK_VALIDATION_STORE_KEY ).setIsShowingReviewed( true ); + expect( select( BLOCK_VALIDATION_STORE_KEY ).getIsShowingReviewed() ).toBe( true ); + + dispatch( BLOCK_VALIDATION_STORE_KEY ).setIsShowingReviewed( false ); + expect( select( BLOCK_VALIDATION_STORE_KEY ).getIsShowingReviewed() ).toBe( false ); + + expect( select( BLOCK_VALIDATION_STORE_KEY ).getAMPCompatibilityBroken() ).toBe( false ); + + dispatch( BLOCK_VALIDATION_STORE_KEY ).setValidationErrors( rawValidationErrors ); + + expect( select( BLOCK_VALIDATION_STORE_KEY ).getAMPCompatibilityBroken() ).toBe( true ); + expect( select( BLOCK_VALIDATION_STORE_KEY ).getValidationErrors() ).toHaveLength( 8 ); + expect( select( BLOCK_VALIDATION_STORE_KEY ).getReviewedValidationErrors() ).toHaveLength( 3 ); + expect( select( BLOCK_VALIDATION_STORE_KEY ).getUnreviewedValidationErrors() ).toHaveLength( 5 ); + } ); +} ); diff --git a/assets/src/block-validation/test/use-validation-error-state-updates.js b/assets/src/block-validation/test/use-validation-error-state-updates.js new file mode 100644 index 00000000000..9357cd35eca --- /dev/null +++ b/assets/src/block-validation/test/use-validation-error-state-updates.js @@ -0,0 +1,221 @@ +/** + * External dependencies + */ +import { act } from 'react-dom/test-utils'; + +/** + * WordPress dependencies + */ +import { render } from '@wordpress/element'; +import { select } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { convertErrorSourcesToArray, maybeAddClientIdToValidationError, useValidationErrorStateUpdates } from '../use-validation-error-state-updates'; +import { BLOCK_VALIDATION_STORE_KEY, createStore } from '../store'; + +jest.mock( '@wordpress/data/build/components/use-select', () => { + return () => ( { + blockOrder: [], + currentPost: { id: 1 }, + getBlock: () => null, + getBlocks: () => [], + validationErrorsFromPost: require( './__data__/raw-validation-errors' ).rawValidationErrors, + } ); +} ); + +createStore( { + validationErrors: [], +} ); + +let container; + +describe( 'useValidationErrorStateUpdates', () => { + beforeEach( () => { + container = document.createElement( 'div' ); + document.body.appendChild( container ); + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it( 'updates state', () => { + expect( select( BLOCK_VALIDATION_STORE_KEY ).getValidationErrors() ).toHaveLength( 0 ); + + function ComponentContainingHook() { + useValidationErrorStateUpdates(); + + return null; + } + + act( () => { + render( + , + container, + ); + } ); + + expect( select( BLOCK_VALIDATION_STORE_KEY ).getValidationErrors() ).toHaveLength( 8 ); + } ); +} ); + +describe( 'convertValidationErrorSourcesToArray', () => { + it( 'converts an object to an array', () => { + const validationError = { + error: { + sources: { + 0: 'error1', + 1: 'error2', + 3: 'error3', + }, + }, + }; + + convertErrorSourcesToArray( validationError ); + expect( validationError ).toMatchObject( + { + error: { + sources: [ 'error1', 'error2', 'error3' ], + }, + }, + ); + } ); +} ); + +describe( 'maybeAddClientIdToValidationError', () => { + it( 'does nothing if the source has no name or block index', () => { + let testValidationError = {}; + + maybeAddClientIdToValidationError( + { + validationError: testValidationError, + source: { + block_name: 'my-block', + post_id: 88, + }, + currentPostId: 88, + blockOrder: [ 'client-id-1', 'client-id-2' ], + getBlock: () => ( { name: 'my-block' } ), + }, + ); + + expect( testValidationError ).toMatchObject( {} ); + + testValidationError = {}; + maybeAddClientIdToValidationError( + { + validationError: testValidationError, + source: { + block_content_index: 1, + post_id: 88, + }, + blockOrder: [ 'client-id-1', 'client-id-2' ], + getBlock: () => ( { name: 'my-block' } ), + }, + ); + expect( testValidationError ).toMatchObject( {} ); + } ); + + it( 'does nothing if the source post ID doesn\'t match the validation error ID', () => { + const testValidationError = {}; + + maybeAddClientIdToValidationError( + { + validationError: testValidationError, + source: { + block_name: 'my-block', + block_content_index: 1, + post_id: 88, + }, + currentPostId: 77, + blockOrder: [ 'client-id-1', 'client-id-2' ], + getBlock: () => ( { name: 'my-block' } ), + }, + ); + + expect( testValidationError ).toMatchObject( {} ); + } ); + + it( 'does nothing if the block index is not in the block order array', () => { + const testValidationError = {}; + + maybeAddClientIdToValidationError( + { + validationError: testValidationError, + source: { + block_name: 'my-block', + block_content_index: 3, + post_id: 88, + }, + currentPostId: 88, + blockOrder: [ 'client-id-1', 'client-id-2' ], + getBlock: () => ( { name: 'my-block' } ), + }, + ); + + expect( testValidationError ).toMatchObject( {} ); + } ); + + it( 'does nothing if no block is found', () => { + const testValidationError = {}; + + maybeAddClientIdToValidationError( + { + validationError: testValidationError, + source: { + block_name: 'my-block', + block_content_index: 1, + post_id: 88, + }, + currentPostId: 88, + blockOrder: [ 'client-id-1', 'client-id-2' ], + getBlock: () => null, + }, + ); + + expect( testValidationError ).toMatchObject( {} ); + } ); + + it( 'does nothing if the real block name doesn\'t match the source block name', () => { + const testValidationError = {}; + + maybeAddClientIdToValidationError( + { + validationError: testValidationError, + source: { + block_name: 'my-block', + block_content_index: 1, + post_id: 88, + }, + currentPostId: 88, + blockOrder: [ 'client-id-1', 'client-id-2' ], + getBlock: () => ( { name: 'some-other-block' } ), + }, + ); + + expect( testValidationError ).toMatchObject( {} ); + } ); + + it( 'adds the client ID if there is a match', () => { + const testValidationError = {}; + + maybeAddClientIdToValidationError( + { + validationError: testValidationError, + source: { + block_name: 'my-block', + block_content_index: 1, + post_id: 88, + }, + currentPostId: 88, + blockOrder: [ 'client-id-1', 'client-id-2' ], + getBlock: () => ( { name: 'my-block' } ), + }, + ); + + expect( testValidationError ).toMatchObject( { clientId: 'client-id-2' } ); + } ); +} ); diff --git a/assets/src/block-validation/use-validation-error-state-updates.js b/assets/src/block-validation/use-validation-error-state-updates.js new file mode 100644 index 00000000000..7a6dd986a6d --- /dev/null +++ b/assets/src/block-validation/use-validation-error-state-updates.js @@ -0,0 +1,130 @@ +/** + * External dependencies + */ +import { isEqual } from 'lodash'; + +/** + * WordPress dependencies + */ +import { useEffect, useState } from '@wordpress/element'; +import { useSelect, useDispatch } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { AMP_VALIDITY_REST_FIELD_NAME } from './constants'; +import { BLOCK_VALIDATION_STORE_KEY } from './store'; + +/** + * Handles cases where a validationError's `sources` are an object (with numeric keys). + * + * Note: this will no longer be an issue after https://github.com/ampproject/amp-wp/commit/bbb0e495a817a56b37554dfd721170712c92d7b8 + * but is still required for validation errors stored in the database prior to that commit. + * + * @param {Object} validationError + */ +export function convertErrorSourcesToArray( validationError ) { + if ( ! Array.isArray( validationError.error.sources ) ) { + validationError.error.sources = Object.values( validationError.error.sources ); + } +} + +/** + * Attempts to associate a validation error with a block current in the editor. + * + * @param {Object} args + * @param {Object} args.validationError Validation error object. + * @param {Object} args.source A single validation error source. + * @param {number} args.currentPostId The ID of the current post. + * @param {string[]} args.blockOrder Block client IDs in order. + * @param {Function} args.getBlock Store selector to get a block in the current editor by client ID. + */ +export function maybeAddClientIdToValidationError( { validationError, source, currentPostId, blockOrder, getBlock } ) { + if ( ! source.block_name || undefined === source.block_content_index ) { + return; + } + + if ( currentPostId !== source.post_id ) { + return; + } + + // Look up the block ID by index, assuming the blocks of content in the editor are the same as blocks rendered on frontend. + const clientId = blockOrder[ source.block_content_index ]; + if ( ! clientId ) { + return; + } + + // Sanity check that block exists for clientId. + const block = getBlock( clientId ); + if ( ! block ) { + return; + } + + // Check the block type in case a block is dynamically added/removed via the_content filter to cause alignment error. + if ( block.name !== source.block_name ) { + return; + } + + validationError.clientId = clientId; +} + +/** + * Custom hook managing state updates through effect hooks. + * + * Handling state through a context provider might be preferable in other circumstances, but in this case + * using a store is necessary because React context is not passed down over slotfills, and we need multiple + * components within multiple slotfills to have access to the same state. + */ +export function useValidationErrorStateUpdates() { + const [ trackedValidationErrorsFromPost, setTrackedValidationErrorsFromPost ] = useState( [] ); + + const { setValidationErrors } = useDispatch( BLOCK_VALIDATION_STORE_KEY ); + + const { blockOrder, currentPost, getBlock, validationErrorsFromPost } = useSelect( ( select ) => ( { + blockOrder: select( 'core/block-editor' ).getClientIdsWithDescendants(), + currentPost: select( 'core/editor' ).getCurrentPost(), + getBlock: select( 'core/block-editor' ).getBlock, + 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( () => { + if ( ! isEqual( trackedValidationErrorsFromPost, validationErrorsFromPost ) ) { + setTrackedValidationErrorsFromPost( validationErrorsFromPost ); + } + }, [ trackedValidationErrorsFromPost, validationErrorsFromPost ] ); + + /* + * Adds clientIds to the validation errors that are associated with blocks. + */ + useEffect( () => { + const newValidationErrors = trackedValidationErrorsFromPost.map( ( validationError ) => { + if ( ! validationError.error.sources ) { + return validationError; + } + + convertErrorSourcesToArray( validationError ); + + for ( const source of validationError.error.sources ) { + // The loop can finish if the validation error (which is passed by reference below) has obtained a clientId. + if ( 'clientId' in validationError ) { + break; + } + + maybeAddClientIdToValidationError( { + validationError, + source, + getBlock, + blockOrder, + currentPostId: currentPost.id, + } ); + } + + return validationError; + } ); + + setValidationErrors( newValidationErrors ); + }, [ blockOrder, currentPost.id, getBlock, setValidationErrors, trackedValidationErrorsFromPost ] ); +} diff --git a/assets/src/block-validation/with-amp-toolbar-button.js b/assets/src/block-validation/with-amp-toolbar-button.js new file mode 100644 index 00000000000..02009ffaa0f --- /dev/null +++ b/assets/src/block-validation/with-amp-toolbar-button.js @@ -0,0 +1,56 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * WordPress dependencies + */ +import { useSelect } from '@wordpress/data'; +import { createHigherOrderComponent } from '@wordpress/compose'; + +/** + * Internal dependencies + */ +import { BLOCK_VALIDATION_STORE_KEY } from './store'; +import { AMPToolbarButton } from './amp-toolbar-button'; + +/** + * Adds the AMPToolbarButton to blocks that have one or more unreviewed validation errors. + * + * @param {Object} props + * @param {Function} props.BlockEdit Block edit function. + * @param {string} props.clientId Client ID. + */ +function BlockEditWithToolbar( props ) { + const { BlockEdit, clientId } = props; + + const count = useSelect( + ( select ) => ( select( BLOCK_VALIDATION_STORE_KEY ).getUnreviewedValidationErrors() || [] ) + .filter( ( { clientId: validationErrorClientId } ) => clientId === validationErrorClientId ) + .length || 0, + [ clientId ], + ); + + return ( + <> + { 0 < count && + + } + + + + ); +} +BlockEditWithToolbar.propTypes = { + BlockEdit: PropTypes.func.isRequired, + clientId: PropTypes.string.isRequired, +}; + +/** + * Filters the block edit function of all blocks. + */ +export const withAMPToolbarButton = createHigherOrderComponent( + ( BlockEdit ) => ( props ) => , + 'BlockEditWithAMPToolbar', +); diff --git a/assets/src/components/reader-theme-selection/style.css b/assets/src/components/reader-theme-selection/style.css index d92ad68d0f9..3a927bf559a 100644 --- a/assets/src/components/reader-theme-selection/style.css +++ b/assets/src/components/reader-theme-selection/style.css @@ -6,12 +6,18 @@ .choose-reader-theme__grid { display: grid; grid-gap: 40px; +} + +@media screen and (min-width: 600px) { - @media screen and (min-width: 600px) { + .choose-reader-theme__grid { grid-template-columns: repeat(2, minmax(0, 1fr)); } +} + +@media screen and (min-width: 1100px) { - @media screen and (min-width: 1100px) { + .choose-reader-theme__grid { grid-template-columns: repeat(3, minmax(0, 1fr)); } } diff --git a/assets/src/settings-page/style.css b/assets/src/settings-page/style.css index c1008f47dcb..20c3f1c3676 100644 --- a/assets/src/settings-page/style.css +++ b/assets/src/settings-page/style.css @@ -290,8 +290,11 @@ li.error-kept { .supported-templates__fields { display: grid; gap: 0; +} - @media (min-width: 783px) { +@media (min-width: 783px) { + + .supported-templates__fields { gap: 1.5rem; grid-template-columns: repeat(2, 1fr); } diff --git a/babel.config.js b/babel.config.js index cd678983d94..4724d9cbc76 100644 --- a/babel.config.js +++ b/babel.config.js @@ -11,7 +11,21 @@ module.exports = function( api ) { plugins: [ ...config.plugins, '@babel/plugin-proposal-class-properties', - 'inline-react-svg', + [ + 'inline-react-svg', + { + svgo: { + plugins: [ + { + cleanupIDs: { + minify: false, // Prevent duplicate SVG IDs from minification. + }, + }, + ], + + }, + }, + ], ], sourceMaps: true, env: { @@ -19,7 +33,21 @@ module.exports = function( api ) { plugins: [ ...config.plugins, '@babel/plugin-proposal-class-properties', - 'inline-react-svg', + [ + 'inline-react-svg', + { + svgo: { + plugins: [ + { + cleanupIDs: { + minify: false, // Prevent duplicate SVG IDs from minification. + }, + }, + ], + + }, + }, + ], 'transform-react-remove-prop-types', ], }, diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 4fe716cc76c..632213442e5 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -6,6 +6,8 @@ * @since 0.6 */ +use AmpProject\AmpWP\Services; + /** * Post meta box class. * @@ -213,26 +215,8 @@ public function enqueue_block_assets() { return; } - // Gutenberg v5.4 was bundled with WP 5.2, which is the earliest release known to work without errors. - $gb_supported = defined( 'GUTENBERG_VERSION' ) && version_compare( GUTENBERG_VERSION, '5.4.0', '>=' ); - $wp_supported = ! $gb_supported && version_compare( get_bloginfo( 'version' ), '5.2', '>=' ); - - // Let the user know that block editor functionality is not available if the current Gutenberg or WordPress version is not supported. - if ( ! $gb_supported && ! $wp_supported ) { - if ( current_user_can( 'manage_options' ) ) { - wp_add_inline_script( - 'wp-edit-post', - sprintf( - 'wp.domReady( - function () { - wp.data.dispatch( "core/notices" ).createWarningNotice( %s ) - } - );', - wp_json_encode( __( 'AMP functionality is not available since your version of the Block Editor is too old. Please either update WordPress core to the latest version or activate the Gutenberg plugin. As a last resort, you may use the Classic Editor plugin instead.', 'amp' ) ) - ) - ); - } - + $editor_support = Services::get( 'editor.editor_support' ); + if ( ! $editor_support->editor_supports_amp_block_editor_features() ) { return; } diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 6d10ab12331..739a1987de8 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -5,10 +5,10 @@ * @package AMP */ +use AmpProject\AmpWP\DevTools\BlockSources; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Icon; use AmpProject\AmpWP\Option; -use AmpProject\AmpWP\PluginRegistry; use AmpProject\AmpWP\QueryVar; use AmpProject\AmpWP\Services; use AmpProject\Attribute; @@ -704,6 +704,7 @@ public static function get_amp_validity_rest_field( $post_data, $field_name, $re 'status' => $result['status'], 'term_status' => $result['term_status'], 'forced' => $result['forced'], + 'term_id' => $result['term']->term_id, ]; } } @@ -1188,7 +1189,7 @@ static function ( $enqueued_script_source ) use ( $script_handle ) { } } - $sources = array_unique( $sources, SORT_REGULAR ); + $sources = array_values( array_unique( $sources, SORT_REGULAR ) ); return $sources; } @@ -2389,6 +2390,13 @@ public static function enqueue_block_validation() { return; } + $editor_support = Services::get( 'editor.editor_support' ); + + // Block validation script uses features only available beginning with WP 5.3. + if ( ! $editor_support->editor_supports_amp_block_editor_features() ) { + return; // @codeCoverageIgnore + } + $slug = 'amp-block-validation'; $asset_file = AMP__DIR__ . '/assets/js/' . $slug . '.asset.php'; @@ -2413,14 +2421,40 @@ public static function enqueue_block_validation() { wp_styles()->add_data( $slug, 'rtl', 'replace' ); + $block_sources = Services::has( 'dev_tools.block_sources' ) ? Services::get( 'dev_tools.block_sources' ) : null; + + $plugin_registry = Services::get( 'plugin_registry' ); + + $plugin_names = array_map( + static function ( $plugin ) { + return isset( $plugin['Name'] ) ? $plugin['Name'] : ''; + }, + $plugin_registry->get_plugins() + ); + $data = [ - 'isSanitizationAutoAccepted' => self::is_sanitization_auto_accepted(), + 'HTML_ATTRIBUTE_ERROR_TYPE' => AMP_Validation_Error_Taxonomy::HTML_ATTRIBUTE_ERROR_TYPE, + 'HTML_ELEMENT_ERROR_TYPE' => AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE, + 'JS_ERROR_TYPE' => AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE, + 'CSS_ERROR_TYPE' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, + 'VALIDATION_ERROR_NEW_REJECTED_STATUS' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS, + 'VALIDATION_ERROR_NEW_ACCEPTED_STATUS' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS, + 'VALIDATION_ERROR_ACK_REJECTED_STATUS' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_REJECTED_STATUS, + 'VALIDATION_ERROR_ACK_ACCEPTED_STATUS' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_ACCEPTED_STATUS, + 'isSanitizationAutoAccepted' => self::is_sanitization_auto_accepted(), + 'blockSources' => $block_sources ? $block_sources->get_block_sources() : null, + 'pluginNames' => $plugin_names, + 'themeName' => wp_get_theme()->get( 'Name' ), + 'themeSlug' => wp_get_theme()->get_stylesheet(), ]; - wp_localize_script( + wp_add_inline_script( $slug, - 'ampBlockValidation', - $data + sprintf( + 'var ampBlockValidation = %s;', + wp_json_encode( $data ) + ), + 'before' ); if ( function_exists( 'wp_set_script_translations' ) ) { diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index 88ac36848e7..5aa189ab7b6 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -67,11 +67,13 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'amp_slug_customization_watcher' => AmpSlugCustomizationWatcher::class, 'css_transient_cache.ajax_handler' => Admin\ReenableCssTransientCachingAjaxAction::class, 'css_transient_cache.monitor' => BackgroundTask\MonitorCssTransientCaching::class, + 'dev_tools.block_sources' => DevTools\BlockSources::class, 'dev_tools.callback_reflection' => DevTools\CallbackReflection::class, 'dev_tools.error_page' => DevTools\ErrorPage::class, 'dev_tools.file_reflection' => DevTools\FileReflection::class, 'dev_tools.likely_culprit_detector' => DevTools\LikelyCulpritDetector::class, 'dev_tools.user_access' => DevTools\UserAccess::class, + 'editor.editor_support' => Editor\EditorSupport::class, 'extra_theme_and_plugin_headers' => ExtraThemeAndPluginHeaders::class, 'mobile_redirection' => MobileRedirection::class, 'obsolete_block_attribute_remover' => ObsoleteBlockAttributeRemover::class, diff --git a/src/DevTools/BlockSources.php b/src/DevTools/BlockSources.php new file mode 100644 index 00000000000..9e04a143fd9 --- /dev/null +++ b/src/DevTools/BlockSources.php @@ -0,0 +1,170 @@ +plugin_registry = $plugin_registry; + $this->likely_culprit_detector = $likely_culprit_detector; + } + + /** + * Runs on instantiation. + */ + public function register() { + if ( empty( $this->get_block_sources() ) ) { + add_filter( 'register_block_type_args', [ $this, 'capture_block_type_source' ] ); + + // All blocks should be registered well before admin_enqueue_scripts. + add_action( 'admin_enqueue_scripts', [ $this, 'cache_block_sources' ], PHP_INT_MAX ); + } + + add_action( 'activated_plugin', [ $this, 'clear_block_sources_cache' ] ); + add_action( 'after_switch_theme', [ $this, 'clear_block_sources_cache' ] ); + add_action( 'upgrader_process_complete', [ $this, 'clear_block_sources_cache' ] ); + } + + /** + * Registers the google font style. + * + * @param array $args Array of arguments for registering a block type. + * @return array Filtered block type args. + */ + public function capture_block_type_source( $args ) { + if ( isset( $this->get_block_sources()[ $args['name'] ] ) ) { + return $args; + } + + $likely_culprit = $this->likely_culprit_detector->analyze_backtrace(); + + if ( in_array( $likely_culprit[ FileReflection::SOURCE_TYPE ], [ FileReflection::TYPE_PLUGIN, FileReflection::TYPE_MU_PLUGIN ], true ) ) { + $plugin = $this->plugin_registry->get_plugin_from_slug( + $likely_culprit[ FileReflection::SOURCE_NAME ], + FileReflection::TYPE_MU_PLUGIN === $likely_culprit[ FileReflection::SOURCE_TYPE ] + ); + + $likely_culprit['title'] = isset( $plugin['data']['Title'] ) ? $plugin['data']['Title'] : $likely_culprit[ FileReflection::SOURCE_NAME ]; + } elseif ( FileReflection::TYPE_THEME === $likely_culprit[ FileReflection::SOURCE_TYPE ] ) { + $theme = wp_get_theme( $likely_culprit['name'] ); + $likely_culprit['title'] = $theme->get( 'Name' ) ?: $likely_culprit[ FileReflection::SOURCE_NAME ]; + } else { + $likely_culprit['title'] = __( 'WordPress core', 'amp' ); + } + + $this->block_sources[ $args['name'] ] = $likely_culprit; + + return $args; + } + + /** + * Saves the block source data to cache. + */ + public function cache_block_sources() { + set_transient( __CLASS__ . self::CACHE_KEY, $this->block_sources, self::CACHE_TIMEOUT ); + } + + /** + * Clears the cached block source data. + */ + public function clear_block_sources_cache() { + delete_transient( __CLASS__ . self::CACHE_KEY ); + } + + /** + * Retrieves block source data from cache. + */ + private function set_block_sources_from_cache() { + $from_cache = get_transient( __CLASS__ . self::CACHE_KEY ); + + $this->block_sources = is_array( $from_cache ) ? $from_cache : []; + } + + /** + * Retrieves block source data. + * + * @return array + */ + public function get_block_sources() { + if ( is_null( $this->block_sources ) ) { + $this->set_block_sources_from_cache(); + } + + return $this->block_sources; + } +} diff --git a/src/Editor/EditorSupport.php b/src/Editor/EditorSupport.php new file mode 100644 index 00000000000..76bc1d0d356 --- /dev/null +++ b/src/Editor/EditorSupport.php @@ -0,0 +1,113 @@ +is_block_editor ) || false === $screen->is_block_editor ) { + return; + } + + if ( ! in_array( get_post_type(), AMP_Post_Type_Support::get_eligible_post_types(), true ) ) { + return; + } + + if ( $this->editor_supports_amp_block_editor_features() ) { + return; + } + + if ( current_user_can( 'manage_options' ) ) { + wp_add_inline_script( + 'wp-edit-post', + sprintf( + 'wp.domReady( + function () { + wp.data.dispatch( "core/notices" ).createWarningNotice( %s ) + } + );', + wp_json_encode( __( 'AMP functionality is not available since your version of the Block Editor is too old. Please either update WordPress core to the latest version or activate the Gutenberg plugin. As a last resort, you may use the Classic Editor plugin instead.', 'amp' ) ) + ) + ); + } + } + + /** + * Returns whether the editor in the current environment supports plugin features. + * + * @return bool + */ + public function editor_supports_amp_block_editor_features() { + // Check for plugin constant here as well as in the function because editor features won't work in + // supported WP versions if an old, unsupported GB version is overriding the editor. + if ( defined( 'GUTENBERG_VERSION' ) ) { + return $this->has_support_from_gutenberg_plugin(); + } + + return $this->has_support_from_core(); + } + + /** + * Returns whether the Gutenberg plugin provides minimal support. + * + * @return bool + */ + public function has_support_from_gutenberg_plugin() { + return defined( 'GUTENBERG_VERSION' ) && version_compare( GUTENBERG_VERSION, self::GB_MIN_VERSION, '>=' ); + } + + /** + * Returns whether WP core provides minimum Gutenberg support. + * + * @return bool + */ + public function has_support_from_core() { + return version_compare( get_bloginfo( 'version' ), self::WP_MIN_VERSION, '>=' ); + } +} diff --git a/src/PluginRegistry.php b/src/PluginRegistry.php index e55e38eb73a..fa52918a8b3 100644 --- a/src/PluginRegistry.php +++ b/src/PluginRegistry.php @@ -103,6 +103,7 @@ public function get_plugins( $active_only = false, $omit_core = true ) { * the plugins directory. * * @param string $plugin_slug Plugin slug. + * @param bool $must_use Whether the slug is for a must-use plugin. * @return array|null { * Plugin data if found, otherwise null. * @@ -110,8 +111,8 @@ public function get_plugins( $active_only = false, $omit_core = true ) { * @type array $data Plugin data. * } */ - public function get_plugin_from_slug( $plugin_slug ) { - $plugins = $this->get_plugins_data(); + public function get_plugin_from_slug( $plugin_slug, $must_use = false ) { + $plugins = $must_use ? $this->get_mu_plugins_data() : $this->get_plugins_data(); if ( isset( $plugins[ $plugin_slug ] ) ) { return [ 'file' => $plugin_slug, @@ -140,4 +141,14 @@ private function get_plugins_data() { $this->plugin_folder ? '/' . trim( $this->plugin_folder, '/' ) : '' ); } + + /** + * Gets the MU plugins on the site. + * + * @return array[] + */ + private function get_mu_plugins_data() { + require_once ABSPATH . 'wp-admin/includes/plugin.php'; + return get_mu_plugins(); + } } diff --git a/src/Services.php b/src/Services.php index d0dfbcc7d60..f7b562008d6 100644 --- a/src/Services.php +++ b/src/Services.php @@ -55,6 +55,16 @@ public static function get( $service ) { return self::get_container()->get( $service ); } + /** + * Check if a particular service has been registered in the service container. + * + * @param string $service Service ID to retrieve. + * @return bool + */ + public static function has( $service ) { + return self::get_container()->has( $service ); + } + /** * Get an instance of the plugin. * diff --git a/tests/js/jest.config.js b/tests/js/jest.config.js index 39d6dbc1ec8..5cb23db8f0e 100644 --- a/tests/js/jest.config.js +++ b/tests/js/jest.config.js @@ -21,6 +21,7 @@ module.exports = { ], modulePathIgnorePatterns: [ '/assets/src/components/.*/__mocks__', + '/assets/src/components/.*/__data__', ], coverageReporters: [ 'lcov' ], coverageDirectory: '/build/logs', diff --git a/tests/php/src/DevTools/BlockSourcesTest.php b/tests/php/src/DevTools/BlockSourcesTest.php new file mode 100644 index 00000000000..beb0a7af13c --- /dev/null +++ b/tests/php/src/DevTools/BlockSourcesTest.php @@ -0,0 +1,192 @@ +instance = $this->get_new_instance(); + } + + /** + * @return BlockSources. + */ + public function get_new_instance() { + $plugin_registry = new PluginRegistry(); + return new BlockSources( $plugin_registry, new LikelyCulpritDetector( new FileReflection( $plugin_registry ) ) ); + } + + /** + * Tear down method. Runs after each test. + * + * @inheritdoc + */ + public function tearDown() { + parent::tearDown(); + + $this->instance->clear_block_sources_cache(); + } + + /** @covers ::is_needed() */ + public function test_is_needed() { + if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { + $this->assertFalse( BlockSources::is_needed() ); + } else { + $this->assertFalse( BlockSources::is_needed() ); + set_current_screen( 'edit.php' ); + $this->assertTrue( BlockSources::is_needed() ); + } + } + + public function test__construct() { + $this->assertInstanceOf( BlockSources::class, $this->instance ); + $this->assertInstanceOf( Service::class, $this->instance ); + $this->assertInstanceOf( Registerable::class, $this->instance ); + $this->assertInstanceOf( Conditional::class, $this->instance ); + } + + /** @covers ::register */ + public function test_register() { + $this->instance->register(); + + $this->assertEquals( 10, has_filter( 'register_block_type_args', [ $this->instance, 'capture_block_type_source' ] ) ); + $this->assertEquals( PHP_INT_MAX, has_action( 'admin_enqueue_scripts', [ $this->instance, 'cache_block_sources' ] ) ); + $this->assertEquals( 10, has_action( 'activated_plugin', [ $this->instance, 'clear_block_sources_cache' ] ) ); + $this->assertEquals( 10, has_action( 'after_switch_theme', [ $this->instance, 'clear_block_sources_cache' ] ) ); + $this->assertEquals( 10, has_action( 'upgrader_process_complete', [ $this->instance, 'clear_block_sources_cache' ] ) ); + } + + /** + * @covers ::capture_block_type_source() + * @covers ::get_block_sources() + */ + public function test_capture_block_type_source() { + if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { + $this->markTestSkipped( 'Detecting block sources requires WordPress 5.5.' ); + } + + $this->instance->clear_block_sources_cache(); + $this->instance->register(); + + // Test registration of a core block. + register_block_type( 'core/test-block' ); + + $this->assertEquals( + [ + 'core/test-block' => [ + 'name' => '', + 'type' => '', + 'title' => 'WordPress core', + ], + ], + $this->instance->get_block_sources() + ); + + require_once MockPluginEnvironment::BAD_PLUGINS_DIR . '/' . MockPluginEnvironment::BAD_BLOCK_PLUGIN_FILE; + + $this->assertEquals( + [ + 'core/test-block' => [ + 'name' => '', + 'type' => '', + 'title' => 'WordPress core', + ], + // @todo How to test this with a real plugin in the plugins directory. + 'bad/bad-block' => [ + 'name' => '', + 'type' => '', + 'title' => 'WordPress core', + ], + ], + $this->instance->get_block_sources() + ); + } + + /** + * Provides whether to test with object cache off or on. + */ + public function get_using_object_cache() { + return [ + [ true ], + [ false ], + ]; + } + + /** + * @covers ::get_block_sources() + * @covers ::cache_block_sources() + * @covers ::clear_block_sources_cache() + * @covers ::set_block_sources_from_cache() + * @dataProvider get_using_object_cache + * + * @param bool $using_object_cache Using object cache. + */ + public function test_caching( $using_object_cache ) { + $original_using_object_cache = wp_using_ext_object_cache( $using_object_cache ); + + $this->assertEquals( [], $this->instance->get_block_sources() ); + + $test_data = [ + 'core/test-block' => [ + 'source' => 'core', + 'name' => null, + ], + 'unknown/block' => [ + 'source' => 'unknown', + 'name' => null, + ], + ]; + + $this->set_private_property( $this->instance, 'block_sources', $test_data ); + $this->instance->cache_block_sources(); + + $this->instance = $this->get_new_instance(); + $this->assertEquals( $test_data, $this->instance->get_block_sources() ); + + $this->instance->clear_block_sources_cache(); + + $this->instance = $this->get_new_instance(); + $this->assertEquals( [], $this->instance->get_block_sources() ); + + wp_using_ext_object_cache( $original_using_object_cache ); + } +} diff --git a/tests/php/src/Editor/EditorSupportTest.php b/tests/php/src/Editor/EditorSupportTest.php new file mode 100644 index 00000000000..b2a14951aac --- /dev/null +++ b/tests/php/src/Editor/EditorSupportTest.php @@ -0,0 +1,157 @@ +instance = new EditorSupport(); + } + + public function test_it_can_be_initialized() { + $this->assertInstanceOf( EditorSupport::class, $this->instance ); + $this->assertInstanceOf( Service::class, $this->instance ); + $this->assertInstanceOf( Registerable::class, $this->instance ); + } + + /** @covers ::register() */ + public function test_register() { + $this->instance->register(); + + $this->assertEquals( 99, has_action( 'admin_enqueue_scripts', [ $this->instance, 'maybe_show_notice' ] ) ); + } + + /** @covers ::has_support_from_gutenberg_plugin */ + public function test_has_support_from_gutenberg_plugin() { + if ( defined( 'GUTENBERG_VERSION' ) ) { + $this->assertTrue( $this->instance->has_support_from_gutenberg_plugin() ); + } else { + if ( version_compare( get_bloginfo( 'version' ), EditorSupport::WP_MIN_VERSION, '>=' ) ) { + $this->assertTrue( $this->instance->has_support_from_core() ); + } else { + $this->assertFalse( $this->instance->has_support_from_core() ); + } + } + } + + public function test_editor_supports_amp_block_editor_features() { + if ( defined( 'GUTENBERG_VERSION' ) ) { + $this->assertTrue( $this->instance->editor_supports_amp_block_editor_features() ); + } else { + if ( version_compare( get_bloginfo( 'version' ), EditorSupport::WP_MIN_VERSION, '>=' ) ) { + $this->assertTrue( $this->instance->editor_supports_amp_block_editor_features() ); + } else { + $this->assertFalse( $this->instance->editor_supports_amp_block_editor_features() ); + } + } + } + + /** @covers ::has_support_from_core() */ + public function test_has_support_from_core() { + if ( version_compare( get_bloginfo( 'version' ), EditorSupport::WP_MIN_VERSION, '>=' ) ) { + $this->assertTrue( $this->instance->has_support_from_core() ); + } else { + $this->assertFalse( $this->instance->has_support_from_core() ); + } + } + + /** @covers ::maybe_show_notice() */ + public function test_dont_show_notice_if_no_screen_defined() { + $this->instance->maybe_show_notice(); + $this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) ); + } + + /** @covers ::maybe_show_notice() */ + public function test_dont_show_notice_for_unsupported_post_type() { + global $post; + + set_current_screen( 'edit.php' ); + register_post_type( 'my-post-type' ); + $post = $this->factory()->post->create( [ 'post_type' => 'my-post-type' ] ); + setup_postdata( get_post( $post ) ); + + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $this->instance->maybe_show_notice(); + $this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) ); + unset( $GLOBALS['current_screen'] ); + unset( $GLOBALS['wp_scripts'] ); + } + + /** @covers ::maybe_show_notice() */ + public function test_show_notice_for_supported_post_type() { + global $post; + + if ( version_compare( get_bloginfo( 'version' ), '5.3', '<=' ) ) { + $this->markTestSkipped(); + } + + set_current_screen( 'edit.php' ); + $post = $this->factory()->post->create(); + setup_postdata( get_post( $post ) ); + + wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $this->instance->maybe_show_notice(); + if ( $this->instance->editor_supports_amp_block_editor_features() ) { + $this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) ); + } else { + $this->assertContains( 'AMP f', wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) ); + } + unset( $GLOBALS['current_screen'] ); + unset( $GLOBALS['wp_scripts'] ); + } + + /** @covers ::maybe_show_notice() */ + public function test_maybe_show_notice_for_unsupported_user() { + global $post; + + set_current_screen( 'edit.php' ); + $post = $this->factory()->post->create(); + setup_postdata( get_post( $post ) ); + + $this->instance->maybe_show_notice(); + + $this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) ); + unset( $GLOBALS['current_screen'] ); + unset( $GLOBALS['wp_scripts'] ); + } + + /** @covers ::maybe_show_notice() */ + public function test_maybe_show_notice_with_cpt_supporting_gutenberg_but_not_amp() { + global $post; + + if ( ! $this->instance->editor_supports_amp_block_editor_features() ) { + $this->markTestSkipped(); + } + + register_post_type( + 'my-gb-post-type', + [ + 'public' => true, + 'show_in_rest' => true, + 'supports' => [ 'editor' ], + ] + ); + + set_current_screen( 'edit.php' ); + $post = $this->factory()->post->create( [ 'post_type' => 'my-gutenberg-post-type' ] ); + setup_postdata( get_post( $post ) ); + + $this->instance->maybe_show_notice(); + $this->assertFalse( wp_scripts()->print_inline_script( 'wp-edit-post', 'after', false ) ); + unset( $GLOBALS['current_screen'] ); + unset( $GLOBALS['wp_scripts'] ); + } +} diff --git a/tests/php/src/PluginRegistryTest.php b/tests/php/src/PluginRegistryTest.php index 244f7c8be18..b8ecaa7d838 100644 --- a/tests/php/src/PluginRegistryTest.php +++ b/tests/php/src/PluginRegistryTest.php @@ -69,6 +69,34 @@ public function test_get_plugins() { ); } + /** @covers ::get_mu_plugins_data() */ + public function test_get_mu_plugins_data() { + $plugin_registry = new PluginRegistry(); + + $plugins = $this->call_private_method( $plugin_registry, 'get_mu_plugins_data' ); + $this->assertInternalType( 'array', $plugins ); + foreach ( $plugins as $plugin_data ) { + $this->assertEqualSets( + [ + 'Name', + 'PluginURI', + 'Version', + 'Description', + 'Author', + 'AuthorURI', + 'TextDomain', + 'DomainPath', + 'Network', + 'RequiresWP', + 'RequiresPHP', + 'Title', + 'AuthorName', + ], + array_keys( $plugin_data ) + ); + } + } + /** @covers ::get_plugin_from_slug() */ public function test_get_plugin_from_slug() { $this->populate_plugins(); diff --git a/tests/php/test-class-amp-meta-box.php b/tests/php/test-class-amp-meta-box.php index 77ce0c7b242..d7f9ee89d04 100644 --- a/tests/php/test-class-amp-meta-box.php +++ b/tests/php/test-class-amp-meta-box.php @@ -115,6 +115,9 @@ public function test_enqueue_admin_assets() { * @covers ::enqueue_block_assets() */ public function test_enqueue_block_assets() { + set_current_screen( 'post.php' ); + get_current_screen()->is_block_editor = true; + if ( ! function_exists( 'register_block_type' ) ) { $this->markTestSkipped( 'The block editor is not available' ); } @@ -178,6 +181,7 @@ public function test_enqueue_block_assets() { foreach ( $expected_localized_values as $localized_value ) { $this->assertContains( $localized_value, $data ); } + unset( $GLOBALS['post'], $GLOBALS['current_screen'] ); } /** @covers ::get_featured_image_dimensions() */ diff --git a/tests/php/validation/test-class-amp-validation-manager.php b/tests/php/validation/test-class-amp-validation-manager.php index caae1d06067..be8093ae96c 100644 --- a/tests/php/validation/test-class-amp-validation-manager.php +++ b/tests/php/validation/test-class-amp-validation-manager.php @@ -651,10 +651,11 @@ public function test_get_amp_validity_rest_field() { ); $this->assertArrayHasKey( 'results', $field ); $this->assertArrayHasKey( 'review_link', $field ); + $this->assertEquals( $field['results'], array_map( - static function ( $error ) { + static function ( $error ) use ( $field ) { return [ 'sanitized' => false, 'title' => 'Unknown error (test)', @@ -662,6 +663,7 @@ static function ( $error ) { 'status' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS, 'term_status' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS, 'forced' => false, + 'term_id' => $field['results'][0]['term_id'], ]; }, $errors @@ -2532,6 +2534,8 @@ public function test_print_plugin_notice() { public function test_enqueue_block_validation() { if ( ! function_exists( 'register_block_type' ) ) { $this->markTestSkipped( 'The block editor is not available.' ); + } elseif ( version_compare( get_bloginfo( 'version' ), '5.3', '<' ) ) { + $this->markTestSkipped( 'Block editor is too old.' ); } global $post; @@ -2559,9 +2563,11 @@ public function test_enqueue_block_validation() { 'wp-components', 'wp-compose', 'wp-data', + 'wp-edit-post', 'wp-element', 'wp-hooks', 'wp-i18n', + 'wp-plugins', 'wp-polyfill', ]; diff --git a/webpack.config.js b/webpack.config.js index ddb20a51398..23c7fa17b2e 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -94,6 +94,7 @@ const blockEditor = { externals: { // Make localized data importable. 'amp-block-editor-data': 'ampBlockEditor', + 'amp-block-validation': 'ampBlockValidation', }, entry: { 'amp-block-editor': './assets/src/block-editor/index.js',