Internationalization

[joshmn]

wip

[afogel]

this is :fire:

[joshmn]

have at it! it's why i assigned this to you :)

fwiw: i don't have any i18n experience; i don't know best practices, conventions, etc. most of the setup is stolen from ben as mentioned in a comment.

On Fri, Aug 25, 2023 at 4:00 AM Ariel @.***> wrote:

@.**** requested changes on this pull request.

Overall looks great. one small thing to update and a few small questions about organizing the custom i18n config module.

In app/models/ahoy_captain/i18n_config.rb https://github.com/joshmn/ahoy_captain/pull/28#discussion_r1305387487:

@@ -0,0 +1,23 @@ +module AhoyCaptain

• class I18nConfig < ::I18n::Config
• BACKEND = I18n::Backend::Simple.new
• AVAILABLE_LOCALES = AhoyCaptain::Engine.root.join("config/locales/ahoy_captain").glob("*.yml").map { |path| File.basename(path, ".yml").to_sym }.uniq
• AVAILABLE_LOCALES_SET = AVAILABLE_LOCALES.inject(Set.new) { |set, locale| set << locale.to_s << locale.to_sym }
• def backend
• BACKEND
• end
• def available_locales

Why do we have two methods, available_locales and available_locales_set? Why not consolidate into a single available_locales method?

In app/models/ahoy_captain/i18n_config.rb https://github.com/joshmn/ahoy_captain/pull/28#discussion_r1305389399:

@@ -0,0 +1,23 @@ +module AhoyCaptain

• class I18nConfig < ::I18n::Config
• BACKEND = I18n::Backend::Simple.new
• AVAILABLE_LOCALES = AhoyCaptain::Engine.root.join("config/locales/ahoy_captain").glob("*.yml").map { |path| File.basename(path, ".yml").to_sym }.uniq

Maybe this is logic we want to add to the initialize method rather than keeping in a constant?

In app/components/ahoy_captain/comparison_link_component.rb https://github.com/joshmn/ahoy_captain/pull/28#discussion_r1305391141:

   (link_to "Custom period", "javascript:customComparisonModal.showModal()", class: selected(:custom)),

• (link_to "Year-over-year", AhoyCaptain::Engine.routes.url_helpers.root_path(**helpers.search_params.merge(comparison: :year)), class: selected(:year)),

if you're already updating, change hardcoded strings to an i18n lookup, e.g., "Year-over-year" -> ".year-over-year"

— Reply to this email directly, view it on GitHub https://github.com/joshmn/ahoy_captain/pull/28#pullrequestreview-1595256799, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPPFEG7VQHJSK73OVFOS2TXXBSS3ANCNFSM6AAAAAA33VB7E4 . You are receiving this because you authored the thread.Message ID: @.***>