From 7c15cc9a8d5bb95d0592bdd833b9227b10d662be Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Thu, 1 Jul 2021 15:28:01 +0200 Subject: [PATCH 01/58] Add `validated-urls` endpoint returning stylesheets data --- .../URLValidationRESTController.php | 101 +++++++++++- src/Validation/ValidatedUrlDataProvider.php | 149 ++++++++++++++++++ .../URLValidationRESTControllerTest.php | 105 ++++++++++++ 3 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 src/Validation/ValidatedUrlDataProvider.php diff --git a/src/Validation/URLValidationRESTController.php b/src/Validation/URLValidationRESTController.php index df7f657943c..13c9d1b41ff 100644 --- a/src/Validation/URLValidationRESTController.php +++ b/src/Validation/URLValidationRESTController.php @@ -63,7 +63,7 @@ public static function get_registration_action() { * Constructor. * * @param URLValidationProvider $url_validation_provider URLValidationProvider instance. - * @param UserAccess $dev_tools_user_access UserAccess instance. + * @param UserAccess $dev_tools_user_access UserAccess instance. */ public function __construct( URLValidationProvider $url_validation_provider, UserAccess $dev_tools_user_access ) { $this->namespace = 'amp/v1'; @@ -104,6 +104,29 @@ public function register() { ] ); + register_rest_route( + $this->namespace, + '/validated-urls/(?P[\d]+)', + [ + 'args' => [ + 'id' => [ + 'description' => __( 'Post ID for the AMP validated URL post.', 'amp' ), + 'required' => true, + 'type' => 'integer', + 'minimum' => 1, + 'validate_callback' => [ $this, 'validate_amp_validated_url_post_id_param' ], + ], + ], + [ + 'args' => $this->get_endpoint_args_for_item_schema( WP_REST_Server::READABLE ), + 'methods' => WP_REST_Server::READABLE, + 'callback' => [ $this, 'get_validated_url' ], + 'permission_callback' => [ $this, 'get_item_permissions_check' ], + ], + 'schema' => [ $this, 'get_public_item_schema' ], + ] + ); + // @todo Additional endpoint to validate a URL (from a URL rather than a post ID). } @@ -143,6 +166,38 @@ public function validate_post_id_param( $id, $request, $param ) { return true; } + /** + * Validate AMP validated URL post ID param. + * + * @param string|int $id Post ID. + * @param WP_REST_Request $request REST request. + * @param string $param Param name ('id'). + * @return true|WP_Error True on valid, WP_Error otherwise. + */ + public function validate_amp_validated_url_post_id_param( $id, $request, $param ) { + // First enforce the schema to ensure $id is an integer greater than 0. + $validity = rest_validate_request_arg( $id, $request, $param ); + if ( is_wp_error( $validity ) ) { + return $validity; + } + + // Make sure the post exists and is of correct post type. + $post = get_post( (int) $id ); + if ( + ! $post instanceof WP_Post + || + AMP_Validated_URL_Post_Type::POST_TYPE_SLUG !== get_post_type( $post ) + ) { + return new WP_Error( + 'rest_amp_validated_url_post_invalid_id', + __( 'Invalid post ID.', 'default' ), + [ 'status' => 404 ] + ); + } + + return true; + } + /** * Checks whether the current user can view AMP validation results. * @@ -161,6 +216,24 @@ public function create_item_permissions_check( $request ) { // phpcs:ignore Vari return true; } + /** + * Checks whether the current user can view results of a URL AMP validation. + * + * @param WP_REST_Request $request Full details about the request. + * @return true|WP_Error True if the request has permission; WP_Error object otherwise. + */ + public function get_item_permissions_check( $request ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable + if ( ! $this->dev_tools_user_access->is_user_enabled() ) { + return new WP_Error( + 'amp_rest_no_dev_tools', + __( 'Sorry, you do not have access to dev tools for the AMP plugin for WordPress.', 'amp' ), + [ 'status' => rest_authorization_required_code() ] + ); + } + + return true; + } + /** * Validate preview nonce. * @@ -240,6 +313,32 @@ public function validate_post_url( $request ) { return rest_ensure_response( $this->filter_response_by_context( $data, $request['context'] ) ); } + /** + * Returns validation information about a URL. + * + * @param WP_REST_Request $request Full details about the request. + * + * @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure. + */ + public function get_validated_url( $request ) { + $post_id = (int) $request['id']; + $validated_url = new ValidatedUrlDataProvider( $post_id ); + + if ( is_wp_error( $validated_url->is_valid() ) ) { + return $validated_url->is_valid(); + } + + $data = [ + 'id' => $validated_url->get_id(), + 'url' => $validated_url->get_url(), + 'date' => $validated_url->get_date(), + 'author' => $validated_url->get_author(), + 'stylesheets' => $validated_url->get_stylesheets(), + ]; + + return rest_ensure_response( $this->filter_response_by_context( $data, $request['context'] ) ); + } + /** * Retrieves the schema for plugin options provided by the endpoint. * diff --git a/src/Validation/ValidatedUrlDataProvider.php b/src/Validation/ValidatedUrlDataProvider.php new file mode 100644 index 00000000000..bc9a883e12b --- /dev/null +++ b/src/Validation/ValidatedUrlDataProvider.php @@ -0,0 +1,149 @@ +post = get_post( $post_id ); + } + + /** + * Check if the validated URL data instance is valid. + * + * @return bool|WP_Error + */ + public function is_valid() { + if ( empty( $this->post ) ) { + return new WP_Error( + 'amp_validated_url_missing_id', + __( 'Unable to retrieve validation data for this ID.', 'amp' ), + [ 'status' => 404 ] + ); + } + + return false; + } + + /** + * Get validated URL ID. + * + * @return int|null + */ + public function get_id() { + if ( ! $this->post ) { + return null; + } + + return $this->post->ID; + } + + /** + * Get the URL that was validated. + * + * @return string|null + */ + public function get_url() { + if ( ! $this->post ) { + return null; + } + + return AMP_Validated_URL_Post_Type::get_url_from_post( $this->post ); + } + + /** + * Get the date that the URL was validated. + * + * @return string|null + */ + public function get_date() { + if ( ! $this->post ) { + return null; + } + + return $this->post->post_date; + } + + /** + * Get the user that last validated the URL. + * + * @return int|null + */ + public function get_author() { + if ( ! $this->post ) { + return null; + } + + return (int) $this->post->post_author; + } + + /** + * Get the validated URL stylesheets data. + * + * @return array|WP_Error + */ + public function get_stylesheets() { + if ( null !== $this->stylesheets ) { + return $this->stylesheets; + } + + $stylesheets = get_post_meta( $this->get_id(), AMP_Validated_URL_Post_Type::STYLESHEETS_POST_META_KEY, true ); + + if ( empty( $stylesheets ) ) { + return new WP_Error( + 'amp_validated_url_stylesheets_no_longer_available', + __( 'Stylesheet information for this URL is no longer available. Such data is automatically deleted after a week to reduce database storage. It is of little value to store long-term given that it becomes stale as themes and plugins are updated. To obtain the latest stylesheet information, recheck this URL.', 'amp' ), + [ 'status' => 404 ] + ); + } + + $stylesheets = json_decode( $stylesheets, true ); + + if ( ! is_array( $stylesheets ) ) { + return new WP_Error( + 'amp_validated_url_stylesheets_missing', + __( 'Unable to retrieve stylesheets data for this URL.', 'amp' ), + [ 'status' => 404 ] + ); + } + + $this->stylesheets = $stylesheets; + + return $this->stylesheets; + } +} diff --git a/tests/php/src/Validation/URLValidationRESTControllerTest.php b/tests/php/src/Validation/URLValidationRESTControllerTest.php index bfce599b4dc..3f02a58acac 100644 --- a/tests/php/src/Validation/URLValidationRESTControllerTest.php +++ b/tests/php/src/Validation/URLValidationRESTControllerTest.php @@ -81,6 +81,18 @@ public function test_create_item_permissions_check() { $this->assertTrue( $this->controller->create_item_permissions_check( new WP_REST_Request( 'POST', '/amp/v1/validate-post-url/' ) ) ); } + /** @covers ::get_item_permissions_check() */ + public function test_get_item_permissions_check() { + $this->assertWPError( $this->controller->get_item_permissions_check( new WP_REST_Request( 'GET', '/amp/v1/validated-urls/' ) ) ); + + wp_set_current_user( self::factory()->user->create( [ 'role' => 'author' ] ) ); + $this->assertWPError( $this->controller->get_item_permissions_check( new WP_REST_Request( 'GET', '/amp/v1/validated-urls/' ) ) ); + + wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + $this->user_access->set_user_enabled( wp_get_current_user(), true ); + $this->assertTrue( $this->controller->get_item_permissions_check( new WP_REST_Request( 'GET', '/amp/v1/validated-urls/' ) ) ); + } + /** @covers ::is_valid_preview_nonce() */ public function test_is_valid_preview_nonce() { $user_id = self::factory()->user->create( [ 'role' => 'author' ] ); @@ -226,6 +238,99 @@ public function test_validate_post_url( $params, $post_type, $user_role, $expect } } + /** @return array */ + public function get_data_for_test_get_validated_url() { + return [ + 'not_int' => [ + 'foo', + null, + 'administrator', + 'rest_no_route', + ], + + 'too_small' => [ + -1, + null, + 'administrator', + 'rest_no_route', + ], + + 'empty_post' => [ + 0, + null, + 'administrator', + 'rest_invalid_param', + ], + + 'revision_id' => [ + '{{id}}', + 'revision', + 'administrator', + 'rest_invalid_param', + ], + + 'post_id' => [ + '{{id}}', + 'post', + 'administrator', + 'rest_invalid_param', + ], + + 'as_author' => [ + '{{id}}', + \AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, + 'author', + 'amp_rest_no_dev_tools', + ], + + 'amp_validated_url_id' => [ + '{{id}}', + \AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, + 'administrator', + false, + ], + ]; + } + + /** + * @dataProvider get_data_for_test_get_validated_url() + * @covers ::get_validated_url() + * @covers ::validate_amp_validated_url_post_id_param() + * + * @param string|int $post_id Post ID. + * @param string|null $post_type Post type. + * @param string $user_role User role. + * @param false|string $expected_error Expected error. + */ + public function test_get_validated_url( $post_id, $post_type, $user_role, $expected_error ) { + add_filter( 'amp_dev_tools_user_default_enabled', '__return_true' ); + $user_id = self::factory()->user->create( [ 'role' => $user_role ] ); + + wp_set_current_user( $user_id ); + + if ( isset( $post_id ) && '{{id}}' === $post_id && $post_type ) { + $post_id = self::factory()->post->create( compact( 'post_type' ) ); + } + + $this->controller->register(); + $request = new WP_REST_Request( 'GET', '/amp/v1/validated-urls/' . $post_id ); + $response = rest_get_server()->dispatch( $request ); + + if ( false === $expected_error ) { + $this->assertFalse( $response->is_error() ); + $data = $response->get_data(); + $this->assertEquals( $data['id'], $post_id ); + $this->assertArrayHasKey( 'url', $data ); + $this->assertArrayHasKey( 'date', $data ); + $this->assertArrayHasKey( 'author', $data ); + $this->assertArrayHasKey( 'stylesheets', $data ); + } else { + $this->assertTrue( $response->is_error() ); + $error = $response->as_error(); + $this->assertEquals( $expected_error, $error->get_error_code() ); + } + } + /** * Tests URLValidationRESTController::get_item_schema. * From 73568d9aec1b4bbea364f0ac4c67439927e7ea0d Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Mon, 5 Jul 2021 16:24:04 +0200 Subject: [PATCH 02/58] Introduce `ValidatedUrlData` that is returned by the `ValidatedUrlDataProvider` --- .../URLValidationRESTController.php | 39 ++++-- src/Validation/ValidatedUrlData.php | 132 ++++++++++++++++++ src/Validation/ValidatedUrlDataProvider.php | 122 +--------------- .../URLValidationRESTControllerTest.php | 3 +- 4 files changed, 165 insertions(+), 131 deletions(-) create mode 100644 src/Validation/ValidatedUrlData.php diff --git a/src/Validation/URLValidationRESTController.php b/src/Validation/URLValidationRESTController.php index 13c9d1b41ff..b793dfc1624 100644 --- a/src/Validation/URLValidationRESTController.php +++ b/src/Validation/URLValidationRESTController.php @@ -43,6 +43,13 @@ final class URLValidationRESTController extends WP_REST_Controller implements De */ private $dev_tools_user_access; + /** + * ValidatedUrlDataProvider instance. + * + * @var ValidatedUrlDataProvider + */ + private $validated_url_data_provider; + /** * Response schema. * @@ -62,13 +69,15 @@ public static function get_registration_action() { /** * Constructor. * - * @param URLValidationProvider $url_validation_provider URLValidationProvider instance. - * @param UserAccess $dev_tools_user_access UserAccess instance. + * @param URLValidationProvider $url_validation_provider URLValidationProvider instance. + * @param UserAccess $dev_tools_user_access UserAccess instance. + * @param ValidatedUrlDataProvider $validated_url_data_provider ValidatedUrlDataProvider instance. */ - public function __construct( URLValidationProvider $url_validation_provider, UserAccess $dev_tools_user_access ) { - $this->namespace = 'amp/v1'; - $this->url_validation_provider = $url_validation_provider; - $this->dev_tools_user_access = $dev_tools_user_access; + public function __construct( URLValidationProvider $url_validation_provider, UserAccess $dev_tools_user_access, ValidatedUrlDataProvider $validated_url_data_provider ) { + $this->namespace = 'amp/v1'; + $this->url_validation_provider = $url_validation_provider; + $this->dev_tools_user_access = $dev_tools_user_access; + $this->validated_url_data_provider = $validated_url_data_provider; } /** @@ -321,19 +330,19 @@ public function validate_post_url( $request ) { * @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure. */ public function get_validated_url( $request ) { - $post_id = (int) $request['id']; - $validated_url = new ValidatedUrlDataProvider( $post_id ); + $post_id = (int) $request['id']; + $validated_url_data = $this->validated_url_data_provider->for_id( $post_id ); - if ( is_wp_error( $validated_url->is_valid() ) ) { - return $validated_url->is_valid(); + if ( is_wp_error( $validated_url_data ) ) { + return $validated_url_data; } $data = [ - 'id' => $validated_url->get_id(), - 'url' => $validated_url->get_url(), - 'date' => $validated_url->get_date(), - 'author' => $validated_url->get_author(), - 'stylesheets' => $validated_url->get_stylesheets(), + 'id' => $validated_url_data->get_id(), + 'url' => $validated_url_data->get_url(), + 'date' => $validated_url_data->get_date(), + 'author' => $validated_url_data->get_author(), + 'stylesheets' => $validated_url_data->get_stylesheets(), ]; return rest_ensure_response( $this->filter_response_by_context( $data, $request['context'] ) ); diff --git a/src/Validation/ValidatedUrlData.php b/src/Validation/ValidatedUrlData.php new file mode 100644 index 00000000000..c5eec9049a5 --- /dev/null +++ b/src/Validation/ValidatedUrlData.php @@ -0,0 +1,132 @@ +post = $post; + } + + /** + * Get validated URL ID. + * + * @return int|null + */ + public function get_id() { + if ( ! $this->post->ID ) { + return null; + } + + return $this->post->ID; + } + + /** + * Get the URL that was validated. + * + * @return string|null + */ + public function get_url() { + if ( ! $this->post ) { + return null; + } + + return AMP_Validated_URL_Post_Type::get_url_from_post( $this->post ); + } + + /** + * Get the date that the URL was validated. + * + * @return string|null + */ + public function get_date() { + if ( ! $this->post->post_date ) { + return null; + } + + return $this->post->post_date; + } + + /** + * Get the user that last validated the URL. + * + * @return int|null + */ + public function get_author() { + if ( ! $this->post->post_author ) { + return null; + } + + return (int) $this->post->post_author; + } + + /** + * Get the validated URL stylesheets data. + * + * @return array|WP_Error + */ + public function get_stylesheets() { + if ( null !== $this->stylesheets ) { + return $this->stylesheets; + } + + $stylesheets = get_post_meta( $this->get_id(), AMP_Validated_URL_Post_Type::STYLESHEETS_POST_META_KEY, true ); + + if ( empty( $stylesheets ) ) { + return new WP_Error( + 'amp_validated_url_stylesheets_no_longer_available', + __( 'Stylesheet information for this URL is no longer available. Such data is automatically deleted after a week to reduce database storage. It is of little value to store long-term given that it becomes stale as themes and plugins are updated. To obtain the latest stylesheet information, recheck this URL.', 'amp' ), + [ 'status' => 404 ] + ); + } + + $stylesheets = json_decode( $stylesheets, true ); + + if ( ! is_array( $stylesheets ) ) { + return new WP_Error( + 'amp_validated_url_stylesheets_missing', + __( 'Unable to retrieve stylesheets data for this URL.', 'amp' ), + [ 'status' => 404 ] + ); + } + + $this->stylesheets = $stylesheets; + + return $this->stylesheets; + } +} diff --git a/src/Validation/ValidatedUrlDataProvider.php b/src/Validation/ValidatedUrlDataProvider.php index bc9a883e12b..1f477eeb88c 100644 --- a/src/Validation/ValidatedUrlDataProvider.php +++ b/src/Validation/ValidatedUrlDataProvider.php @@ -8,9 +8,7 @@ namespace AmpProject\AmpWP\Validation; -use AMP_Validated_URL_Post_Type; use WP_Error; -use WP_Post; /** * ValidatedUrlDataProvider class. @@ -21,35 +19,16 @@ final class ValidatedUrlDataProvider { /** - * Validated URL post. - * - * @var WP_Post - */ - private $post; - - /** - * Validated URL stylesheets data parsed from the JSON string in post meta. - * - * @var array|null - */ - private $stylesheets = null; - - /** - * ValidatedUrlDataProvider constructor. + * Provide validated URL data for a given post ID. * * @param int $post_id Post ID. - */ - public function __construct( $post_id ) { - $this->post = get_post( $post_id ); - } - - /** - * Check if the validated URL data instance is valid. * - * @return bool|WP_Error + * @return ValidatedUrlData|WP_Error Validated URL data. */ - public function is_valid() { - if ( empty( $this->post ) ) { + public function for_id( $post_id ) { + $post = get_post( $post_id ); + + if ( empty( $post ) ) { return new WP_Error( 'amp_validated_url_missing_id', __( 'Unable to retrieve validation data for this ID.', 'amp' ), @@ -57,93 +36,6 @@ public function is_valid() { ); } - return false; - } - - /** - * Get validated URL ID. - * - * @return int|null - */ - public function get_id() { - if ( ! $this->post ) { - return null; - } - - return $this->post->ID; - } - - /** - * Get the URL that was validated. - * - * @return string|null - */ - public function get_url() { - if ( ! $this->post ) { - return null; - } - - return AMP_Validated_URL_Post_Type::get_url_from_post( $this->post ); - } - - /** - * Get the date that the URL was validated. - * - * @return string|null - */ - public function get_date() { - if ( ! $this->post ) { - return null; - } - - return $this->post->post_date; - } - - /** - * Get the user that last validated the URL. - * - * @return int|null - */ - public function get_author() { - if ( ! $this->post ) { - return null; - } - - return (int) $this->post->post_author; - } - - /** - * Get the validated URL stylesheets data. - * - * @return array|WP_Error - */ - public function get_stylesheets() { - if ( null !== $this->stylesheets ) { - return $this->stylesheets; - } - - $stylesheets = get_post_meta( $this->get_id(), AMP_Validated_URL_Post_Type::STYLESHEETS_POST_META_KEY, true ); - - if ( empty( $stylesheets ) ) { - return new WP_Error( - 'amp_validated_url_stylesheets_no_longer_available', - __( 'Stylesheet information for this URL is no longer available. Such data is automatically deleted after a week to reduce database storage. It is of little value to store long-term given that it becomes stale as themes and plugins are updated. To obtain the latest stylesheet information, recheck this URL.', 'amp' ), - [ 'status' => 404 ] - ); - } - - $stylesheets = json_decode( $stylesheets, true ); - - if ( ! is_array( $stylesheets ) ) { - return new WP_Error( - 'amp_validated_url_stylesheets_missing', - __( 'Unable to retrieve stylesheets data for this URL.', 'amp' ), - [ 'status' => 404 ] - ); - } - - $this->stylesheets = $stylesheets; - - return $this->stylesheets; + return new ValidatedUrlData( $post ); } } diff --git a/tests/php/src/Validation/URLValidationRESTControllerTest.php b/tests/php/src/Validation/URLValidationRESTControllerTest.php index 3f02a58acac..050e84eb3d7 100644 --- a/tests/php/src/Validation/URLValidationRESTControllerTest.php +++ b/tests/php/src/Validation/URLValidationRESTControllerTest.php @@ -12,6 +12,7 @@ use AmpProject\AmpWP\Tests\TestCase; use AmpProject\AmpWP\Validation\URLValidationProvider; use AmpProject\AmpWP\Validation\URLValidationRESTController; +use AmpProject\AmpWP\Validation\ValidatedUrlDataProvider; use WP_REST_Controller; use WP_REST_Request; @@ -47,7 +48,7 @@ public function setUp() { do_action( 'rest_api_init' ); $this->user_access = new UserAccess(); - $this->controller = new URLValidationRESTController( new URLValidationProvider(), $this->user_access ); + $this->controller = new URLValidationRESTController( new URLValidationProvider(), $this->user_access, new ValidatedUrlDataProvider() ); add_filter( 'pre_http_request', [ $this, 'get_validate_response' ] ); } From 8aba364ecf9322f59b4062666311cd39b78a5dd7 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Mon, 5 Jul 2021 20:32:07 +0200 Subject: [PATCH 03/58] Add Validated URL JavaScript entry point --- .../validated-url-provider/index.js | 102 ++++++++++++++++++ assets/src/validated-url-page/index.js | 94 ++++++++++++++++ .../class-amp-validated-url-post-type.php | 48 +++++++++ webpack.config.js | 7 +- 4 files changed, 248 insertions(+), 3 deletions(-) create mode 100644 assets/src/components/validated-url-provider/index.js create mode 100644 assets/src/validated-url-page/index.js diff --git a/assets/src/components/validated-url-provider/index.js b/assets/src/components/validated-url-provider/index.js new file mode 100644 index 00000000000..bdea46188ac --- /dev/null +++ b/assets/src/components/validated-url-provider/index.js @@ -0,0 +1,102 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * WordPress dependencies + */ +import { + createContext, + useContext, + useEffect, + useRef, + useState, +} from '@wordpress/element'; +import apiFetch from '@wordpress/api-fetch'; + +/** + * Internal dependencies + */ +import { ErrorContext } from '../error-context-provider'; +import { useAsyncError } from '../../utils/use-async-error'; + +export const ValidatedUrl = createContext(); + +/** + * Validated URL data context provider. + * + * @param {Object} props Component props. + * @param {any} props.children Component children. + * @param {boolean} props.hasErrorBoundary Whether the component is wrapped in an error boundary. + * @param {number} props.postId Validated URL post ID. + * @param {string} props.validatedUrlsRestPath REST endpoint to retrieve validated URL data. + */ +export function ValidatedUrlProvider( { children, hasErrorBoundary = false, postId, validatedUrlsRestPath } ) { + const [ validatedUrl, setValidatedUrl ] = useState( {} ); + const [ fetchingValidatedUrl, setFetchingValidatedUrl ] = useState( null ); + + const { error, setError } = useContext( ErrorContext ); + const { setAsyncError } = useAsyncError(); + + /** + * This component sets state inside async functions. + * Use this ref to prevent state updates after unmount. + */ + const hasUnmounted = useRef( false ); + useEffect( () => () => { + hasUnmounted.current = true; + }, [] ); + + /** + * Fetches validated URL data. + */ + useEffect( () => { + if ( error || Object.keys( validatedUrl ).length || fetchingValidatedUrl ) { + return; + } + + ( async () => { + setFetchingValidatedUrl( true ); + + try { + const fetchedValidatedUrl = await apiFetch( { + path: `${ validatedUrlsRestPath }/${ postId }`, + } ); + + if ( hasUnmounted.current === true ) { + return; + } + + setValidatedUrl( fetchedValidatedUrl ); + } catch ( e ) { + if ( hasUnmounted.current === true ) { + return; + } + + setError( e ); + + if ( hasErrorBoundary ) { + setAsyncError( e ); + } + + return; + } + + setFetchingValidatedUrl( false ); + } )(); + }, [ error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); + + return ( + + { children } + + ); +} + +ValidatedUrlProvider.propTypes = { + children: PropTypes.any, + hasErrorBoundary: PropTypes.bool, + postId: PropTypes.number, + validatedUrlsRestPath: PropTypes.string, +}; diff --git a/assets/src/validated-url-page/index.js b/assets/src/validated-url-page/index.js new file mode 100644 index 00000000000..0f1c52fd0f6 --- /dev/null +++ b/assets/src/validated-url-page/index.js @@ -0,0 +1,94 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +import { + APP_ROOT_ID, + POST_ID, + VALIDATED_URLS_REST_PATH, +} from 'amp-settings'; // From WP inline script. + +/** + * WordPress dependencies + */ +import domReady from '@wordpress/dom-ready'; +import { render, useContext } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { Loading } from '../components/loading'; +import { ErrorBoundary } from '../components/error-boundary'; +import { ErrorContextProvider } from '../components/error-context-provider'; +import { ErrorScreen } from '../components/error-screen'; +import { ValidatedUrl, ValidatedUrlProvider } from '../components/validated-url-provider'; + +let errorHandler; + +/** + * Context providers for the settings page. + * + * @param {Object} props Component props. + * @param {any} props.children Context consumers. + */ +function Providers( { children } ) { + global.removeEventListener( 'error', errorHandler ); + + return ( + + + + { children } + + + + ); +} +Providers.propTypes = { + children: PropTypes.any, +}; + +/** + * Validated URL page application root. + */ +function Root() { + const { fetchingValidatedUrl, validatedUrl } = useContext( ValidatedUrl ); + + if ( fetchingValidatedUrl !== false ) { + return ; + } + + return ( +

+ { validatedUrl.url } +

+ ); +} + +domReady( () => { + const root = document.getElementById( APP_ROOT_ID ); + + if ( ! root ) { + return; + } + + errorHandler = ( event ) => { + // Handle only own errors. + if ( event.filename && /amp-validated-url-page(\.min)?\.js/.test( event.filename ) ) { + render( , root ); + } + }; + + global.addEventListener( 'error', errorHandler ); + + render( + + + , + root, + ); +} ); diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 83e1ac542fd..530338512cb 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -90,6 +90,20 @@ class AMP_Validated_URL_Post_Type { */ const EDIT_POST_SCRIPT_HANDLE = 'amp-validated-url-post-edit-screen'; + /** + * The handle for the AMP validated URL page script. + * + * @var string + */ + const AMP_VALIDATED_URL_PAGE_SCRIPT_HANDLE = 'amp-validated-url-page'; + + /** + * HTML ID for the app root element. + * + * @var string + */ + const AMP_VALIDATED_URL_PAGE_APP_ROOT_ID = 'amp-validated-url-root'; + /** * The query arg for the number of URLs tested. * @@ -1986,6 +2000,8 @@ static function( $result ) { * Enqueue scripts for the edit post screen. */ public static function enqueue_edit_post_screen_scripts() { + global $post; + $current_screen = get_current_screen(); if ( 'post' !== $current_screen->base || self::POST_TYPE_SLUG !== $current_screen->post_type ) { return; @@ -2007,6 +2023,35 @@ public static function enqueue_edit_post_screen_scripts() { true ); + // React-based validated URL page component. + $asset_file = AMP__DIR__ . '/assets/js/' . self::AMP_VALIDATED_URL_PAGE_SCRIPT_HANDLE . '.asset.php'; + $asset = require $asset_file; + $dependencies = $asset['dependencies']; + $version = $asset['version']; + + wp_enqueue_script( + self::AMP_VALIDATED_URL_PAGE_SCRIPT_HANDLE, + amp_get_asset_url( 'js/' . self::AMP_VALIDATED_URL_PAGE_SCRIPT_HANDLE . '.js' ), + $dependencies, + $version, + true + ); + + $validated_url_page_data = [ + 'APP_ROOT_ID' => self::AMP_VALIDATED_URL_PAGE_APP_ROOT_ID, + 'POST_ID' => $post->ID, + 'VALIDATED_URLS_REST_PATH' => '/amp/v1/validated-urls', + ]; + + wp_add_inline_script( + self::AMP_VALIDATED_URL_PAGE_SCRIPT_HANDLE, + sprintf( + 'var ampSettings = %s;', + wp_json_encode( $validated_url_page_data ) + ), + 'before' + ); + // @todo This is likely dead code. $current_screen = get_current_screen(); if ( $current_screen && 'post' === $current_screen->base && self::POST_TYPE_SLUG === $current_screen->post_type ) { @@ -2280,6 +2325,9 @@ public static function print_status_meta_box( $post ) { * @return void */ public static function print_stylesheets_meta_box( $post ) { + ?> +
+ ID, self::STYLESHEETS_POST_META_KEY, true ); if ( empty( $stylesheets ) ) { printf( diff --git a/webpack.config.js b/webpack.config.js index e9479f4e094..460e3dff5b9 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -280,12 +280,13 @@ const onboardingWizard = { ], }; -const settingsPage = { +const adminPages = { ...sharedConfig, entry: { 'wp-api-fetch': './assets/src/polyfills/api-fetch.js', 'wp-components': '@wordpress/components/build-style/style.css', 'amp-settings': './assets/src/settings-page', + 'amp-validated-url-page': './assets/src/validated-url-page', }, externals: { 'amp-settings': 'ampSettings', @@ -328,7 +329,7 @@ const settingsPage = { // Ensure assets generated by `DependencyExtractionWebpackPlugin` are also ignored for styles entries. new IgnoreExtraneousStyleAssets(), new WebpackBar( { - name: 'Settings page', + name: 'Admin Pages', color: '#67b255', } ), ], @@ -402,7 +403,7 @@ module.exports = [ customizer, wpPolyfills, onboardingWizard, - settingsPage, + adminPages, styles, mobileRedirection, ]; From e43d2963695ea0611211d740cb33a92eb0041a3b Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 7 Jul 2021 11:26:12 +0200 Subject: [PATCH 04/58] Use local variable for storing current post --- .../class-amp-validated-url-post-type.php | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 530338512cb..4d9f3dd0923 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -2000,8 +2000,6 @@ static function( $result ) { * Enqueue scripts for the edit post screen. */ public static function enqueue_edit_post_screen_scripts() { - global $post; - $current_screen = get_current_screen(); if ( 'post' !== $current_screen->base || self::POST_TYPE_SLUG !== $current_screen->post_type ) { return; @@ -2023,6 +2021,22 @@ public static function enqueue_edit_post_screen_scripts() { true ); + $post = get_post(); + + // @todo This is likely dead code. + $current_screen = get_current_screen(); + if ( $current_screen && 'post' === $current_screen->base && self::POST_TYPE_SLUG === $current_screen->post_type ) { + $data = [ + 'amp_enabled' => self::is_amp_enabled_on_post( $post ), + ]; + + wp_localize_script( + self::EDIT_POST_SCRIPT_HANDLE, + 'ampValidation', + $data + ); + } + // React-based validated URL page component. $asset_file = AMP__DIR__ . '/assets/js/' . self::AMP_VALIDATED_URL_PAGE_SCRIPT_HANDLE . '.asset.php'; $asset = require $asset_file; @@ -2052,21 +2066,6 @@ public static function enqueue_edit_post_screen_scripts() { 'before' ); - // @todo This is likely dead code. - $current_screen = get_current_screen(); - if ( $current_screen && 'post' === $current_screen->base && self::POST_TYPE_SLUG === $current_screen->post_type ) { - $post = get_post(); - $data = [ - 'amp_enabled' => self::is_amp_enabled_on_post( $post ), - ]; - - wp_localize_script( - self::EDIT_POST_SCRIPT_HANDLE, - 'ampValidation', - $data - ); - } - if ( function_exists( 'wp_set_script_translations' ) ) { wp_set_script_translations( self::EDIT_POST_SCRIPT_HANDLE, 'amp' ); } elseif ( function_exists( 'wp_get_jed_locale_data' ) || function_exists( 'gutenberg_get_jed_locale_data' ) ) { From 22aae9e0a21d08bd2f2c0111b668a6044c28bf7f Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 7 Jul 2021 11:27:12 +0200 Subject: [PATCH 05/58] Remove dead code --- .../class-amp-validated-url-post-type.php | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 4d9f3dd0923..27483f2b330 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -2021,22 +2021,6 @@ public static function enqueue_edit_post_screen_scripts() { true ); - $post = get_post(); - - // @todo This is likely dead code. - $current_screen = get_current_screen(); - if ( $current_screen && 'post' === $current_screen->base && self::POST_TYPE_SLUG === $current_screen->post_type ) { - $data = [ - 'amp_enabled' => self::is_amp_enabled_on_post( $post ), - ]; - - wp_localize_script( - self::EDIT_POST_SCRIPT_HANDLE, - 'ampValidation', - $data - ); - } - // React-based validated URL page component. $asset_file = AMP__DIR__ . '/assets/js/' . self::AMP_VALIDATED_URL_PAGE_SCRIPT_HANDLE . '.asset.php'; $asset = require $asset_file; @@ -2051,6 +2035,7 @@ public static function enqueue_edit_post_screen_scripts() { true ); + $post = get_post(); $validated_url_page_data = [ 'APP_ROOT_ID' => self::AMP_VALIDATED_URL_PAGE_APP_ROOT_ID, 'POST_ID' => $post->ID, From 8824f8302b1c56da19296c776cc44eac983bf0e5 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 7 Jul 2021 18:33:17 +0200 Subject: [PATCH 06/58] Fix and update unit tests --- ...test-class-amp-validated-url-post-type.php | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 4dd90c855ed..8ea2089fbb5 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -1320,14 +1320,19 @@ public function test_enqueue_edit_post_screen_scripts() { AMP_Validated_URL_Post_Type::enqueue_edit_post_screen_scripts(); $this->assertTrue( wp_script_is( 'autosave', 'enqueued' ) ); $this->assertFalse( wp_script_is( 'amp-validated-url-post-edit-screen', 'enqueued' ) ); + $this->assertFalse( wp_script_is( 'amp-validated-url-page', 'enqueued' ) ); - global $pagenow; + global $pagenow, $post; $pagenow = 'post.php'; + $post = self::factory()->post->create(); set_current_screen( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ); AMP_Validated_URL_Post_Type::enqueue_edit_post_screen_scripts(); $this->assertFalse( wp_script_is( 'autosave', 'enqueued' ) ); $this->assertTrue( wp_script_is( 'amp-validated-url-post-edit-screen', 'enqueued' ) ); + $this->assertTrue( wp_script_is( 'amp-validated-url-page', 'enqueued' ) ); + $this->assertStringContains( 'var ampSettings', get_echo( 'wp_print_scripts' ) ); $pagenow = null; + $post = null; } /** @@ -1470,6 +1475,21 @@ public function test_print_status_meta_box() { $this->assertStringContainsString( 'misc-pub-section', $output ); } + /** + * Test for print_stylesheets_meta_box() + * + * @covers \AMP_Validated_URL_Post_Type::print_stylesheets_meta_box() + */ + public function test_print_stylesheets_meta_box() { + AMP_Validation_Manager::init(); + wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + $validated_url_post = self::factory()->post->create_and_get( [ 'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ] ); + $output = get_echo( [ 'AMP_Validated_URL_Post_Type', 'print_stylesheets_meta_box' ], [ $validated_url_post ] ); + + $this->assertStringContains( '
', $output ); + } + /** * Test for render_single_url_list_table() * From 56ef32f8e9e7d7c04d9593b97f9fa7a105494864 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 7 Jul 2021 21:10:59 +0200 Subject: [PATCH 07/58] Calculate stylesheets sizes and add to app context --- .../validated-url-provider/index.js | 5 +- .../src/validated-url-page/helpers/index.js | 35 ++++++++++++ .../helpers/test/calculateStylesheetSizes.js | 56 +++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 assets/src/validated-url-page/helpers/index.js create mode 100644 assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js diff --git a/assets/src/components/validated-url-provider/index.js b/assets/src/components/validated-url-provider/index.js index bdea46188ac..cc4a64f2955 100644 --- a/assets/src/components/validated-url-provider/index.js +++ b/assets/src/components/validated-url-provider/index.js @@ -20,6 +20,7 @@ import apiFetch from '@wordpress/api-fetch'; */ import { ErrorContext } from '../error-context-provider'; import { useAsyncError } from '../../utils/use-async-error'; +import { calculateStylesheetSizes } from '../../validated-url-page/helpers'; export const ValidatedUrl = createContext(); @@ -34,6 +35,7 @@ export const ValidatedUrl = createContext(); */ export function ValidatedUrlProvider( { children, hasErrorBoundary = false, postId, validatedUrlsRestPath } ) { const [ validatedUrl, setValidatedUrl ] = useState( {} ); + const [ stylesheetSizes, setStylesheetSizes ] = useState( {} ); const [ fetchingValidatedUrl, setFetchingValidatedUrl ] = useState( null ); const { error, setError } = useContext( ErrorContext ); @@ -69,6 +71,7 @@ export function ValidatedUrlProvider( { children, hasErrorBoundary = false, post } setValidatedUrl( fetchedValidatedUrl ); + setStylesheetSizes( calculateStylesheetSizes( fetchedValidatedUrl?.stylesheets ) ); } catch ( e ) { if ( hasUnmounted.current === true ) { return; @@ -88,7 +91,7 @@ export function ValidatedUrlProvider( { children, hasErrorBoundary = false, post }, [ error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); return ( - + { children } ); diff --git a/assets/src/validated-url-page/helpers/index.js b/assets/src/validated-url-page/helpers/index.js new file mode 100644 index 00000000000..f05ba3f1e36 --- /dev/null +++ b/assets/src/validated-url-page/helpers/index.js @@ -0,0 +1,35 @@ +/** + * Calculate stylesheets sizes. + * + * Calculates total CSS size prior and after the minification based on the + * stylesheets data. + * + * @param {Array} stylesheets List of stylesheets. + * @return {Object|null} Stylesheets sizes data or null. + */ +export function calculateStylesheetSizes( stylesheets ) { + if ( ! stylesheets || stylesheets?.length === 0 ) { + return null; + } + + return stylesheets.reduce( ( sizes, stylesheet ) => { + const key = stylesheet.included === true ? 'included' : 'excluded'; + + return { + ...sizes, + [ key ]: { + originalSize: sizes[ key ].originalSize + stylesheet.original_size, + finalSize: sizes[ key ].finalSize + stylesheet.final_size, + }, + }; + }, { + included: { + originalSize: 0, + finalSize: 0, + }, + excluded: { + originalSize: 0, + finalSize: 0, + }, + } ); +} diff --git a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js new file mode 100644 index 00000000000..0a200712eb8 --- /dev/null +++ b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js @@ -0,0 +1,56 @@ +/** + * Internal dependencies + */ +import { calculateStylesheetSizes } from '..'; + +describe( 'calculateStylesheetSizes', () => { + it( 'returns null if no stylesheets are provided', () => { + expect( calculateStylesheetSizes() ).toBeNull(); + expect( calculateStylesheetSizes( [] ) ).toBeNull(); + } ); + + it( 'returns correct sizes prior and after minification', () => { + const stylesheets = [ + { + original_size: 20, + final_size: 3, + included: true, + }, + { + original_size: 10, + final_size: 2, + included: true, + }, + { + original_size: 10, + final_size: 0, + included: true, + }, + { + original_size: 200, + final_size: 30, + included: false, + }, + { + original_size: 100, + final_size: 20, + included: false, + }, + { + original_size: 100, + final_size: 0, + included: false, + }, + ]; + expect( calculateStylesheetSizes( stylesheets ) ).toMatchObject( { + included: { + originalSize: 40, + finalSize: 5, + }, + excluded: { + originalSize: 400, + finalSize: 50, + }, + } ); + } ); +} ); From e71fbf1130a554e09b7a43b0471c6e28d943e7b7 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 7 Jul 2021 21:12:44 +0200 Subject: [PATCH 08/58] Add `StylesheetsSummary` component --- .../components/stylesheets-summary/index.js | 52 +++++++++++++++++++ assets/src/validated-url-page/index.js | 7 ++- 2 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 assets/src/validated-url-page/components/stylesheets-summary/index.js diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js new file mode 100644 index 00000000000..bc471a77077 --- /dev/null +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -0,0 +1,52 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * WordPress dependencies + */ +import { __, _x } from '@wordpress/i18n'; + +export default function StylesheetsSummary( { stylesheetSizes } ) { + return ( + + + + + + + + + + + +
+ { __( 'Total CSS size prior to minification:', 'amp' ) } + + { stylesheetSizes.included.originalSize } + + { _x( 'B', 'abbreviation for bytes', 'amp' ) } + +
+ { __( 'Total CSS size after minification:', 'amp' ) } + + { stylesheetSizes.included.finalSize } + + { _x( 'B', 'abbreviation for bytes', 'amp' ) } + +
+ ); +} +StylesheetsSummary.propTypes = { + stylesheetSizes: PropTypes.shape( { + included: PropTypes.shape( { + originalSize: PropTypes.number, + finalSize: PropTypes.number, + } ), + excluded: PropTypes.shape( { + originalSize: PropTypes.number, + finalSize: PropTypes.number, + } ), + } ), +}; diff --git a/assets/src/validated-url-page/index.js b/assets/src/validated-url-page/index.js index 0f1c52fd0f6..1bf3e7f359a 100644 --- a/assets/src/validated-url-page/index.js +++ b/assets/src/validated-url-page/index.js @@ -22,6 +22,7 @@ import { ErrorBoundary } from '../components/error-boundary'; import { ErrorContextProvider } from '../components/error-context-provider'; import { ErrorScreen } from '../components/error-screen'; import { ValidatedUrl, ValidatedUrlProvider } from '../components/validated-url-provider'; +import StylesheetsSummary from './components/stylesheets-summary'; let errorHandler; @@ -56,16 +57,14 @@ Providers.propTypes = { * Validated URL page application root. */ function Root() { - const { fetchingValidatedUrl, validatedUrl } = useContext( ValidatedUrl ); + const { fetchingValidatedUrl, stylesheetSizes } = useContext( ValidatedUrl ); if ( fetchingValidatedUrl !== false ) { return ; } return ( -

- { validatedUrl.url } -

+ ); } From bd11a06da90314adc5db73e322f8d23f079b74d6 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Thu, 8 Jul 2021 11:19:26 +0200 Subject: [PATCH 09/58] Add locale-aware number formatting helper --- assets/src/utils/locale/index.js | 46 +++++++++++++++++++ assets/src/utils/locale/test/index.js | 26 +++++++++++ assets/src/utils/number-format/README.md | 21 +++++++++ assets/src/utils/number-format/index.js | 16 +++++++ .../components/stylesheets-summary/index.js | 11 ++++- 5 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 assets/src/utils/locale/index.js create mode 100644 assets/src/utils/locale/test/index.js create mode 100644 assets/src/utils/number-format/README.md create mode 100644 assets/src/utils/number-format/index.js diff --git a/assets/src/utils/locale/index.js b/assets/src/utils/locale/index.js new file mode 100644 index 00000000000..7dc06c7965e --- /dev/null +++ b/assets/src/utils/locale/index.js @@ -0,0 +1,46 @@ +/** + * WordPress dependencies + */ +import { __experimentalGetSettings } from '@wordpress/date'; //eslint-disable-line @wordpress/no-unsafe-wp-apis + +/** + * Clean up WP locale so it matches the format expected by browsers. + * + * @param {string} locale - Locale given by WordPress. + * @return {string} Browser-formatted locale. + */ +export const cleanLocale = ( locale ) => { + const regex = /^([a-z]{2,3})(_[a-z]{2}|_[a-z][a-z0-9]{4,7})?(?:_.*)?$/i; + + // Search for the correct locale format: + // e.g. af, arq, fr_FR, pap_CW, de_DE_formal, art_xpirate + const localeRegex = locale.match( regex ); + + // Locale was set to something that seems invalid, fallback to en-US. + if ( ! localeRegex ) { + return 'en-US'; + } + + return ( + // Keep only the language and the region, and replace the underscore used in WP locale by an hyphen. + `${ localeRegex[ 1 ] }${ localeRegex[ 2 ] ? localeRegex[ 2 ] : '' }`.replace( '_', '-' ) + ); +}; + +/** + * Current user locale, or browser locale as fallback. + * + * @return {string} Formatted user locale (e.g. `en-US` or `fr-FR`). + */ +export const getUserLocale = () => { + const { + l10n: { locale }, + } = __experimentalGetSettings(); + + if ( locale ) { + return cleanLocale( locale ); + } + + // Fallback to the browser locale if necessary. + return global?.window?.navigator?.language ?? 'en-US'; +}; diff --git a/assets/src/utils/locale/test/index.js b/assets/src/utils/locale/test/index.js new file mode 100644 index 00000000000..08a6aae4e46 --- /dev/null +++ b/assets/src/utils/locale/test/index.js @@ -0,0 +1,26 @@ +/** + * Internal dependencies + */ +import { cleanLocale } from '../'; + +describe( 'cleanLocale', () => { + const testLocales = [ + [ 'af', 'af' ], + [ 'arq', 'arq' ], + [ 'fr_FR', 'fr-FR' ], + [ 'pap_CW', 'pap-CW' ], + [ 'de_DE_formal', 'de-DE' ], + [ 'art_xpirate', 'art-xpirate' ], + [ 'art_xemoji', 'art-xemoji' ], + [ 'pt_PT_ao90', 'pt-PT' ], + [ 'deDE', 'en-US' ], + [ 'foobarde_DE', 'en-US' ], // Language should never be more than 3 chars long. + [ 'en_alotofchars', 'en' ], // region or variant tags should not be more than 8 chars. + ]; + + testLocales.forEach( ( testLocale ) => { + it( `${ testLocale[ 0 ] } is cleaned into ${ testLocale[ 1 ] }`, () => { + expect( cleanLocale( testLocale[ 0 ] ) ).toBe( testLocale[ 1 ] ); + } ); + } ); +} ); diff --git a/assets/src/utils/number-format/README.md b/assets/src/utils/number-format/README.md new file mode 100644 index 00000000000..e8bdfb16482 --- /dev/null +++ b/assets/src/utils/number-format/README.md @@ -0,0 +1,21 @@ +numberFormat +========= + +This utility function can be used to format numbers so they are displayed in the format expected for a specific locale. + +For example, a large number such as `123456` would be displayed as `123,456` in US English, and as `123 456` in FR French. + +The function relies on `Intl.NumberFormat` to format the numbers, based on the locale information available in WordPress, or based on the browser locale as a fallback. + +## General Usage: + +```js +import { numberFormat } from 'components/number-format'; + +render() { + const number = '123456'; + return ( + <>{ numberFormat( number ) } + ); +} +``` diff --git a/assets/src/utils/number-format/index.js b/assets/src/utils/number-format/index.js new file mode 100644 index 00000000000..2c9b7815069 --- /dev/null +++ b/assets/src/utils/number-format/index.js @@ -0,0 +1,16 @@ +/** + * Internal dependencies + */ +import { getUserLocale } from '../locale'; + +/** + * Format a number using the locale in use by the user viewing the page. + * + * @param {number} number The number to format. + * @return {string} Formatted number. + */ +export const numberFormat = ( number ) => { + const locale = getUserLocale(); + + return new Intl.NumberFormat( locale ).format( number ); +}; diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index bc471a77077..3ed3a7996ee 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -8,6 +8,11 @@ import PropTypes from 'prop-types'; */ import { __, _x } from '@wordpress/i18n'; +/** + * Internal dependencies + */ +import { numberFormat } from '../../../utils/number-format'; + export default function StylesheetsSummary( { stylesheetSizes } ) { return ( @@ -17,7 +22,8 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { { __( 'Total CSS size prior to minification:', 'amp' ) } @@ -34,11 +30,7 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { { __( 'Total CSS size after minification:', 'amp' ) } From bf35648cc10366da7cf5cf5c86fe416540a375d2 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Thu, 8 Jul 2021 20:41:23 +0200 Subject: [PATCH 11/58] Add CSS budget-related values to stylesheets summary table --- .../validated-url-provider/index.js | 23 +++++- .../components/stylesheets-summary/index.js | 41 +++++++++- .../src/validated-url-page/helpers/index.js | 71 +++++++++++++---- .../helpers/test/calculateStylesheetSizes.js | 77 ++++++++++++++++--- assets/src/validated-url-page/index.js | 13 +++- .../class-amp-validated-url-post-type.php | 9 +++ 6 files changed, 201 insertions(+), 33 deletions(-) diff --git a/assets/src/components/validated-url-provider/index.js b/assets/src/components/validated-url-provider/index.js index cc4a64f2955..71ba4081878 100644 --- a/assets/src/components/validated-url-provider/index.js +++ b/assets/src/components/validated-url-provider/index.js @@ -29,11 +29,18 @@ export const ValidatedUrl = createContext(); * * @param {Object} props Component props. * @param {any} props.children Component children. + * @param {number} props.cssBudgetBytes CSS budget value in bytes. * @param {boolean} props.hasErrorBoundary Whether the component is wrapped in an error boundary. * @param {number} props.postId Validated URL post ID. * @param {string} props.validatedUrlsRestPath REST endpoint to retrieve validated URL data. */ -export function ValidatedUrlProvider( { children, hasErrorBoundary = false, postId, validatedUrlsRestPath } ) { +export function ValidatedUrlProvider( { + children, + cssBudgetBytes, + hasErrorBoundary = false, + postId, + validatedUrlsRestPath, +} ) { const [ validatedUrl, setValidatedUrl ] = useState( {} ); const [ stylesheetSizes, setStylesheetSizes ] = useState( {} ); const [ fetchingValidatedUrl, setFetchingValidatedUrl ] = useState( null ); @@ -71,7 +78,7 @@ export function ValidatedUrlProvider( { children, hasErrorBoundary = false, post } setValidatedUrl( fetchedValidatedUrl ); - setStylesheetSizes( calculateStylesheetSizes( fetchedValidatedUrl?.stylesheets ) ); + setStylesheetSizes( calculateStylesheetSizes( fetchedValidatedUrl?.stylesheets, cssBudgetBytes ) ); } catch ( e ) { if ( hasUnmounted.current === true ) { return; @@ -88,10 +95,17 @@ export function ValidatedUrlProvider( { children, hasErrorBoundary = false, post setFetchingValidatedUrl( false ); } )(); - }, [ error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); + }, [ cssBudgetBytes, error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); return ( - + { children } ); @@ -99,6 +113,7 @@ export function ValidatedUrlProvider( { children, hasErrorBoundary = false, post ValidatedUrlProvider.propTypes = { children: PropTypes.any, + cssBudgetBytes: PropTypes.number, hasErrorBoundary: PropTypes.bool, postId: PropTypes.number, validatedUrlsRestPath: PropTypes.string, diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index 54831f90737..b2ce98df791 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -6,14 +6,22 @@ import PropTypes from 'prop-types'; /** * WordPress dependencies */ -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; /** * Internal dependencies */ import FormattedMemoryValue from '../../../components/formatted-memory-value'; +import { numberFormat } from '../../../utils/number-format'; -export default function StylesheetsSummary( { stylesheetSizes } ) { +/** + * Render stylesheets summary table. + * + * @param {Object} props Component props. + * @param {number} props.cssBudgetBytes CSS budget value in bytes. + * @param {Object} props.stylesheetSizes Stylesheet sizes object. + */ +export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } ) { return (
- { stylesheetSizes.included.originalSize } + { numberFormat( stylesheetSizes.included.originalSize ) } + { ' ' } { _x( 'B', 'abbreviation for bytes', 'amp' ) } @@ -28,7 +34,8 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { { __( 'Total CSS size after minification:', 'amp' ) } - { stylesheetSizes.included.finalSize } + { numberFormat( stylesheetSizes.included.finalSize ) } + { ' ' } { _x( 'B', 'abbreviation for bytes', 'amp' ) } From 79a4ac7dd31310f44b2a9bff1a33645d31795516 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Thu, 8 Jul 2021 17:10:57 +0200 Subject: [PATCH 10/58] Introduce `FormattedMemoryValue` with tests --- .../formatted-memory-value/index.js | 57 +++++++++++++ .../formatted-memory-value/test/index.js | 84 +++++++++++++++++++ .../components/stylesheets-summary/index.js | 16 +--- 3 files changed, 145 insertions(+), 12 deletions(-) create mode 100644 assets/src/components/formatted-memory-value/index.js create mode 100644 assets/src/components/formatted-memory-value/test/index.js diff --git a/assets/src/components/formatted-memory-value/index.js b/assets/src/components/formatted-memory-value/index.js new file mode 100644 index 00000000000..645be1a20c8 --- /dev/null +++ b/assets/src/components/formatted-memory-value/index.js @@ -0,0 +1,57 @@ +/** + * WordPress dependencies + */ +import { __, _x } from '@wordpress/i18n'; + +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * Internal dependencies + */ +import { numberFormat } from '../../utils/number-format'; + +function getMemoryUnit( unit ) { + if ( ! unit || typeof unit !== 'string' ) { + return null; + } + + switch ( unit.toLowerCase() ) { + case 'b': + return { + name: __( 'bytes', 'amp' ), + abbreviation: _x( 'B', 'abbreviation for bytes', 'amp' ), + }; + case 'kb': + return { + name: __( 'kilobytes', 'amp' ), + abbreviation: _x( 'kB', 'abbreviation for kilobytes', 'amp' ), + }; + default: + return null; + } +} + +export default function FormattedMemoryValue( { value, unit } ) { + const memoryUnit = getMemoryUnit( unit ); + + return ( + <> + { numberFormat( value ) } + { memoryUnit && ( + <> + { ' ' } + + { memoryUnit.abbreviation } + + + ) } + + ); +} +FormattedMemoryValue.propTypes = { + value: PropTypes.oneOfType( [ PropTypes.number, PropTypes.string ] ).isRequired, + unit: PropTypes.oneOf( [ 'b', 'B', 'kb', 'kB', 'KB' ] ), +}; diff --git a/assets/src/components/formatted-memory-value/test/index.js b/assets/src/components/formatted-memory-value/test/index.js new file mode 100644 index 00000000000..ae7b70c422a --- /dev/null +++ b/assets/src/components/formatted-memory-value/test/index.js @@ -0,0 +1,84 @@ +/** + * WordPress dependencies + */ +import { render } from '@wordpress/element'; + +/** + * External dependencies + */ +import { act } from 'react-dom/test-utils'; + +/** + * Internal dependencies + */ +import FormattedMemoryValue from '..'; + +let container; + +describe( 'FormattedMemoryValue', () => { + beforeEach( () => { + container = document.createElement( 'div' ); + document.body.appendChild( container ); + } ); + + afterEach( () => { + document.body.removeChild( container ); + container = null; + } ); + + it( 'prints bare value if no unit is provided', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.textContent ).toBe( '123' ); + } ); + + it( 'prints correct output if value is a string', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.textContent ).toBe( '234' ); + } ); + + it( 'prints correct value and unit', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.textContent ).toBe( '345 B' ); + expect( container.querySelector( 'abbr' ) ).not.toBeNull(); + expect( container.querySelector( 'abbr' ).title ).toBe( 'bytes' ); + } ); + + it.each( [ + [ 'bytes', 'B', 'B' ], + [ 'bytes', 'b', 'B' ], + [ 'kilobytes', 'kB', 'kB' ], + [ 'kilobytes', 'kb', 'kB' ], + [ 'kilobytes', 'KB', 'kB' ], + ] )( + 'prints correct %s value and unit for the following input unit: %s', + ( fullName, inputUnit, abbreviation ) => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( 'abbr' ).title ).toBe( fullName ); + expect( container.querySelector( 'abbr' ).textContent ).toBe( abbreviation ); + }, + ); +} ); diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index 3ed3a7996ee..54831f90737 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -6,12 +6,12 @@ import PropTypes from 'prop-types'; /** * WordPress dependencies */ -import { __, _x } from '@wordpress/i18n'; +import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { numberFormat } from '../../../utils/number-format'; +import FormattedMemoryValue from '../../../components/formatted-memory-value'; export default function StylesheetsSummary( { stylesheetSizes } ) { return ( @@ -22,11 +22,7 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { { __( 'Total CSS size prior to minification:', 'amp' ) } - { numberFormat( stylesheetSizes.included.originalSize ) } - { ' ' } - - { _x( 'B', 'abbreviation for bytes', 'amp' ) } - +
- { numberFormat( stylesheetSizes.included.finalSize ) } - { ' ' } - - { _x( 'B', 'abbreviation for bytes', 'amp' ) } - +
@@ -33,19 +41,48 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { + + + + + + + +
+ { __( 'Percentage of used CSS budget', 'amp' ) } + { cssBudgetBytes && [ ' (', , ')' ] } + { ':' } + + { `${ numberFormat( parseFloat( stylesheetSizes.budgetUsed * 100 ).toFixed( 1 ) ) }%` } +
+ { sprintf( + // translators: %d stands for the number of stylesheets + __( 'Excluded minified CSS size (%d stylesheets):', 'amp' ), + stylesheetSizes.excluded.stylesheets.length, + ) } + + +
); } StylesheetsSummary.propTypes = { + cssBudgetBytes: PropTypes.number, stylesheetSizes: PropTypes.shape( { included: PropTypes.shape( { originalSize: PropTypes.number, finalSize: PropTypes.number, + stylesheets: PropTypes.arrayOf( PropTypes.string ), + } ), + excessive: PropTypes.shape( { + stylesheets: PropTypes.arrayOf( PropTypes.string ), } ), excluded: PropTypes.shape( { originalSize: PropTypes.number, finalSize: PropTypes.number, + stylesheets: PropTypes.arrayOf( PropTypes.string ), } ), + budgetUsed: PropTypes.number, } ), }; diff --git a/assets/src/validated-url-page/helpers/index.js b/assets/src/validated-url-page/helpers/index.js index f05ba3f1e36..a176b07c0f5 100644 --- a/assets/src/validated-url-page/helpers/index.js +++ b/assets/src/validated-url-page/helpers/index.js @@ -5,31 +5,76 @@ * stylesheets data. * * @param {Array} stylesheets List of stylesheets. + * @param {number} cssBudgetBytes CSS budget value in bytes. * @return {Object|null} Stylesheets sizes data or null. */ -export function calculateStylesheetSizes( stylesheets ) { +export function calculateStylesheetSizes( stylesheets, cssBudgetBytes ) { if ( ! stylesheets || stylesheets?.length === 0 ) { return null; } - return stylesheets.reduce( ( sizes, stylesheet ) => { - const key = stylesheet.included === true ? 'included' : 'excluded'; - - return { - ...sizes, - [ key ]: { - originalSize: sizes[ key ].originalSize + stylesheet.original_size, - finalSize: sizes[ key ].finalSize + stylesheet.final_size, - }, - }; - }, { + const initialState = { included: { originalSize: 0, finalSize: 0, + stylesheets: [], + }, + excessive: { + stylesheets: [], }, excluded: { originalSize: 0, finalSize: 0, + stylesheets: [], }, - } ); + budgetUsed: 0, + }; + + const result = stylesheets + // Determine which stylesheets are included based on their priorities. + .sort( ( a, b ) => a.priority - b.priority ) + .reduce( ( sizes, stylesheet ) => { + // Skip duplicate stylesheets and invalid groups. + if ( stylesheet?.duplicate || stylesheet.group !== 'amp-custom' ) { + return sizes; + } + + // Excluded stylesheet. + if ( ! stylesheet.included ) { + return { + ...sizes, + excluded: { + originalSize: sizes.excluded.originalSize + stylesheet.original_size, + finalSize: sizes.excluded.finalSize + stylesheet.final_size, + stylesheets: [ + ...sizes.excluded.stylesheets, + stylesheet.hash, + ], + }, + }; + } + + const isExcessive = sizes.included.finalSize + stylesheet.final_size >= cssBudgetBytes; + + return { + ...sizes, + included: { + originalSize: sizes.included.originalSize + stylesheet.original_size, + finalSize: sizes.included.finalSize + stylesheet.final_size, + stylesheets: ! isExcessive + ? [ ...sizes.included.stylesheets, stylesheet.hash ] + : sizes.included.stylesheets, + }, + excessive: { + stylesheets: isExcessive + ? [ ...sizes.excessive.stylesheets, stylesheet.hash ] + : sizes.excessive.stylesheets, + }, + }; + }, initialState ); + + // Calculate CSS budget used. + result.budgetUsed = ( result.included.finalSize + result.excluded.finalSize ) / cssBudgetBytes; + + return result; } diff --git a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js index 0a200712eb8..a7ee211fabf 100644 --- a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js +++ b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js @@ -12,45 +12,98 @@ describe( 'calculateStylesheetSizes', () => { it( 'returns correct sizes prior and after minification', () => { const stylesheets = [ { - original_size: 20, - final_size: 3, + hash: 'excessive-1', included: true, + priority: 10, + group: 'amp-custom', + original_size: 200, + final_size: 30, }, { - original_size: 10, - final_size: 2, + hash: 'included', included: true, + priority: 1, + group: 'amp-custom', + original_size: 100, + final_size: 20, }, { - original_size: 10, - final_size: 0, + hash: 'excessive-2', included: true, + priority: 100, + group: 'amp-custom', + original_size: 100, + final_size: 0, }, { + hash: 'excluded-3', + included: false, + priority: 90, + group: 'amp-custom', original_size: 200, final_size: 30, - included: false, }, { + hash: 'excluded-1', + included: false, + priority: 5, + group: 'amp-custom', original_size: 100, final_size: 20, - included: false, }, { + hash: 'excluded-2', + included: false, + priority: 10, + group: 'amp-custom', original_size: 100, final_size: 0, - included: false, }, ]; - expect( calculateStylesheetSizes( stylesheets ) ).toMatchObject( { + expect( calculateStylesheetSizes( stylesheets, 25 ) ).toMatchObject( { included: { - originalSize: 40, - finalSize: 5, + originalSize: 400, + finalSize: 50, + stylesheets: [ 'included' ], + }, + excessive: { + stylesheets: [ 'excessive-1', 'excessive-2' ], }, excluded: { originalSize: 400, finalSize: 50, + stylesheets: [ 'excluded-1', 'excluded-2', 'excluded-3' ], }, + budgetUsed: 4, } ); } ); + + it( 'ignores groups other than amp-custom and duplicate stylesheets', () => { + const stylesheets = [ + { + hash: 'included', + group: 'amp-custom', + included: true, + priority: 1, + original_size: 100, + final_size: 20, + }, + { + hash: 'ignored', + group: 'foo-bar', + included: true, + priority: 100, + }, + { + hash: 'included', + group: 'amp-custom', + duplicate: true, + priority: 1, + }, + ]; + + const result = calculateStylesheetSizes( stylesheets, 75000 ); + expect( result.included.stylesheets ).toHaveLength( 1 ); + expect( result.included.stylesheets ).toContain( 'included' ); + } ); } ); diff --git a/assets/src/validated-url-page/index.js b/assets/src/validated-url-page/index.js index 1bf3e7f359a..712a789f77d 100644 --- a/assets/src/validated-url-page/index.js +++ b/assets/src/validated-url-page/index.js @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import { APP_ROOT_ID, + CSS_BUDGET_BYTES, POST_ID, VALIDATED_URLS_REST_PATH, } from 'amp-settings'; // From WP inline script. @@ -39,6 +40,7 @@ function Providers( { children } ) { ; } return ( - + ); } diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 27483f2b330..56606d28a39 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -2035,9 +2035,18 @@ public static function enqueue_edit_post_screen_scripts() { true ); + // @todo: Determine $css_budget_bytes value in a better place. + $css_budget_bytes = null; + foreach ( AMP_Allowed_Tags_Generated::get_allowed_tag( 'style' ) as $spec_rule ) { + if ( isset( $spec_rule[ AMP_Rule_Spec::TAG_SPEC ]['spec_name'] ) && AMP_Style_Sanitizer::STYLE_AMP_CUSTOM_SPEC_NAME === $spec_rule[ AMP_Rule_Spec::TAG_SPEC ]['spec_name'] ) { + $css_budget_bytes = $spec_rule[ AMP_Rule_Spec::CDATA ]['max_bytes']; + } + } + $post = get_post(); $validated_url_page_data = [ 'APP_ROOT_ID' => self::AMP_VALIDATED_URL_PAGE_APP_ROOT_ID, + 'CSS_BUDGET_BYTES' => $css_budget_bytes, 'POST_ID' => $post->ID, 'VALIDATED_URLS_REST_PATH' => '/amp/v1/validated-urls', ]; From 74c873271afce39dc9a6f3e472c8006c25b1b6b7 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 18:28:41 +0200 Subject: [PATCH 12/58] Move icons to parent `components` directory --- .../components/amp-document-status/index.js | 2 +- .../components/amp-validation-status/status-notification.js | 2 +- .../with-amp-toolbar-button/amp-toolbar-button.js | 2 +- assets/src/block-validation/plugins/amp-block-validation.js | 2 +- assets/src/{block-validation => }/components/icon/index.js | 6 +++--- assets/src/{block-validation => }/components/icon/style.css | 0 .../src/{block-validation => }/components/icon/test/icon.js | 0 7 files changed, 7 insertions(+), 7 deletions(-) rename assets/src/{block-validation => }/components/icon/index.js (91%) rename assets/src/{block-validation => }/components/icon/style.css (100%) rename assets/src/{block-validation => }/components/icon/test/icon.js (100%) diff --git a/assets/src/block-validation/components/amp-document-status/index.js b/assets/src/block-validation/components/amp-document-status/index.js index d1d5002e968..1424fffd1c3 100644 --- a/assets/src/block-validation/components/amp-document-status/index.js +++ b/assets/src/block-validation/components/amp-document-status/index.js @@ -11,7 +11,7 @@ import { useDispatch, useSelect } from '@wordpress/data'; import AMPValidationErrorsKeptIcon from '../../../../images/amp-validation-errors-kept.svg'; import BellIcon from '../../../../images/bell-icon.svg'; import { BLOCK_VALIDATION_STORE_KEY } from '../../store'; -import { StatusIcon } from '../icon'; +import { StatusIcon } from '../../../components/icon'; import { SidebarNotification } from '../sidebar-notification'; import { useAMPDocumentToggle } from '../../hooks/use-amp-document-toggle'; import { useErrorsFetchingStateChanges } from '../../hooks/use-errors-fetching-state-changes'; diff --git a/assets/src/block-validation/components/amp-validation-status/status-notification.js b/assets/src/block-validation/components/amp-validation-status/status-notification.js index 1705c3b4b1c..bbccd3447e9 100644 --- a/assets/src/block-validation/components/amp-validation-status/status-notification.js +++ b/assets/src/block-validation/components/amp-validation-status/status-notification.js @@ -10,7 +10,7 @@ import { useDispatch, useSelect } from '@wordpress/data'; */ import AMPValidationErrorsKeptIcon from '../../../../images/amp-validation-errors-kept.svg'; import { BLOCK_VALIDATION_STORE_KEY } from '../../store'; -import { StatusIcon } from '../icon'; +import { StatusIcon } from '../../../components/icon'; import { SidebarNotification } from '../sidebar-notification'; import { useErrorsFetchingStateChanges } from '../../hooks/use-errors-fetching-state-changes'; diff --git a/assets/src/block-validation/components/with-amp-toolbar-button/amp-toolbar-button.js b/assets/src/block-validation/components/with-amp-toolbar-button/amp-toolbar-button.js index 4dedb69ceb7..31dd08a52f1 100644 --- a/assets/src/block-validation/components/with-amp-toolbar-button/amp-toolbar-button.js +++ b/assets/src/block-validation/components/with-amp-toolbar-button/amp-toolbar-button.js @@ -13,7 +13,7 @@ import { useDispatch } from '@wordpress/data'; /** * Internal dependencies */ -import { ToolbarIcon } from '../icon'; +import { ToolbarIcon } from '../../../components/icon'; import { PLUGIN_NAME, SIDEBAR_NAME } from '../../plugins/amp-block-validation'; import { useAMPDocumentToggle } from '../../hooks/use-amp-document-toggle'; diff --git a/assets/src/block-validation/plugins/amp-block-validation.js b/assets/src/block-validation/plugins/amp-block-validation.js index f9e40007c03..d6d89dacf55 100644 --- a/assets/src/block-validation/plugins/amp-block-validation.js +++ b/assets/src/block-validation/plugins/amp-block-validation.js @@ -9,7 +9,7 @@ import { useSelect } from '@wordpress/data'; * Internal dependencies */ import { BLOCK_VALIDATION_STORE_KEY } from '../store'; -import { MoreMenuIcon, ToolbarIcon } from '../components/icon'; +import { MoreMenuIcon, ToolbarIcon } from '../../components/icon'; import { Sidebar } from '../components/sidebar'; import { InvalidBlockOutline } from '../components/invalid-block-outline'; import { usePostDirtyStateChanges } from '../hooks/use-post-dirty-state-changes'; diff --git a/assets/src/block-validation/components/icon/index.js b/assets/src/components/icon/index.js similarity index 91% rename from assets/src/block-validation/components/icon/index.js rename to assets/src/components/icon/index.js index a3c15e91441..f954d23fc80 100644 --- a/assets/src/block-validation/components/icon/index.js +++ b/assets/src/components/icon/index.js @@ -7,9 +7,9 @@ import PropTypes from 'prop-types'; * Internal dependencies */ import './style.css'; -import AMPLogoIcon from '../../../../images/amp-logo-icon.svg'; -import AMPToolbarIcon from '../../../../images/amp-icon-toolbar.svg'; -import AMPToolbarIconBroken from '../../../../images/amp-toolbar-icon-broken.svg'; +import AMPLogoIcon from '../../../images/amp-logo-icon.svg'; +import AMPToolbarIcon from '../../../images/amp-icon-toolbar.svg'; +import AMPToolbarIconBroken from '../../../images/amp-toolbar-icon-broken.svg'; /** * Plugin icon. diff --git a/assets/src/block-validation/components/icon/style.css b/assets/src/components/icon/style.css similarity index 100% rename from assets/src/block-validation/components/icon/style.css rename to assets/src/components/icon/style.css diff --git a/assets/src/block-validation/components/icon/test/icon.js b/assets/src/components/icon/test/icon.js similarity index 100% rename from assets/src/block-validation/components/icon/test/icon.js rename to assets/src/components/icon/test/icon.js From 7fac19b86b985e5d612085f59d6472fe5f1de37e Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 19:02:57 +0200 Subject: [PATCH 13/58] Introduce validation status icon component --- assets/images/amp-alert.svg | 4 +-- assets/images/amp-valid.svg | 2 +- assets/images/amp-warning.svg | 3 ++ assets/src/components/icon/index.js | 24 +++++++++++++ assets/src/components/icon/style.css | 27 +++++++++++++++ assets/src/components/icon/test/icon.js | 46 ++++++++++++++++++++++++- 6 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 assets/images/amp-warning.svg diff --git a/assets/images/amp-alert.svg b/assets/images/amp-alert.svg index 24acfb27d90..66b8bbe1706 100644 --- a/assets/images/amp-alert.svg +++ b/assets/images/amp-alert.svg @@ -1,3 +1,3 @@ - - + + diff --git a/assets/images/amp-valid.svg b/assets/images/amp-valid.svg index 2a49b68e38e..c64a3e34589 100644 --- a/assets/images/amp-valid.svg +++ b/assets/images/amp-valid.svg @@ -1,3 +1,3 @@ - + diff --git a/assets/images/amp-warning.svg b/assets/images/amp-warning.svg new file mode 100644 index 00000000000..65cebfd4796 --- /dev/null +++ b/assets/images/amp-warning.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/src/components/icon/index.js b/assets/src/components/icon/index.js index f954d23fc80..0d319409c84 100644 --- a/assets/src/components/icon/index.js +++ b/assets/src/components/icon/index.js @@ -10,6 +10,9 @@ import './style.css'; import AMPLogoIcon from '../../../images/amp-logo-icon.svg'; import AMPToolbarIcon from '../../../images/amp-icon-toolbar.svg'; import AMPToolbarIconBroken from '../../../images/amp-toolbar-icon-broken.svg'; +import AMPValidIcon from '../../../images/amp-valid.svg'; +import AMPWarningIcon from '../../../images/amp-warning.svg'; +import AMPAlertIcon from '../../../images/amp-alert.svg'; /** * Plugin icon. @@ -94,3 +97,24 @@ export function StatusIcon( { broken = false } ) { StatusIcon.propTypes = { broken: PropTypes.bool, }; + +/** + * The warning icon with an exclamation mark symbol. + * + * @param {Object} props + * @param {string} props.type Icon type. + * @param {boolean} props.boxed Whether the icon should be contained in a box. + */ +export function ValidationStatusIcon( { type, boxed = false } ) { + return ( + + { type === 'valid' && } + { type === 'warning' && } + { type === 'error' && } + + ); +} +ValidationStatusIcon.propTypes = { + type: PropTypes.oneOf( [ 'valid', 'warning', 'error' ] ).isRequired, + boxed: PropTypes.bool, +}; diff --git a/assets/src/components/icon/style.css b/assets/src/components/icon/style.css index f8d8f96e2bb..56a0be05f9c 100644 --- a/assets/src/components/icon/style.css +++ b/assets/src/components/icon/style.css @@ -79,3 +79,30 @@ .is-pressed .amp-error-count-badge { border-color: #1e1e1e; } + +/* + * AMP validation status icons. + */ +.amp-validation-status-icon { + display: inline-block; + height: 16px; + vertical-align: middle; + width: 16px; +} + +.amp-validation-status-icon--boxed { + border-radius: 5px; + padding: 2px; +} + +.amp-validation-status-icon--boxed.amp-validation-status-icon--valid { + background-color: #d3e5e5; +} + +.amp-validation-status-icon--boxed.amp-validation-status-icon--warning { + background-color: #fff6dd; +} + +.amp-validation-status-icon--boxed.amp-validation-status-icon--error { + background-color: #f7ded4; +} diff --git a/assets/src/components/icon/test/icon.js b/assets/src/components/icon/test/icon.js index 9fca433ca51..0403835c82e 100644 --- a/assets/src/components/icon/test/icon.js +++ b/assets/src/components/icon/test/icon.js @@ -11,7 +11,7 @@ import { render } from '@wordpress/element'; /** * Internal dependencies */ -import { MoreMenuIcon, ToolbarIcon, StatusIcon } from '../index'; +import { MoreMenuIcon, ToolbarIcon, StatusIcon, ValidationStatusIcon } from '../index'; let container; @@ -116,4 +116,48 @@ describe( 'Icons', () => { expect( container.querySelector( '.amp-status-icon--broken' ) ).not.toBeNull(); } ); + + it( 'renders the valid ValidationStatusIcon', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-validation-status-icon--valid' ) ).not.toBeNull(); + } ); + + it( 'renders the warning ValidationStatusIcon', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-validation-status-icon--warning' ) ).not.toBeNull(); + } ); + + it( 'renders the error ValidationStatusIcon', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-validation-status-icon--error' ) ).not.toBeNull(); + } ); + + it( 'renders the boxed ValidationStatusIcon', () => { + act( () => { + render( + , + container, + ); + } ); + + expect( container.querySelector( '.amp-validation-status-icon--boxed' ) ).not.toBeNull(); + } ); } ); From a9e3c39a3bc8764207e7dd1d866f5619bb4eb8b0 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 19:08:37 +0200 Subject: [PATCH 14/58] Add status icons to stylesheets summary table --- .../validated-url-provider/index.js | 7 +- .../components/stylesheets-summary/index.js | 14 +++- .../src/validated-url-page/helpers/index.js | 16 +++- .../helpers/test/calculateStylesheetSizes.js | 80 ++++++++++++++++++- assets/src/validated-url-page/index.js | 2 + .../class-amp-validated-url-post-type.php | 16 +++- 6 files changed, 120 insertions(+), 15 deletions(-) diff --git a/assets/src/components/validated-url-provider/index.js b/assets/src/components/validated-url-provider/index.js index 71ba4081878..9597a0b65c7 100644 --- a/assets/src/components/validated-url-provider/index.js +++ b/assets/src/components/validated-url-provider/index.js @@ -30,6 +30,7 @@ export const ValidatedUrl = createContext(); * @param {Object} props Component props. * @param {any} props.children Component children. * @param {number} props.cssBudgetBytes CSS budget value in bytes. + * @param {number} props.cssBudgetWarningPercentage CSS budget warning level percentage. * @param {boolean} props.hasErrorBoundary Whether the component is wrapped in an error boundary. * @param {number} props.postId Validated URL post ID. * @param {string} props.validatedUrlsRestPath REST endpoint to retrieve validated URL data. @@ -37,6 +38,7 @@ export const ValidatedUrl = createContext(); export function ValidatedUrlProvider( { children, cssBudgetBytes, + cssBudgetWarningPercentage, hasErrorBoundary = false, postId, validatedUrlsRestPath, @@ -78,7 +80,7 @@ export function ValidatedUrlProvider( { } setValidatedUrl( fetchedValidatedUrl ); - setStylesheetSizes( calculateStylesheetSizes( fetchedValidatedUrl?.stylesheets, cssBudgetBytes ) ); + setStylesheetSizes( calculateStylesheetSizes( fetchedValidatedUrl?.stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) ); } catch ( e ) { if ( hasUnmounted.current === true ) { return; @@ -95,7 +97,7 @@ export function ValidatedUrlProvider( { setFetchingValidatedUrl( false ); } )(); - }, [ cssBudgetBytes, error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); + }, [ cssBudgetBytes, cssBudgetWarningPercentage, error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); return ( - { `${ numberFormat( parseFloat( stylesheetSizes.budgetUsed * 100 ).toFixed( 1 ) ) }%` } + { `${ numberFormat( parseFloat( stylesheetSizes.budget.usage ).toFixed( 1 ) ) }%` } + { ' ' } + { stylesheetSizes.budget.status === 'exceeded' && } + { stylesheetSizes.budget.status === 'warning' && } + { stylesheetSizes.budget.status === 'valid' && } @@ -83,6 +88,9 @@ StylesheetsSummary.propTypes = { finalSize: PropTypes.number, stylesheets: PropTypes.arrayOf( PropTypes.string ), } ), - budgetUsed: PropTypes.number, + budget: PropTypes.shape( { + usage: PropTypes.number, + status: PropTypes.oneOf( [ 'valid', 'warning', 'exceeded' ] ), + } ), } ), }; diff --git a/assets/src/validated-url-page/helpers/index.js b/assets/src/validated-url-page/helpers/index.js index a176b07c0f5..c020ba53b87 100644 --- a/assets/src/validated-url-page/helpers/index.js +++ b/assets/src/validated-url-page/helpers/index.js @@ -6,9 +6,10 @@ * * @param {Array} stylesheets List of stylesheets. * @param {number} cssBudgetBytes CSS budget value in bytes. + * @param {number} cssBudgetWarningPercentage CSS budget warning level percentage. * @return {Object|null} Stylesheets sizes data or null. */ -export function calculateStylesheetSizes( stylesheets, cssBudgetBytes ) { +export function calculateStylesheetSizes( stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) { if ( ! stylesheets || stylesheets?.length === 0 ) { return null; } @@ -27,7 +28,10 @@ export function calculateStylesheetSizes( stylesheets, cssBudgetBytes ) { finalSize: 0, stylesheets: [], }, - budgetUsed: 0, + budget: { + usage: 0, + status: 'valid', + }, }; const result = stylesheets @@ -74,7 +78,13 @@ export function calculateStylesheetSizes( stylesheets, cssBudgetBytes ) { }, initialState ); // Calculate CSS budget used. - result.budgetUsed = ( result.included.finalSize + result.excluded.finalSize ) / cssBudgetBytes; + result.budget.usage = ( result.included.finalSize + result.excluded.finalSize ) / cssBudgetBytes * 100; + + if ( result.budget.usage > 100 ) { + result.budget.status = 'exceeded'; + } else if ( result.budget.usage > cssBudgetWarningPercentage ) { + result.budget.status = 'warning'; + } return result; } diff --git a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js index a7ee211fabf..7776839dc7f 100644 --- a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js +++ b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js @@ -60,7 +60,7 @@ describe( 'calculateStylesheetSizes', () => { final_size: 0, }, ]; - expect( calculateStylesheetSizes( stylesheets, 25 ) ).toMatchObject( { + expect( calculateStylesheetSizes( stylesheets, 25, 20 ) ).toMatchObject( { included: { originalSize: 400, finalSize: 50, @@ -74,7 +74,6 @@ describe( 'calculateStylesheetSizes', () => { finalSize: 50, stylesheets: [ 'excluded-1', 'excluded-2', 'excluded-3' ], }, - budgetUsed: 4, } ); } ); @@ -102,8 +101,83 @@ describe( 'calculateStylesheetSizes', () => { }, ]; - const result = calculateStylesheetSizes( stylesheets, 75000 ); + const result = calculateStylesheetSizes( stylesheets, 75000, 80 ); expect( result.included.stylesheets ).toHaveLength( 1 ); expect( result.included.stylesheets ).toContain( 'included' ); } ); + + it( 'sets the exceeded budget values correctly', () => { + const stylesheets = [ + { + hash: '1', + group: 'amp-custom', + included: true, + priority: 10, + original_size: 100, + final_size: 50, + }, + { + hash: '2', + group: 'amp-custom', + included: true, + priority: 10, + original_size: 100, + final_size: 50, + }, + ]; + + const result = calculateStylesheetSizes( stylesheets, 50, 80 ); + expect( result.budget.usage ).toBe( 200 ); + expect( result.budget.status ).toBe( 'exceeded' ); + } ); + + it( 'sets the warning budget values correctly', () => { + const stylesheets = [ + { + hash: '1', + group: 'amp-custom', + included: true, + priority: 10, + original_size: 100, + final_size: 50, + }, + { + hash: '2', + group: 'amp-custom', + included: true, + priority: 10, + original_size: 100, + final_size: 50, + }, + ]; + + const result = calculateStylesheetSizes( stylesheets, 200, 40 ); + expect( result.budget.usage ).toBe( 50 ); + expect( result.budget.status ).toBe( 'warning' ); + } ); + + it( 'sets the valid budget values correctly', () => { + const stylesheets = [ + { + hash: '1', + group: 'amp-custom', + included: true, + priority: 10, + original_size: 100, + final_size: 50, + }, + { + hash: '2', + group: 'amp-custom', + included: true, + priority: 10, + original_size: 100, + final_size: 50, + }, + ]; + + const result = calculateStylesheetSizes( stylesheets, 200, 60 ); + expect( result.budget.usage ).toBe( 50 ); + expect( result.budget.status ).toBe( 'valid' ); + } ); } ); diff --git a/assets/src/validated-url-page/index.js b/assets/src/validated-url-page/index.js index 712a789f77d..1a883154ee4 100644 --- a/assets/src/validated-url-page/index.js +++ b/assets/src/validated-url-page/index.js @@ -5,6 +5,7 @@ import PropTypes from 'prop-types'; import { APP_ROOT_ID, CSS_BUDGET_BYTES, + CSS_BUDGET_WARNING_PERCENTAGE, POST_ID, VALIDATED_URLS_REST_PATH, } from 'amp-settings'; // From WP inline script. @@ -41,6 +42,7 @@ function Providers( { children } ) { self::AMP_VALIDATED_URL_PAGE_APP_ROOT_ID, - 'CSS_BUDGET_BYTES' => $css_budget_bytes, - 'POST_ID' => $post->ID, - 'VALIDATED_URLS_REST_PATH' => '/amp/v1/validated-urls', + 'APP_ROOT_ID' => self::AMP_VALIDATED_URL_PAGE_APP_ROOT_ID, + 'CSS_BUDGET_BYTES' => $css_budget_bytes, + 'CSS_BUDGET_WARNING_PERCENTAGE' => AMP_Style_Sanitizer::CSS_BUDGET_WARNING_PERCENTAGE, + 'POST_ID' => $post->ID, + 'VALIDATED_URLS_REST_PATH' => '/amp/v1/validated-urls', ]; wp_add_inline_script( From cd81f7b6dbb633da5d86b3497eb3b32a3302b6fb Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 19:18:08 +0200 Subject: [PATCH 15/58] Add warning and error notices related to CSS usage --- .../components/stylesheets-summary/index.js | 105 ++++++++++-------- 1 file changed, 59 insertions(+), 46 deletions(-) diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index e53eba12c90..6c404157aac 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -14,6 +14,7 @@ import { __, sprintf } from '@wordpress/i18n'; import { numberFormat } from '../../../utils/number-format'; import FormattedMemoryValue from '../../../components/formatted-memory-value'; import { ValidationStatusIcon } from '../../../components/icon'; +import { AMPNotice, NOTICE_SIZE_LARGE, NOTICE_TYPE_WARNING, NOTICE_TYPE_ERROR } from '../../../components/amp-notice'; /** * Render stylesheets summary table. @@ -24,52 +25,64 @@ import { ValidationStatusIcon } from '../../../components/icon'; */ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } ) { return ( - - - - - - - - - - - - - - - - - - - -
- { __( 'Total CSS size prior to minification:', 'amp' ) } - - -
- { __( 'Total CSS size after minification:', 'amp' ) } - - -
- { __( 'Percentage of used CSS budget', 'amp' ) } - { cssBudgetBytes && [ ' (', , ')' ] } - { ':' } - - { `${ numberFormat( parseFloat( stylesheetSizes.budget.usage ).toFixed( 1 ) ) }%` } - { ' ' } - { stylesheetSizes.budget.status === 'exceeded' && } - { stylesheetSizes.budget.status === 'warning' && } - { stylesheetSizes.budget.status === 'valid' && } -
- { sprintf( - // translators: %d stands for the number of stylesheets - __( 'Excluded minified CSS size (%d stylesheets):', 'amp' ), - stylesheetSizes.excluded.stylesheets.length, - ) } - - -
+ <> + + + + + + + + + + + + + + + + + + + +
+ { __( 'Total CSS size prior to minification:', 'amp' ) } + + +
+ { __( 'Total CSS size after minification:', 'amp' ) } + + +
+ { __( 'Percentage of used CSS budget', 'amp' ) } + { cssBudgetBytes && [ ' (', , ')' ] } + { ':' } + + { `${ numberFormat( parseFloat( stylesheetSizes.budget.usage ).toFixed( 1 ) ) }%` } + { ' ' } + { stylesheetSizes.budget.status === 'exceeded' && } + { stylesheetSizes.budget.status === 'warning' && } + { stylesheetSizes.budget.status === 'valid' && } +
+ { sprintf( + // translators: %d stands for the number of stylesheets + __( 'Excluded minified CSS size (%d stylesheets):', 'amp' ), + stylesheetSizes.excluded.stylesheets.length, + ) } + + +
+ { stylesheetSizes.budget.status === 'warning' && ( + + { __( 'You are nearing the limit of the CSS budget. Once this limit is reached, stylesheets deemed of lesser priority will be excluded from the page. Please review the stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } + + ) } + { stylesheetSizes.budget.status === 'exceeded' && ( + + { __( 'You have exceeded the CSS budget. Stylesheets deemed of lesser priority have been excluded from the page. Please review the flagged stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } + + ) } + ); } StylesheetsSummary.propTypes = { From bf49de94de16a29ac5dbadb118fe8d0733c33846 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 19:37:55 +0200 Subject: [PATCH 16/58] Use flags instead of string prop --- assets/src/components/icon/index.js | 36 +++++++++++++------ assets/src/components/icon/test/icon.js | 8 ++--- .../components/stylesheets-summary/index.js | 6 ++-- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/assets/src/components/icon/index.js b/assets/src/components/icon/index.js index 0d319409c84..424a99687d9 100644 --- a/assets/src/components/icon/index.js +++ b/assets/src/components/icon/index.js @@ -99,22 +99,38 @@ StatusIcon.propTypes = { }; /** - * The warning icon with an exclamation mark symbol. + * Renders the validation status icon. * * @param {Object} props - * @param {string} props.type Icon type. - * @param {boolean} props.boxed Whether the icon should be contained in a box. + * @param {boolean} props.isError Flag indicating the icon is for an error status. + * @param {boolean} props.isWarning Flag indicating the icon is for a warning status. + * @param {boolean} props.isValid Flag indicating the icon is for a valid status. + * @param {boolean} props.isBoxed Whether the icon should be contained in a box. */ -export function ValidationStatusIcon( { type, boxed = false } ) { +export function ValidationStatusIcon( { isError, isWarning, isValid, isBoxed = false } ) { + let type; + + if ( isError ) { + type = 'error'; + } else if ( isWarning ) { + type = 'warning'; + } else if ( isValid ) { + type = 'valid'; + } else { + return null; + } + return ( - - { type === 'valid' && } - { type === 'warning' && } - { type === 'error' && } + + { isValid && } + { isWarning && } + { isError && } ); } ValidationStatusIcon.propTypes = { - type: PropTypes.oneOf( [ 'valid', 'warning', 'error' ] ).isRequired, - boxed: PropTypes.bool, + isError: PropTypes.bool, + isWarning: PropTypes.bool, + isValid: PropTypes.bool, + isBoxed: PropTypes.bool, }; diff --git a/assets/src/components/icon/test/icon.js b/assets/src/components/icon/test/icon.js index 0403835c82e..2944b6e0193 100644 --- a/assets/src/components/icon/test/icon.js +++ b/assets/src/components/icon/test/icon.js @@ -120,7 +120,7 @@ describe( 'Icons', () => { it( 'renders the valid ValidationStatusIcon', () => { act( () => { render( - , + , container, ); } ); @@ -131,7 +131,7 @@ describe( 'Icons', () => { it( 'renders the warning ValidationStatusIcon', () => { act( () => { render( - , + , container, ); } ); @@ -142,7 +142,7 @@ describe( 'Icons', () => { it( 'renders the error ValidationStatusIcon', () => { act( () => { render( - , + , container, ); } ); @@ -153,7 +153,7 @@ describe( 'Icons', () => { it( 'renders the boxed ValidationStatusIcon', () => { act( () => { render( - , + , container, ); } ); diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index 6c404157aac..92f0d0f1884 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -53,9 +53,9 @@ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } { `${ numberFormat( parseFloat( stylesheetSizes.budget.usage ).toFixed( 1 ) ) }%` } { ' ' } - { stylesheetSizes.budget.status === 'exceeded' && } - { stylesheetSizes.budget.status === 'warning' && } - { stylesheetSizes.budget.status === 'valid' && } + { stylesheetSizes.budget.status === 'exceeded' && } + { stylesheetSizes.budget.status === 'warning' && } + { stylesheetSizes.budget.status === 'valid' && } From b6ed86716a526a9445b4c1fad2b955356ae5fc2f Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 19:44:36 +0200 Subject: [PATCH 17/58] Use flags for stylesheets budget status designators --- .../components/stylesheets-summary/index.js | 34 +++++++++++++++---- .../src/validated-url-page/helpers/index.js | 10 ++++-- .../helpers/test/calculateStylesheetSizes.js | 13 ++++--- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index 92f0d0f1884..b3913d55865 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -14,7 +14,17 @@ import { __, sprintf } from '@wordpress/i18n'; import { numberFormat } from '../../../utils/number-format'; import FormattedMemoryValue from '../../../components/formatted-memory-value'; import { ValidationStatusIcon } from '../../../components/icon'; -import { AMPNotice, NOTICE_SIZE_LARGE, NOTICE_TYPE_WARNING, NOTICE_TYPE_ERROR } from '../../../components/amp-notice'; +import { + AMPNotice, + NOTICE_SIZE_LARGE, + NOTICE_TYPE_WARNING, + NOTICE_TYPE_ERROR, +} from '../../../components/amp-notice'; +import { + STYLESHEETS_BUDGET_STATUS_VALID, + STYLESHEETS_BUDGET_STATUS_WARNING, + STYLESHEETS_BUDGET_STATUS_EXCEEDED, +} from '../../helpers'; /** * Render stylesheets summary table. @@ -53,9 +63,15 @@ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } { `${ numberFormat( parseFloat( stylesheetSizes.budget.usage ).toFixed( 1 ) ) }%` } { ' ' } - { stylesheetSizes.budget.status === 'exceeded' && } - { stylesheetSizes.budget.status === 'warning' && } - { stylesheetSizes.budget.status === 'valid' && } + { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_EXCEEDED && ( + + ) } + { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_WARNING && ( + + ) } + { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_VALID && ( + + ) } @@ -72,12 +88,12 @@ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } - { stylesheetSizes.budget.status === 'warning' && ( + { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_WARNING && ( { __( 'You are nearing the limit of the CSS budget. Once this limit is reached, stylesheets deemed of lesser priority will be excluded from the page. Please review the stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } ) } - { stylesheetSizes.budget.status === 'exceeded' && ( + { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_EXCEEDED && ( { __( 'You have exceeded the CSS budget. Stylesheets deemed of lesser priority have been excluded from the page. Please review the flagged stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } @@ -103,7 +119,11 @@ StylesheetsSummary.propTypes = { } ), budget: PropTypes.shape( { usage: PropTypes.number, - status: PropTypes.oneOf( [ 'valid', 'warning', 'exceeded' ] ), + status: PropTypes.oneOf( [ + STYLESHEETS_BUDGET_STATUS_VALID, + STYLESHEETS_BUDGET_STATUS_WARNING, + STYLESHEETS_BUDGET_STATUS_EXCEEDED, + ] ), } ), } ), }; diff --git a/assets/src/validated-url-page/helpers/index.js b/assets/src/validated-url-page/helpers/index.js index c020ba53b87..ef097edb07f 100644 --- a/assets/src/validated-url-page/helpers/index.js +++ b/assets/src/validated-url-page/helpers/index.js @@ -1,3 +1,7 @@ +export const STYLESHEETS_BUDGET_STATUS_VALID = 'valid'; +export const STYLESHEETS_BUDGET_STATUS_WARNING = 'warning'; +export const STYLESHEETS_BUDGET_STATUS_EXCEEDED = 'exceeded'; + /** * Calculate stylesheets sizes. * @@ -30,7 +34,7 @@ export function calculateStylesheetSizes( stylesheets, cssBudgetBytes, cssBudget }, budget: { usage: 0, - status: 'valid', + status: STYLESHEETS_BUDGET_STATUS_VALID, }, }; @@ -81,9 +85,9 @@ export function calculateStylesheetSizes( stylesheets, cssBudgetBytes, cssBudget result.budget.usage = ( result.included.finalSize + result.excluded.finalSize ) / cssBudgetBytes * 100; if ( result.budget.usage > 100 ) { - result.budget.status = 'exceeded'; + result.budget.status = STYLESHEETS_BUDGET_STATUS_EXCEEDED; } else if ( result.budget.usage > cssBudgetWarningPercentage ) { - result.budget.status = 'warning'; + result.budget.status = STYLESHEETS_BUDGET_STATUS_WARNING; } return result; diff --git a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js index 7776839dc7f..eba9459aaba 100644 --- a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js +++ b/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js @@ -1,7 +1,12 @@ /** * Internal dependencies */ -import { calculateStylesheetSizes } from '..'; +import { + calculateStylesheetSizes, + STYLESHEETS_BUDGET_STATUS_VALID, + STYLESHEETS_BUDGET_STATUS_WARNING, + STYLESHEETS_BUDGET_STATUS_EXCEEDED, +} from '..'; describe( 'calculateStylesheetSizes', () => { it( 'returns null if no stylesheets are provided', () => { @@ -128,7 +133,7 @@ describe( 'calculateStylesheetSizes', () => { const result = calculateStylesheetSizes( stylesheets, 50, 80 ); expect( result.budget.usage ).toBe( 200 ); - expect( result.budget.status ).toBe( 'exceeded' ); + expect( result.budget.status ).toBe( STYLESHEETS_BUDGET_STATUS_EXCEEDED ); } ); it( 'sets the warning budget values correctly', () => { @@ -153,7 +158,7 @@ describe( 'calculateStylesheetSizes', () => { const result = calculateStylesheetSizes( stylesheets, 200, 40 ); expect( result.budget.usage ).toBe( 50 ); - expect( result.budget.status ).toBe( 'warning' ); + expect( result.budget.status ).toBe( STYLESHEETS_BUDGET_STATUS_WARNING ); } ); it( 'sets the valid budget values correctly', () => { @@ -178,6 +183,6 @@ describe( 'calculateStylesheetSizes', () => { const result = calculateStylesheetSizes( stylesheets, 200, 60 ); expect( result.budget.usage ).toBe( 50 ); - expect( result.budget.status ).toBe( 'valid' ); + expect( result.budget.status ).toBe( STYLESHEETS_BUDGET_STATUS_VALID ); } ); } ); From 8f81c07bc88b6b50513e004c01aaa91e4e783502 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 20:02:10 +0200 Subject: [PATCH 18/58] Use more accurate naming convention --- .../validated-url-provider/index.js | 5 +-- .../components/stylesheets-summary/index.js | 41 +++++++++++-------- .../src/validated-url-page/helpers/index.js | 29 ++++++------- ...etSizes.js => calculateStylesheetStats.js} | 33 ++++++++------- assets/src/validated-url-page/index.js | 6 +-- 5 files changed, 61 insertions(+), 53 deletions(-) rename assets/src/validated-url-page/helpers/test/{calculateStylesheetSizes.js => calculateStylesheetStats.js} (75%) diff --git a/assets/src/components/validated-url-provider/index.js b/assets/src/components/validated-url-provider/index.js index 9597a0b65c7..c84cb82bada 100644 --- a/assets/src/components/validated-url-provider/index.js +++ b/assets/src/components/validated-url-provider/index.js @@ -20,7 +20,7 @@ import apiFetch from '@wordpress/api-fetch'; */ import { ErrorContext } from '../error-context-provider'; import { useAsyncError } from '../../utils/use-async-error'; -import { calculateStylesheetSizes } from '../../validated-url-page/helpers'; +import { calculateStylesheetStats } from '../../validated-url-page/helpers'; export const ValidatedUrl = createContext(); @@ -80,7 +80,7 @@ export function ValidatedUrlProvider( { } setValidatedUrl( fetchedValidatedUrl ); - setStylesheetSizes( calculateStylesheetSizes( fetchedValidatedUrl?.stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) ); + setStylesheetSizes( calculateStylesheetStats( fetchedValidatedUrl?.stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) ); } catch ( e ) { if ( hasUnmounted.current === true ) { return; @@ -102,7 +102,6 @@ export function ValidatedUrlProvider( { return ( @@ -43,7 +52,7 @@ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } { __( 'Total CSS size prior to minification:', 'amp' ) } @@ -51,25 +60,25 @@ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } { __( 'Total CSS size after minification:', 'amp' ) } @@ -79,21 +88,21 @@ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } { sprintf( // translators: %d stands for the number of stylesheets __( 'Excluded minified CSS size (%d stylesheets):', 'amp' ), - stylesheetSizes.excluded.stylesheets.length, + excluded.stylesheets.length, ) }
- +
- +
{ __( 'Percentage of used CSS budget', 'amp' ) } - { cssBudgetBytes && [ ' (', , ')' ] } + { budgetBytes && [ ' (', , ')' ] } { ':' } - { `${ numberFormat( parseFloat( stylesheetSizes.budget.usage ).toFixed( 1 ) ) }%` } + { `${ numberFormat( parseFloat( actualPercentage ).toFixed( 1 ) ) }%` } { ' ' } - { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_EXCEEDED && ( + { status === STYLESHEETS_BUDGET_STATUS_EXCEEDED && ( ) } - { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_WARNING && ( + { status === STYLESHEETS_BUDGET_STATUS_WARNING && ( ) } - { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_VALID && ( + { status === STYLESHEETS_BUDGET_STATUS_VALID && ( ) } - +
- { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_WARNING && ( + { status === STYLESHEETS_BUDGET_STATUS_WARNING && ( { __( 'You are nearing the limit of the CSS budget. Once this limit is reached, stylesheets deemed of lesser priority will be excluded from the page. Please review the stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } ) } - { stylesheetSizes.budget.status === STYLESHEETS_BUDGET_STATUS_EXCEEDED && ( + { status === STYLESHEETS_BUDGET_STATUS_EXCEEDED && ( { __( 'You have exceeded the CSS budget. Stylesheets deemed of lesser priority have been excluded from the page. Please review the flagged stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } @@ -102,7 +111,6 @@ export default function StylesheetsSummary( { cssBudgetBytes, stylesheetSizes } ); } StylesheetsSummary.propTypes = { - cssBudgetBytes: PropTypes.number, stylesheetSizes: PropTypes.shape( { included: PropTypes.shape( { originalSize: PropTypes.number, @@ -117,8 +125,9 @@ StylesheetsSummary.propTypes = { finalSize: PropTypes.number, stylesheets: PropTypes.arrayOf( PropTypes.string ), } ), - budget: PropTypes.shape( { - usage: PropTypes.number, + usage: PropTypes.shape( { + actualPercentage: PropTypes.number, + budgetBytes: PropTypes.number, status: PropTypes.oneOf( [ STYLESHEETS_BUDGET_STATUS_VALID, STYLESHEETS_BUDGET_STATUS_WARNING, diff --git a/assets/src/validated-url-page/helpers/index.js b/assets/src/validated-url-page/helpers/index.js index ef097edb07f..7c7f9d8174a 100644 --- a/assets/src/validated-url-page/helpers/index.js +++ b/assets/src/validated-url-page/helpers/index.js @@ -3,17 +3,17 @@ export const STYLESHEETS_BUDGET_STATUS_WARNING = 'warning'; export const STYLESHEETS_BUDGET_STATUS_EXCEEDED = 'exceeded'; /** - * Calculate stylesheets sizes. + * Calculate stylesheets stats. * - * Calculates total CSS size prior and after the minification based on the - * stylesheets data. + * Calculates total CSS size and other stats prior and after the minification + * based on the stylesheets data. * * @param {Array} stylesheets List of stylesheets. - * @param {number} cssBudgetBytes CSS budget value in bytes. - * @param {number} cssBudgetWarningPercentage CSS budget warning level percentage. + * @param {number} budgetBytes CSS budget value in bytes. + * @param {number} budgetWarningPercentage CSS budget warning level percentage. * @return {Object|null} Stylesheets sizes data or null. */ -export function calculateStylesheetSizes( stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) { +export function calculateStylesheetStats( stylesheets, budgetBytes, budgetWarningPercentage ) { if ( ! stylesheets || stylesheets?.length === 0 ) { return null; } @@ -32,8 +32,9 @@ export function calculateStylesheetSizes( stylesheets, cssBudgetBytes, cssBudget finalSize: 0, stylesheets: [], }, - budget: { - usage: 0, + usage: { + actualPercentage: 0, + budgetBytes, status: STYLESHEETS_BUDGET_STATUS_VALID, }, }; @@ -62,7 +63,7 @@ export function calculateStylesheetSizes( stylesheets, cssBudgetBytes, cssBudget }; } - const isExcessive = sizes.included.finalSize + stylesheet.final_size >= cssBudgetBytes; + const isExcessive = sizes.included.finalSize + stylesheet.final_size >= budgetBytes; return { ...sizes, @@ -82,12 +83,12 @@ export function calculateStylesheetSizes( stylesheets, cssBudgetBytes, cssBudget }, initialState ); // Calculate CSS budget used. - result.budget.usage = ( result.included.finalSize + result.excluded.finalSize ) / cssBudgetBytes * 100; + result.usage.actualPercentage = ( result.included.finalSize + result.excluded.finalSize ) / budgetBytes * 100; - if ( result.budget.usage > 100 ) { - result.budget.status = STYLESHEETS_BUDGET_STATUS_EXCEEDED; - } else if ( result.budget.usage > cssBudgetWarningPercentage ) { - result.budget.status = STYLESHEETS_BUDGET_STATUS_WARNING; + if ( result.usage.actualPercentage > 100 ) { + result.usage.status = STYLESHEETS_BUDGET_STATUS_EXCEEDED; + } else if ( result.usage.actualPercentage > budgetWarningPercentage ) { + result.usage.status = STYLESHEETS_BUDGET_STATUS_WARNING; } return result; diff --git a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js b/assets/src/validated-url-page/helpers/test/calculateStylesheetStats.js similarity index 75% rename from assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js rename to assets/src/validated-url-page/helpers/test/calculateStylesheetStats.js index eba9459aaba..e54d4374876 100644 --- a/assets/src/validated-url-page/helpers/test/calculateStylesheetSizes.js +++ b/assets/src/validated-url-page/helpers/test/calculateStylesheetStats.js @@ -2,16 +2,16 @@ * Internal dependencies */ import { - calculateStylesheetSizes, + calculateStylesheetStats, STYLESHEETS_BUDGET_STATUS_VALID, STYLESHEETS_BUDGET_STATUS_WARNING, STYLESHEETS_BUDGET_STATUS_EXCEEDED, } from '..'; -describe( 'calculateStylesheetSizes', () => { +describe( 'calculateStylesheetStats', () => { it( 'returns null if no stylesheets are provided', () => { - expect( calculateStylesheetSizes() ).toBeNull(); - expect( calculateStylesheetSizes( [] ) ).toBeNull(); + expect( calculateStylesheetStats() ).toBeNull(); + expect( calculateStylesheetStats( [] ) ).toBeNull(); } ); it( 'returns correct sizes prior and after minification', () => { @@ -65,7 +65,7 @@ describe( 'calculateStylesheetSizes', () => { final_size: 0, }, ]; - expect( calculateStylesheetSizes( stylesheets, 25, 20 ) ).toMatchObject( { + expect( calculateStylesheetStats( stylesheets, 25, 20 ) ).toMatchObject( { included: { originalSize: 400, finalSize: 50, @@ -106,7 +106,7 @@ describe( 'calculateStylesheetSizes', () => { }, ]; - const result = calculateStylesheetSizes( stylesheets, 75000, 80 ); + const result = calculateStylesheetStats( stylesheets, 75000, 80 ); expect( result.included.stylesheets ).toHaveLength( 1 ); expect( result.included.stylesheets ).toContain( 'included' ); } ); @@ -131,9 +131,10 @@ describe( 'calculateStylesheetSizes', () => { }, ]; - const result = calculateStylesheetSizes( stylesheets, 50, 80 ); - expect( result.budget.usage ).toBe( 200 ); - expect( result.budget.status ).toBe( STYLESHEETS_BUDGET_STATUS_EXCEEDED ); + const result = calculateStylesheetStats( stylesheets, 50, 80 ); + expect( result.usage.budgetBytes ).toBe( 50 ); + expect( result.usage.actualPercentage ).toBe( 200 ); + expect( result.usage.status ).toBe( STYLESHEETS_BUDGET_STATUS_EXCEEDED ); } ); it( 'sets the warning budget values correctly', () => { @@ -156,9 +157,10 @@ describe( 'calculateStylesheetSizes', () => { }, ]; - const result = calculateStylesheetSizes( stylesheets, 200, 40 ); - expect( result.budget.usage ).toBe( 50 ); - expect( result.budget.status ).toBe( STYLESHEETS_BUDGET_STATUS_WARNING ); + const result = calculateStylesheetStats( stylesheets, 200, 40 ); + expect( result.usage.budgetBytes ).toBe( 200 ); + expect( result.usage.actualPercentage ).toBe( 50 ); + expect( result.usage.status ).toBe( STYLESHEETS_BUDGET_STATUS_WARNING ); } ); it( 'sets the valid budget values correctly', () => { @@ -181,8 +183,9 @@ describe( 'calculateStylesheetSizes', () => { }, ]; - const result = calculateStylesheetSizes( stylesheets, 200, 60 ); - expect( result.budget.usage ).toBe( 50 ); - expect( result.budget.status ).toBe( STYLESHEETS_BUDGET_STATUS_VALID ); + const result = calculateStylesheetStats( stylesheets, 200, 60 ); + expect( result.usage.budgetBytes ).toBe( 200 ); + expect( result.usage.actualPercentage ).toBe( 50 ); + expect( result.usage.status ).toBe( STYLESHEETS_BUDGET_STATUS_VALID ); } ); } ); diff --git a/assets/src/validated-url-page/index.js b/assets/src/validated-url-page/index.js index 1a883154ee4..a2c92ffde2f 100644 --- a/assets/src/validated-url-page/index.js +++ b/assets/src/validated-url-page/index.js @@ -62,7 +62,6 @@ Providers.propTypes = { */ function Root() { const { - cssBudgetBytes, fetchingValidatedUrl, stylesheetSizes, } = useContext( ValidatedUrl ); @@ -72,10 +71,7 @@ function Root() { } return ( - + ); } From e7ba4428901c9960b8c0ee8d03814ba21d984b6f Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Fri, 9 Jul 2021 20:13:31 +0200 Subject: [PATCH 19/58] Fix minor calculation bugs and update copy --- .../components/stylesheets-summary/index.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index 25b201da29e..bafdacecfc1 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -6,7 +6,7 @@ import PropTypes from 'prop-types'; /** * WordPress dependencies */ -import { __, sprintf } from '@wordpress/i18n'; +import { __, _n, sprintf } from '@wordpress/i18n'; /** * Internal dependencies @@ -52,7 +52,7 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { { __( 'Total CSS size prior to minification:', 'amp' ) } - + @@ -60,7 +60,7 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { { __( 'Total CSS size after minification:', 'amp' ) } - + @@ -87,7 +87,12 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { { sprintf( // translators: %d stands for the number of stylesheets - __( 'Excluded minified CSS size (%d stylesheets):', 'amp' ), + _n( + 'Excluded minified CSS size (%d stylesheet):', + 'Excluded minified CSS size (%d stylesheets):', + excluded.stylesheets.length, + 'amp', + ), excluded.stylesheets.length, ) } @@ -104,7 +109,10 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { ) } { status === STYLESHEETS_BUDGET_STATUS_EXCEEDED && ( - { __( 'You have exceeded the CSS budget. Stylesheets deemed of lesser priority have been excluded from the page. Please review the flagged stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } + { excluded.stylesheets.length === 0 + ? __( 'You have exceeded the CSS budget. The page will not be served as a valid AMP page.', 'amp' ) + : __( 'You have exceeded the CSS budget. Stylesheets deemed of lesser priority have been excluded from the page.', 'amp' ) } + { __( 'Please review the flagged stylesheets below and determine if the current theme or a particular plugin is including excessive CSS.', 'amp' ) } ) } From 3503fad2aced25db6c2b0faf1be40b70c0ebbb16 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 14 Jul 2021 13:48:03 +0200 Subject: [PATCH 20/58] Add missing/stale stylesheets errors --- .../validated-url-provider/index.js | 6 +- .../components/stylesheets-summary/index.js | 8 +- .../src/validated-url-page/helpers/index.js | 2 +- assets/src/validated-url-page/index.js | 16 ++-- assets/src/validated-url-page/stylesheets.js | 74 +++++++++++++++++++ .../class-amp-validated-url-post-type.php | 1 + src/Validation/ValidatedUrlData.php | 6 +- 7 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 assets/src/validated-url-page/stylesheets.js diff --git a/assets/src/components/validated-url-provider/index.js b/assets/src/components/validated-url-provider/index.js index c84cb82bada..5b091fa67c8 100644 --- a/assets/src/components/validated-url-provider/index.js +++ b/assets/src/components/validated-url-provider/index.js @@ -44,7 +44,7 @@ export function ValidatedUrlProvider( { validatedUrlsRestPath, } ) { const [ validatedUrl, setValidatedUrl ] = useState( {} ); - const [ stylesheetSizes, setStylesheetSizes ] = useState( {} ); + const [ stylesheetStats, setStylesheetStats ] = useState(); const [ fetchingValidatedUrl, setFetchingValidatedUrl ] = useState( null ); const { error, setError } = useContext( ErrorContext ); @@ -80,7 +80,7 @@ export function ValidatedUrlProvider( { } setValidatedUrl( fetchedValidatedUrl ); - setStylesheetSizes( calculateStylesheetStats( fetchedValidatedUrl?.stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) ); + setStylesheetStats( calculateStylesheetStats( fetchedValidatedUrl?.stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) ); } catch ( e ) { if ( hasUnmounted.current === true ) { return; @@ -103,7 +103,7 @@ export function ValidatedUrlProvider( { diff --git a/assets/src/validated-url-page/components/stylesheets-summary/index.js b/assets/src/validated-url-page/components/stylesheets-summary/index.js index bafdacecfc1..8daa7a27a75 100644 --- a/assets/src/validated-url-page/components/stylesheets-summary/index.js +++ b/assets/src/validated-url-page/components/stylesheets-summary/index.js @@ -30,9 +30,9 @@ import { * Render stylesheets summary table. * * @param {Object} props Component props. - * @param {Object} props.stylesheetSizes Stylesheet sizes object. + * @param {Object} props.stats Stylesheet stats object. */ -export default function StylesheetsSummary( { stylesheetSizes } ) { +export default function StylesheetsSummary( { stats } ) { const { included, excluded, @@ -41,7 +41,7 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { budgetBytes, status, }, - } = stylesheetSizes; + } = stats; return ( <> @@ -119,7 +119,7 @@ export default function StylesheetsSummary( { stylesheetSizes } ) { ); } StylesheetsSummary.propTypes = { - stylesheetSizes: PropTypes.shape( { + stats: PropTypes.shape( { included: PropTypes.shape( { originalSize: PropTypes.number, finalSize: PropTypes.number, diff --git a/assets/src/validated-url-page/helpers/index.js b/assets/src/validated-url-page/helpers/index.js index 7c7f9d8174a..4bbca28fc61 100644 --- a/assets/src/validated-url-page/helpers/index.js +++ b/assets/src/validated-url-page/helpers/index.js @@ -14,7 +14,7 @@ export const STYLESHEETS_BUDGET_STATUS_EXCEEDED = 'exceeded'; * @return {Object|null} Stylesheets sizes data or null. */ export function calculateStylesheetStats( stylesheets, budgetBytes, budgetWarningPercentage ) { - if ( ! stylesheets || stylesheets?.length === 0 ) { + if ( ! stylesheets || stylesheets?.errors || stylesheets?.length === 0 ) { return null; } diff --git a/assets/src/validated-url-page/index.js b/assets/src/validated-url-page/index.js index a2c92ffde2f..69fcef11bd1 100644 --- a/assets/src/validated-url-page/index.js +++ b/assets/src/validated-url-page/index.js @@ -19,12 +19,11 @@ import { render, useContext } from '@wordpress/element'; /** * Internal dependencies */ -import { Loading } from '../components/loading'; import { ErrorBoundary } from '../components/error-boundary'; import { ErrorContextProvider } from '../components/error-context-provider'; import { ErrorScreen } from '../components/error-screen'; import { ValidatedUrl, ValidatedUrlProvider } from '../components/validated-url-provider'; -import StylesheetsSummary from './components/stylesheets-summary'; +import Stylesheets from './stylesheets'; let errorHandler; @@ -63,15 +62,16 @@ Providers.propTypes = { function Root() { const { fetchingValidatedUrl, - stylesheetSizes, + stylesheetStats, + validatedUrl, } = useContext( ValidatedUrl ); - if ( fetchingValidatedUrl !== false ) { - return ; - } - return ( - + ); } diff --git a/assets/src/validated-url-page/stylesheets.js b/assets/src/validated-url-page/stylesheets.js new file mode 100644 index 00000000000..96c146df292 --- /dev/null +++ b/assets/src/validated-url-page/stylesheets.js @@ -0,0 +1,74 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; +import { RECHECK_URL } from 'amp-settings'; // From WP inline script. + +/** + * WordPress dependencies + */ +import { __, sprintf } from '@wordpress/i18n'; +import { RawHTML } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { Loading } from '../components/loading'; +import { + AMPNotice, + NOTICE_SIZE_LARGE, + NOTICE_TYPE_INFO, +} from '../components/amp-notice'; +import StylesheetsSummary from './components/stylesheets-summary'; + +/** + * Stylesheets validation data. + * + * @param {Object} props Component props. + * @param {boolean} props.fetching Flag indicating if stylesheets data is being fetched. + * @param {Object} props.stats Stylesheets stats object. + * @param {Object|Array} props.stylesheets Array of stylesheets details or an object containing errors. + */ +export default function Stylesheets( { + fetching, + stats, + stylesheets, +} ) { + if ( fetching !== false ) { + return ; + } + + if ( stylesheets?.errors?.amp_validated_url_stylesheets_no_longer_available ) { + return ( + + + { sprintf( + /* translators: placeholder is URL to recheck the post */ + __( 'Stylesheet information for this URL is no longer available. Such data is automatically deleted after a week to reduce database storage. It is of little value to store long-term given that it becomes stale as themes and plugins are updated. To obtain the latest stylesheet information, recheck this URL.', 'amp' ), + `${ RECHECK_URL }#amp_stylesheets`, + ) } + + + ); + } + + if ( stylesheets?.errors?.amp_validated_url_stylesheets_missing || stylesheets?.length === 0 || ! stats ) { + return ( + + { __( 'Unable to retrieve stylesheets data for this URL.', 'amp' ) } + + ); + } + + return ; +} +Stylesheets.propTypes = { + fetching: PropTypes.bool, + stats: PropTypes.object, + stylesheets: PropTypes.oneOf( [ + PropTypes.array, + PropTypes.shape( { + errors: PropTypes.object, + } ), + ] ), +}; diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 95745a45d50..38ab7d60528 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -2056,6 +2056,7 @@ public static function enqueue_edit_post_screen_scripts() { 'CSS_BUDGET_BYTES' => $css_budget_bytes, 'CSS_BUDGET_WARNING_PERCENTAGE' => AMP_Style_Sanitizer::CSS_BUDGET_WARNING_PERCENTAGE, 'POST_ID' => $post->ID, + 'RECHECK_URL' => self::get_recheck_url( $post ), 'VALIDATED_URLS_REST_PATH' => '/amp/v1/validated-urls', ]; diff --git a/src/Validation/ValidatedUrlData.php b/src/Validation/ValidatedUrlData.php index c5eec9049a5..2a2a24baf61 100644 --- a/src/Validation/ValidatedUrlData.php +++ b/src/Validation/ValidatedUrlData.php @@ -110,8 +110,7 @@ public function get_stylesheets() { if ( empty( $stylesheets ) ) { return new WP_Error( 'amp_validated_url_stylesheets_no_longer_available', - __( 'Stylesheet information for this URL is no longer available. Such data is automatically deleted after a week to reduce database storage. It is of little value to store long-term given that it becomes stale as themes and plugins are updated. To obtain the latest stylesheet information, recheck this URL.', 'amp' ), - [ 'status' => 404 ] + __( 'Stylesheet information for this URL is no longer available. Such data is automatically deleted after a week to reduce database storage. It is of little value to store long-term given that it becomes stale as themes and plugins are updated. To obtain the latest stylesheet information, recheck this URL.', 'amp' ) ); } @@ -120,8 +119,7 @@ public function get_stylesheets() { if ( ! is_array( $stylesheets ) ) { return new WP_Error( 'amp_validated_url_stylesheets_missing', - __( 'Unable to retrieve stylesheets data for this URL.', 'amp' ), - [ 'status' => 404 ] + __( 'Unable to retrieve stylesheets data for this URL.', 'amp' ) ); } From cd96844bbbeb1d695aebf24e554893a50bc70383 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 14 Jul 2021 13:56:31 +0200 Subject: [PATCH 21/58] Add notice when CSS processing is limited --- .../src/components/_amp-stylesheet-list.scss | 1 + .../src/components/_amp-stylesheet-summary.scss | 2 +- assets/src/validated-url-page/stylesheets.js | 17 +++++++++++++++-- .../class-amp-validated-url-post-type.php | 1 + 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/assets/css/src/components/_amp-stylesheet-list.scss b/assets/css/src/components/_amp-stylesheet-list.scss index 53a3fb632c4..b1a776dfa33 100644 --- a/assets/css/src/components/_amp-stylesheet-list.scss +++ b/assets/css/src/components/_amp-stylesheet-list.scss @@ -1,4 +1,5 @@ .amp-stylesheet-list { + margin: 1em 0; th { overflow-wrap: normal; diff --git a/assets/css/src/components/_amp-stylesheet-summary.scss b/assets/css/src/components/_amp-stylesheet-summary.scss index 4ff217add55..5a2e6d5338f 100644 --- a/assets/css/src/components/_amp-stylesheet-summary.scss +++ b/assets/css/src/components/_amp-stylesheet-summary.scss @@ -1,5 +1,5 @@ .amp-stylesheet-summary { - margin-bottom: 1em; + margin: 1em 0; span.amp-icon { font-size: 16px; diff --git a/assets/src/validated-url-page/stylesheets.js b/assets/src/validated-url-page/stylesheets.js index 96c146df292..77b8f826502 100644 --- a/assets/src/validated-url-page/stylesheets.js +++ b/assets/src/validated-url-page/stylesheets.js @@ -2,7 +2,10 @@ * External dependencies */ import PropTypes from 'prop-types'; -import { RECHECK_URL } from 'amp-settings'; // From WP inline script. +import { + HAS_REQUIRED_PHP_CSS_PARSER, + RECHECK_URL, +} from 'amp-settings'; // From WP inline script. /** * WordPress dependencies @@ -18,6 +21,7 @@ import { AMPNotice, NOTICE_SIZE_LARGE, NOTICE_TYPE_INFO, + NOTICE_TYPE_WARNING, } from '../components/amp-notice'; import StylesheetsSummary from './components/stylesheets-summary'; @@ -60,7 +64,16 @@ export default function Stylesheets( { ); } - return ; + return ( + <> + { ! HAS_REQUIRED_PHP_CSS_PARSER && ( + + { __( 'AMP CSS processing is limited because a conflicting version of PHP-CSS-Parser has been loaded by another plugin or theme. Tree shaking is not available.', 'amp' ) } + + ) } + + + ); } Stylesheets.propTypes = { fetching: PropTypes.bool, diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 38ab7d60528..750fd8e2606 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -2055,6 +2055,7 @@ public static function enqueue_edit_post_screen_scripts() { 'APP_ROOT_ID' => self::AMP_VALIDATED_URL_PAGE_APP_ROOT_ID, 'CSS_BUDGET_BYTES' => $css_budget_bytes, 'CSS_BUDGET_WARNING_PERCENTAGE' => AMP_Style_Sanitizer::CSS_BUDGET_WARNING_PERCENTAGE, + 'HAS_REQUIRED_PHP_CSS_PARSER' => AMP_Style_Sanitizer::has_required_php_css_parser(), 'POST_ID' => $post->ID, 'RECHECK_URL' => self::get_recheck_url( $post ), 'VALIDATED_URLS_REST_PATH' => '/amp/v1/validated-urls', From 3693b471c65293896336aeae2107c36e238ae679 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 14 Jul 2021 14:00:20 +0200 Subject: [PATCH 22/58] Fix `propTypes` definition --- assets/src/validated-url-page/stylesheets.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assets/src/validated-url-page/stylesheets.js b/assets/src/validated-url-page/stylesheets.js index 77b8f826502..61b0579c895 100644 --- a/assets/src/validated-url-page/stylesheets.js +++ b/assets/src/validated-url-page/stylesheets.js @@ -78,8 +78,8 @@ export default function Stylesheets( { Stylesheets.propTypes = { fetching: PropTypes.bool, stats: PropTypes.object, - stylesheets: PropTypes.oneOf( [ - PropTypes.array, + stylesheets: PropTypes.oneOfType( [ + PropTypes.arrayOf( PropTypes.object ), PropTypes.shape( { errors: PropTypes.object, } ), From 6652eb7409cda5b18893e386c77bcf85619178a3 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 14 Jul 2021 18:20:10 +0200 Subject: [PATCH 23/58] Calculate stylesheets stats in separate side effect --- .../validated-url-provider/index.js | 19 +++++++++++++++++-- .../src/validated-url-page/helpers/index.js | 2 +- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/assets/src/components/validated-url-provider/index.js b/assets/src/components/validated-url-provider/index.js index 5b091fa67c8..1706d8cf4da 100644 --- a/assets/src/components/validated-url-provider/index.js +++ b/assets/src/components/validated-url-provider/index.js @@ -80,7 +80,6 @@ export function ValidatedUrlProvider( { } setValidatedUrl( fetchedValidatedUrl ); - setStylesheetStats( calculateStylesheetStats( fetchedValidatedUrl?.stylesheets, cssBudgetBytes, cssBudgetWarningPercentage ) ); } catch ( e ) { if ( hasUnmounted.current === true ) { return; @@ -97,7 +96,23 @@ export function ValidatedUrlProvider( { setFetchingValidatedUrl( false ); } )(); - }, [ cssBudgetBytes, cssBudgetWarningPercentage, error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); + }, [ error, fetchingValidatedUrl, hasErrorBoundary, postId, setAsyncError, setError, validatedUrl, validatedUrlsRestPath ] ); + + /** + * Calculate stylesheet stats. + */ + useEffect( () => { + if ( + ! validatedUrl?.stylesheets || + validatedUrl.stylesheets?.errors || + ! Array.isArray( validatedUrl.stylesheets ) || + validatedUrl.stylesheets.length === 0 + ) { + return; + } + + setStylesheetStats( calculateStylesheetStats( [ ...validatedUrl.stylesheets ], cssBudgetBytes, cssBudgetWarningPercentage ) ); + }, [ cssBudgetBytes, cssBudgetWarningPercentage, validatedUrl.stylesheets ] ); return ( Date: Wed, 14 Jul 2021 18:21:59 +0200 Subject: [PATCH 24/58] Add stylesheet details table (WIP) --- .../src/components/_amp-stylesheet-list.scss | 4 - assets/src/components/icon/index.js | 7 +- .../components/stylesheets-table/head.js | 48 +++++++ .../components/stylesheets-table/index.js | 60 ++++++++ .../components/stylesheets-table/row.js | 135 ++++++++++++++++++ assets/src/validated-url-page/stylesheets.js | 2 + .../class-amp-validated-url-post-type.php | 2 +- src/Validation/ValidatedUrlData.php | 67 +++++++++ 8 files changed, 318 insertions(+), 7 deletions(-) create mode 100644 assets/src/validated-url-page/components/stylesheets-table/head.js create mode 100644 assets/src/validated-url-page/components/stylesheets-table/index.js create mode 100644 assets/src/validated-url-page/components/stylesheets-table/row.js diff --git a/assets/css/src/components/_amp-stylesheet-list.scss b/assets/css/src/components/_amp-stylesheet-list.scss index b1a776dfa33..2b04bc08775 100644 --- a/assets/css/src/components/_amp-stylesheet-list.scss +++ b/assets/css/src/components/_amp-stylesheet-list.scss @@ -42,10 +42,6 @@ text-align: center; } - .column-percentage { - text-align: center; - } - .column-source { width: 25%; } diff --git a/assets/src/components/icon/index.js b/assets/src/components/icon/index.js index 424a99687d9..1bb4a5df530 100644 --- a/assets/src/components/icon/index.js +++ b/assets/src/components/icon/index.js @@ -107,7 +107,7 @@ StatusIcon.propTypes = { * @param {boolean} props.isValid Flag indicating the icon is for a valid status. * @param {boolean} props.isBoxed Whether the icon should be contained in a box. */ -export function ValidationStatusIcon( { isError, isWarning, isValid, isBoxed = false } ) { +export function ValidationStatusIcon( { isError, isWarning, isValid, isBoxed = false, ...rest } ) { let type; if ( isError ) { @@ -121,7 +121,10 @@ export function ValidationStatusIcon( { isError, isWarning, isValid, isBoxed = f } return ( - + { isValid && } { isWarning && } { isError && } diff --git a/assets/src/validated-url-page/components/stylesheets-table/head.js b/assets/src/validated-url-page/components/stylesheets-table/head.js new file mode 100644 index 00000000000..7082d545751 --- /dev/null +++ b/assets/src/validated-url-page/components/stylesheets-table/head.js @@ -0,0 +1,48 @@ +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; + +export default function StylesheetsTableHead() { + return ( + + + + + { __( 'Expand/collapse', 'amp' ) } + + + + { __( 'Order', 'amp' ) } + + + { __( 'Original', 'amp' ) } + + + { __( 'Minified', 'amp' ) } + + + { __( 'Final', 'amp' ) } + + + { __( 'Percent', 'amp' ) } + + + { __( 'Priority', 'amp' ) } + + + { __( 'Included', 'amp' ) } + + + { __( 'Markup', 'amp' ) } + + + { __( 'Sources', 'amp' ) } + + + + ); +} diff --git a/assets/src/validated-url-page/components/stylesheets-table/index.js b/assets/src/validated-url-page/components/stylesheets-table/index.js new file mode 100644 index 00000000000..ba51b1af253 --- /dev/null +++ b/assets/src/validated-url-page/components/stylesheets-table/index.js @@ -0,0 +1,60 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * Internal dependencies + */ +import StylesheetsTableHead from './head'; +import StylesheetsTableRow from './row'; + +export default function StylesheetsTable( { stylesheets, stats } ) { + const totalFinalSize = stats.included.finalSize + stats.excluded.finalSize; + + return ( + + + + { stylesheets.map( ( stylesheet, index ) => ( + + ) ) } + +
+ ); +} +StylesheetsTable.propTypes = { + stylesheets: PropTypes.arrayOf( PropTypes.shape( { + hash: PropTypes.string, + original_size: PropTypes.number, + final_size: PropTypes.number, + priority: PropTypes.number, + original_tag: PropTypes.string, + original_tag_abbr: PropTypes.string, + } ) ), + stats: PropTypes.shape( { + included: PropTypes.shape( { + finalSize: PropTypes.number, + stylesheets: PropTypes.array, + } ), + excluded: PropTypes.shape( { + finalSize: PropTypes.number, + stylesheets: PropTypes.array, + } ), + excessive: PropTypes.shape( { + stylesheets: PropTypes.array, + } ), + } ), +}; diff --git a/assets/src/validated-url-page/components/stylesheets-table/row.js b/assets/src/validated-url-page/components/stylesheets-table/row.js new file mode 100644 index 00000000000..b8f5716e9d9 --- /dev/null +++ b/assets/src/validated-url-page/components/stylesheets-table/row.js @@ -0,0 +1,135 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * WordPress dependencies + */ +import { useState } from '@wordpress/element'; +import { __ } from '@wordpress/i18n'; +import { Button } from '@wordpress/components'; + +/** + * Internal dependencies + */ +import FormattedMemoryValue from '../../../components/formatted-memory-value'; +import { ValidationStatusIcon } from '../../../components/icon'; + +export default function StylesheetsTableRow( { + finalSize, + index, + isExcessive, + isExcluded, + isIncluded, + originalSize, + orignalTag, + orignalTagAbbr, + priority, + totalFinalSize, +} ) { + const [ expanded, setExpanded ] = useState( false ); + const toggle = () => setExpanded( ( value ) => ! value ); + + const minifiedRatio = originalSize === 0 ? 1 : finalSize / originalSize; + const minifiedPercentage = `${ minifiedRatio <= 1 ? '-' : '+' }${ Math.abs( ( 1.0 - minifiedRatio ) * 100 ).toFixed( 1 ) }%`; + + return ( + <> + + +