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

feat: add lazy load to i18n #952

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

gminetoma
Copy link
Contributor

@gminetoma gminetoma commented Dec 24, 2024

Resolves #889

🔧 What changed

We no longer need the file './i18n.options.ts' to configure i18n.

The configuration is now handled directly in the Nuxt config file.

We are loading the last language used via cookies; therefore, we need to disable the language selector and display a loading indicator for the user until the loading process is complete.

The browser's language code uses "-", so we need to apply a small adjustment to align with our backend typing, which uses "_".

📑 References

Nuxt i18n Lazy-load translations
Nuxt Vue I18n API

📸 Screenshots

  • Before

Recording.2024-12-24.211500.mp4
  • After

Recording.2024-12-24.210736.mp4

@gminetoma gminetoma added enhancement New feature or request i18n localization labels Dec 24, 2024
@gminetoma gminetoma linked an issue Dec 24, 2024 that may be closed by this pull request
3 tasks
Copy link

netlify bot commented Dec 24, 2024

Deploy Preview for findadoc ready!

Name Link
🔨 Latest commit 89c9b84
🔍 Latest deploy log https://app.netlify.com/sites/findadoc/deploys/677e14e17fbde300083fa1cb
😎 Deploy Preview https://deploy-preview-952--findadoc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@gminetoma gminetoma marked this pull request as ready for review December 24, 2024 12:23
@gminetoma gminetoma force-pushed the feat/lazy-load-localization branch 2 times, most recently from ff3647f to fe58df6 Compare December 24, 2024 12:46
Copy link
Contributor

@ermish ermish left a comment

Choose a reason for hiding this comment

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

nice work @gminetoma ! good find for lazy loading them and thinking of the loading state ✨


// getLocaleCookie to set the initial locale, if there is no cookie, it will default based on the nuxt.config.js
// HACK: Browser's code language is using "-" instead of "_".
const localeCookie = getLocaleCookie() ? getLocaleCookie().replace('-', '_') : Locale.EnUs
Copy link
Contributor

Choose a reason for hiding this comment

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

if having the dash in there is hacky, could we just remove it? I don't think we technically need it, it was just there to try and match a standard. This would simplify the code and remove the hack 😸

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry @ermish, we cannot remove it because our location is based on the backend's data type 'Locale'. The backend uses '_' while the browser and Nuxt use '-' for locale. We are saving the cookie in the browser's format and then parsing it to our format. We could fix this by changing the server's data type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I think we should change the servers type but let's get democracy in here! @Anissa3005 @theyokohamalife @NabbeunNabi cast ye's opinion 🏴‍☠️

Copy link
Contributor

Choose a reason for hiding this comment

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

I think changing the server's type would also be better as an overall choice. Makes sense to me to match the browser, since most modern browsers use IETF BCP 47 language tags.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in favor of changing it in the backend. @gminetoma it would be a good idea to include a link to the Nuxt docs in your PR description so reviewers can reference it easily.

i18n/index.ts Show resolved Hide resolved
We no longer need the file './i18n.options.ts' to configure i18n.

The configuration is now handled directly in the Nuxt config file.

We are loading the last language used via cookies; therefore, we need to
disable the language selector and display a loading indicator for the user
until the loading process is complete.

The browser's language code uses "-", so we need to apply a small
adjustment to align with our backend typing, which uses "_".

Co-authored-by: Will <[email protected]>
@gminetoma gminetoma force-pushed the feat/lazy-load-localization branch from fe58df6 to 3b7c9f1 Compare January 7, 2025 06:53
@gminetoma gminetoma requested a review from ermish January 7, 2025 06:57
@ermish ermish marked this pull request as draft January 13, 2025 11:47
@ermish
Copy link
Contributor

ermish commented Jan 13, 2025

@gminetoma moving this to Draft for now until the backend is updated 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FEAT: Lazy load localization
4 participants