-
Notifications
You must be signed in to change notification settings - Fork 291
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.
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
Fix missing translations #1227
Fix missing translations #1227
Changes from 6 commits
6fa7a85
7f92afb
33c6f66
8696d4c
8fe1c26
21c84f1
41de4ee
f40b189
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,8 +144,10 @@ class SetupModule extends Component { | |
> | ||
{ | ||
! blockedByParentModule ? | ||
/* translators: %s: module name */ | ||
sprintf( __( 'Set up %s', 'google-site-kit' ), name ) : | ||
sprintf( __( 'Setup Analytics to gain access to %s', 'google-site-kit' ), name ) | ||
/* translators: %s: module name */ | ||
sprintf( __( 'Set up Analytics to gain access to %s', 'google-site-kit' ), name ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed from "Setup Analytics" to "Set up Analytics" for consistency, but also because it is the correct form to use here. This caused VRT to fail, hence updates for those as well. |
||
} | ||
</Link> | ||
</p> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1102,7 +1102,8 @@ class AnalyticsSetup extends Component { | |
} | ||
|
||
{ !! existingTag && | ||
<p>{ sprintf( __( 'An existing analytics tag was found on your site with the id %s. If later on you decide to replace this tag, Site Kit can place the new tag for you. Make sure you remove the old tag first.', 'google-site-kit' ), existingTag ) }</p> | ||
/* translators: %s: Analytics tag ID */ | ||
<p>{ sprintf( __( 'An existing analytics tag was found on your site with the ID %s. If later on you decide to replace this tag, Site Kit can place the new tag for you. Make sure you remove the old tag first.', 'google-site-kit' ), existingTag ) }</p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Capitalized "ID" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I doubt that this translator comment is going to be picked up. It should be inside the brackets, preceding either <p>{
sprintf(
/* translators: %s: Analytics tag ID */
__( 'An existing analytics tag was found on your site with the ID %s. If later on you decide to replace this tag, Site Kit can place the new tag for you. Make sure you remove the old tag first.', 'google-site-kit' ),
existingTag
)
}</p> |
||
} | ||
|
||
{ this.renderErrorOrNotice() } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,12 +45,18 @@ class SearchConsoleDashboardWidgetSiteStats extends Component { | |
|
||
setOptions() { | ||
const { selectedStats, series, vAxes } = this.props; | ||
const { pageTitle } = global.googlesitekit; | ||
|
||
const pageTitle = global.googlesitekit.pageTitle && global.googlesitekit.pageTitle.length ? sprintf( __( 'Search Traffic Summary for %s', 'google-site-kit' ), decodeHtmlEntity( global.googlesitekit.pageTitle ) ) : __( 'Search Traffic Summary', 'google-site-kit' ); | ||
let title = __( 'Search Traffic Summary', 'google-site-kit' ); | ||
|
||
if ( pageTitle && pageTitle.length ) { | ||
/* translators: %s: page title */ | ||
title = sprintf( __( 'Search Traffic Summary for %s', 'google-site-kit' ), decodeHtmlEntity( pageTitle ) ); | ||
} | ||
Comment on lines
+50
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ESLint rules weren't properly detecting the translators string in a multiline ternary so I split it up this way, which is probably cleaner anyway 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense 👍 With the ternary it wouldn't be clear which |
||
|
||
const options = { | ||
chart: { | ||
title: pageTitle, | ||
title, | ||
}, | ||
curveType: 'line', | ||
height: 270, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect because strings can't be properly translated when concatenated. This also renders oddly in some cases, e.g. in Español where the translation is missing the space at the end so the text and the link are running together.