Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creates i18n infrastructure #40

Merged
merged 5 commits into from
Oct 12, 2023
Merged

Creates i18n infrastructure #40

merged 5 commits into from
Oct 12, 2023

Conversation

gbdubs
Copy link
Contributor

@gbdubs gbdubs commented Oct 12, 2023

  • Adds vue-i18n and nuxt-i18n, configured such that:
    • we'll have the locale in the url (/fr/admin) etc. This is easy to change if RMI wants to later.
    • we store the user's preference in a cookie, and that cookie works (at least locally) to deliver an SSR'ed i18n page.
    • we guess the user's location based on accept-lang and a few other heuristics, but honor a user's choice once made.
    • we offer english pages without prefix (/admin and /en/admin go to the same place)
    • the language files are lazy-loaded, so only current language gets pulled in by using VueI18nVitePlugin - you can see in the network tab when we switch langauges we fetch the lang/*.json?import file.
    • configures the fallback to be English, with the expectation that we won't actually write any of the other-language strings until the pre-launch stage of the project.
  • Creates a pattern that I hope we use everywhere for i18n.
    • uses objectNotation: true to allow Prefix.Suffix to be a nested property in JSON.
    • Groups language strings by file prefix - seems like an appropriate thing for now.
    • uses prefix with t in the template to create a helper tt to avoid having to write prefixes.
    • Generally uses the full string as the translation key, but uses a placeholder for longer strings.
    • Implements this pattern on the Nav + Footer + a few other places so you can see what it looks like.
  • Re-configures the footer to support a language selector. I decided on the footer over the header because as a I looked around and most functional websites put the selector in the footer, while most informational/one-time websites put it in the header.
  • I went with flags in the selector mostly for visibility/legibility. Without flags it's hard to draw the eye to the selector. I don't love it for a bunch of colonial reasons, but I think given how out-of-view it is after the first selection, I think it's probably a reasonable tradeoff.
  • Creates a one-time toast to alert folks to the fact that we support multiple languages.

@gbdubs gbdubs requested a review from bcspragu October 12, 2023 14:14
Copy link
Collaborator

@bcspragu bcspragu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, main general comment is that not all links have been updated [1], so you occassionally just get dumped back to the default language.

Is there a scheme (within vue/nuxt-i18n) we can use that's a little more bulletproof? It's easy to forget localePath when adding links/buttons/general nav

[1] /pages/admin/index.vue and /pages/admin/pacta-version/index.vue were the two I noticed

frontend/components/locale/Selector.vue Show resolved Hide resolved
frontend/lang/en.json Outdated Show resolved Hide resolved
frontend/lang/en.json Outdated Show resolved Hide resolved
frontend/layouts/default.vue Outdated Show resolved Hide resolved
@@ -19,6 +19,7 @@ interface PACTAError extends NuxtError {
}

export const createErrorWithRemediation = (err: string | Partial<NuxtError>, r: Remediation): PACTAError => {
console.log(err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove? This should get logged up the stack if it's thrown correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't showing up in logs, and it's proving challenging to debug stack traces without them.

frontend/nuxt.config.ts Outdated Show resolved Hide resolved
@@ -41,4 +44,24 @@ export default defineNuxtConfig({
],
dirs: ['globalimports'],
},
i18n: {
// TODO(grady) Set a Base URL once we have it for SEO https://i18n.nuxtjs.org/guide/seo#:~:text=You%20must%20also%20set%20the%20baseUrl%20option%20to%20your%20production%20domain%20in%20order%20to%20make%20alternate%20URLs%20fully%2Dqualified%3A
// baseUrl: 'https://my-nuxt-app.com'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we do have a base URL! In dev, it's https://pacta.dev.rmi.siliconally.dev. Seems like this should be populated from envs/ like other env-specific config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GC fixed

frontend/package.json Show resolved Hide resolved
@gbdubs
Copy link
Contributor Author

gbdubs commented Oct 12, 2023

Looks great, main general comment is that not all links have been updated [1], so you occassionally just get dumped back to the default language.

Is there a scheme (within vue/nuxt-i18n) we can use that's a little more bulletproof? It's easy to forget localePath when adding links/buttons/general nav

[1] /pages/admin/index.vue and /pages/admin/pacta-version/index.vue were the two I noticed

Great question + suggestion. I ended up creating some middleware that tests for this, and logs in the console when we fail to meet it. We can beef that up if/when we need to. That (in tandem with making the prefix consistent) will make these navigation errors a lot easier to spot and remediate, though I wish we could do something more compiley.

@gbdubs gbdubs enabled auto-merge (squash) October 12, 2023 22:26
@gbdubs gbdubs merged commit 4def329 into main Oct 12, 2023
2 checks passed
@gbdubs gbdubs mentioned this pull request Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants