Internationalization

[joshmn]

wip

[afogel]

this is 🔥

[afogel]

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

[afogel]

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

[afogel]

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

[afogel]

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

[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 <#28 (comment)>: > @@ -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 <#28 (comment)>: > @@ -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 <#28 (comment)>: > (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 <#28 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPPFEG7VQHJSK73OVFOS2TXXBSS3ANCNFSM6AAAAAA33VB7E4> . You are receiving this because you authored the thread.Message ID: ***@***.***>