From 7fdb552675bb3e0a70b6a402fc8f26f725ecf5ad Mon Sep 17 00:00:00 2001 From: Dhaval Parekh Date: Fri, 25 Mar 2022 17:49:33 +0530 Subject: [PATCH 1/7] Replace dangerouslySetInnerHTML with createInterpolateElement for settings page --- assets/src/settings-page/analytics.js | 47 ++++++++------ .../src/settings-page/paired-url-structure.js | 23 ++++--- .../src/settings-page/plugin-suppression.js | 13 +++- assets/src/settings-page/site-review.js | 61 +++++++++++-------- .../src/settings-page/supported-templates.js | 25 ++++---- 5 files changed, 99 insertions(+), 70 deletions(-) diff --git a/assets/src/settings-page/analytics.js b/assets/src/settings-page/analytics.js index 8f242d5051d..5a5722d3e0d 100644 --- a/assets/src/settings-page/analytics.js +++ b/assets/src/settings-page/analytics.js @@ -10,8 +10,8 @@ import { ANALYTICS_VENDORS_LIST } from 'amp-settings'; */ import { Icon, plus, trash } from '@wordpress/icons'; import { __, sprintf } from '@wordpress/i18n'; -import { useContext, useEffect, useRef } from '@wordpress/element'; -import { Button, PanelRow, BaseControl, VisuallyHidden } from '@wordpress/components'; +import { createInterpolateElement, useContext, useEffect, useRef } from '@wordpress/element'; +import { Button, TextControl, PanelRow, BaseControl, VisuallyHidden } from '@wordpress/components'; /** * Internal dependencies @@ -226,24 +226,31 @@ export function Analytics() { { __( 'Learn about analytics for AMP.', 'amp' ) } -

documentation for %2$s as well as the plugin\'s analytics documentation. Each analytics configuration supplied below must take the form of a JSON object beginning with a %4$s and ending with a %5$s. Do not include any HTML tags like %6$s or %7$s. For the type field, supply one of the available analytics vendors or leave it blank for in-house analytics. For Google Analytics specifically, the type should be %9$s. For Google Tag Manager please consider using Site Kit by Google plugin.', 'amp' ), - __( 'https://amp.dev/documentation/components/amp-analytics/', 'amp' ), - 'amp-analytics', - __( 'https://amp-wp.org/documentation/getting-started/analytics/', 'amp' ), - '{', - '}', - '<amp-analytics>', - '<script>', - __( 'https://amp.dev/documentation/guides-and-tutorials/optimize-and-measure/configure-analytics/analytics-vendors/', 'amp' ), - `${ GOOGLE_ANALYTICS_VENDOR }`, - __( 'https://wordpress.org/plugins/google-site-kit/', 'amp' ), - ), - } } - /> +

+ { + createInterpolateElement( + sprintf( + /* translators: 1: amp-analytics, 2: {, 3: }, 4: amp-analytics tag, 5: script tag, 6: googleanalytics. */ + __( 'Please see AMP project\'s documentation for %1$s as well as the plugin\'s analytics documentation. Each analytics configuration supplied below must take the form of a JSON object beginning with a %2$s and ending with a %3$s. Do not include any HTML tags like %4$s or %5$s. For the type field, supply one of the available analytics vendors or leave it blank for in-house analytics. For Google Analytics specifically, the type should be %6$s. For Google Tag Manager please consider using Site Kit by Google plugin.', 'amp' ), + 'amp-analytics', + '{', + '}', + '<amp-analytics>', + '<script>', + `${ GOOGLE_ANALYTICS_VENDOR }`, + ), + { + /* eslint-disable jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. */ + AnalyticsDocsUrl: , + PluginAnalyticsDocsUrl: , + VendorDocsUrl: , + SiteKitUrl: , + /* eslint-enable jsx-a11y/anchor-has-content */ + code: , + }, + ) + } +

{ Object.entries( analytics || {} ).map( ( [ key, { type, config } ], index ) => ( ) } -

Note: Changing the paired URL structure can cause AMP pages to temporarily disappear from search results until your site is re-indexed. If you\'re migrating from another AMP plugin with a different paired URL structure, then you may want to change this setting. Otherwise we recommend leaving it as is. Learn more', 'amp' ), - __( 'https://amp-wp.org/?p=10004', 'amp' ), - ), - } } - /> +

