diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php index c65e3ae2f92..4167f17bfd8 100644 --- a/.phpstorm.meta.php +++ b/.phpstorm.meta.php @@ -8,6 +8,7 @@ // TODO: I'd like to use AmpWpPlugin::SERVICES directly here but it doesn't seem to work. map( [ 'admin.analytics_menu' => \AmpProject\AmpWP\Admin\AnalyticsOptionsSubmenu::class, + 'admin.after_activation_site_scan' => Admin\AfterActivationSiteScan::class, 'admin.google_fonts' => \AmpProject\AmpWP\Admin\GoogleFonts::class, 'admin.onboarding_menu' => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenu::class, 'admin.onboarding_wizard' => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenuPage::class, @@ -45,7 +46,6 @@ 'paired_routing' => \AmpProject\AmpWP\PairedRouting::class, 'paired_url' => \AmpProject\AmpWP\PairedUrl::class, 'plugin_activation_notice' => \AmpProject\AmpWP\Admin\PluginActivationNotice::class, - 'plugin_activation_site_scan' => \AmpProject\AmpWP\Admin\PluginActivationSiteScan::class, 'plugin_registry' => \AmpProject\AmpWP\PluginRegistry::class, 'plugin_suppression' => \AmpProject\AmpWP\PluginSuppression::class, 'reader_theme_loader' => \AmpProject\AmpWP\ReaderThemeLoader::class, diff --git a/assets/src/admin/site-scan-notice/index.js b/assets/src/admin/site-scan-notice/index.js index ff73fd6d559..22c577a9ba4 100644 --- a/assets/src/admin/site-scan-notice/index.js +++ b/assets/src/admin/site-scan-notice/index.js @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import { APP_ROOT_ID, + APP_ROOT_SIBLING_ID, OPTIONS_REST_PATH, SCANNABLE_URLS_REST_PATH, VALIDATE_NONCE, @@ -24,6 +25,7 @@ import { ErrorContextProvider } from '../../components/error-context-provider'; import { OptionsContextProvider } from '../../components/options-context-provider'; import { PluginsContextProvider } from '../../components/plugins-context-provider'; import { SiteScanContextProvider } from '../../components/site-scan-context-provider'; +import { ThemesContextProvider } from '../../components/themes-context-provider'; import { ErrorScreen } from '../../components/error-screen'; import { ErrorBoundary } from '../../components/error-boundary'; import { SiteScanNotice } from './notice'; @@ -50,12 +52,14 @@ function Providers( { children } ) { populateDefaultValues={ false } > - - { children } - + + + { children } + + @@ -67,10 +71,18 @@ Providers.propTypes = { }; domReady( () => { - const root = document.getElementById( APP_ROOT_ID ); + let root = document.getElementById( APP_ROOT_ID ); if ( ! root ) { - return; + const rootSibling = document.getElementById( APP_ROOT_SIBLING_ID ); + + if ( ! rootSibling ) { + return; + } + + root = document.createElement( 'div' ); + root.setAttribute( 'id', APP_ROOT_ID ); + rootSibling.after( root ); } errorHandler = ( event ) => { diff --git a/assets/src/admin/site-scan-notice/notice.js b/assets/src/admin/site-scan-notice/notice.js index 1560794c33b..5cbc5be1599 100644 --- a/assets/src/admin/site-scan-notice/notice.js +++ b/assets/src/admin/site-scan-notice/notice.js @@ -1,11 +1,3 @@ -/** - * External dependencies - */ -import { - AMP_COMPATIBLE_PLUGINS_URL, - SETTINGS_LINK, -} from 'amp-site-scan-notice'; // From WP inline script. - /** * WordPress dependencies */ @@ -24,12 +16,8 @@ import { AmpAdminNotice, } from '../../components/amp-admin-notice'; import { Loading } from '../../components/loading'; -import { isExternalUrl } from '../../common/helpers/is-external-url'; import { PluginsWithAmpIncompatibility } from './plugins-with-amp-incompatibility'; - -// Define Plugin Suppression link. -const PLUGIN_SUPPRESSION_LINK = new URL( SETTINGS_LINK ); -PLUGIN_SUPPRESSION_LINK.hash = 'plugin-suppression'; +import { ThemeWithAmpIncompatibility } from './theme-with-amp-incompatibility'; export function SiteScanNotice() { const { @@ -41,6 +29,7 @@ export function SiteScanNotice() { isInitializing, isReady, pluginsWithAmpIncompatibility, + themesWithAmpIncompatibility, startSiteScan, } = useContext( SiteScan ); @@ -72,32 +61,29 @@ export function SiteScanNotice() { ); } - if ( isCompleted && pluginsWithAmpIncompatibility.length === 0 ) { - return ( - -

