Merge remote-tracking branch 'origin/develop' into develop

src/languageHandler.js

@@ -126,6 +126,8 @@ export function substitute(text, variables, tags) {
  * @return a React <span> component if any non-strings were used in substitutions, otherwise a string
  */
 export function replaceByRegexes(text, mapping) {
+    // We initially store our output as an array of strings and objects (e.g. React components).
+    // This will then be converted to a string or a <span> at the end
     const output = [text];
 
     // If we insert any components we need to wrap the output in a span. React doesn't like just an array of components.
@@ -135,53 +137,64 @@ export function replaceByRegexes(text, mapping) {
         // TODO: Cache regexps
         const regexp = new RegExp(regexpString);
 
-        // convert the last element in 'output' into 3 elements (pre-text, sub function, post-text).
+        // Loop over what output we have so far and perform replacements
-        // Rinse and repeat for other patterns (using post-text).
+        // We look for matches: if we find one, we get three parts: everything before the match, the replaced part,
-        const inputText = output.pop();
+        // and everything after the match. Insert all three into the output. We need to do this because we can insert objects.
-        const match = inputText.match(regexp);
+        // Otherwise there would be no need for the splitting and we could do simple replcement.
-        if (!match) {
+        for (const outputIndex in output) {
-            output.push(inputText); // Push back input
+            const inputText = output[outputIndex];
-
+            if (typeof inputText !== 'string') { // We might have inserted objects earlier, don't try to replace them
-            // Missing matches is entirely possible because you might choose to show some variables only in the case
+                continue;
-            // of e.g. plurals. It's still a bit suspicious, and could be due to an error, so log it.
-            // However, not showing count is so common that it's not worth logging. And other commonly unused variables
-            // here, if there are any.
-            if (regexpString !== '%\\(count\\)s') {
-                console.log(`Could not find ${regexp} in ${inputText}`);
             }
-            continue;
-        }
-        const capturedGroups = match.slice(2);
 
-        // Return the raw translation before the *match* followed by the return value of sub() followed
+            const match = inputText.match(regexp);
-        // by the raw translation after the *match* (not captured group).
+            if (!match) {
+                // Missing matches is entirely possible because you might choose to show some variables only in the case
+                // of e.g. plurals. It's still a bit suspicious, and could be due to an error, so log it.
+                // However, not showing count is so common that it's not worth logging. And other commonly unused variables
+                // here, if there are any.
+                if (regexpString !== '%\\(count\\)s') {
+                    console.log(`Could not find ${regexp} in ${inputText}`);
+                }
+                continue;
+            }
 
-        const head = inputText.substr(0, match.index);
+            const capturedGroups = match.slice(2);
-        if (head !== '') { // Don't push empty nodes, they are of no use
-            output.push(head);
-        }
 
-        let replaced;
+            // The textual part before the match
-        // If substitution is a function, call it
+            const head = inputText.substr(0, match.index);
-        if (mapping[regexpString] instanceof Function) {
-            replaced = mapping[regexpString].apply(null, capturedGroups);
-        } else {
-            replaced = mapping[regexpString];
-        }
 
-        // Here we also need to check that it actually is a string before comparing against one
+            // The textual part after the match
-        // The head and tail are always strings
+            const tail = inputText.substr(match.index + match[0].length);
-        if (typeof replaced !== 'string' || replaced !== '') {
-            output.push(replaced);
-        }
 
-        if (typeof replaced === 'object') {
+            let replaced;
-            shouldWrapInSpan = true;
+            // If substitution is a function, call it
-        }
+            if (mapping[regexpString] instanceof Function) {
+                replaced = mapping[regexpString].apply(null, capturedGroups);
+            } else {
+                replaced = mapping[regexpString];
+            }
 
-        const tail = inputText.substr(match.index + match[0].length);
+            if (typeof replaced === 'object') {
-        if (tail !== '') {
+                shouldWrapInSpan = true;
-            output.push(tail);
+            }
+
+            output.splice(outputIndex, 1); // Remove old element
+
+            // Insert in reverse order as splice does insert-before and this way we get the final order correct
+            if (tail !== '') {
+                output.splice(outputIndex, 0, tail);
+            }
+
+            // Here we also need to check that it actually is a string before comparing against one
+            // The head and tail are always strings
+            if (typeof replaced !== 'string' || replaced !== '') {
+                output.splice(outputIndex, 0, replaced);
+            }
+
+            if (head !== '') { // Don't push empty nodes, they are of no use
+                output.splice(outputIndex, 0, head);
+            }
         }
     }
 

test/i18n-test/languageHandler-test.js

@@ -65,4 +65,9 @@ describe('languageHandler', function() {
         expect(languageHandler._t(text, {}, { 'StartChatButton': () => <i>foo</i> }))
             .toEqual(<span>Press <i>foo</i> to start a chat with someone</span>);
     });
+
+    it('replacements in the wrong order', function() {
+        const text = '%(var1)s %(var2)s';
+        expect(languageHandler._t(text, { var2: 'val2', var1: 'val1' })).toBe('val1 val2');
+    });
 });