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

Align docs to the new Translation Service #694

Merged
merged 8 commits into from
Nov 30, 2017
Merged

Align docs to the new Translation Service #694

merged 8 commits into from
Nov 30, 2017

Conversation

ma2ciek
Copy link
Contributor

@ma2ciek ma2ciek commented Nov 28, 2017

Suggested merge commit message (convention)

Docs: Aligned docs and snippets to the Translation Service v2. Closes #624.


Additional information

Requires ckeditor/ckeditor5-dev#314.
TODO: Update snippet-adapter's dependencies after ckeditor5-dev releases.
TODO: Update package.json after ckeditor5-dev releases.

@ma2ciek ma2ciek changed the title Aligned docs to the new Translation Service. Align docs to the new Translation Service Nov 28, 2017
toolbar: [ 'headings', 'bold', 'italic', 'custombutton' ],

// UI language. Language codes follow the https://en.wikipedia.org/wiki/ISO_639-1 format.
lang: 'en'
Copy link
Member

Choose a reason for hiding this comment

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

One problem I see here is that the editor's config option is called lang while the Webpack plugin's language. This will lead people misconfiguring one or the other.

Since additionalLangs and langs sound at least bad, I think it'll be better if we used the full language word in all cases. So we need to change it in the editor's code.

@@ -15,7 +15,7 @@ See the demo of the editor in German:

## Building the editor using a specific language

Currently, it is only possible to change the UI language at the build stage. A single build of the editor supports only the language which was defined in the [CKEditor 5 webpack plugin](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-webpack-plugin)'s configuration.
Currently, it is possible to change the UI language at the build stage and after the build. A single build of the editor supports the language which was defined in the [CKEditor 5 webpack plugin](https://www.npmjs.com/package/@ckeditor/ckeditor5-dev-webpack-plugin)'s configuration. See the whole translation process to see how you can change the language later.
Copy link
Member

Choose a reason for hiding this comment

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

This guide needs to be pretty much rewritten rather than patched. Since the whole picture changed dramatically we need to put focus on different things.

  1. The current intro can stay.

  2. But then we should explain how to initialize the editor from CDN/zip/npm with a different language. This is what most people will want to do. Please also review other guides like installation.md and other places where we list files available in a build because that list needs to be updated (and perhaps link to the ui-language.md guide):

    image

  3. Then we can provide a few more details on what other options people have:

    • compiling an optimised editor (single language case),
    • compiling a subset of languages (TBH, I'm not sure what's the use case for that – perhaps speeding up the process and reducing the number of files – but let's mention it anyway),
    • handling multiple entry points scenario (as @ma2ciek noticed that has certain limitations)
  4. At the end we can mention that compiling editor translations is only available for Webpack and we will be extending support for other bundlers in the future. If we want to mention those tickets about translations services we should explain why we mention them. Something like – if you'd like to tinker with one of the bundlers you can read about CKEditor 5's translation service here and here.

outputDirectory: 'lang',

// Optional flag indicates breaking the build process when the error is found.
throwErrorOnMissingTranslation: false
Copy link
Member

Choose a reason for hiding this comment

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

Is false a default value? In documentation, in code samples, we should present real usage and no one will change false to false :P Plus, the comment above is unclear now – if you'd turn it on here, we could simply document that it "Will cause the building process to fail if a translation for one of the strings is missing".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I just wanted to list available options with their default values.

Copy link
Member

Choose a reason for hiding this comment

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

You can mention the default value of each option in a comment.

<script src="../build/ckeditor.js"></script>
<script src="../build/lang/pl.js"></script>
<script>
ClassicEditor.create( document.querySelector( '#editor' ), {
Copy link
Member

Choose a reason for hiding this comment

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

Better Consistent formatting:

ClassicEditor
    .create( {
    } )
    .then()
    .catch();

npm-debug.log Outdated
@@ -0,0 +1,45 @@
0 info it worked if it ends with ok
Copy link
Contributor

Choose a reason for hiding this comment

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

I dunno why I was reading this commit but this caught my attention :): npm-debug.log


### CDN

To use different language than default one, you need to load the editor together with the preferred language:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can specify, that English will be the main language at CDN.

@szymonkups szymonkups merged commit 0a97bbe into master Nov 30, 2017
@szymonkups szymonkups deleted the t/624 branch November 30, 2017 11:13
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.

4 participants