- { __( 'No AMP compatibility issues detected.', 'amp' ) } -

-
- ); - } + if ( isCompleted ) { + let elements = [ + pluginsWithAmpIncompatibility.length > 0 ? : null, + themesWithAmpIncompatibility.length > 0 ? : null, + ]; + + // Display the theme information at the top when on the `themes.php` screen. + if ( document.location.href.includes( 'themes.php' ) ) { + elements = elements.reverse(); + } + + elements = elements.filter( Boolean ); - if ( isCompleted && pluginsWithAmpIncompatibility.length > 0 ) { return ( - - -
- - { __( 'Review Plugin Suppression', 'amp' ) } - - - { __( 'View AMP-Compatible Plugins', 'amp' ) } - -
+ + { elements.length ? elements : ( +

+ { __( 'No AMP compatibility issues detected.', 'amp' ) } +

+ ) }
); } diff --git a/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js b/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js index dcc035b0aa9..4e05863e142 100644 --- a/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js +++ b/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js @@ -2,18 +2,24 @@ * External dependencies */ import PropTypes from 'prop-types'; +import { AMP_COMPATIBLE_PLUGINS_URL, SETTINGS_LINK } from 'amp-site-scan-notice'; // From WP inline script. /** * WordPress dependencies */ import { useContext } from '@wordpress/element'; -import { _n, sprintf } from '@wordpress/i18n'; +import { __, _n, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ import { Plugins } from '../../components/plugins-context-provider'; import { getPluginSlugFromFile } from '../../common/helpers/get-plugin-slug-from-file'; +import { isExternalUrl } from '../../common/helpers/is-external-url'; + +// Define Plugin Suppression link. +const PLUGIN_SUPPRESSION_LINK = new URL( SETTINGS_LINK ); +PLUGIN_SUPPRESSION_LINK.hash = 'plugin-suppression'; /** * Render a DETAILS element for each plugin causing AMP incompatibilities. @@ -77,6 +83,18 @@ export function PluginsWithAmpIncompatibility( { pluginsWithAmpIncompatibility } ) ) } +
+ + { __( 'Review Plugin Suppression', 'amp' ) } + + + { __( 'View AMP-Compatible Plugins', 'amp' ) } + +
); } @@ -87,5 +105,5 @@ PluginsWithAmpIncompatibility.propTypes = { slug: PropTypes.string, urls: PropTypes.arrayOf( PropTypes.string ), } ), - ), + ).isRequired, }; diff --git a/assets/src/admin/site-scan-notice/theme-with-amp-incompatibility.js b/assets/src/admin/site-scan-notice/theme-with-amp-incompatibility.js new file mode 100644 index 00000000000..b45a821770d --- /dev/null +++ b/assets/src/admin/site-scan-notice/theme-with-amp-incompatibility.js @@ -0,0 +1,67 @@ +/** + * WordPress dependencies + */ +import { createInterpolateElement, useContext } from '@wordpress/element'; +import { __, sprintf } from '@wordpress/i18n'; + +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +import { AMP_COMPATIBLE_THEMES_URL } from 'amp-site-scan-notice'; // From WP inline script. + +/** + * Internal dependencies + */ +import { Themes } from '../../components/themes-context-provider'; +import { isExternalUrl } from '../../common/helpers/is-external-url'; + +/** + * Render a message saying a theme is not AMP compatible. + * + * @param {Object} props Component props. + * @param {Object} props.themeWithAmpIncompatibility Theme with AMP incompatibilities. + */ +export function ThemeWithAmpIncompatibility( { themeWithAmpIncompatibility } ) { + const { fetchingThemes, themes } = useContext( Themes ); + + if ( fetchingThemes ) { + return null; + } + + const themeMeta = themes.find( ( theme ) => theme.stylesheet === themeWithAmpIncompatibility.slug ); + const themeName = themeMeta?.name?.rendered ?? themeMeta?.name; + + return ( + <> +

