i18n

[rreusser]

Opening this for a discussion of i18n, both for this and for the public plotly.js repo.

There are a few different ways to accomplish this. In semi-pseudocode, the i18n function fundamentally looks like:

function _( key ) {
  if (key in dictionary) {
    return dictionary[key]
  } else {
    return key
  }
}

The input is the string itself, which has pros and cons. The dictionary looks like:

{
  "es": {
    "Trace": "Rastro",
    "Click to toggle visibility": "Haga clic para activar la visibilidad",
    ...etc
  },
  "ru": {
    ...
  }
}

You'd use this like:

_("Trace")

The downside is that "Trace" (nominative) and "Trace" (accusative) have different translations in some languages. If this were the case, you could be more explicit and make nontrivial use of an english dictionary to use keys instead of the strings themselves.

{
  "en": {
    "Trace label in style panel": "Trace"
  },
  "es": {
    "Trace label in style panel": "Rastro",
    "Click to toggle visibility": "Haga clic para activar la visibilidad",
    ...etc
  },
  "ru": {
    ...
  }
}

_("Trace label in the style panel")

Or maybe panels.style.trace or whatever you prefer. It's a bit free-form.

At any rate, you'd pass in a dictionary which would get deep-merged into whatever default dictionaries plotly.js has built in. The fallback is builtin dictionary values or just a pass-through i18n function.

For plotly, the dictionary would be passed through config as, e.g. {"es": {...}} and the desired language would be specified as, e.g "language": "es". As in:

Plotly.plot(gd, {
  data: {...},
  config: {
    language: "es",
    dictionary: {
      es: {
        key: value
      }
    }
  }
})

(locale instead of language? dictionaries, plural? I at least like the idea of being able to pass multiple dictionaries in one parameter so that changing the language would be possible.)

For plotly.js it could also be possible to use Plotly.Register. As in:

Plotly.Register(require('plotly.js/dictionaries/en.json'))
Plotly.Register(require('plotly.js/dictionaries/es.json'))

Plotly.plot(gd, {config: {language: 'es'}})

For react components, the editor would probably receive a dictionary and expose it through context. All components would inherit from a base component that makes the dictionary available so that it's not global and so that the dictionary doesn't need to trickle down through props explicitly. Then instead of importing _, you'd inherit from the base component and have this._(...) automatically available.

/cc @bpostlethwaite @alexcjohnson @etpinard

[bpostlethwaite]

Inheriting from a none-base component has implications for testing, code sharing and readability. So does using context. Enzyme, the popular React testing framework requires every component requiring context to be explicitly declared in each test.

Translation is an edge-case as far as most development and maintenance efforts are concerned (aside from wrapping all strings in _). Inheriting from something aside from Component or requiring every component handle context adds a fixed and highly visible complexity cost for every component.

What about something like initializing _ with a dictionary during app setup? _.register(dictionary). And then nothing need change beyond importing _ and using it to wrap every string? This way the complexity is hidden within something unconnected to React.

[rreusser]

Thanks for the feedback! I agree wholeheartedly. Class inheritance is unequivocally bad and context is basically global variables. Unfortunately, I'm somewhat far down the path of using both due to the fact that we're using JSX directly to create the interface. That makes minimizing the wires that need to be hooked up to every field somewhat desirable. The one alternative would be trickling some central object all the way through (sorta like dispatch), though IMO it's basically equivalent as long a what is happening is stable and documented. For reference, The panel definition currently looks like this.

I initially did precisely _.register, except then you're replacing a context variable (which is confined to its context) with an actual global variable, which I think is perhaps less desirable. It would be possible to just pass it manually, though again, I don't necessarily see the benefit as long as what's going on is minimal, well-defined, and documented.

[rreusser]

I've been thinking about this, and I think maybe the right answer is somewhere between the two. I'm going to look at using some higher order components (à la redux connect()) in order to eliminate class inheritance and at the very least fully encapsulate the use of context.

