ember-intl upgrade side effects

[jackcmeyer]

Expected behavior:
Warning message should have no HTML in it.
Actual behavior:
Warning message has HTML in it.

Steps to reproduce:
New Inventory Item Add

1. Login
2. Click on Inventory Received
3. Click on add button
4. View the message with HTML in it

OR
New Patient Quick Add

1. Login
2. Click on Medication -> New Request
3. Enter patient name that doesn't exist
4. Fill in other information
5. View error message

Screenshots (if applicable):
New Inventory Item Add

New Patient Quick Add

OS and Browser:
MacOS, Chrome 70, Firefox, Safari

• warning messages which use HTML to render are showing the raw html in the message
• unable to select first lanaguage
• navigation bar items not being translated
• warnings with Hebrew in the console and some Hebrew messages not rendering
• default to English for missing translations
• different results between missing translations and empty translations
• loss of support of right-to-left languages

[MatthewDorner]

That's interesting, I'm not reproducing it. It just appears in a bold font. And I used Chromium 70 in Ubuntu. Also if you check the text in translations.js, it has <br> tags that aren't appearing in your screenshot, although it also isn't rendering line breaks at those points.

[jackcmeyer]

@MatthewDorner
here it is in Firefox

and in Safari...

[jackcmeyer]

I was poking around in the translations files...

Could it be related to the recent work on translations? After pulling in the most recent changes, I ran npm install. Not sure if it's related or not.

[MatthewDorner]

I just realized my previous comment was using code before #1553. Just tried with the most recent code and now I'm reproducing, so yeah that must be it.

[jackcmeyer]

I just went back to commit ee9bcc0 and was able to render the messages with the proper styling.

[jackcmeyer]

Looks like this might be able to fix it:
https://www.npmjs.com/package/ember-intl#format-html-message

[MatthewDorner]

There appear to be a few other side-effects of the change to ember-intl:

• language select control doesn't work the same as before, not able to select the very top language (chinese? not sure)
• navigation bar items not being translated
• some warnings appear on npm start, but I tested the appointments.messages.appointmentSaved and login.messages.signIn one and they are still being displayed

[jackcmeyer]

I did a quick test to fix the New Inventory Item message issue using htmlSafe=true. And it appears to work.

Here is the change in app/inventory/quick-add/template.hbs

  <div class="alert alert-warning" role="alert">{{t 'inventory.messages.itemNotFound' item=model.name htmlSafe=true}}</div>

[jackcmeyer]

Looks like the warning messages in the console have to do with the 8th language in the list. I was not able to get the appointments.messages.appointmentSaved to work.

The warning message has an Israeli flag and has he as a code, so I think the language is Hebrew

[MatthewDorner]

Also we have lost the "default to English for missing translation" behavior, and we're getting different behavior if the translation key itself is missing from the JSON vs. if it is just a empty string, although that could be fixed with a script. Russian has a bunch of empty string translations but Thailand has a lot of missing ones.

[MatthewDorner]

Well none of these things individually seem like they will be too hard to fix. If you'd like to work on it, I will be happy to help.

[jackcmeyer]

Sure, I'd like to work on it!

[jackcmeyer]

language select control doesn't work the same as before, not able to select the very top language (chinese? not sure)

I was able to select the top language and see the translations take place. Any special steps to reproduce this one?

[MatthewDorner]

Happens for me on Firefox and Chromium. I think it's because the action only happens when you change the selection. The actual difference is that before, the default selected value would be your current language, and now it's always defaulting to the first one instead:

I'll start looking at the navigation bar not being translated issue.

Edit: Since the language control is part of the navigation bar, this might just be happening for the same reason the nav bar items aren't being translated.

[jackcmeyer]

Gotcha. I'm looking at the HTML rendering one.

[chadian]

This is related to my implementation of ember-intl. Sorry for the bugs that were introduced. I can investigate and work on some of these issues, too.

• The htmlSafe as you've discovered will fix any of the translations relying on html, this slipped my mind.
• The old translations had null for missing translations, I changed these to empty string because a null value wasn't allowed by ember-intl
• I will have to double-check what is happening with the select, it might need to pick the default value from the service on init.

[chadian]

Missing Translations

To be consistent with missing translations we should probably:

1. Remove all empty strings and rely on the warnings of missing strings report at build time
2. Let missing translations fallback to the missing translations message util:

// app/utils/intl/missing-message.js:

export default function missingMessage(key, locales, options) {
  throw new Error(`[ember-intl] Missing translation for key: "${key}" for locales: "${locales}"`);
}

What do you think?

[jackcmeyer]