+ { createInterpolateElement( + sprintf( + // translators: %s stands for a theme name. + __( 'AMP compatibility issues discovered with the %s theme.', 'amp' ), + themeName, + ), + { + b: , + }, + ) } +

+ + + ); +} + +ThemeWithAmpIncompatibility.propTypes = { + themeWithAmpIncompatibility: PropTypes.shape( { + slug: PropTypes.string, + urls: PropTypes.arrayOf( PropTypes.string ), + } ).isRequired, +}; diff --git a/src/Admin/PluginActivationSiteScan.php b/src/Admin/AfterActivationSiteScan.php similarity index 72% rename from src/Admin/PluginActivationSiteScan.php rename to src/Admin/AfterActivationSiteScan.php index 936e30fed32..3da7c9ac6dd 100644 --- a/src/Admin/PluginActivationSiteScan.php +++ b/src/Admin/AfterActivationSiteScan.php @@ -1,6 +1,6 @@ $this->get_amp_compatible_plugins_url(), + 'AMP_COMPATIBLE_THEMES_URL' => $this->get_amp_compatible_themes_url(), 'APP_ROOT_ID' => self::APP_ROOT_ID, + 'APP_ROOT_SIBLING_ID' => self::APP_ROOT_SIBLING_ID, 'OPTIONS_REST_PATH' => '/amp/v1/options', 'SETTINGS_LINK' => menu_page_url( AMP_Options_Manager::OPTION_NAME, false ), 'SCANNABLE_URLS_REST_PATH' => '/amp/v1/scannable-urls', @@ -180,6 +202,22 @@ protected function get_amp_compatible_plugins_url() { return 'https://amp-wp.org/ecosystem/plugins/'; } + /** + * Get a URL to AMP compatible themes directory. + * + * For users capable of installing themes, the link should lead to the Theme install page. + * Other users will be directed to the themes page on amp-wp.org. + * + * @return string URL to AMP compatible themes directory. + */ + protected function get_amp_compatible_themes_url() { + if ( current_user_can( 'switch_themes' ) ) { + return admin_url( '/theme-install.php?tab=amp-compatible' ); + } + + return 'https://amp-wp.org/ecosystem/themes/'; + } + /** * Adds REST paths to preload. */ @@ -197,6 +235,11 @@ protected function add_preload_rest_paths() { [ 'author', 'name', 'plugin', 'status', 'version' ], '/wp/v2/plugins' ), + add_query_arg( + '_fields', + [ 'author', 'name', 'status', 'stylesheet', 'version' ], + '/wp/v2/themes' + ), '/wp/v2/users/me', ]; diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index 01575d06ecd..89e98c66f28 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -70,6 +70,7 @@ final class AmpWpPlugin extends ServiceBasedPlugin { */ const SERVICES = [ 'admin.analytics_menu' => Admin\AnalyticsOptionsSubmenu::class, + 'admin.after_activation_site_scan' => Admin\AfterActivationSiteScan::class, 'admin.google_fonts' => Admin\GoogleFonts::class, 'admin.onboarding_menu' => Admin\OnboardingWizardSubmenu::class, 'admin.onboarding_wizard' => Admin\OnboardingWizardSubmenuPage::class, @@ -108,7 +109,6 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'paired_routing' => PairedRouting::class, 'paired_url' => PairedUrl::class, 'plugin_activation_notice' => Admin\PluginActivationNotice::class, - 'plugin_activation_site_scan' => Admin\PluginActivationSiteScan::class, 'plugin_registry' => PluginRegistry::class, 'plugin_suppression' => PluginSuppression::class, 'reader_theme_loader' => ReaderThemeLoader::class, diff --git a/tests/php/src/Admin/PluginActivationSiteScanTest.php b/tests/php/src/Admin/AfterActivationSiteScanTest.php similarity index 63% rename from tests/php/src/Admin/PluginActivationSiteScanTest.php rename to tests/php/src/Admin/AfterActivationSiteScanTest.php index 907ac93102c..81d76496088 100644 --- a/tests/php/src/Admin/PluginActivationSiteScanTest.php +++ b/tests/php/src/Admin/AfterActivationSiteScanTest.php @@ -1,13 +1,13 @@ plugin_activation_site_scan = $this->injector->make( PluginActivationSiteScan::class ); + $this->after_activation_site_scan = $this->injector->make( AfterActivationSiteScan::class ); delete_option( 'amp-options' ); } @@ -57,11 +57,11 @@ public function tearDown() { /** @covers ::__construct() */ public function test__construct() { - $this->assertInstanceOf( PluginActivationSiteScan::class, $this->plugin_activation_site_scan ); - $this->assertInstanceOf( Conditional::class, $this->plugin_activation_site_scan ); - $this->assertInstanceOf( Delayed::class, $this->plugin_activation_site_scan ); - $this->assertInstanceOf( Service::class, $this->plugin_activation_site_scan ); - $this->assertInstanceOf( Registerable::class, $this->plugin_activation_site_scan ); + $this->assertInstanceOf( AfterActivationSiteScan::class, $this->after_activation_site_scan ); + $this->assertInstanceOf( Conditional::class, $this->after_activation_site_scan ); + $this->assertInstanceOf( Delayed::class, $this->after_activation_site_scan ); + $this->assertInstanceOf( Service::class, $this->after_activation_site_scan ); + $this->assertInstanceOf( Registerable::class, $this->after_activation_site_scan ); } /** @return array */ @@ -121,6 +121,42 @@ public function get_data_to_test_is_needed() { 'expected' => true, 'role' => 'administrator', ], + 'plugins_screen_with_get_activated' => [ + 'screen_hook' => 'plugins.php', + 'query_params' => [ 'activated' ], + 'expected' => false, + 'role' => 'administrator', + ], + 'themes_screen_no_get_vars' => [ + 'screen_hook' => 'themes.php', + 'query_params' => [], + 'expected' => false, + 'role' => 'administrator', + ], + 'admin_index_with_get_activated' => [ + 'screen_hook' => 'index.php', + 'query_params' => [ 'activated' ], + 'expected' => false, + 'role' => 'administrator', + ], + 'themes_screen_with_get_activated' => [ + 'screen_hook' => 'themes.php', + 'query_params' => [ 'activated' ], + 'expected' => true, + 'role' => 'administrator', + ], + 'themes_screen_with_get_activated_not_admin' => [ + 'screen_hook' => 'themes.php', + 'query_params' => [ 'activated' ], + 'expected' => false, + 'role' => 'editor', + ], + 'themes_screen_with_get_activate' => [ + 'screen_hook' => 'themes.php', + 'query_params' => [ 'activate' ], + 'expected' => false, + 'role' => 'administrator', + ], ]; } @@ -149,25 +185,25 @@ public function test_is_needed( $screen_hook, $query_params, $expected, $role ) } wp_set_current_user( self::factory()->user->create( compact( 'role' ) ) ); - $this->assertEquals( $expected, PluginActivationSiteScan::is_needed() ); + $this->assertEquals( $expected, AfterActivationSiteScan::is_needed() ); } /** - * Tests PluginActivationSiteScan::register + * Tests AfterActivationSiteScan::register * * @covers ::register */ public function test_register_with_cap() { - $this->plugin_activation_site_scan->register(); - $this->assertEquals( 10, has_action( 'pre_current_active_plugins', [ $this->plugin_activation_site_scan, 'render_notice' ] ) ); - $this->assertEquals( 10, has_action( 'admin_enqueue_scripts', [ $this->plugin_activation_site_scan, 'enqueue_assets' ] ) ); + $this->after_activation_site_scan->register(); + $this->assertEquals( 10, has_action( 'pre_current_active_plugins', [ $this->after_activation_site_scan, 'render_notice' ] ) ); + $this->assertEquals( 10, has_action( 'admin_enqueue_scripts', [ $this->after_activation_site_scan, 'enqueue_assets' ] ) ); } /** * @covers ::render_notice */ public function test_render_notice() { - $this->assertStringContainsString( 'id="amp-site-scan-notice"', get_echo( [ $this->plugin_activation_site_scan, 'render_notice' ] ) ); + $this->assertStringContainsString( 'id="amp-site-scan-notice"', get_echo( [ $this->after_activation_site_scan, 'render_notice' ] ) ); } /** @return array */ @@ -204,16 +240,17 @@ function ( $caps, $cap ) { $handle = 'amp-site-scan-notice'; - $rest_preloader = $this->get_private_property( $this->plugin_activation_site_scan, 'rest_preloader' ); + $rest_preloader = $this->get_private_property( $this->after_activation_site_scan, 'rest_preloader' ); $this->assertCount( 0, $this->get_private_property( $rest_preloader, 'paths' ) ); - $this->plugin_activation_site_scan->enqueue_assets(); + $this->after_activation_site_scan->enqueue_assets(); $this->assertTrue( wp_script_is( $handle ) ); $this->assertTrue( wp_style_is( $handle ) ); $script_before = implode( '', wp_scripts()->get_data( $handle, 'before' ) ); $this->assertStringContainsString( 'var ampSiteScanNotice', $script_before ); $this->assertStringContainsString( 'AMP_COMPATIBLE_PLUGINS_URL', $script_before ); + $this->assertStringContainsString( 'AMP_COMPATIBLE_THEMES_URL', $script_before ); $this->assertStringContainsString( 'VALIDATE_NONCE', $script_before ); if ( $can_validate ) { $this->assertStringContainsString( AMP_Validation_Manager::get_amp_validate_nonce(), $script_before ); @@ -227,6 +264,7 @@ function ( $caps, $cap ) { '/amp/v1/options', '/amp/v1/scannable-urls?_fields%5B0%5D=url&_fields%5B1%5D=amp_url&_fields%5B2%5D=type&_fields%5B3%5D=label', '/wp/v2/plugins?_fields%5B0%5D=author&_fields%5B1%5D=name&_fields%5B2%5D=plugin&_fields%5B3%5D=status&_fields%5B4%5D=version', + '/wp/v2/themes?_fields%5B0%5D=author&_fields%5B1%5D=name&_fields%5B2%5D=status&_fields%5B3%5D=stylesheet&_fields%5B4%5D=version', '/wp/v2/users/me', ], $this->get_private_property( $rest_preloader, 'paths' ) @@ -239,9 +277,20 @@ function ( $caps, $cap ) { */ public function test_get_amp_compatible_plugins_url() { wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $this->assertStringContainsString( '/plugin-install.php?tab=amp-compatible', $this->call_private_method( $this->plugin_activation_site_scan, 'get_amp_compatible_plugins_url' ) ); + $this->assertStringContainsString( '/plugin-install.php?tab=amp-compatible', $this->call_private_method( $this->after_activation_site_scan, 'get_amp_compatible_plugins_url' ) ); + + wp_set_current_user( self::factory()->user->create( [ 'role' => 'author' ] ) ); + $this->assertSame( 'https://amp-wp.org/ecosystem/plugins/', $this->call_private_method( $this->after_activation_site_scan, 'get_amp_compatible_plugins_url' ) ); + } + + /** + * @covers ::get_amp_compatible_themes_url + */ + public function test_get_amp_compatible_themes_url() { + wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->assertStringContainsString( '/theme-install.php?tab=amp-compatible', $this->call_private_method( $this->after_activation_site_scan, 'get_amp_compatible_themes_url' ) ); wp_set_current_user( self::factory()->user->create( [ 'role' => 'author' ] ) ); - $this->assertSame( 'https://amp-wp.org/ecosystem/plugins/', $this->call_private_method( $this->plugin_activation_site_scan, 'get_amp_compatible_plugins_url' ) ); + $this->assertSame( 'https://amp-wp.org/ecosystem/themes/', $this->call_private_method( $this->after_activation_site_scan, 'get_amp_compatible_themes_url' ) ); } }