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

React i18next #8406

Merged
merged 29 commits into from
Nov 28, 2022
Merged

React i18next #8406

merged 29 commits into from
Nov 28, 2022

Conversation

pettinarip
Copy link
Member

@pettinarip pettinarip commented Oct 29, 2022

Description

Implement react-i18next by using the next plugin https://github.com/microapps/gatsby-plugin-react-i18next

Known issues:

  • Custom 404 pages are not working on the GC env. Still under investigation but I don’t think it is a blocker. Its working fine on Netlify env.

Related Issue

#8345

@github-actions github-actions bot added content 🖋️ This involves copy additions or edits dependencies 📦 Changes related to project dependencies review needed 👀 tooling 🔧 Changes related to tooling of the project labels Oct 29, 2022
@gatsby-cloud
Copy link

gatsby-cloud bot commented Oct 29, 2022

✅ ethereum-org-website-dev deploy preview ready

@pettinarip pettinarip changed the title [WIP] React i18next React i18next Nov 2, 2022
}

return <Layout {...props}>{element}</Layout>
}
Copy link
Member Author

Choose a reason for hiding this comment

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

gatsby-browser & gatsby-ssr content was replaced by the usage of gatsby-plugin-layout.

dangerouslySetInnerHTML={{
__html: t("page-staking-hierarchy-solo-p1"),
}}
/>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a temporary implementation. In the next iteration we are planning in using the following approach: https://react.i18next.com/latest/trans-component#using-with-react-components where we will also need to modify our translation files.

@@ -494,7 +478,16 @@ export const listImage = graphql`
`

export const query = graphql`
query WalletsPage {
query WalletsPage($language: String!) {
locales: allLocale(filter: { language: { eq: $language } }) {
Copy link
Member Author

Choose a reason for hiding this comment

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

On each page, we need to fetch the selected language strings.

@pettinarip pettinarip marked this pull request as ready for review November 4, 2022 13:49
@minimalsm
Copy link
Contributor

When reviewing I noticed a bug with our link styling for links embedded as HTML in JSON files - #8547.

@wackerow
Copy link
Member

Noticed our intl tutorials aren't loading properly
image

@pettinarip
Copy link
Member Author

Noticed our intl tutorials aren't loading properly

Uhmm good catch @wackerow! I'm looking into it now. Thanks

@minimalsm
Copy link
Contributor

minimalsm commented Nov 17, 2022

Noticed this on the other PR, but it's also happening here.

If a string is not available in a language, I would expect it to fall back to English strings. This is what happens in this PR:

Screenshot 2022-11-17 at 19 01 16

English

Chinese - A language with translated JSON strings

Slovenian - A language without translated JSON files

@pettinarip
Copy link
Member Author

Noticed this on the other PR, but it's also happening here.

If a string is not available in a language, I would expect it to fall back to English strings. This is what happens in this PR:

Good catch @minimalsm I thought I had that covered! will take a look, thanks!

@pettinarip pettinarip marked this pull request as draft November 22, 2022 13:34
@pettinarip pettinarip marked this pull request as ready for review November 23, 2022 15:47
i18n lib change - use htmr to parse html inside json files #8585
Copy link
Member

@corwintines corwintines left a comment

Choose a reason for hiding this comment

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

LGTM, nice one

Noting, we need to update /docs/best-practices.md

Copy link
Contributor

@minimalsm minimalsm left a comment

Choose a reason for hiding this comment

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

LGTM :)

@pettinarip pettinarip merged commit 930cb24 into dev Nov 28, 2022
@pettinarip pettinarip deleted the react-i18next branch November 28, 2022 13:33
@pettinarip
Copy link
Member Author

@corwintines I'll create a new PR with those updates 👍🏼 thanks for pointing that out.

@corwintines corwintines mentioned this pull request Nov 28, 2022
@pettinarip pettinarip mentioned this pull request Nov 29, 2022
@pettinarip pettinarip restored the react-i18next branch December 8, 2022 21:54
@pettinarip pettinarip mentioned this pull request Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits dependencies 📦 Changes related to project dependencies tooling 🔧 Changes related to tooling of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants