Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Implement __experimentalCreateInterpolateElement for translations. #1736

Merged
merged 5 commits into from
Feb 17, 2020
Merged

Implement __experimentalCreateInterpolateElement for translations. #1736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd51286
Improved placeholders of translatable string
claudiosanches Oct 14, 2019
8d453c3
Improved one more string
claudiosanches Oct 14, 2019
64b841e
Include wordpress element package
mikejolley Feb 14, 2020
5624837
Implement __experimentalCreateInterpolateElement for translations
mikejolley Feb 14, 2020
8502c4d
Dump fragments
mikejolley Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 42 additions & 37 deletions assets/js/blocks/reviews/edit-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
/**
* External dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { Fragment, RawHTML } from '@wordpress/element';
import { escapeHTML } from '@wordpress/escape-html';
import { __ } from '@wordpress/i18n';
import { __experimentalCreateInterpolateElement } from 'wordpress-element';
import {
Notice,
ToggleControl,
Expand Down Expand Up @@ -36,7 +35,7 @@ export const getBlockControls = ( editMode, setAttributes ) => (

export const getSharedReviewContentControls = ( attributes, setAttributes ) => {
return (
<Fragment>
<>
<ToggleControl
label={ __( 'Product rating', 'woo-gutenberg-products-block' ) }
checked={ attributes.showReviewRating }
Expand All @@ -51,21 +50,24 @@ export const getSharedReviewContentControls = ( attributes, setAttributes ) => {
className="wc-block-reviews__notice"
isDismissible={ false }
>
<RawHTML>
{ sprintf(
escapeHTML(
/* translators: A notice that links to WooCommerce settings. */
__(
'Product rating is disabled in your %sstore settings%s.',
'woo-gutenberg-products-block'
)
{ __experimentalCreateInterpolateElement(
__(
'Product rating is disabled in your <a>store settings</a>.',
'woo-gutenberg-products-block'
),
{
a: (
// eslint-disable-next-line jsx-a11y/anchor-has-content
<a
href={ getAdminLink(
'admin.php?page=wc-settings&tab=products'
) }
target="_blank"
rel="noopener noreferrer"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use both noopener and noreferrer here (and in other places in this pull), could we just use noreferrer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misread the linting rule and thought both were required. Referer could be useful (?) so Ill use noopener.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm actually the rule does enforce both. eslint(react/jsx-no-target-blank) You cannot just use one or the other...

Copy link
Contributor

@nerrad nerrad Feb 17, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Interesting, from this it appears this targets both because of older browser support (but in this context we probably want noreferrer anyways). I can't think of any reason offhand why doing both creates a problem, so let's just keep it as is for now.

Copy link
Contributor

@nerrad nerrad Feb 17, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's an issue that got started around this rule enforcing both :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference: a similar discussion happened in Storefront repo. In the end we ended up using rel="noreferrer". We don't have this ESLint rule so it was possible to use only one in that case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the links. I think we're okay using both to keep eslint happy until that issue is fixed. There doesn't appear to be consequences in doing so.

/>
),
`<a href="${ getAdminLink(
'admin.php?page=wc-settings&tab=products'
) }" target="_blank">`,
'</a>'
) }
</RawHTML>
}
) }
</Notice>
) }
<ToggleControl
Expand Down Expand Up @@ -105,7 +107,7 @@ export const getSharedReviewContentControls = ( attributes, setAttributes ) => {
}
/>
{ attributes.showReviewImage && (
<Fragment>
<>
<ToggleButtonControl
label={ __(
'Review image',
Expand Down Expand Up @@ -137,26 +139,29 @@ export const getSharedReviewContentControls = ( attributes, setAttributes ) => {
className="wc-block-reviews__notice"
isDismissible={ false }
>
<RawHTML>
{ sprintf(
escapeHTML(
/* translators: A notice that links to WordPress settings. */
__(
'Reviewer photo is disabled in your %ssite settings%s.',
'woo-gutenberg-products-block'
)
{ __experimentalCreateInterpolateElement(
__(
'Reviewer photo is disabled in your <a>site settings</a>.',
'woo-gutenberg-products-block'
),
{
a: (
// eslint-disable-next-line jsx-a11y/anchor-has-content
<a
href={ getAdminLink(
'options-discussion.php'
) }
target="_blank"
rel="noopener noreferrer"
/>
),
`<a href="${ getAdminLink(
'options-discussion.php'
) }" target="_blank">`,
'</a>'
) }
</RawHTML>
}
) }
</Notice>
) }
</Fragment>
</>
) }
</Fragment>
</>
);
};

Expand All @@ -165,7 +170,7 @@ export const getSharedReviewListControls = ( attributes, setAttributes ) => {
const maxPerPage = 20;

return (
<Fragment>
<>
<ToggleControl
label={ __( 'Order by', 'woo-gutenberg-products-block' ) }
checked={ attributes.showOrderby }
Expand Down Expand Up @@ -219,6 +224,6 @@ export const getSharedReviewListControls = ( attributes, setAttributes ) => {
min={ minPerPage }
/>
) }
</Fragment>
</>
);
};
22 changes: 22 additions & 0 deletions package-lock.json
View file
Open in desktop

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

3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@
"react-number-format": "4.3.1",
"trim-html": "0.1.9",
"use-debounce": "3.3.0",
"wordpress-components": "npm:@wordpress/components@8.5.0"
"wordpress-components": "npm:@wordpress/components@8.5.0",
"wordpress-element": "npm:@wordpress/element@2.11.0"
},
"husky": {
"hooks": {
Expand Down