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

(WIP) [www] Extract UI text strings to YAML files so they can be translated #20803

Closed
wants to merge 3 commits into from
Closed

Conversation

tesseralis
Copy link
Contributor

@tesseralis tesseralis commented Jan 23, 2020

Description

Extract (some) UI text to YAML so they can be translated.

Just a proof-of-concept with a few files translated. I'm still not sure what the best way to go about this is. See inline comments.

Localization Process

  1. Our script adds the directory www/src/data/strings as a source to be pulled in to gatsby-i18n-source.
  2. Translators translate the new YAML strings
  3. These translations are pulled in using gatsby-source-git

Related Issues

Fixes: #19102

@tesseralis tesseralis requested a review from LekoArts January 23, 2020 00:26
export default class MarkdownPageFooter extends React.Component {
constructor() {
super()
this.state = { feedbackSubmitted: false }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know what this was used for. Text searching the rest of www yielded no results, so I'm assuming it's something old or copy-pasta'd.

Copy link
Contributor

Choose a reason for hiding this comment

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

There used to be a "Was this doc helpful" element in the footer of each page, it was replaced by the hovering widget in the right bottom corner. So I'd say it's save to delete, yeah.

prevAndNext:
prevLabel: Previous
nextLabel: Next
searchForm:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't update search-form.js because it's a class component that's too detailed and I didn't feel like making a render props version of useLocalizedStrings for this WIP.

export default function useLocalizedStrings(component) {
const currentLocale = React.useContext(LocaleContext)

const { allStringsYaml } = useStaticQuery(graphql`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only way I've found to make this work so far, but I don't like it.

Usually, you just provide whatever i18n library you have with a set of key-value pairs, and it takes care of it. But since we're pulling in translations from different repos, they have to be sourced, which means the only way (that I know of) to reference the content is through a GraphQL query, which means listing all the individual keys and values.

This means whenever you add a new text string, you have to reference the key three times: once in strings/components.yml, once in this file, and once in the component itself. It's tedious and error prone.

It feels like you should be able to limit the query to whatever file you're working on, e.g.:

query($locale: String) {
  stringsYaml(filter: { locale: $locale }) {
    nodes {
      docsTableOfContents { title }
    }
  }
}

But we can't have variables in useStaticQuery, so no way to limit to the current locale.

Copy link
Contributor

@ascorbic ascorbic Jan 23, 2020

Choose a reason for hiding this comment

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

Are you able to use a flat yaml structure and source key/value pairs, rather than this structure scoped by component? e.g. a structure like this:

Locales:
    -
        locale: en_GB
        strings:
            - {key: SIDEBAR_LABEL_EXPAND, value: Expand}
            - {key: SIDEBAR_LABEL_COLLAPSE, value: Collapse}
    -
        locale: en_US
        strings:
            - {key: SIDEBAR_LABEL_EXPAND, value: Expand}
            - {key: SIDEBAR_LABEL_COLLAPSE, value: Collapse}

And then you can just query :

query {
      allStringsYaml {
        nodes {
          locale
          strings {
            key
            value
          }
        }
      }
}

...or something? Then change the hook to have an interface more like a conventional i18n library like

const {t} = useLocalizedStrings(currentLocale);
return <button>{t('SIDEBAR_LABEL_COLLAPSE')}</button>

`

Copy link
Contributor

Choose a reason for hiding this comment

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

Matt's suggestion is surely not as contextual "clean"/understandable as a structure like sorting it by components but I think it's long-term better.

I'd suggest writing up a guideline in what syntax to write those keys, like BEM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed that a flat file seems to be the most common way that i18n libraries do things and should be the way to go (though some do have support for namespaces!).

Turns out that the issue of "Maps in GraphQL" is a pretty controversial one (the example is also about i18n!) @TylerBarnes brought up the idea of getting the files using getNode in gatsby-node and passing that into page context. Which option would be better?

Copy link
Contributor

Choose a reason for hiding this comment

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

The nice thing about using page context is you wouldn't be loading extra data, though I'm not sure how much it matters in the context of this project (not sure how many translations there are). You could do a hybrid between that suggestion and @ascorbic's suggestion where you pass the relevant strings to page context and pass them into the hook:

const { localizationStrings } = data.pageContext
const { t } = useLocalizedStrings(currentLocale, localizationStrings);
return <button>{t('SIDEBAR_LABEL_COLLAPSE')}</button>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The actual pages are translated separate from the UI text strings, and in fact, are already accounted for! Yes, we're creating a new page for each language, so we get all that SSR built-in.

I'm thinking of using the pageContext approach too, and should have an updated PR using it today.

Copy link
Contributor Author

@tesseralis tesseralis Jan 25, 2020

Choose a reason for hiding this comment

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

I’m running into an issue where the page won't update after changing my content. E.g. I'll change "Table of Contents" to "Table of Elements" but the change won't be reflected in the doc unless I clear the cache. I think it might be related to #16477? If so that would put a damper on our plans to use pageContext.

Copy link
Contributor

@ascorbic ascorbic Jan 27, 2020

Choose a reason for hiding this comment

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

Great that you're way ahead of me on the handling of actual pages. Others will know better than me, but it certainly does sound like #16477 could be the issue. What you could do is rather than passing the whole lot in context, you just pass the locale name and then do the query in the pageQuery, filtering by locale. That would mean that the context wouldn't change, and changes to translations should be picked up in the page query.

One concern I do have though is that this whole approach is quite wasteful, as it's keeping all of the strings in the page data for every page, so you need to load it again whenever you navigate. I wonder if it's better to go the other way, and rather than passing the specific locale to the page, you load all of the locales and put them in a context provider in a wrapRootElement function. Your custom hook could then use that root context, which has all of the locales, and which would be preserved during page navigations (I think: is that right?). Even though the initial load is larger because of it holding all of the locales, this should be more than made up for by the fact it doesn't need to load them in every page data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering that as well. Does wrapRootElement have access to page/static queries?

Another thing I was thinking of was putting it in wrapPageElement through gatsby-plugin-layout. Since that function has access to location, it can detect the locale from the path and get the right strings from that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it has access to page queries, but static queries should be fine if it wraps it in a component that has the static query

@tesseralis
Copy link
Contributor Author

closing in favor of #21024

@tesseralis tesseralis closed this Jan 30, 2020
@tesseralis tesseralis deleted the ui-text branch January 30, 2020 18:21
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.

Translate UI text
4 participants