-
-
Notifications
You must be signed in to change notification settings - Fork 11.4k
🌐 Merged fragment translation keys in Portal notifications #27175
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
Changes from all commits
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 |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import React from 'react'; | ||
| import Interpolate from '@doist/react-interpolate'; | ||
| import Frame from './frame'; | ||
| import AppContext from '../app-context'; | ||
| import NotificationStyle from './notification.styles'; | ||
|
|
@@ -40,19 +41,34 @@ const NotificationText = ({type, status, context}) => { | |
| } else if (type === 'signin' && status === 'error') { | ||
| return ( | ||
| <p> | ||
| {t('Could not sign in. Login link expired.')} <br /><a href={signinPortalLink} target="_parent">{t('Click here to retry')}</a> | ||
| <Interpolate | ||
| mapping={{ | ||
| a: <a href={signinPortalLink} target="_parent" /> | ||
| }} | ||
| string={t('Could not sign in. Login link expired. <br /><a>Click here to retry</a>')} | ||
| /> | ||
| </p> | ||
| ); | ||
| } else if (type === 'signup' && status === 'success') { | ||
| return ( | ||
| <p> | ||
| {t('You\'ve successfully subscribed to')} <br /><strong>{context.site.title}</strong> | ||
| <Interpolate | ||
| mapping={{ | ||
| strong: <strong /> | ||
| }} | ||
| string={t('You\'ve successfully subscribed to <br /><strong>{siteTitle}</strong>', {siteTitle: context.site.title})} | ||
|
Member
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. ok, this breaks my heuristic that a br means a new string... 😆 Is having the br necessary here? Will it survive a variety of languages (which could be adjusted by translators) and site title lengths (which cannot? Would it be better to let the browser make the decision about when to break the line? And does the layout break if a translator removes it? I'm imagining that the zh and zh-Hant translators would probably remove it, for example. |
||
| /> | ||
| </p> | ||
| ); | ||
| } else if (type === 'signup-paid' && status === 'success') { | ||
| return ( | ||
| <p> | ||
| {t('You\'ve successfully subscribed to')} <br /><strong>{context.site.title}</strong> | ||
| <Interpolate | ||
| mapping={{ | ||
| strong: <strong /> | ||
| }} | ||
| string={t('You\'ve successfully subscribed to <br /><strong>{siteTitle}</strong>', {siteTitle: context.site.title})} | ||
| /> | ||
| </p> | ||
| ); | ||
| } else if (type === 'updateEmail' && status === 'success') { | ||
|
|
@@ -70,13 +86,23 @@ const NotificationText = ({type, status, context}) => { | |
| } else if (type === 'signup' && status === 'error') { | ||
| return ( | ||
| <p> | ||
| {t('Signup error: Invalid link')}<br /><a href={singupPortalLink} target="_parent">{t('Click here to retry')}</a> | ||
| <Interpolate | ||
| mapping={{ | ||
| a: <a href={singupPortalLink} target="_parent" /> | ||
| }} | ||
| string={t('Signup error: Invalid link.<br /><a>Click here to retry</a>')} | ||
| /> | ||
| </p> | ||
| ); | ||
| } else if (type === 'signup-paid' && status === 'error') { | ||
| return ( | ||
| <p> | ||
| {t('Signup error: Invalid link')}<br /><a href={singupPortalLink} target="_parent">{t('Click here to retry')}</a> | ||
| <Interpolate | ||
| mapping={{ | ||
| a: <a href={singupPortalLink} target="_parent" /> | ||
| }} | ||
| string={t('Signup error: Invalid link.<br /><a>Click here to retry</a>')} | ||
| /> | ||
| </p> | ||
| ); | ||
| } else if (type === 'stripe:checkout' && status === 'success') { | ||
|
|
||
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.
To play devil's advocate, the fact that there's a br might really suggest that these are two separate ideas that can be safely translated as separate strings. And then we'd keep the enforced common phrasing for "Click here to retry", which really does seem to be a separate idea.
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.
Looking at this with fresh eyes and I think you're right, this would revert this PR and make it a no-op so I'll close this PR
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.
@rob-ghost - I think it's perhaps worth keeping "You've successfully subscribed to {siteTitle}". That's an improvement :)