[errai-dev] Pull Request: i18n support

[Eric Wittmann]

Nice work, Jonathan.  Thoughts inline...

On 04/05/2013 06:38 PM, Jonathan Fuerth wrote:
> 1. I renamed the attribute data-i18n-skip=true to data-role=dummy, and
> added tests

+1 - awesome

> a. Eric suggested a heuristic for omitting certain elements from the
> auto-generated translation bundle: if an element has a data-field
> attribute, we should disregard even if its contents are text only. In
> GroceryList, this heuristic was correct everywhere except <button>
> elements. A workaround (not super-awesome, but it does work) is:
>
>    <button data-field="fooButton"><span>Button Label</span></button>
>
> Better suggestions?

As long as there is an i18n attribute that takes precedence (e.g. 
data-i18n-key="blah") I don't see why we couldn't exclude certain 
elements from the heuristic.

That said, once Errai supports id and css based @DataField, the 
heuristic won't work as well.

> c. What about eliminating the @Bundle annotation? We could search for
> bundles either in the same package as the @Templated class, or in the
> same package as the HTML template it points to.

I'm all for this.  The @Bundle annotation is really more of a holdover 
from when I was trying to implement this as a decorator.

> d. Long bits of text with a little bit of markup in the millde are
> problematic. Consider this example:
> <snip>
> And then allow HTML markup in translation values? We could treat them as
> SafeHtml...

If you add 'data-i18n-key="my-custom-key"' to the <p> element of that 
single thought, then the entire thing will get translated - mixed 
content or no.  In addition, I'm setting the innerHtml, so any mixed 
content in the translation will work.  Sort of the opposite of your 
suggestion of a data-inline attribute.

> e. I think we should automatically consider placeholder and title
> attributes as translatable. Perhaps there's a case for allowing end
> users to override this default in ErraiApp.properties or in an attribute
> of the @Bundle annotation if we keep it. I'm mostly thinking of custom
> data-* attributes as a motivation for this.

I believe placeholder and title attributes are already being picked up 
as translatable.

> f. It would be kind of us to include a reusable language chooser widget
> with well-documented CSS classnames. Seems like everyone is going to
> need one of these.

+1

> g. The default strings are already in the templates themselves. Is it
> really actually useful to restate them in a default messages file?

+1 - the default translation file is especially less useful if the 
@Bundle annotation goes away (which I think it should).

> This widget (or TranslationService.setLocale()) should also
> take care of translating the existing live DOM contents to the new language.

+1 - I hadn't considered live-updating the DOM with the new language. 
But there's certainly nothing stopping this.

> i. Bug: the i18n key scanner picked up "Declarative is Good" as a keys
> in the following:
>
>      <h3><i class="icon-thumbs-up"></i>Declarative is Good</h3>
>
> But the i18n translator did not apply the translation from the bundle.

I suspect this is a jtidy issue.  I believe jtidy will do all kinds of 
shennanigans with the HTML.  Perhaps there are some flags that will 
minimize that.

> j. I *love* the _missing JSON file that appears under .errai. Brilliant!

:)