reactioncommerce / example-storefront

Example Storefront is Reaction Commerce’s headless ecommerce storefront - Next.js, GraphQL, React. Built using Apollo Client and the commerce-focused React UI components provided in the Storefront Component Library (reactioncommerce/reaction-component-library). It connects with Reaction backend with the GraphQL API.
https://www.mailchimp.com/developer/open-commerce
Apache License 2.0
604 stars 288 forks source link

i18n support #524

Closed janus-reith closed 2 years ago

janus-reith commented 5 years ago

I added i18next support to the starterkit by wiring up https://github.com/isaachinman/next-i18next.

Initially I started with german. The strings are located as static json files in the starterkit, the translations from the backend are not reused (Wonder if there would be an efficient way to do that / If it makes sense).

Do you have a roadmap on i18n support in the starterkit, or would you accept PR`s?

delagroove commented 5 years ago

@janus-reith if you send a PR can you tag me? i can work on a Spanish translation of it based on this work

janus-reith commented 5 years ago

@delagroove Sure, although I'm still waiting for some kind of response from the team...

loan-laux commented 5 years ago

I'm highly in favor of this. Would be great to get feedback from the core team!

HarisSpahija commented 4 years ago

Bumping this. With upcomming Hacktoberfest. Maybe this would be a good issue to solve for storefront?

HarisSpahija commented 4 years ago

@janus-reith is there a chance you could share your implementation?

janus-reith commented 4 years ago

Sure, I would just need to find the time for that and could create a repo. Some other module updates were involved so I need to verify what is actually relevant here first.

Actually I think we could really improve this now that https://github.com/reactioncommerce/reaction/pull/5514 is merged and see if we can pull the translated strings from the backend again like it used to be with the v1.x versions.

Currently I am bundling them as static resources in the storefront code.

HarisSpahija commented 4 years ago

@janus-reith Let's collaborate on this? We currently implemented next-i18next in our repository, maybe we can help?

nnnnat commented 4 years ago

Hey y'all wanted to chime in on this topic. We do not have any plans to include i18n support to the example storefront but we would be very open to merging in a community built solution. I'll defer to @aldeed on the approaches outlined above but I don't see pulling translations from the API being way to do this and having storefront translations local (maybe via next-i18next?) as the better solution.

aldeed commented 4 years ago

@nnnnat The API translations are available, but the general thinking was that clients would probably host their own translation files and if necessary would merge from both sources. There are some benefits to API-side translations, like not having to keep in sync in multiple places if you have multiple clients. It would be fine for now for storefront to have its own files.

As far as how the client code works and how it determines the language, it should follow the pattern that is in the latest Reaction Admin release. Determine lang based on the header sent by the browser, with ability to override with lng query string.

HarisSpahija commented 4 years ago

@aldeed Even though it sounds like a cool feature, we recognized in our store front that remote i18 locales hosting can be a big pain. In general you would always want to have a fallback in case the connection drops.

On your second point, i18next has a HOC that keeps track of its own selected language. Do you think it would be good enough to let next-18next check its own language, or should we add it to the current global state managment?

Also, should we immediately add translations by extracting strings from all components/pages to a locales file?

aldeed commented 4 years ago

@HarisSpahija I18next has a feature where you can compile remote translations into your static page, which is one way to have fallback. But all translations are loaded on startup in a typical setup, so it's likely not much of an issue.

Reaction Admin uses default detection, but limited to only "querystring" and "navigator" methods to avoid too much confusion. That's my recommendation for this project, too. See https://github.com/reactioncommerce/reaction/blob/master/client/modules/i18n/startup.js#L53-L62 (Pretty much all i18n init is in that file.)

Reaction Admin uses i18next.language everywhere, or uses i18next.t, which automatically uses i18next.language. I recommend that rather than global state since it's the standard way. There is no need for reactive updates based on language changing unless you are going to add a dropdown language selector and want to do it without refreshing the page, but I'm not sure that's necessary. A dropdown could just set lng in query string and refresh the whole page.

If it's determined that we really need reactive language state, it could be dual-stored into global state with some code like this in the init file:

i18next.on("languageChanged", (lng) => {
  // set lng global state
});

I would avoid the HOC because we're moving everything toward classless components and React Hooks.

Immediately moving all text out of components and into locales file would be great. I know there are some places where the text is coded into the components in https://github.com/reactioncommerce/reaction-component-library. Those components should be updated to take string props that override that text. There is a separate issue about this here: https://github.com/reactioncommerce/reaction-component-library/issues/409#issuecomment-483776520

HarisSpahija commented 4 years ago

I think its good to split this issue into multiple tasks for translating the pages and adding i18next support. Once those 2 things are merged we can include community translations.

I wont have time this week to do this, but I will start working on it as soon as I have time!

janus-reith commented 4 years ago

The UIStore already has a field language: https://github.com/reactioncommerce/example-storefront/blob/4e1c281ec5de541ec6b22c52c38e6e2e6e072a1c/src/lib/stores/UIStore.js#L32

I think currently it is only used for the order status labels which can be localized already. I remember facing issues when I tried to initialize that field with the value of i18.language as it was undefined initially, I need to recheck how I actually solved it in the end.

Regarding the Dropdown and changing the language without a page reload, that's exactly how I approached this.

About the translation availability: For some error messages this might be helpful, but for a running store I don't see cases where the shop would be usable and the backend therefore available but the translations where not. Maybe need to see if nextjs is caching the locales on its server between requests or always calls the Reaction Backend.

Should we create a separate plugin like storefront-i18n that contains the translations? It probably would get messy if we split namespaces between the plugins again.

HarisSpahija commented 4 years ago

@janus-reith Its a nice usecase to enable languages from the backend (like what is available in the storefront), but have the languages being switchable in the frontend. Like @aldeed said, having a route set the language is a nice solution

aldeed commented 4 years ago

@janus-reith I forgot about that UIStore language. I believe @kieckhafer added that with the goal of eventually supporting this I18N stuff.

You could use i18next init/changeLanguage to manage the current language, but have an onLanguageChanged listener in which you set the UIStore language to keep them in sync.

janus-reith commented 4 years ago

@aldeed thanks for the link, I never noticed these events exist. Probably the cleanest way then to set the language in the UI Store, will try that out.

HarisSpahijaPon commented 4 years ago

Recently I have been working on adding translation overrides for the component library. How are we going to pass translations to these components? Originate them from the example storefront code or from the reaction api?

aldeed commented 4 years ago

@HarisSpahijaPon I'd lean toward starting with separate files in example storefront. The general thought is that companies with multiple different clients will prefer to maintain common translations in one place (the API that they all connect to anyway). But I think the easier path to get started would be to have them local to this project, and then from there potentially identify some that should be moved to an API plugin as a future optimization.

Also, there is an updated i18n article in the docs if it's helpful: https://docs.reactioncommerce.com/docs/next/internationalization#supporting-multiple-languages-in-a-reaction-client Feel free to submit a PR to further clarify the article as you're doing this.

janus-reith commented 4 years ago

@HarisSpahijaPon Now that your PRs on the component library allow us to pass the labels/funcs, let's create a new branch that adds nexti18next as a dependency and intializes i18n with the locales from the static folder as discussed. Maybe really just settle for browser detection of the language and url subpaths first to avoid the need to decide yet how to visually display a language toggle in the storefront?( Not sure how specific the requirements would be laid out here.) Then we need to figure out how the current next routes would play along with the i18n specific paths, I did not use them in my implementation yet, maybe it will work out easily.

HarisSpahijaPon commented 4 years ago

Basically we have 4 steps that we could follow to implement i18n.

1. Check URL subpath: test.com/en/productGrid and test.com/fr/productGrid
2. Check browser language: https://github.com/i18next/i18next-browser-languageDetector ?
3. Check cookies preference, we already make use of jscookie. Why not reuse this?
4. Toggle button (would require a new component to component library)

Do we know already if there will be a connection with the backend in the future?

@janus-reith, feel free to hit me up on https://twitter.com/spahija_haris to set up a call so we can work this out.

HarisSpahijaPon commented 4 years ago

For people that current want to translate their example storefront. There is a current alternative way that @aldeed provided here: https://github.com/reactioncommerce/reaction-component-library/issues/409#issuecomment-564131174

Quote:

@HarisSpahija I didn't test this, but it should work as long as you provide a component (e.g. a function that returns JSX).

{
  MiniCartSummary: (props) => <MiniCartSummary checkoutButtonText={i18next.t("MiniCartSummary.checkoutButtonText")} footerMessageText={i18next.t("MiniCartSummary.footerMessageText")} {...props} />
}

You'd have to make a slightly more complicated component that uses useTranslation hook to get a properly reactive version of i18next.t function, but this is the general idea.

HarisSpahijaPon commented 4 years ago

Hey guys, best wishes for this year!

Do you guys have a goal set for 3.0 on how to tackle i18n @aldeed ? I was planning on adding i18next and added a translation file for every component. From there on we can always create a common.json translation file for every shared translation

aldeed commented 4 years ago

@nnnnat Do you have any thoughts or guidelines developed?

The translations in the 3.0 API are primarily for Reaction Admin UI, though a storefront could request them and merge with its own translations as needed. So I think this storefront would be free to do whatever pattern makes sense. Since it does have its own server, I think remote loading the same way Reaction Admin does would be best. NextJS can probably statically server the JSON files. This will keep the client bundle small.

HarisSpahijaPon commented 4 years ago

While working on the Migration to v4 issue #618 I noticed that MaterialUI has i18n support. Maybe worth while to look into this?

https://medium.com/material-ui/material-ui-v4-is-out-4b7587d1e701

https://material-ui.com/guides/localization/

aldeed commented 4 years ago

MUI localization should be used, but it seems it would be supplemental to i18next rather than in place of it. You'd have to determine the MUI locale based on the i18next config and then pass it to the theme? Not sure exactly.

HarisSpahijaPon commented 4 years ago

Oh yes but ofcourse, they should be parallel to each other. The MUI locale has a remote i18n config but you can pass your own i18n. It is mostly used for translations on the components themselves such as helper texts (for example on <TablePagination />) The main feature of MUI i10l that is relevant to us is for Typography. Adding Arabic right to left will support a big potential group of users.

I do think we should target the current users of example-storefront first by introducing basic i18next features, maybe keep the MUI l10n for #618 ?

I havent been able to make any progress due to upgrading to 3.0 and would love to have some help with this guiding me in the right direction. @nnnnat Do you have any thoughts or guidelines developed?

HarisSpahijaPon commented 4 years ago

Hey guys, an update on this. Currently this is how you can implement next-i18next in your environment i18n is implemented trough the next-i18next library that can be found here:
https://github.com/isaachinman/next-i18next

i18n consists out of 3 components:

Config

The configuration for next-18next is located in src/i18n.js. i18n.js

const NextI18Next = require("next-i18next").default;

const languages = ["nl"]; // For us we only have Dutch as a language. You can change/add this to any language. For example "Pirate"

const options = {
  otherLanguages: ["en"],
  defaultLanguage: "en",
  localePath: typeof window !== "undefined" ? "static/locales" : "src/static/locales",
  fallbackLng: "en"
};

const NextI18NextInstance = new NextI18Next(options);

NextI18NextInstance.i18n.languages = languages;

module.exports = NextI18NextInstance;

Component HOC

next.i18next can be set by creating a HOC or Experimental Decorator withTranslation . In _app.js the next.i18next HOC can be defined like so. Using the i18next instance from src/i18n.js.

Now i18next can be set on every component where translation is needed like so.

import React, { Component } from "react";
import { withStyles } from "@material-ui/core/styles";
import PropTypes from "prop-types";
import { withTranslation } from "../../../i18n";

const styles = () => ({

});

@withTranslation("ExampleNameSpace")
@withStyles(styles, { name: "ExampleComponent" })
class ExampleComponent extends Component {
  static propTypes = {
    classes: PropTypes.object,
    t: PropTypes.func.isRequired
  };

  render() {
    const { t } = this.props;
    return (
      <div>
        {t("")} // <----- Translation added here with the t func from props
      </div>
    );
  }
}

ExampleComponent.getInitialProps = async () => ({
  namespacesRequired: ["ExampleNameSpace"]
});

export default ExampleComponent;

Make sure to use the @withTranslation(“{namespacehere}”) is used when adding translations. You can also add multiple namespaces by adding more namespaces in @withTranslation(). For example with common: @withTranslation(“common”, “ExampleNameSpace”). Remember that there are no duplicate string names inside the JSON files from i18next allowed. If you do have duplicated make sure to only import 1 and add the name space in a different way (using getInitialProps)

With Multiple languages:

import React, { Component } from "react";
import { withStyles } from "@material-ui/core/styles";
import PropTypes from "prop-types";
import { withTranslation } from "../../../i18n";

const styles = () => ({

});

@withTranslation("ExampleNameSpace")
@withStyles(styles, { name: "ExampleComponent" })
class ExampleComponent extends Component {
  static propTypes = {
    classes: PropTypes.object,
    t: PropTypes.func.isRequired
  };

  render() {
    const { t } = this.props;
    return (
      <div>
        {t("Hello")} // Returns "Hello"
        {t("ExampleNameSpace2:Hello")} // Returns "Good Day!"
      </div>
    );
  }
}

ExampleComponent.getInitialProps = async () => ({
  namespacesRequired: ["ExampleNameSpace", "ExampleNameSpace2"]
});

export default ExampleComponent;

// ExampleNameSpace
// {
//  "Hello": "Hello"
// }

// ExampleNameSpace2
// {
//  "Hello": "Good day!"
// }

I created a snippet available for Reaction i18next translated component. Add this to your user snippets in vscode

{
    // Created by Haris Spahija
    // Github: harisspahija@gmail.com
    // ce: haris.spahija@hybrit.org
    "Reaction Class Component Translated": {
        "scope": "javascript,typescript",
        "prefix": "reactionct",
        "body": [
            "import React, { Component } from 'react';",
            "import { withStyles } from '@material-ui/core/styles';",
            "import PropTypes from 'prop-types';",
            "import { withTranslation } from '../../../i18n';",
            "", 
            "const styles = () => ({",
            "", 
            "});",
            "", 
            "@withTranslation('$1')",
            "@withStyles(styles, { name: '$TM_FILENAME_BASE' })",
            "class $TM_FILENAME_BASE extends Component {",
            "\tstatic propTypes = {",
            "\t\tclasses: PropTypes.object,",
            "\t\tt: PropTypes.func.isRequired",
            "\t};",
            "",
            "\trender() {",
            "\tconst { t } = this.props;",
            "\t\treturn (",
            "\t\t\t<div>",
            "\t\t\t\t{t('$2')}",
            "\t\t\t</div>",
            "\t\t);",
            "\t}",
            "}",
            "",
            "$TM_FILENAME_BASE.getInitialProps = async () => ({",
            "\tnamespacesRequired: ['$1']",
            "});",
            "",
            "export default $TM_FILENAME_BASE"      
        ],
        "description": "Creates a Reaction component with HOC withStyles and Translations"
    }
}

Static Translation Files By default, next-i18next expects your translations to be organised as such:

.
└── public
    └── static
        └── locales
            ├── en
            |   └── common.json
            └── de
                └── common.json

This structure can also be seen in the simple example.

If you want to structure your translations/namespaces in a custom way, you will need to pass modified localePath and localeStructure values into the initialisation config.

If translations are not found in config.localePath or public/static/locales an attempt will be made to find the locales in static/locales, if found a deprecation warning will be logged.

I hope this helps for people still trying to implement i18n! Also if there are people willing to help implement this to the current example-storefront and test it. Would be really appreciated!

CristianCucunuba commented 4 years ago

We implemented next-i18next the same way you did and it's working as well, but we have a warning that says,we haven't declared the namespacesRequired specially when the component uses multiple HOC's. We use @withTranslation always as the first HOC, but the warning keeps displaying inside the other HOC's. Have you seen this problem?

ps. Thanks for the snippet

HarisSpahija commented 4 years ago

@CristianCucunuba I think for debugging your @withTranslation you can best go and post an issue on the next-i18next github repo.

HarisSpahija commented 4 years ago

@aldeed I think it will be a good idea to add a basic implementation of the english language and then merge it into example-storefront. After that we can allow individual pull requests for multiple languages.

It would be an amazing help to add some labels to this. If anyone wants to make a PR with the implementation of my snippet would be great!