Re: [errai-dev] Pull Request: i18n support
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!
:)