@chadian missing translations use to fall back to English. Is this still possible?

[MatthewDorner]

We added the fall back to English pretty recently, and the change was made here: #1458.

I'd prefer keeping it that way since the error message doesn't help those trying to use the software, and we will be missing translations for a while. We don't really need them to show up as warnings during application launch either, since there will be a ton of them.

[chadian]

Ah yeah, that makes sense. I was just digging through the ember-intl issues and there's a solution that landed 3 weeks ago:
ember-intl/ember-intl#584

Changing to:
return run(() => this.set('intl.locale', [selectedLanguage, 'en']));
in #setApplicationLanguage language-preference.js.

Can you confirm that's the behavior you expect? I've tested it with the Thai locale selected. We would still want to remove all empty strings from the translation files, too.

[MatthewDorner]

@chadian Just tested myself, looks good!

[jackcmeyer]

@MatthewDorner do you want us to open a PR for each task on this issue as we get them done?

[MatthewDorner]

Fewer PRs would be better, these don't look like they'll get too complicated individually.

[MatthewDorner]

However if that makes it more confusing to keep track of who's doing what where, you can make them individually, doesn't really matter to me.

[chadian]

I have a branch with a fix for:

• remove empty string translations
• setting a missing translation message to an empty string
• Adding the fallback locale

I can split them into separate PR's though.

[MatthewDorner]

Together is fine if they're already together.

[jackcmeyer]

I've got a branch for the htmlSafe and I'm trying to fix the hebrew translations

[chadian]

Also addressed as part of my #1562 pull request, related to the migration toember-intl:

• Fixed loss of support for right-to-left languages.
• Fixed navigation bar translations not being updated.

Not addressed by my pull request is that translated strings that are set via actions like setSectionHeader aren't bound to locale translation changes. I'm not sure if this bug existed before with ember-i18n since it's the same behavior but maybe it's something we should address in a new issue. I am wondering if maybe we should do a refresh of the route on locale change...

[chadian]

@MatthewDorner Looks like my PR closed the issue, sorry about that. Maybe we can reopen it for the remaining checklist @jackcmeyer has setup and track the remaining issues.

Missing translations

As a follow up to the question from @MatthewDorner about missing translations in these files I think we will need some sort of tooling. I was thinking we can use en.json as a source of truth and then maybe flatten the structure into something like:

flatten_en_reference.json

{
  "admin.address.address1Label": "Address 1 Label"
}

flatten_fr_missing.json

{
  "admin.address.address1Label": ""
}

Only missing from en.json would should up in the flattened. We would be able to run the reverse feed of the flatten_fr_missing.json back into the fr.json. If there are any tools that do something similar out of the box that would be nice, too.

I have a few friends that can help out with a few of the missing translations and have reached out already so if we could get a friendlier format we could fill in a bit of what's missing.

Urdu font change when switching to ember-intl
I am not sure why it changed, but maybe first we can verify if it's something that changes the meaning, context, readability, usability of the app? If it's okay we can just leave it, otherwise I can create an issue and investigate further.

[jackcmeyer]

@chadian and @MatthewDorner I've updated the task list to align with items that have been fixed and merged to master.

[chadian]

@jackcmeyer I think "unable to select first language" was fixed in my PR:
https://github.com/HospitalRun/hospitalrun-frontend/pull/1562/files#diff-4a152be7cf0c07e5f4bde6a37a912d69R56

(at least I hope so!)

[jackcmeyer]

@chadian sorry, must have missed this. I've also updated the list after my PR was merged. Last thing to do is the Hebrew translation warnings.

[MatthewDorner]

Yeah that one was fixed.

Hebrew translation can be a new issue, as can the issue where some text is not updated when you change the locale. I can make those.

You can compare the Urdu text before and after, I'm pretty confident it's not affecting readability, but I'd like to try a little to figure it out. Maybe not make an issue for it yet.

As far as missing translations scripts, I think the output from ember-intl where it lists all missing translations is good, but if you think something more advanced would be worth it we can re-open #1523 and work on it there.

[MatthewDorner]

I was trying to get hospitalrun-server working with a current frontend. So I built the frontend with ember build --environment=production, copied to prod folder, and pointed the frontend npm dependency in hospitalrun-server to my git branch with my new build, but I am getting this error when I try to access the app running with hospitalrun-server:

It's the same error whether or not I enable sourcemaps in ember-cli-build.js, though we've had them disabled so far. It works however if I do the build without --environment=production.

[MatthewDorner]

Regarding the above, I think I have it fixed by changing broccoli-serviceworker excludePaths setting in config/environment.js, will PR after testing a little more.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.