This repository has been archived by the owner. It is now read-only.

[BACKUP] new I18n guide #2340

Closed
wants to merge 22 commits into
base: master
from

Conversation

@Foxandxss
Member

Foxandxss commented Sep 14, 2016

This supersedes #2309

Now superseded by #2491. Close it when that PR is merged.

@googlebot

This comment has been minimized.

Show comment
Hide comment
@googlebot

googlebot Sep 14, 2016

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot commented Sep 14, 2016

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@googlebot

This comment has been minimized.

Show comment
Hide comment
@googlebot

googlebot Sep 14, 2016

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot commented Sep 14, 2016

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@ocombe

This comment has been minimized.

Show comment
Hide comment
@ocombe

ocombe Sep 14, 2016

Collaborator

@googlebot I confirm (is this enough?)

Collaborator

ocombe commented Sep 14, 2016

@googlebot I confirm (is this enough?)

@vicb

This comment has been minimized.

Show comment
Hide comment
@vicb

vicb Sep 14, 2016

Contributor

@ocombe no, it will stay red forever when the PR author is not the same as the commit author

Contributor

vicb commented Sep 14, 2016

@ocombe no, it will stay red forever when the PR author is not the same as the commit author

@vicb

I think this is missing

  • translating attrs: i18n-<attr name>
  • translating sibling nodes
    • <!--i18n:m|d --> ... <!--/i18n --> or
    • <ng-container i18n>
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
@filipesilva

This comment has been minimized.

Show comment
Hide comment
@filipesilva

filipesilva Sep 24, 2016

Member

Heya @vicb, I'm picking up this chapter for now to get it ready for our testing setup and some more edits. I'm not too familiar with the i18n functionality aside from the chapter itself, so that's my starting point.

Member

filipesilva commented Sep 24, 2016

Heya @vicb, I'm picking up this chapter for now to get it ready for our testing setup and some more edits. I'm not too familiar with the i18n functionality aside from the chapter itself, so that's my starting point.

@vicb

This comment has been minimized.

Show comment
Hide comment
@vicb

vicb Sep 24, 2016

Contributor

No pb, added a bunch of comments ping me if you need more info.

We should make it clear that i18n is supported equally well for JiT and AoT.

Contributor

vicb commented Sep 24, 2016

No pb, added a bunch of comments ping me if you need more info.

We should make it clear that i18n is supported equally well for JiT and AoT.

@filipesilva

This comment has been minimized.

Show comment
Hide comment
@filipesilva

filipesilva Sep 26, 2016

Member

The guide is heavily revised, I know that @wardbell wants to review but I would also appreciate @vicb, @ocombe and @Foxandxss to have a look. We aim to merge soon though.

We plan to further enhance it post AC, but for now the main priority is to get something simple and practical that users can follow.

Notably, I removed the pipe sections as per @vicb comment:

Usage of pipes should be discouraged. They should only be used by the compiler to transform codes.
If you use the pipes you basically can not i18n your app because different langs use different plural category

I also removed sections that talked about dynamic language switching. Angular itself doesn't support it and we don't have a server setup example to show. Also, as per @vicb:

Angular always use the static approach for both AoT and JiT

However, as @ocombe noted, this is a very common request and we should incorporate it in the guide in some form when revising.

@vicb I did not make the ICU messages note you mentioned simply because I don't know what it is nor what to say about it.

Member

filipesilva commented Sep 26, 2016

The guide is heavily revised, I know that @wardbell wants to review but I would also appreciate @vicb, @ocombe and @Foxandxss to have a look. We aim to merge soon though.

We plan to further enhance it post AC, but for now the main priority is to get something simple and practical that users can follow.

Notably, I removed the pipe sections as per @vicb comment:

Usage of pipes should be discouraged. They should only be used by the compiler to transform codes.
If you use the pipes you basically can not i18n your app because different langs use different plural category

I also removed sections that talked about dynamic language switching. Angular itself doesn't support it and we don't have a server setup example to show. Also, as per @vicb:

Angular always use the static approach for both AoT and JiT

