Thanks @jujes. This is a missing feature and one that someone in the community could (relatively) quickly tackle. I'll label appropriately.

@tangollama I don't have a good sense of the changes required to implement this, but I'm pretty keen to learn. I have a fair amount of questions before getting started though.

@richiethomas
Hopefully these links will be helpful, although I am unsure
https://eviltrout.com/2013/11/24/i18n-in-ember.html
https://github.com/jamesarosen/ember-i18n

@tchan yeah looks like that's currently a dependency of the project. I'll definitely need to read through the package documentation as part of playing this story.

@tangollama since there's no design spec to work off, is there a certain location where the language toggle should go? Or should I just come up with something and submit it for consideration? My first thought was adding a select element to the dropdown that appears when clicking the "Settings" gear.

Thanks @tangollama. Follow-up question- in the past I've used an npm package called ember-power-select for use cases like this. It's powerful and well-maintained. I'm thinking about using it in the implementation of the language select dropdown, but since it would be a new dependency I wanted to run it by y'all first.

More info here- https://github.com/cibernox/ember-power-select

If you'd rather I just hard-code the languages for now so the dependency decision can be postponed, I could also do that.

@richiethomas I would prefer to not add another select component to the project. We currently use ember-rapid-forms which has a select component (em-select) and there also is a another component, ember-select-list, in the project.

@jkleinsc gotcha, wasn't aware of those other 2 dependencies. I'll investigate those instead.

@jkleinsc I'm running into problems with the navigation.js mixin. When I toggle the language in my dropdown menu, the change propogates to the i18n service, which causes all the relevant components to re-render except for the sidebar navigation. This is because the text values for these elements are populated by the localizedNavItems property. This bug occurs even when I add the i18n service as a dependency of the computed property (i.e. localizedNavItems: Ember.computed('i18n', ...)).

I don't see any authorization logic (i.e. userCanDoFoobar) inside this method, so I'm not sure what this method adds that we couldn't get from having the navItems key/value pairs in the translations files. Any light you could shed on this would be appreciated.

@richiethomas I think you need Ember.computed('i18n.locale', ....) instead of Ember.computed('i18n', ....).

As far as localizedNavItems is concerned, it is an artifact to the way the navigation was localized. I would be happy if we changed the navigation to define translation keys instead of text that gets automagically translated. There really isn't a good reason for it being the way it is right now.

@jkleinsc yep, i18n.locale did the trick! I now have a working version of the language toggle. Just need tests to support it. :-)

As for localizedNavItems, I'm happy to refactor this out as part of this story. Just to confirm, my understanding is that I should only need to move the navItems hash logic into the translations files, and then refactor the views to use the t helper method instead of this computed property. Does that sound correct to you as well?

If you're not sure off the top of your head, don't worry about it. I've committed the working version to my local and so I'm free to mess around until I get the refactoring right. :-)

@richiethomas That sounds correct to me. I would also verify that there isn't anything funky in the tests with those, but I would just make the change and then just verify if the tests are passing.

@jkleinsc cool, thanks. Going back to the i18n vs i18n.locale question for a second, this implies that Ember only watches for shallow changes to dependencies such as i18n, not deeply-nested changes. I assume this is a feature in Ember, not a bug, but in my mind the most intuitive implementation would be to watch for deep changes (for this exact case as well as similar cases). This implies that I'm missing some broader context that more seasoned devs like the Ember core team thought of. Do you have a sense of why shallow seems to be the preferred implementation here, or am I better off asking Stack Overflow? Let me know if I should rephrase my question.

@richiethomas good question. Yes, this is the way Ember works. I think (my own 2c) there is a good reason why: computed properties are cached and are only recalculated when the dependent keys change. If it went deep on an object every time anything changed on the object, that means that the computed property would be unnecessarily recalculated on every change, instead of just focusing on the changes that the computed property depended on. If you had a rather large object you could up needlessly recalculating endlessly.

Having said that, there is a way in Ember to work with aggregate data in computed properties:
https://guides.emberjs.com/v2.13.0/object-model/computed-properties-and-aggregate-data/

Awesome, that makes sense now. Thanks for that, and I'll take a look at the link.

@jkleinsc since the code for this feature has been solely on my machine for awhile, I'd like to try to get the initial version (before refactoring out the localizedNavItems method) up to a branch for a review (and possibly a merge?). I find it more manageable to get code reviews for smaller chunks of code in stages, and this way if you decide to merge it, the feature is available to users sooner. How does that sound? NB: I'm still writing tests for the feature but I'm making progress on them.

@richiethomas I wholeheartedly agree. Smaller PRs are much easier to review.

@jkleinsc PR posted here: #1109

Screenshots below:

Before clicking the "Settings" Gear:

Clicking the "Settings" gear shows the language select dropdown:

Selecting a language:

Language changed from English to French (note the change in sidebar and table header translations, but page title has not changed):

After page refresh, page title has changed too:

P.S.- ignore the <script>alert("Hi")</script> in the Patient Name column. I was doing some security testing. :-p

To address the original issue that prompted @jujes to report this- the locales/es/translations.js file is missing quite a few of the values required to have a first-class Spanish-language version of the app. I could probably go on Google Translate and fill in the missing blanks, but I don't think that would do the app or the language justice. Given the potential worst-case scenario of a mis-translation in a medical app such as this, a native Spanish speaker should probably take on that work instead. @jkleinsc what are your thoughts?

@richiethomas missing translations should be filed as separate issues so that we can solicit folks with good understanding of the language to be translated to work on them.

Resolved by #1109.

I know that the translation files need a lot to be added but I just found the missing translations for the dropdown to be a bit strange. Like so

I had a bit of time tonight so I'll try to contribute :)