+ { + createInterpolateElement( + __( 'When using the Transitional or Reader template modes, your site is in a “Paired AMP” configuration. Your site\'s canonical URLs are non-AMP, and the separate AMP versions of your pages have AMP-specific URLs. The structure of a paired AMP URL is not important, whether using a query parameter or path suffix. The use of a query parameter is the most compatible across various sites and it has the benefit of not resulting in a 404 if the AMP plugin is deactivated. Note: Changing the paired URL structure can cause AMP pages to temporarily disappear from search results until your site is re-indexed. If you\'re migrating from another AMP plugin with a different paired URL structure, then you may want to change this setting. Otherwise we recommend leaving it as is. Learn more', 'amp' ), + { + em: , + // eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. + a: , + }, + ) + } +

{ ! endpointSuffixAvailable && ( diff --git a/assets/src/settings-page/plugin-suppression.js b/assets/src/settings-page/plugin-suppression.js index 099ae443270..bae781e9833 100644 --- a/assets/src/settings-page/plugin-suppression.js +++ b/assets/src/settings-page/plugin-suppression.js @@ -7,7 +7,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { useContext, Fragment } from '@wordpress/element'; +import { useContext, Fragment, createInterpolateElement } from '@wordpress/element'; import { __, sprintf, _n } from '@wordpress/i18n'; import { autop } from '@wordpress/autop'; import { format, dateI18n } from '@wordpress/date'; @@ -158,7 +158,16 @@ function ValidationErrorDetails( { errors } ) { ) }> - + + { + createInterpolateElement( + error.title, + { + code: , + }, + ) + } + diff --git a/assets/src/settings-page/site-review.js b/assets/src/settings-page/site-review.js index 044f122bd7d..be90a504cf1 100644 --- a/assets/src/settings-page/site-review.js +++ b/assets/src/settings-page/site-review.js @@ -1,9 +1,9 @@ /** * WordPress dependencies */ -import { __, sprintf } from '@wordpress/i18n'; +import { __ } from '@wordpress/i18n'; import { Button } from '@wordpress/components'; -import { useContext } from '@wordpress/element'; +import { createInterpolateElement, useContext } from '@wordpress/element'; /** * Internal dependencies @@ -74,30 +74,39 @@ export function SiteReview() { { __( 'Need help?', 'amp' ) }
    - { /* dangerouslySetInnerHTML reason: Injection of a link. */ } -
  • support forums', 'amp' ), - 'https://wordpress.org/support/plugin/amp/#new-topic-0', - ), - } } /> - { /* dangerouslySetInnerHTML reason: Injection of a link. */ } -
  • template mode', 'amp' ), - '#template-modes', - ), - } } /> - { /* dangerouslySetInnerHTML reason: Injection of a link. */ } -
  • Learn more how the AMP plugin works', 'amp' ), - 'https://amp-wp.org/documentation/how-the-plugin-works/', - ), - } } /> +
  • + { + createInterpolateElement( + __( 'Reach out in the support forums', 'amp' ), + { + // eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. + a: , + }, + ) + } +
    • +
  • + { + createInterpolateElement( + __( 'Try a different template mode', 'amp' ), + { + // eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. + a: , + }, + ) + } +
    • +
  • + { + createInterpolateElement( + __( 'Learn more how the AMP plugin works', 'amp' ), + { + // eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. + a: , + }, + ) + } +
{ previewPermalink && ( diff --git a/assets/src/settings-page/supported-templates.js b/assets/src/settings-page/supported-templates.js index f34eb33b1ac..b9565be3996 100644 --- a/assets/src/settings-page/supported-templates.js +++ b/assets/src/settings-page/supported-templates.js @@ -6,8 +6,8 @@ import PropTypes from 'prop-types'; /** * WordPress dependencies */ -import { __, sprintf } from '@wordpress/i18n'; -import { useContext } from '@wordpress/element'; +import { __ } from '@wordpress/i18n'; +import { useContext, createInterpolateElement } from '@wordpress/element'; import { CheckboxControl } from '@wordpress/components'; /** @@ -246,16 +246,17 @@ export function SupportedTemplatesFieldset() { { ! allTemplatesSupported ? ( <> - { /* dangerouslySetInnerHTML reason: Link embedded in translation string. */ } -

Template Hierarchy:', 'amp' ), - 'https://developer.wordpress.org/themes/basics/template-hierarchy/', - ), - } } - /> +

+ { + createInterpolateElement( + __( 'Limit AMP on a subset of the WordPress Template Hierarchy:', 'amp' ), + { + // eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. + a: , + }, + ) + } +

From 19723916cc717fd1d74e2945b1f89e9f8b19b810 Mon Sep 17 00:00:00 2001 From: Dhaval Parekh Date: Sun, 27 Mar 2022 21:14:27 +0530 Subject: [PATCH 2/7] Replace dangerouslySetInnerHTML with createInterpolateElement for site scan notice, amp-support, onboarding-wizard --- .../plugins-with-amp-incompatibility.js | 34 ++++++----- .../themes-with-amp-incompatibility.js | 34 ++++++----- assets/src/components/amp-support/index.js | 19 +++--- assets/src/components/error-screen/index.js | 23 ++++--- .../components/template-mode-option/index.js | 13 ++-- .../use-template-mode-recommendation/index.js | 23 +++++-- .../src/onboarding-wizard/pages/done/index.js | 61 +++++++++++-------- .../pages/template-mode/index.js | 26 ++++---- assets/src/settings-page/analytics.js | 18 +++--- 9 files changed, 149 insertions(+), 102 deletions(-) diff --git a/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js b/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js index 6b854772ed8..301c7bf3c2d 100644 --- a/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js +++ b/assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js @@ -7,7 +7,7 @@ import { AMP_COMPATIBLE_PLUGINS_URL, SETTINGS_LINK } from 'amp-site-scan-notice' /** * WordPress dependencies */ -import { useContext, useMemo } from '@wordpress/element'; +import { createInterpolateElement, useContext, useMemo } from '@wordpress/element'; import { __, _n, sprintf } from '@wordpress/i18n'; /** @@ -57,22 +57,26 @@ export function PluginsWithAmpIncompatibility( { pluginsWithAmpIncompatibility } key={ pluginWithAmpIncompatibility.slug } className="amp-site-scan-notice__source-details" > - %1$s on %2$d URL', - '%1$s on %2$d URLs', + + { + createInterpolateElement( + sprintf( + /* translators: 1: plugin name; 2: number of URLs with AMP validation issues. */ + _n( + '%1$s on %2$d URL', + '%1$s on %2$d URLs', + pluginWithAmpIncompatibility.urls.length, + 'amp', + ), + pluginNames[ pluginWithAmpIncompatibility.slug ], pluginWithAmpIncompatibility.urls.length, - 'amp', ), - pluginNames[ pluginWithAmpIncompatibility.slug ], - pluginWithAmpIncompatibility.urls.length, - ), - } } - /> + { + b: , + }, + ) + } +
diff --git a/assets/src/onboarding-wizard/pages/template-mode/index.js b/assets/src/onboarding-wizard/pages/template-mode/index.js index 69daa3e4569..6bd767a86be 100644 --- a/assets/src/onboarding-wizard/pages/template-mode/index.js +++ b/assets/src/onboarding-wizard/pages/template-mode/index.js @@ -1,8 +1,8 @@ /** * WordPress dependencies */ -import { useEffect, useContext } from '@wordpress/element'; -import { __, sprintf } from '@wordpress/i18n'; +import { useEffect, useContext, createInterpolateElement } from '@wordpress/element'; +import { __ } from '@wordpress/i18n'; /** * Internal dependencies @@ -39,15 +39,19 @@ export function TemplateMode() {

{ __( 'Template Modes', 'amp' ) }

- { /* dangerouslySetInnerHTML reason: Injection of links. */ } -

AMP experience with different modes and availability of AMP components in the ecosystem.', 'amp' ), - 'https://amp-wp.org/documentation/getting-started/template-modes/', - 'https://amp-wp.org/ecosystem/', - ), - } } /> +

+ { + createInterpolateElement( + __( 'Based on site scan results the AMP plugin provides the following choices. Learn more about the AMP experience with different modes and availability of AMP components in the ecosystem.', 'amp' ), + { + /* eslint-disable jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. */ + GettingStartedUrl: , + EcosystemUrl: , + /* eslint-enable jsx-a11y/anchor-has-content */ + }, + ) + } +

Site Kit by Google. This plugin configures analytics for both non-AMP and AMP pages alike, avoiding the need to manually provide a separate AMP configuration here. Nevertheless, for documentation on manual configuration see Adding Analytics to your AMP pages.', 'amp' ), - __( 'https://wordpress.org/plugins/google-site-kit/', 'amp' ), - __( 'https://developers.google.com/analytics/devguides/collection/amp-analytics/', 'amp' ), +const GOOGLE_ANALYTICS_NOTICE = createInterpolateElement( + __( 'For Google Analytics or Google Tag Manager please consider using Site Kit by Google. This plugin configures analytics for both non-AMP and AMP pages alike, avoiding the need to manually provide a separate AMP configuration here. Nevertheless, for documentation on manual configuration see Adding Analytics to your AMP pages.', 'amp' ), + { + /* eslint-disable jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string. */ + GoogleSiteKitUrl: , + GoogleAnalyticsDevGuideUrl: , + /* eslint-enable jsx-a11y/anchor-has-content */ + }, ); const vendorConfigs = { @@ -164,8 +167,9 @@ function AnalyticsEntry( { entryIndex, onChange, onDelete, type = '', config = ' { vendorConfigs[ type ]?.notice && ( - { /* dangerouslySetInnerHTML reason: Injection of links. */ } - + + { vendorConfigs[ type ].notice } + ) } From ce91686e310d700b68c227fbaf1dc14a13a1d487 Mon Sep 17 00:00:00 2001 From: Dhaval Parekh Date: Sun, 27 Mar 2022 21:23:19 +0530 Subject: [PATCH 3/7] Update JS test cases --- .../test/__snapshots__/index.js.snap | 18 ++++++++++------- .../test/__snapshots__/index.js.snap | 20 ++++++------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/assets/src/components/error-screen/test/__snapshots__/index.js.snap b/assets/src/components/error-screen/test/__snapshots__/index.js.snap index e3c710923ca..bfe441115b6 100644 --- a/assets/src/components/error-screen/test/__snapshots__/index.js.snap +++ b/assets/src/components/error-screen/test/__snapshots__/index.js.snap @@ -17,13 +17,17 @@ exports[`ErrorScreen matches snapshot 1`] = ` } } /> -

support forum.", - } - } - /> +

+ Please submit details to our + + support forum + + . +

Details diff --git a/assets/src/components/template-mode-option/test/__snapshots__/index.js.snap b/assets/src/components/template-mode-option/test/__snapshots__/index.js.snap index 8dfd7d8836b..24bfc460f93 100644 --- a/assets/src/components/template-mode-option/test/__snapshots__/index.js.snap +++ b/assets/src/components/template-mode-option/test/__snapshots__/index.js.snap @@ -432,13 +432,9 @@ exports[`TemplateModeOption matches snapshot 2`] = ` className="template-mode-selection__details" >

- + + Reader info +

- + + Component details + Date: Mon, 28 Mar 2022 12:49:48 +0530 Subject: [PATCH 4/7] Update message of AMP Analytics on settings page --- assets/src/settings-page/analytics.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/assets/src/settings-page/analytics.js b/assets/src/settings-page/analytics.js index 2de3d816411..9fb0d52be53 100644 --- a/assets/src/settings-page/analytics.js +++ b/assets/src/settings-page/analytics.js @@ -239,8 +239,6 @@ export function Analytics() { 'amp-analytics', '{', '}', - '<amp-analytics>', - '<script>', `${ GOOGLE_ANALYTICS_VENDOR }`, ), { @@ -251,6 +249,16 @@ export function Analytics() { SiteKitUrl: , /* eslint-enable jsx-a11y/anchor-has-content */ code: , + AmpAnalyticsTag: ( + + { '' } + + ), + ScriptTag: ( + + { '