However, as @ocombe noted, this is a very common request and we should incorporate it in the guide in some form when revising.

@vicb I did not make the ICU messages note you mentioned simply because I don't know what it is nor what to say about it.

@vicb

Added some "brief" comments, ping me if you need more info

@@ -0,0 +1,13 @@
/// <reference path='../_protractor/e2e.d.ts' />
'use strict';

This comment has been minimized.

@vicb

vicb Sep 26, 2016

Contributor

Is this required in TS ?

@vicb

vicb Sep 26, 2016

Contributor

Is this required in TS ?

This comment has been minimized.

@filipesilva

filipesilva Sep 26, 2016

Member

In our case yes, because we there's a couple of es2015 things that node requires 'use strict'; to support.

@filipesilva

filipesilva Sep 26, 2016

Member

In our case yes, because we there's a couple of es2015 things that node requires 'use strict'; to support.

@NgModule({
imports: [ BrowserModule ],
declarations: [ AppComponent ],
bootstrap: [ AppComponent ]

This comment has been minimized.

@vicb

vicb Sep 26, 2016

Contributor

nit: trailing coma

@vicb

vicb Sep 26, 2016

Contributor

nit: trailing coma

This comment has been minimized.

@filipesilva

filipesilva Sep 26, 2016

Member

We don't enforce it in the examples.

@filipesilva

filipesilva Sep 26, 2016

Member

We don't enforce it in the examples.

@@ -0,0 +1,5 @@
// #docregion
export const TRANSLATION = `

This comment has been minimized.

@vicb

vicb Sep 26, 2016

Contributor

explain what this is ?

@vicb

vicb Sep 26, 2016

Contributor

explain what this is ?

This comment has been minimized.

@filipesilva

filipesilva Sep 26, 2016

Member

A file we need for example purpose, since our docs examples come from real files. Later in the guide we insert the content of the the .xlf file.

@filipesilva

filipesilva Sep 26, 2016

Member

A file we need for example purpose, since our docs examples come from real files. Later in the guide we insert the content of the the .xlf file.

@@ -20,7 +20,8 @@
"test:webpack": "karma start karma.webpack.conf.js",
"build:webpack": "rimraf dist && webpack --config config/webpack.prod.js --bail",
"build:cli": "ng build",
"build:aot": "ngc -p tsconfig-aot.json && rollup -c rollup.js"
"build:aot": "ngc -p tsconfig-aot.json && rollup -c rollup.js",
"extract": "ng-xi18n"

This comment has been minimized.

@vicb

vicb Sep 26, 2016

Contributor

what is the added value of the alias ?

@vicb

vicb Sep 26, 2016

Contributor

what is the added value of the alias ?

This comment has been minimized.

@filipesilva

filipesilva Sep 26, 2016

Member

Not having to do ./node_modules/.bin/ng-xi18n.

@filipesilva

filipesilva Sep 26, 2016

Member

Not having to do ./node_modules/.bin/ng-xi18n.

Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
```
This XML element represents the translation of our header tag.
You might have a different string in `id="af2ccf4b5dba59616e92cf1531505af02da8f6d2"`, this is normal.

This comment has been minimized.

@vicb

vicb Sep 26, 2016

Contributor

The id depends on the content of the message and it's meaning

@vicb

vicb Sep 26, 2016

Contributor

The id depends on the content of the message and it's meaning

This comment has been minimized.

@filipesilva

filipesilva Sep 26, 2016

Member

Added

@filipesilva
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade
Show outdated Hide outdated public/docs/ts/latest/guide/i18n.jade

filipesilva added some commits Sep 26, 2016

@wardbell

This comment has been minimized.

Show comment
Hide comment
@wardbell

wardbell Sep 27, 2016

Contributor

Now superseded by #2491

Contributor

wardbell commented Sep 27, 2016

Now superseded by #2491

@wardbell wardbell closed this Sep 27, 2016

@wardbell wardbell reopened this Sep 27, 2016

@wardbell wardbell changed the title from I18n guide to [BACKUP] new I18n guide Sep 27, 2016

@wardbell wardbell closed this Oct 5, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.