Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace use of dangerouslySetInnerHTML with createInterpolateElement #7000

Merged
merged 7 commits into from
Apr 20, 2022
Merged

Replace use of dangerouslySetInnerHTML with createInterpolateElement #7000

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7fdb552
Replace dangerouslySetInnerHTML with createInterpolateElement for set…
dhaval-parekh Mar 25, 2022
1972391
Replace dangerouslySetInnerHTML with createInterpolateElement for sit…
dhaval-parekh Mar 27, 2022
ce91686
Update JS test cases
dhaval-parekh Mar 27, 2022
1e9bf82
Update message of AMP Analytics on settings page
dhaval-parekh Mar 28, 2022
3303a53
Fix eslint issues
dhaval-parekh Apr 19, 2022
ff8e928
Apply suggestions from PR
dhaval-parekh Apr 20, 2022
e3134fd
Use Link for suffix instead of Url
dhaval-parekh Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions assets/src/admin/site-scan-notice/plugins-with-amp-incompatibility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -57,22 +57,26 @@ export function PluginsWithAmpIncompatibility( { pluginsWithAmpIncompatibility }
key={ pluginWithAmpIncompatibility.slug }
className="amp-site-scan-notice__source-details"
>
<summary
className="amp-site-scan-notice__source-summary"
dangerouslySetInnerHTML={ {
__html: sprintf(
/* translators: 1: plugin name; 2: number of URLs with AMP validation issues. */
_n(
'<b>%1$s</b> on %2$d URL',
'<b>%1$s</b> on %2$d URLs',
<summary className="amp-site-scan-notice__source-summary">
{
createInterpolateElement(
sprintf(
/* translators: 1: plugin name; 2: number of URLs with AMP validation issues. */
_n(
'<b>%1$s</b> on %2$d URL',
'<b>%1$s</b> on %2$d URLs',
pluginWithAmpIncompatibility.urls.length,
'amp',
),
pluginNames[ pluginWithAmpIncompatibility.slug ],
pluginWithAmpIncompatibility.urls.length,
'amp',
),
pluginNames[ pluginWithAmpIncompatibility.slug ],
pluginWithAmpIncompatibility.urls.length,
),
} }
/>
{
b: <b />,
},
)
}
</summary>
<ul className="amp-site-scan-notice__urls-list">
{ pluginWithAmpIncompatibility.urls.map( ( url ) => (
<li key={ url }>
Expand Down
34 changes: 19 additions & 15 deletions assets/src/admin/site-scan-notice/themes-with-amp-incompatibility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { AMP_COMPATIBLE_THEMES_URL } from 'amp-site-scan-notice'; // From WP inl
/**
* WordPress dependencies
*/
import { useContext, useMemo } from '@wordpress/element';
import { createInterpolateElement, useContext, useMemo } from '@wordpress/element';
import { __, _n, sprintf } from '@wordpress/i18n';

/**
Expand Down Expand Up @@ -52,22 +52,26 @@ export function ThemesWithAmpIncompatibility( { themesWithAmpIncompatibility } )
key={ themeWithAmpIncompatibility.slug }
className="amp-site-scan-notice__source-details"
>
<summary
className="amp-site-scan-notice__source-summary"
dangerouslySetInnerHTML={ {
__html: sprintf(
/* translators: 1: theme name; 2: number of URLs with AMP validation issues. */
_n(
'<b>%1$s</b> on %2$d URL',
'<b>%1$s</b> on %2$d URLs',
<summary className="amp-site-scan-notice__source-summary">
{
createInterpolateElement(
sprintf(
/* translators: 1: theme name; 2: number of URLs with AMP validation issues. */
_n(
'<b>%1$s</b> on %2$d URL',
'<b>%1$s</b> on %2$d URLs',
themeWithAmpIncompatibility.urls.length,
'amp',
),
themeNames[ themeWithAmpIncompatibility.slug ],
themeWithAmpIncompatibility.urls.length,
'amp',
),
themeNames[ themeWithAmpIncompatibility.slug ],
themeWithAmpIncompatibility.urls.length,
),
} }
/>
{
b: <b />,
},
)
}
</summary>
<ul className="amp-site-scan-notice__urls-list">
{ themeWithAmpIncompatibility.urls.map( ( url ) => (
<li key={ url }>
Expand Down
19 changes: 10 additions & 9 deletions assets/src/components/amp-support/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import PropTypes from 'prop-types';
/**
* WordPress dependencies
*/
import { useState, useEffect } from '@wordpress/element';
import { createInterpolateElement, useState, useEffect } from '@wordpress/element';
import { Button, ExternalLink } from '@wordpress/components';
import apiFetch from '@wordpress/api-fetch';
import { __, sprintf } from '@wordpress/i18n';
Expand Down Expand Up @@ -85,16 +85,17 @@ export function AMPSupport( props ) {
<h2 className="amp-support__heading">
{ __( 'AMP Support', 'amp' ) }
</h2>
{ /* dangerouslySetInnerHTML reason: Injection of links. */ }
<p dangerouslySetInnerHTML={
<p>
{
__html: sprintf(
/* translators: %s is the URL to create a new support topic */
__( 'In order to best assist you, please tap the Send Data button below to send the following site information to our private database. Once you have done so, copy the the resulting Support UUID in the blue box that appears and include the ID in a new <a href="%s" rel="noreferrer" target="_blank">support forum topic</a>. You do not have to submit data to get support, but our team will be able to help you more effectively if you do so.', 'amp' ),
'https://wordpress.org/support/plugin/amp/#new-topic-0',
),
createInterpolateElement(
__( 'In order to best assist you, please tap the Send Data button below to send the following site information to our private database. Once you have done so, copy the the resulting Support UUID in the blue box that appears and include the ID in a new <a>support forum topic</a>. You do not have to submit data to get support, but our team will be able to help you more effectively if you do so.', 'amp' ),
dhaval-parekh marked this conversation as resolved.
Show resolved Hide resolved
{
// eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string.
a: <a href="https://wordpress.org/support/plugin/amp/#new-topic-0" rel="noreferrer" target="_blank" />,
},
)
}
} />
</p>

<ValidationResultsNotice data={ data } args={ args } ampValidatedPostCount={ ampValidatedPostCount } />

Expand Down
23 changes: 13 additions & 10 deletions assets/src/components/error-screen/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import PropTypes from 'prop-types';
/**
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { __ } from '@wordpress/i18n';
import { Panel } from '@wordpress/components';
import { useState } from '@wordpress/element';
import { createInterpolateElement, useState } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -41,14 +41,17 @@ export function ErrorScreen( { error, finishLinkLabel, finishLinkUrl, title } )
__html: message || __( 'There was an error loading the page.', 'amp' ),
} } />

{ /* dangerouslySetInnerHTML reason: The message contains a link to the AMP support forum. */ }
<p dangerouslySetInnerHTML={ {
__html: sprintf(
// translators: %s is the AMP support forum URL.
__( 'Please submit details to our <a href="%s" target="_blank" rel="noreferrer noopener">support forum</a>.', 'amp' ),
__( 'https://wordpress.org/support/plugin/amp/', 'amp' ),
),
} } />
<p>
{
createInterpolateElement(
__( 'Please submit details to our <a>support forum</a>.', 'amp' ),
{
// eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string.
a: <a href="https://wordpress.org/support/plugin/amp/" target="_blank" rel="noreferrer noopener" />,
},
)
}
</p>

{ stack && (
<details>
Expand Down
18 changes: 11 additions & 7 deletions assets/src/components/error-screen/test/__snapshots__/index.js.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 8 additions & 5 deletions assets/src/components/template-mode-option/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,17 @@ export function TemplateModeOption( { children, details, detailsUrl, initialOpen
<li
key={ index }
className="template-mode-selection__details-list-item"
/* dangerouslySetInnerHTML reason: `detail` may contain `strong` elements. */
dangerouslySetInnerHTML={ { __html: detail } }
/>
>
{ detail }
</li>
) ) }
</ul>
) }
{ details && ! Array.isArray( details ) && (
<p>
<span dangerouslySetInnerHTML={ { __html: details } } />
<span>
{ details }
</span>
{ detailsUrl && (
<>
{ ' ' }
Expand All @@ -171,7 +173,8 @@ TemplateModeOption.propTypes = {
children: PropTypes.any,
details: PropTypes.oneOfType( [
PropTypes.string,
PropTypes.arrayOf( PropTypes.string ),
PropTypes.array,
PropTypes.object,
dhaval-parekh marked this conversation as resolved.
Show resolved Hide resolved
] ),
detailsUrl: PropTypes.string,
initialOpen: PropTypes.bool,
Expand Down
20 changes: 6 additions & 14 deletions assets/src/components/template-mode-option/test/__snapshots__/index.js.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 19 additions & 4 deletions assets/src/components/use-template-mode-recommendation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useContext, useLayoutEffect, useState } from '@wordpress/element';
import { createInterpolateElement, useContext, useLayoutEffect, useState } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -78,9 +78,24 @@ export function getTemplateModeRecommendation( {
} ) {
/* eslint-disable @wordpress/no-unused-vars-before-return */
const mobileRedirectionNote = __( 'If automatic mobile redirection is enabled, the AMP version of the content will be served on mobile devices. If AMP-to-AMP linking is enabled, once users are on an AMP page, they will continue navigating your AMP content.', 'amp' );
const readerModeDescription = __( 'In Reader mode <b>your site will have a non-AMP and an AMP version</b>, and <b>each version will use its own theme</b>.', 'amp' ) + ' ' + mobileRedirectionNote;
const transitionalModeDescription = __( 'In Transitional mode <b>your site will have a non-AMP and an AMP version</b>, and <b>both will use the same theme</b>.', 'amp' ) + ' ' + mobileRedirectionNote;
const standardModeDescription = __( 'In Standard mode <b>your site will be completely AMP</b> (except in cases where you opt-out of AMP for specific parts of your site), and <b>it will use a single theme</b>.', 'amp' );
const readerModeDescription = createInterpolateElement(
__( 'In Reader mode <b>your site will have a non-AMP and an AMP version</b>, and <b>each version will use its own theme</b>.', 'amp' ) + ' ' + mobileRedirectionNote,
{
b: <b />,
},
);
const transitionalModeDescription = createInterpolateElement(
__( 'In Transitional mode <b>your site will have a non-AMP and an AMP version</b>, and <b>both will use the same theme</b>.', 'amp' ) + ' ' + mobileRedirectionNote,
{
b: <b />,
},
);
const standardModeDescription = createInterpolateElement(
__( 'In Standard mode <b>your site will be completely AMP</b> (except in cases where you opt-out of AMP for specific parts of your site), and <b>it will use a single theme</b>.', 'amp' ),
{
b: <b />,
},
);
const pluginIncompatibilityNote = __( 'To address plugin compatibility issue(s), you may need to use Plugin Suppression to disable incompatible plugins on AMP pages or else select an alternative AMP-compatible plugin.', 'amp' );
const readerNoteWhenThemeIssuesPresent = __( 'Recommended if you want to enable AMP on your site despite the detected compatibility issue(s).', 'amp' );
const transitionalNoteWhenThemeIssuesPresent = __( 'Recommended so you can progressively enable AMP on your site while still making the non-AMP version available to visitors for functionality that is not AMP-compatible. Choose this mode if compatibility issues can be fixed or if your theme degrades gracefully when JavaScript is disabled.', 'amp' );
Expand Down
61 changes: 35 additions & 26 deletions assets/src/onboarding-wizard/pages/done/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { SETTINGS_LINK } from 'amp-settings'; // From WP inline script.
/**
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { useContext, useEffect } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { createInterpolateElement, useContext, useEffect } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -198,30 +198,39 @@ export function Done() {
{ __( 'Need help?', 'amp' ) }
</h2>
<ul className="done__list">
{ /* dangerouslySetInnerHTML reason: Injection of a link. */ }
<li dangerouslySetInnerHTML={ {
__html: sprintf(
/* translators: placeholder is a link to support forum. */
__( 'Reach out in the <a href="%s" target="_blank" rel="noreferrer noopener">support forums</a>', 'amp' ),
'https://wordpress.org/support/plugin/amp/#new-topic-0',
),
} } />
{ /* dangerouslySetInnerHTML reason: Injection of a link. */ }
<li dangerouslySetInnerHTML={ {
__html: sprintf(
/* translators: placeholder is a link to the settings page. */
__( 'Try a different template mode <a href="%s" target="_blank" rel="noreferrer noopener">in settings</a>', 'amp' ),
SETTINGS_LINK,
),
} } />
{ /* dangerouslySetInnerHTML reason: Injection of a link. */ }
<li dangerouslySetInnerHTML={ {
__html: sprintf(
/* translators: placeholder is a link to the plugin site. */
__( '<a href="%s" target="_blank" rel="noreferrer noopener">Learn more</a> how the AMP plugin works', 'amp' ),
'https://amp-wp.org/documentation/how-the-plugin-works/',
),
} } />
<li>
{
createInterpolateElement(
__( 'Reach out in the <a>support forums</a>', 'amp' ),
{
// eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string.
a: <a href="https://wordpress.org/support/plugin/amp/#new-topic-0" target="_blank" rel="noreferrer noopener" />,
},
)
}
</li>
<li>
{
createInterpolateElement(
__( 'Try a different template mode <a>in settings</a>', 'amp' ),
{
// eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string.
a: <a href={ SETTINGS_LINK } target="_blank" rel="noreferrer noopener" />,
},
)
}
</li>
<li>
{
createInterpolateElement(
__( '<a>Learn more</a> how the AMP plugin works', 'amp' ),
{
// eslint-disable-next-line jsx-a11y/anchor-has-content -- Anchor has content defined in the translated string.
a: <a href="https://amp-wp.org/documentation/how-the-plugin-works/" target="_blank" rel="noreferrer noopener" />,
},
)
}
</li>
</ul>
</div>
</div>
Expand Down
Loading