synacor / preact-i18n

Simple localization for Preact.
BSD 3-Clause "New" or "Revised" License
206 stars 18 forks source link

MarkupText only renders first child when using fallback #20

Closed nephix closed 6 years ago

nephix commented 6 years ago

Steps:

  1. Go to https://codesandbox.io/s/1zzm1n6npq
  2. Open en.json and locate news.title property
  3. See the correct output in result window: <span>This <em>should work</em></span>
  4. Change news.title in index.js to news.whatever

Expected Result:

The default text <span>This <b>This doesnt work</b></span> is rendered in the result window

Actual Result:

<span>This </span> is rendered in the result window

azizhk commented 6 years ago

This is because the following code seems to pick only the first child. https://github.com/synacor/preact-i18n/blob/0287d0f1a5eabc050a475cee160ecedd7463145b/src/components/text.js#L44

billneff79 commented 6 years ago

That is likely the issue. I would be happy to review a PR that changes it to just fallback = children and added test cases for a fallback with markup in it.

nephix commented 6 years ago

I changed it to fallback = children and implemented a test case but the issue remained. Not sure what the actual issue is. Can someone else with more experience have a look at it?

azizhk commented 6 years ago

Sorry try changing https://github.com/synacor/preact-i18n/blob/8a43fc770a93aba605af33f7424cebe12053dc82/src/lib/translate-mapping.js#L31

Your working example: https://codesandbox.io/s/31rq37xnn5

nephix commented 6 years ago

Hey @azizhk, thank you. Although that fixes the problem, it lets a bunch of other test cases fail.

nephix commented 6 years ago

Is there a chance of this getting fixed and released any time soon? It's currently blocking us from rolling out our app in another location.

pl12133 commented 6 years ago

@nephix Surely, I would be happy to review a PR with the fix recommended by @azizhk.

nephix commented 6 years ago

@pl12133 sure, but the fix brings a whole lot of other issues when you run the tests that I'm not able to fix.

azizhk commented 6 years ago

Created a PR https://github.com/synacor/preact-i18n/pull/21

billneff79 commented 6 years ago

This is resolved in preact-i18n@1.2.1

gavmck commented 4 years ago

Same issue with the basic <Text> field caused by the same line of code by the looks of it