[bpostlethwaite]

In the case of a dictionary is it worse to pass around a global singleton via requires? It's almost like a constant?

[rreusser]

I would tend to think that yes, it's worse. It's almost a constant, but that would make it for example, actually impossible to show two editors in two different languages on the same page. I can't come up with a good use case for why you'd want to do that (apart from comparing/testing/debugging), but the actual inability indicates to me unnecessary use of global state. For this editor (and for the connection to its plot, in particular), I've been operating under the assumption that global state is 100% prohibited.

[bpostlethwaite]

Yes true. The tradeoffs in a library are different from an end-user application. Ok, thanks for hearing out my concerns!

[rreusser]

Likewise! I really appreciate the feedback. I'm reworking some pieces a bit to make it a bit more predictable/maintainable. Instead of this:

class RangeInput extends BaseField { }

it'll be:

class RangeInput extends Component { }
export default connectEditorField(RangeInput)

That might do some context magic to expose the equivalent of the workspace plot and provide things like the value and fullValue via props, but I think it's just better design to do it this way.

Or even:

export default localize(connectEditorField(RangeInput))

(FWIW how do you think redux does it? 😛)

[bpostlethwaite]

Oh ya for sure and React Router. They are both a PITA to unit test. But whaddyagonnado

[rreusser]

Have had some talks with @bpostlethwaite and @alexcjohnson. Here are some takeaways:

React

• context is probably a reasonable way to encapsulate some details and set up translation. Encapsulating that with a higher order component (HOC) seems best. That ends up looking like:

  class Field extends React.Component {
    render () {
      return <div>{this.props._("Label")}</div>
    }
  }
  
  export default localize(Field)

  where localize returns a higher order component that fetches the dictionary and locale from top-level editor context and exposes that as props.

• Class inheritance has to go. So each field will receive plot context as a HOC as well. That makes it more straightforward to improvise when we encounter situations that don't fit well into the domain specific language (DSL) we're building for specifying the editor. The plot context should be rolled up as a single object (similar to workspacePlot except not global) to minimize context variables floating around.

I18n

• Dictionaries should be registered in plotly.js via Plotly.register(require('plotly.js/i18n/ro')) (i18n? locale? locales?). That makes them available to any use of that Plotly library instance and also greatly simplifies bundling plotly with additional languages.
• The locale is specified in plot config.
• In plotly.js, the call signature of i18n is _(gd, str) which is shorthand for the more verbose but equivalent _(gd._context.dictionaries, gd._context.locale, str).
• Static analysis will either use a regex or an AST to local all uses of i18n and extract the keys. That makes the following code invalid:

  var key = "Label"
  draw(_(gd, key))

  Instead, it must be:

  draw(_(gd, "Label"))

  so that it may be statically analyzed.
• A create-dictionary task will locate keys and create a new i18n file (in pig-latin maybe? 🐷 )
• A check-dictionary (assert-dictionary?) task will locate keys and make sure a given file has all the translations it needs
• Similar tasks will exist for the editor. Whether the developer needs to provide a dictionary for both the plot and the editor or one dictionary for both seems perhaps up to the developer. I think our job is to provide the tasks to make sure they've done it correctly.

[etpinard]

A few comments:

I at least like the idea of being able to pass multiple dictionaries in one parameter so that changing the language would be possible.)

So maybe language (I like locale by the way - which I think is more common) should be a layout attribute?

For plotly.js it could also be possible to use Plotly.Register. As in:

Great idea. I'd vote for putting the dictionaries in plotly.js/lib/ though along side the other require-able modules we expose for Plotly.register.

[rreusser]

Thanks for the feedback! locale it is. Would that look like Plotly.register('plotly.js/lib/locales/en')?

[etpinard]

I'd keep it flat Plotly.register('plotly.js/lib/locale-en')

[rreusser]

Ohh, lib, not src. Of course. Lots of files in a flat directory. Agreed.

[bpostlethwaite]

this is complete