ElixirTeSS / TeSS

Training e-Support Service using Ruby on Rails.
Other
12 stars 12 forks source link

Language of Event feature. #995

Closed cwant closed 1 month ago

cwant commented 1 month ago

Summary of changes

This adds a "Language of Instruction" feature to events, as per #994 The languages supported are the 24 official languages of the EU: https://european-union.europa.eu/principles-countries-history/languages_en

Motivation and context

We (Canada) need this feature for the bilingual instance of TeSS we are working on, so thought it might be better to push this upstream too.

We'll see how this goes, and then add a similar feature for learning materials in the future.

Checklist

cwant commented 1 month ago

Oops, looks like I have config.i18n.raise_on_missing_translations = true turned off here. Will have to either rework the failing test or do something else with the helper function.

fbacall commented 1 month ago

Thanks for this! Not had a chance to have a detailed review yet, but I was wondering did you consider using a gem to provide the list of language codes/names? e.g. https://github.com/xwmx/iso-639

cwant commented 1 month ago

I had not considered getting it from a gem. Some thoughts on that specific gem:

cwant commented 1 month ago

Thanks for the review!

lang-menu

fbacall commented 1 month ago
  • I've flip-flopped on this decision, but I still keep a dictionary to handle the allowed language names (a list with hundreds of names isn't great UX). I can change this if need be. Another option I considered is listing some codes in tess.yml as being the allowable ones. The default I went with is the 24 official EU languages.

I think I like the idea of them being listed in an array in tess.yml, that way the order that the languages appear can also be changed. We can provide a default of the 24 EU languages in tess.example.yml.

  • I'm not super happy with some of the design of I18nData. They go with iso-639-1 (two letter codes instead of three), but for some reason they decided to upper case the codes (e,g., 'EN' instead of 'en'). I decided to go with the lower case version in the standard, but this will be a deviation from the language field in the Trainer model. So yeah, this isn't great. What direction would you prefer to go? (consistency with the other table, or consistency with the standard?)

I agree it's not very nice, although the spec says they are case-insensitive. I think we should go with lower case. We can just down-case the keys from the trainers table when they are read to be consistent (or write a migration to convert the stored data).

  • I capitalize language names in the form/facets, although this might be a deviation from what is done in some locales
  • Some of the language names from I18nData aren't great. Most noticeable is the Greek one

How about we list the EU language names under a language key in en.yml, then when rendering the language name it checks there first and falls back to the I18nData name if it is missing?

cwant commented 1 month ago

This all sounds reasonable. I've integrated all of those suggestions in the latest push.