From 788be67c75e67f5f69c46e2e645d06b39744a5f5 Mon Sep 17 00:00:00 2001 From: Stefan Parviainen Date: Tue, 14 Nov 2017 19:55:47 +0100 Subject: [PATCH] Clarifications --- .../views/messages/SenderProfile.js | 4 +++ src/languageHandler.js | 30 +++++++++++-------- test/i18n-test/languageHandler-test.js | 6 ++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/components/views/messages/SenderProfile.js b/src/components/views/messages/SenderProfile.js index d64c5fe651..2c557bebe2 100644 --- a/src/components/views/messages/SenderProfile.js +++ b/src/components/views/messages/SenderProfile.js @@ -52,6 +52,10 @@ export default function SenderProfile(props) { return (
+ // The text surrounding the user name must be wrapped in order for it to have the correct opacity. + // It is not possible to wrap the whole thing, because the user name might contain flair which should + // be shown at full opacity. Sadly CSS does not make it possible to "reset" opacity so we have to do it + // in parts like this. Sometimes CSS makes me a sad panda :-( { content.props.children[0] ? { content.props.children[0] } : '' } diff --git a/src/languageHandler.js b/src/languageHandler.js index 272b0a4848..d6660be283 100644 --- a/src/languageHandler.js +++ b/src/languageHandler.js @@ -36,7 +36,7 @@ export function _td(s) { } // Wrapper for counterpart's translation function so that it handles nulls and undefineds properly -//Takes the same arguments as counterpart.translate() +// Takes the same arguments as counterpart.translate() function safe_counterpart_translate(...args) { // Horrible hack to avoid https://github.com/vector-im/riot-web/issues/4191 // The interpolation library that counterpart uses does not support undefined/null @@ -66,14 +66,20 @@ function safe_counterpart_translate(...args) { * @param {object} variables Variable substitutions, e.g { foo: 'bar' } * @param {object} tags Tag substitutions e.g. { 'a': (sub) => {sub} } * - * The values to substitute with can be either simple strings, or functions that return the value to use in - * the substitution (e.g. return a React component). In case of a tag replacement, the function receives as - * the argument the text inside the element corresponding to the tag. + * In both variables and tags, the values to substitute with can be either simple strings, React components, + * or functions that return the value to use in the substitution (e.g. return a React component). In case of + * a tag replacement, the function receives as the argument the text inside the element corresponding to the tag. + * + * Use tag substitutions if you need to translate text between tags (e.g. "Click here!"), otherwise + * you will end up with literal "" in your output, rather than HTML. Note that you can also use variable + * substitution to insert React components, but you can't use it to translate text between tags. * * @return a React component if any non-strings were used in substitutions, otherwise a string */ export function _t(text, variables, tags) { - // Don't do subsitutions in counterpart. We hanle it ourselves so we can replace with React components + // Don't do subsitutions in counterpart. We handle it ourselves so we can replace with React components + // However, still pass the variables to counterpart so that it can choose the correct plural if count is given + // It is enough to pass the count variable, but in the future counterpart might make use of other information too const args = Object.assign({ interpolate: false }, variables); // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution) @@ -123,15 +129,18 @@ export function substitute(text, variables, tags) { export function replaceByRegexes(text, mapping) { const output = [text]; - let wrap = false; // Remember if the output needs to be wrapped later + // If we insert any components we need to wrap the output in a span. React doesn't like just an array of components. + let shouldWrapInSpan = false; + for (const regexpString in mapping) { + // TODO: Cache regexps const regexp = new RegExp(regexpString); // convert the last element in 'output' into 3 elements (pre-text, sub function, post-text). // Rinse and repeat for other patterns (using post-text). const inputText = output.pop(); const match = inputText.match(regexp); - if(!match) { + if (!match) { output.push(inputText); // Push back input continue; // Missing matches is entirely possible, because translation might change things } @@ -160,7 +169,7 @@ export function replaceByRegexes(text, mapping) { } if(typeof replaced === 'object') { - wrap = true; + shouldWrapInSpan = true; } const tail = inputText.substr(match.index + match[0].length); @@ -169,10 +178,7 @@ export function replaceByRegexes(text, mapping) { } } - if(wrap) { - // this is a bit of a fudge to avoid the 'Each child in an array or iterator - // should have a unique "key" prop' error: we explicitly pass the generated - // nodes into React.createElement as children of a . + if(shouldWrapInSpan) { return React.createElement('span', null, ...output); } else { return output.join(''); diff --git a/test/i18n-test/languageHandler-test.js b/test/i18n-test/languageHandler-test.js index bdaa431e0a..9c08916235 100644 --- a/test/i18n-test/languageHandler-test.js +++ b/test/i18n-test/languageHandler-test.js @@ -54,6 +54,12 @@ describe('languageHandler', function() { .toEqual((You are now ignoring foo)); }); + it('variable substitution with plain React component', function() { + const text = 'You are now ignoring %(userId)s'; + expect(languageHandler._t(text, { userId: foo })) + .toEqual((You are now ignoring foo)); + }); + it('tag substitution with React component', function() { const text = 'Press to start a chat with someone'; expect(languageHandler._t(text, {}, { 'StartChatButton': () => foo }))