i18n plans?

[zyf722]

I'm working on a project that requires me to allow users to edit and run code in multiple languages in the browser. Then I found this project and it works perfectly for my needs.

I notice that currently all messages and texts are hardcoded in the source code. I think it would be better if we can move all these texts to a separate file so that it's easier to maintain and translate. Any plans or thoughts on this i18n feature?

[hatemhosny]

Thank you, @zyf722 for the suggestion.
This is indeed a good idea.

This is also related to providing a RTL layout, since a significant percentage of LiveCodes users have Arabic as their first language.

[hatemhosny]

I have opened an issue for this here:
#548

[zyf722]

Glad to see this is being considered. If possible, I would like to have a try to work on this.

I think i18next could be a good choice for i18n library based on following reasons:

• It allows to use translations type-safe with TypeScript.
• It supports pluralization and dynamic translations out of the box.
• There is multiple extraction tools to build translation files based on current source code from scratch.
• Online translation services like Crowdin have integration support for it.
• As for implementing RTL layout, it provides a dir function to get the writing direction based on the current language.

I might take some time to get familiar with the codebase and other current existing i18n implementations, then try to make a demo to validate the feasibility of this proposal.

[zyf722]

After some research, I believe the hardest part of this challenge is to find out all hard-coded UI strings out of other string literals. String concatenation shall also be considered since it is indeed a common way to dynamically generate strings.

Frustatingly, there seems to be no easy way to do this. Existing tools either are not designed to handle this task (instead, they basically extract keys like i18n.t('some.key') from source code to json files, not original hard-coded strings) or might get messy with code as they try to turn code into AST and then convert it back.

The only choice left is to manually search for hard-coded strings. But ahead of that, it is necessary to understand where to look for. After a quick look at the codebase, I wonder if all we have to check is src/livecodes/html and src/livecodes/UI folders.

@hatemhosny Any idea if this is the right approach? If so, I will start working on it. Otherwise, please let me know what you think. Thanks!

[hatemhosny]

yes, @zyf722 , I see what you mean.

i18n was not put in mind when developing LiveCodes, so no effort was made to isolate user-facing text.

The only choice left is to manually search for hard-coded strings. But ahead of that, it is necessary to understand where to look for. After a quick look at the codebase, I wonder if all we have to check is src/livecodes/html and src/livecodes/UI folders.

exactly as you say, most UI strings can be found in those 2 folders.

html files in src/livecodes/html are imported as strings by src/livecodes/html/index.ts (using esbuild text loader). So string manipulations can be done there before re-exporting.

Other much fewer strings will be elsewhere, like tool names: https://github.com/live-codes/livecodes/blob/develop/src/livecodes/toolspane/tools.ts#L29C1-L42C5

If you start, I can help or guide for the rest.

Please note that I would prefer lazy-loading language resources for the selected language only.
Also the language should be added as a user config so that it can be saved for later sessions (I would be happy to do that).

Thanks a lot for willing to help.

[zyf722]

Here's some progress about infrastructure and HTML-related work. All changes could be checked here in my fork.

Infrastructure

1. i18next with its backend (mainly to implement lazy loading) and browser language detector are added as dependencies.
2. New build script scripts/i18n.js is added to generate locales folder. Changes are also made in scripts/build.js to integrate this script into current build process.
3. New commit type i18n is used to mark commits related to .json translation files and other non-code-level changes.

HTML

html files in src/livecodes/html are imported as strings by src/livecodes/html/index.ts (using esbuild text loader). So string manipulations can be done there before re-exporting.

As for static HTML, I checked replaceValues function in src/livecodes/html/index.ts, however found it seemingly impossible to apply translation here, as the translation resource might not be loaded yet at this moment.

Current workaround is to add i18n class to the elements that will be localized, and change their inner text to translation key in HTML files. (However, this probably make the HTML files less readable and harder to maintain. I'm considering to use an attribute to store translation key instead, while keeping the inner text as fallback.)

Then, these elements will be processed in their container's corresponding handler in src/livecodes/core.ts. To avoid code duplication, maybe it is better to create a new function to handle this kind of localization and move selector-related code to src/livecodes/UI/selector.ts.

To preview, you may want to change fallbackLng in src/livecodes/i18n/index.ts to zh-CN to see the effect. I don't include that since I'm using Chinese as my browser language so it's not necessary for me.

That's all for now. Please let me know if you have any suggestions or questions.

By the way, is it still available to assign the issue by commenting .take? If so, I will do that soon.

[hatemhosny]

@zyf722
Your approach does work. Nice job.

I have some comments:

• I agree. An attribute (e.g. data-i18n) for translation key is better than putting the key as inner text. The fallback is an added benefit.

• Instead of scattering text replacement logic in each corresponding handler, all screens are loaded in modal.show(). This would be a better place.

• Not all text is inner text. e.g. Tooltips are in data-hint attributes. We may add another attribute for the element property that needs to be replaced (e.g. data-i18n-prop which defaults to innerText).

• Adding i18next library and dependencies adds ~77 kb to bundle (~22kb gzipped). This is probably acceptable for the full app. However, I do not want to add this for embedded playgrounds. Most UI screens with most text are not available for embedded playgrounds anyway. I currently provide 4 bundles (app, embed, lite & headless). I utilize tree-shaking to allow smaller bundles for un-needed features. I think i18n should only be available for the full app and not the others. So may be others, get a mock that just uses the fallback text similar to this.
  To be able to utilize tree-shaking, './i18n' should not be imported anywhere else, and any modules that needs to utilize it (e.g. modal) should get a reference to it (or the mock) as dependency injection.

• I add hash for all files (file checksum) to enable aggressive caching across versions. The hash changes only when file content changes regardless which app version. Then I cache it in the browser. We will need to do the same for locale files. This can be done later. Let's just get it working and we can add that later.

Let me know what you think.

edit: Of course, I would be happy to assign you to this issue. Just add a comment there so that I can assign you.

[zyf722]

Thanks for such a detailed explanation and guidance. I'll move the current logic to modal.show() and switch to use attributes to make it more clear.

Regarding the size and performance considerations, my workaround is indeed a bit rough as I didn't take into account the impact on the bundle size. I agree to enable i18n only for the full app bundle. However, I must admit that I'm not entirely familiar with the concepts of dependency injection and tree-shaking. Could you possibly provide some further information on how to implement these?

Once again, thank you for your assistance!

[hatemhosny]

I will open a PR in your repo with the proposed changes when I have some time.

[hatemhosny]

After checking bundle size, I found that the app minified bundle size increased from 313kb to 385kb after adding the i18n dependencies.
I still think this is unnecessary for most users who will use the English language.
So I built the i18n module separately and lazy-loaded it when another language is selected (in Config.appLanguage or browser settings).
The module is not loaded in embedded playgrounds.

The variable i18n either gives access to the module when loaded or is undefined. TypeScript will force us to check that it is not undefined before using it.

A helper function translate allows us to populate HTMLElements with translations. It can be used like that.

The type AppLanguage strongly types the list of supported languages for use in the SDK.

I have opened a PR in your repo with these changes

[zyf722]

All changes look great to me.

I only have one question about language detection in core.ts when no language is explicitly set. This seems to do the same thing as the language detector plugin (which has more ways to detect the language besides navigator.language). If we decide to use this simpler approach, we could remove the plugin and simplify the setup. What do you think?

Another thing to mention is that i18next offers namespace support which allows to further separate translations into different files. How many localized strings do you roughly estimate in the application? Should separate files be considered for better maintainability?

If there's no other major changes, I shall merge the PR and continue to work on the next steps once we reach a consensus on the above points.

[hatemhosny]

I agree on removing language detection plugin. Other ways for language detection are either not needed or already provided by the app.
e.g. query param - you can override detected browser language like this ?appLanguage=ar.
When we add a UI for manually changing language, the setting will be saved in the browser storage (TODO: we need to add appLanguage here for this to happen.)

Something else we need to consider. Currently the translate helper function only allows to replace a single property (e.g. textContent or title). Some few elements have text in more than one property. We need to think of an elegant way to handle this. However, we can keep this for later.

I think keeping each translation in a single file will be easier to manage and easier for contributors to provide more translations. However, the language-info file is quite large. Can we keep this file in a separate namespace for lazy-loading and have everything else together?

[zyf722]

If we choose to keep language-info in a separate namespace, then there is more steps to make sure it would be loaded when access the screen. i18next.loadNamespaces(ns, callback) provides a way to load the namespace on demand.

[hatemhosny]

To deal with multiple properties, is it possible to use data-i18n as a parent key and store properties needs to be translated space-separatedly in data-i18n-prop?

Nice 👌
Let's go with this

If we choose to keep language-info in a separate namespace, then there is more steps to make sure it would be loaded when access the screen. i18next.loadNamespaces(ns, callback) provides a way to load the namespace on demand.

I think this is reasonable. We probably need to do this once here (we may need to turn it to async). It is then called there.

[zyf722]

I think I found another issue that needs to be discussed.

How shall we handle elements like this (nested elements)? Apparently it is not a good choice to separate them into different keys totally based on tag since the context matters and word order may vary in different languages.

React integration for i18next has a component called Trans that can be used to handle this kind of situation. We might need to implement a similar mechanism in our project.

Are we going to use innerHTML in data-i18n-prop? Would this method pose any potential security risks? If we choose this method, we also need to change escapeValue in i18n.ts to false to prevent escaping.

[hatemhosny]

In this case, I believe setting innerHTML is safe enough since we have full control on the injected HTML in translation files. That would be much simpler than implementing such mechanism.
We are already injecting HTML from source files. This is probably the same.

[zyf722]

Alright, I believe there are no further questions as of now.

The PR will be merged soon and changes we discussed will be carried out. I will return here to provide an update once there is new progress.

[hatemhosny]

We may use Weblate to allow contributions for translation to different languages.
They offer the hosted plan for free for open-source projects. LiveCodes seems to be eligible.

This is a demo for the-algorithms.com: https://github.com/TheAlgorithms/website?tab=readme-ov-file#translating-the-website

We should consider this after extracting text to be translated.

[zyf722]

I only have some experience with Crowdin and another similar platform, but Weblate seems to be a good choice as well 👍

[hatemhosny]

Crowdin also looks awesome. I have no experience with either. You choose the platform to use.

[zyf722]

OK, then I might try starting with Crowdin first.

[zyf722]

New progress here.

Features discussed before have now been implemented, and some new features have been added.

Changes

• Synced with upstream develop branch in livecodes/livecodes repo.
• Removed i18next-language-detector plugin.
• Added appLanguage to getUserConfig in src/livecodes/config/config.ts. Query parameter appLanguage should work now.
  
• Modified translate function as discussed.
• Add lazy-loading support for language-info namespace. The json file will be loaded only when needed.
  
• Changed escapeValue to false in i18n.ts file to support HTML tags in translations.
  • TODO: Need testing.
• Change translation file from .json to .ts in order to support type-safe translations.
  • This will allow us to have some degree of IntelliSense for translations, to check if the attribute is valid. An key object must have at least one of the attributes defined in _I18nAttributes interface (an empty object or using properties not defined will lead to error).
  • Extend _I18nAttributes interface in template.ts to add more possible attributes that can be translated.

    interface _I18nAttributes {
      textContent?: string;
      title?: string
    }

  • i18n.js will now try to load .ts files in src/livecodes/locales folder and use esbuild to compile them in memory, then save the module's default export (the translation object) as .json file in build/livecodes/locales folder.

Comments

Type-safe translations

This is a feature that I come up with when implementing other changes. Please let me know if you have any thoughts on this.

Pre-replaced values in HTML tags

As for HTML tag support, I now find another thing to think about. Take the about message for example, it contains multiple HTML tags with pre-replaced values like {{REPO_URL}}. How should we handle this?

Abstract tags proposal

Besides, containing a large <a> element like <a href="{{DOCS_BASE_URL}}why#client-side" target="_blank" rel="noopener">client-side</a> in translation file seems to be redundant. I'm still thinking using abstract tags and creating a helper function to overwrite them when translating.

For example, we use <0>client-side</0> in translation file, and when translating, we can replace <0> with <a href="{{DOCS_BASE_URL}}why#client-side" target="_blank" rel="noopener"> and </0> with </a>. If necceary (like different target urls for different languages), we can also add attributes in the abstract tags and later use them to overwrite the fallback values.

This is just a thought, and I'm not sure if it's a good idea because more efforts to parse HTML and replace tags are needed.

[zyf722]

As for pre-replaced values, we could utilize interpolation to replace the values in the string. Codes might behave similarly to the replaceValues function. Any ideas to prevent code duplication?

[zyf722]

@hatemhosny Could you please take a look here at your convenience?

[hatemhosny]

Thank you.
I will have a good look and get back to you.

[hatemhosny]

Great work @zyf722 . Thank you.

I like the type-safe solution.

I have started another PR with the following changes:

• extended type-safety to include translation keys.
• renamed template.ts to models.ts to be consistent with other type-only files.
• reduced redundency in translation files. Instead of requiring to supply attribute for all values (and only use them a few times), now the value can also be a string which will then be assumed to be textContent.
  e.g.

const translation = {
 welcome: 'Welcome',
 hello: { textContent: 'Hello', title: 'Hello' }
}

• I have made some changes to the build script:
  • calling esbuild.build inside a nested loop will lead to starting many processes. Instead, it is much more performant to pass all files to a single call as entryPoints.
  • now the output files are placed in the build/livecodes build directory with the pattern translation-{locale}-{filename}.json. TODO: we need to add hash to file names.

Regarding your question, I like your proposal (<0>client-side</0>). Please go ahead implementing that if you think this is the way to go.
If we need to reuse replaceValues function, we can move it to utils module and export it from there.

[zyf722]

Thanks for pointing out these issues and making the changes. I have merged the PR and will continue with finding a solution to replace those predefined values and implementing the abstract tag part.

PS: As there is a .vscode folder in the repo, I suppose you are currently using VSCode. I'm considering making full use of Custom Data Format to provide better Intellisense for data-i18n and data-i18n-props attributes. I'll come back here once there is something could be shown.

[zyf722]

Done with abstract tags and intellisense enhancement. Please check commits for details.

• replaceValues-related code has been refactored to re-use in i18n.
• Two i18n namespaces has been separated to two types in order to have better type checking.
• A new vscode-intellisense npm script has been added to package.json to generate html.html-data.json file in .vscode folder for VSCode intellisense enhancement. Currently it only contains data-i18n and data-i18n-prop attributes and value set for data-i18n-prop attribute, which is generated by extracting fields from I18nAttributes in src/livecodes/i18n/locales/models.ts using regex. Further extensions could be added here to provide better intellisense support as well for other parts of the project, like data-hint and other custom attributes.
• Abstract tags are introduced using DOMParser, which supports overriding specified attributes or adding new ones to corresponding element defined in .html files (e.g. <2 href="https://example.org">open-source</2> will override href attribute of the original element).
  
  • Add data-i18n key for the element and check console to see the abstractified innerHTML and array of extracted tags. Since this is implemented using missingKeyHandler, so make sure the key has not been added to the translation files yet.
    

I believe i18n integration is now functional enough for use. Please let me know if you have any suggestions or questions on above changes. If there is no further feature planned, I will mainly focus on marking elements that need to be translated in HTML files next.

[hatemhosny]

wow! this is impressive.

some minor comments:

• replaceValues-related code has been refactored to re-use in i18n.

Nice 👍

• Two i18n namespaces has been separated to two types in order to have better type checking.

This is fine. You may want to update the comment here to use the new type I18nLangInfoTranslation.

• A new vscode-intellisense npm script has been added ...

Beautiful 😍

You may actually add data-hint as you suggested.

I think the regex used here is too specific and can easily break with minor code change (e.g. if we later use Partial instead of RequireAtLeastOne or change it to interface instead of type). I suggest using a more generic regex like this: I18nAttributes.+?{([\s\S]*?)}

• Abstract tags are introduced using DOMParser, which supports overriding specified attributes or adding new ones to corresponding element defined in .html files (e.g. <2 href="https://example.org">open-source</2> will override href attribute of the original element).

I like this indeed 👍
Just a minor stylistic comment. I tend to use early return pattern, if there is a condition that wraps the rest of a function. It decreases indentations and I feel it is more readable.
So instead of:

const replaceElement = (node: HTMLElement) => {
  if (node.nodeType === Node.ELEMENT_NODE) {
    node.childNodes.forEach((child) => {
      replaceElement(child as HTMLElement);
    });

    const name = node.tagName.toLowerCase();
    if (name !== 'body') {
      // implementation
    }
  }
};

I usually do this:

const replaceElement = (node: HTMLElement) => {
  if (node.nodeType !== Node.ELEMENT_NODE) return;

  node.childNodes.forEach((child) => {
    replaceElement(child as HTMLElement);
  });

  const name = node.tagName.toLowerCase();
  if (name === 'body') return;
  
  // implementation
};

also, instead of:

const attributes =
(node.attributes.length > 0) ?
  Array.from(node.attributes).reduce((acc, attr) => {
    acc[attr.name] = attr.value;
    return acc;
  }, {} as Record<string, string>) : undefined;

I would reverse the condition to deal with the short path (undefined) early and continue with the logic, like this:

const attributes =
  node.attributes.length === 0
    ? undefined
    : Array.from(node.attributes).reduce(
        (acc, attr) => {
          acc[attr.name] = attr.value;
          return acc;
        },
        {} as Record<string, string>,
      );

just to keep the same style with the rest of the codebase.

• Add data-i18n key for the element and check console to see the abstractified innerHTML and array of extracted tags. Since this is implemented using missingKeyHandler, so make sure the key has not been added to the translation files yet.

Very nice 💯

In my local setup, I have prettier configured to run on-save. I noticed some formatting changes (mostly whitspace formatting).
You may want to run npm run fix before commits to run formatters and linters with autofix, to avoid later failure in CI when merging.

Apart from these minor comments, I think we are ready. Please go ahead.

[zyf722]

Got it. Thanks for suggestions on style and readability and I'll fix that soon :)

[zyf722]

New progress in these days. Check here to see all changes.

String-level translation

Added the string-level translation and enhance its intellisense and static checking with type-safety (Reused some code from i18next.d.ts which was done before, so acutally that part is not totally useless 😏 ).

Now we can use window.deps.translateString like this:

window.deps.translateString('namespace:file.key1.subkey1', 'default <strong>value</strong>, {{interpol}}', {
  isHTML: true,
  interpol: "abc"
})

The function is completely type-safe:

• The first argument is key, which is validated with valid keys in i18n/locales/en/.
• The second argument is value. With key provided, the type of value will be narrowed to the value in the translation file to ensure all occurrences of the key have the same value. If this is a HTML string (with isHTML attribute set to true), the value will automatically be abstractified when exporting translation.
• The third argument is interpolation, which could be omitted when the value doesn't contain any interpolation. Otherwise, it should be an object whose attributes are inferred from the value. Moreover, an additional isHTML boolean attribute is added to indicate whether the value contains HTML tags and should be abstractified when exporting translation.

To provide better readability and maintainability, only string-level translation will be used in .ts files when it comes to dynamic content. For static content, element-level translation is still the best choice.

Marked all strings in src/livecodes/UI/*.ts files

Done with almost all known strings of files in src/livecodes/UI and core.ts.

There might still be some strings that are not marked yet, so I'd appreciate it if you could help me review the files and point out anything missing.

Support for .ts files in i18n-export.js

Now the i18n-export.js script can handle .ts files as well. It will use Babel to extract all strings marked with window.deps.translateString and export them to .lokalise.json and .ts files in i18n/locales/tmp.

To add new keys, just mark strings with window.deps.translateString in source files (don't worry if type check complains about missing keys, as these new keys haven't been exported yet), run npm run i18n-export and copy generated .ts files to i18n/locales/en to update the translation files and everything will be fine.

[zyf722]

PS. Don't know if keys (here to take a glance) are elegant and neat enough. Please feel free to suggest any improvements.

[zyf722]

@hatemhosny Could you please take some time to review this? Don't mean to bother, but it coule be better if you could give me some feedback so that I can continue to work on HTML files. Thanks in advance!

[hatemhosny]

hi @zyf722
Sorry for the delay. I have been busy the last few days, and was able to finish a difficult and important update.
I will have a good look and get back to you, hopefully by tomorrow.

[hatemhosny]

wow!
That was a lot of work. I really appreciate the effort and thought you put in this project.
Thank you.

I like the added type-safety and extracting data from ts files (using babel!). Impressive.

I reviewed the changes and have some comments:

• When I start the app I get an error: Cannot read properties of undefined (reading 'translateString')
  This is related to importing this variable which tries to run window.deps.translateString before initApp runs.
  The solution for this would be to change the noUserTemplates variable to a function and then call it here and there.

• When the welcome screen loads with a previous unsaved project, I get this error: TypeError: Cannot set properties of null (setting 'textContent')
  This is related to the translation of welcome.recover.lastModified, does not include the child element: <span id="unsaved-project-last-modified"></span>

PS. Don't know if keys (here to take a glance) are elegant and neat enough. Please feel free to suggest any improvements.

I have no problem with the namings of the keys. Actually, I find them descriptive.
However, I'm a bit concerned about using the file name in the key. Although this makes it easier to find and group keys, but this means that refactoring app code will result in the need to make significant changes in translations. I'm not sure what is the best approach here. I'm just thinking with you.

The big concern I'm having now is that I'm starting to feel overwhelmed with this feature. I'm not 100% sure I know what is going on where.
I know you are making a really impressive effort. I'm just worried about the burden on maintaining this, adding translation for new features, changing old text (and translations), the process of using the cloud service and getting back the translations to the code base.
I'm not sure what would be the solution for this. These are some suggestions:

• Can we simplify the workflow, even if we lose some power?
• Adding clear documentations for different workflows
• Add some code comments describing why and when each script should run.

I want to know your thoughts.
Again, thanks a lot. Much appreciated.

[zyf722]

That was a lot of work. I really appreciate the effort and thought you put in this project.

Firstly, it's my pleasure to contribute to this project. I'm glad you like the changes and the new features :)

I reviewed the changes and have some comments: ...

I'll check and fix these issues as soon as possible. Thanks for pointing them out.

However, I'm a bit concerned about using the file name in the key. Although this makes it easier to find and group keys, but this means that refactoring app code will result in the need to make significant changes in translations.

Indeed, this is a trade-off between maintainability and readability. Well, currently I think it's not a big deal since I believe that the architecture of the app is stable and won't change frequently. If we need to refactor the app, we can always update the keys in the translation files accordingly. But you're right that this might be a potential issue in the future.

Maybe we could use a more semantic top-level key to group the keys about the same thing while they might not strictly organize in the same file. In fact, we are facing this issue at the moment. For instance, broadcast, this word is both used in core.ts and broadcast.ts, while they are not in the same file, which forces us to use two duplicate keys. If we use a semantic top-level key like broadcast (representing all keys about the broadcast feature, not only the file), we can group all keys related to this feature under this key, which makes it easier to manage and maintain. Meanwhile, I believe features are more stable and not easy to be influenced by refactoring, so this approach might be a good choice.

Anyway, I'll keep an eye on this and check some other projects and best practices to see if there's a better solution.

Another related issue is the reuse of keys. As for now I manage them in generic top-level keys, but I find it a little vague to consider a key representing exactly the same thing in different files as generic. How do you think about this? Should we keep it minimal and use more specific keys, or try best to reuse keys as much as possible? (This also correlates with the previous broadcast example)

The big concern I'm having now is that I'm starting to feel overwhelmed with this feature. I'm not 100% sure I know what is going on where. I know you are making a really impressive effort. I'm just worried about the burden on maintaining this, adding translation for new features, changing old text (and translations), the process of using the cloud service and getting back the translations to the code base.

I myself do recognize the complexity of the current workflow and the potential burden on maintaining this feature. Perhaps I need some time to explore experiences from other projects and find a better solution.

As for documents and comments, please rest assured that I could handle this part. Comments will be added soon, and I'll focus on writing detailed documentation and guidelines once I finish these current technical tasks.

[hatemhosny]

Maybe we could use a more semantic top-level key to group the keys about the same thing while they might not strictly organize in the same file. In fact, we are facing this issue at the moment. For instance, broadcast, this word is both used in core.ts and broadcast.ts, while they are not in the same file, which forces us to use two duplicate keys. If we use a semantic top-level key like broadcast (representing all keys about the broadcast feature, not only the file), we can group all keys related to this feature under this key, which makes it easier to manage and maintain. Meanwhile, I believe features are more stable and not easy to be influenced by refactoring, so this approach might be a good choice.

I like the idea of semantic keys. I think naming keys based on features is much better than based on file names. Let's do that.

Another related issue is the reuse of keys. As for now I manage them in generic top-level keys, but I find it a little vague to consider a key representing exactly the same thing in different files as generic. How do you think about this? Should we keep it minimal and use more specific keys, or try best to reuse keys as much as possible? (This also correlates with the previous broadcast example)

I think if we use feature-based keys, this becomes much less of a problem. I prefer not to use the same key unless the text (semantically) represent the exact same thing. This re-use might cause un-intended result if the wording was changed in one place and was not expected to change in the other.

I myself do recognize the complexity of the current workflow and the potential burden on maintaining this feature. Perhaps I need some time to explore experiences from other projects and find a better solution.

As for documents and comments, please rest assured that I could handle this part. Comments will be added soon, and I'll focus on writing detailed documentation and guidelines once I finish these current technical tasks.

Thank you very much. I really appreciate the effort.

[zyf722]

New updates. All commits are listed here.

• Add comments for i18n-related files.
• Fix previous issues and add more type-safety in order to work correctly with abstract HTML tags.
• Minor improvements in i18n-export.js.
• Finish with all translation keys in .html files in src/livecodes/html.

I believe as for now, most of the translation keys are marked and exported. Next I will get to other projects to see how they manage translation keys and try if there's a better solution for our project. Once that's ready, I could start working on the Lokalise integration and the CI/CD pipeline.

P.S. I think I found some typos during the process of adding translation keys to the HTML files. Not sure if they are intentional or not, so I'll list them here for your reference:

• Normalize.CSS in external-resources.html: The dot between Normalize and CSS
• data-hint for Welcome ... in settings-menu.html: The attribute is put in the label, which causes that the hint is only shown when cursor hovering on the label, not the whole li (Compared to Delay).
• sync in sync.html: Not capitalized correctly, causing i18n-export.js to log a warning (Compared to L22)

[hatemhosny]

Thank you.
Added a few comments:
zyf722@1ff478d#r142423670
zyf722@1ff478d#r142423365
zyf722@1ff478d#r142423488

[zyf722]

Thanks for your prompt comments. Necessary changes has now been made in the last three commits.

[hatemhosny]

even more :)
zyf722@e95f0a4#r142424176

[zyf722]

Sorry for my misunderstanding, already fixed in zyf722/livecodes@8f2b2c1 :)

[hatemhosny]

That's great. Thank you 👍

[zyf722]

Apologies for my inactivity in the past month first - I was caught up with some personal matters and couldn't allocate time to work on the project. I'm back now and ready to continue the work 🚀

After digging into some other projects, I found that although the current solution is not perfect, it's still a good start. Other projects either keep the i18n workflow simple and manual (with potential human errors) or to some extent use scripts and integrations which are similar to what we have done.

I believe manually editing translation files and keeping them in sync with the source code would be of much more burden than the current solution - there will be no spelling errors or misplaced quotation marks in the automatically generated files, and accordingly, the effort of review and correction spent on such low-level errors could be transferred to maintaining high-level translation pipelines, which in my opinion is more valuable and meaningful. Besides, although building the translation process can be relatively cumbersome, once configured, there is almost no need to make modifications to the infrastructure itself. So in this comparison, I think it's worth it.

Let's re-list out what changes will be introduced if we adopt the current solution:

1. While developing new features, developers need to mark strings with window.deps.translateString or data-i18n attributes.
2. Once a new feature or version is ready, developers run npm run i18n-export to extract and update all marked strings, then export them to .lokalise.json and .ts files. The script is assumed to handle everything here, so for developers, it's just a one-click process.
3. Developers commit and push the changes to the repository. A webhook could be configured so that Lokalise will automatically pull the changes once a new commit is pushed.
   • Other than using webhooks, setting up a Github action is also feasible. See demo mentioned later for more details.
4. Source texts on Lokalise should now be updated. Translators can start translating the texts.
5. After translations are done, maintainer can trigger a pull request on Lokalise to merge the translations back to the repository. This will trigger an import Github action to run the i18n-import.js script to import translations back to the source code, commit them and push to the pull request branch.
6. Related tests, checks and manual review could be carried out on the pull request branch. If everything is fine, the pull request will be merged and the feature alongside translations will be updated in the main branch.

Throughout the process, the only manual work about i18n is to mark strings in the source code, which is inevitable and necessary. The rest of the process is one-time work, which later can be automated by a single command or a single click. The process is also transparent and traceable with logs and commit history.

P.S. This demo by Lokalise could be a reference on how to bi-directionally pull/push translations between Lokalise and GitHub using only cli and actions.

This is just my personal opinion and I'm open to any suggestions or comments on this. Let me know if you have any concerns or ideas :)

[hatemhosny]

Hi @zyf722
Welcome back :)

That's a great summary. Thank you.
I agree. This seems a reasonable workflow.

I prefer to use github actions for lokalise integration.
This makes it clear (and less magic) for others who fork the repo and want to have a similar workflow. They will just need to set their own tokens and then they are good to go. What do you think?

Also please add this summary to contribution docs (may be with simple code samples for using window.deps.translateString and data-i18n attributes) and link to it here.

It would also be very helpful to add a technical summary for maintainers about the workflow and scripts/generated code/github actions used for i18n.

Thanks a lot. Much appreciated.

[zyf722]

Some progress

Check commits here:

• An --overwrite option is added to i18n-export.js to copy generated .ts files in i18n/locales/tmp to i18n/locales/en directly.
• Removed missingKeyHandler and export for abstractifyHTML since all keys are marked now.
• Changed abstract tag's numbering from deep-first to left-to-right, in order to keep consistent with Lokalise. Previously abstract tags are numbered in a deep-first manner as the DOM tree is traversed, which is not consistent with Lokalise's group highlighting order.
  • 
• Added scripts/i18n-upload.mjs to push translations to Lokalise.

---

Further thoughts about the i18n workflow

During the process of adding the ci script above, I found that the process proposed three days ago still has room for improvement and further automation. Below is a more detailed and refined workflow, based on branching and actions:

1. Same as previous step 1. Developers or Contributors work on a feature or fix and mark strings in the source code.
2. A feature PR is created and reviewed, and reelated tests, checks and manual review could be carried out. I18n stuff won't intervene until a related action (Let's temporarily call it i18n-update) is manually triggered by a comment like .i18n-update-push in the PR by the Maintainer after the feature PR is merged.
3. i18n-update will contain the following steps:
   • Create a new branch based on the source branch name of the feature PR with an i18n/ prefix, then check out to the new branch.
     • e.g. If the feature PR is named feature/awesome, the new branch will be named i18n/feature/awesome.
   • Set up node environment, run npm run i18n-export -- --overwrite to extract all marked strings to i18n/locales/tmp and i18n/locales/en and commit the changes.
   • Run npm run i18n-upload -- <branchName> to push translations to the corresponding branch on Lokalise.
   • Create a comment in the feature PR with the link to the new branch.
4. Same as previous step 4. Translators work on Lokalise and translations are updated.
5. Given that we don't choose to rely on webhooks or Github integration of Lokalise due to its invasiveness, the Maintainer need to manually trigger an action (i18n-update-pull) in Github instead of Lokalise to get translations back to the source code. We may assume here a similar comment .i18n-update-pull in the feature PR also serves as a trigger.
6. i18n-update will contain the following steps:
   • Check out to the branch created in step 3.
   • Run npm run i18n-import to import all translations in all supported languages back to the source code and commit the changes. i18n-import is a ci script just like i18n-export.js and i18n-upload.mjs.
   • Create a new i18n PR to merge the changes back.
   • Mention the new PR in the feature PR as a reference.
7. A final review and merge of the i18n PR will be done by the Maintainer. Considering most reviews and checks have been done on Lokalise, the i18n PR should be relatively easy to review.

Some notes:

• This workflow binds breaking changes of translations to feature PRs (or generally speaking, source PRs), ensuring that the translations are always up-to-date with the source code.
• An important feature of Lokalise used in this workflow is branching, just as we do in the source code. This allows us to manage different versions of translations in different branches, making it possible and clean to work on multiple features simultaneously.
• The reason for separating the feature PR and i18n PR is to keep the feature PR clean and not let the progress of translation hinders the feature development, so that the feature PR can be merged as soon as it's ready.
• To make this workflow work, a special bot account on Lokalise (instead of our personal accounts) may be needed, in order to use its API token to push and pull translations.

This is more detailed and seemingly more complicated than the previous one, but I believe it's more robust and automated. I'd like to hear your thoughts on this and whether it's feasible to implement this. Once we reach a final consensus, I'll summarize it into documentation and guidelines for future reference just as you suggested.

[hatemhosny]

I agree. Please go ahead.

[hatemhosny]

you may also update the PR template to remind contributors to mark strings in the source code, and maintainers to trigger the update (with a link to docs).

[zyf722]

Finally, I am glad to announce that the Lokalise integration is now ready! Check here to see all changes 🚀

I have tried to use rebase to remove the modifications that I introduced but later removed, in order to make the commit history appear cleaner. However, there might still be some commits that are not well-organized, so I might suggest you to directly check the "Files Changed" tab in the compare view of the commits to get a clearer view. If any confusion arise, my apologies for that 😥

Some notable changes

📚 Docs

• The whole i18n workflow is done and related documentation is added.
  • Since some great changes are made these days, I recommend you to check them out (especially the i18n.md) first to get a better understanding and review the whole process.
• A minor alignment issue in the documentation is also fixed.
• Lokalise is added in credit part of related documentation. Sadly there is currently no badge service by Lokalise nor any open API without authentication to get the project status (unlike Crowdin, they offer a badge endpoint to render a "localized" badge), so I wasn't able to add a badge about i18n in the README 😌
• i18next and its http backend are added in vendor-licenses.

⚙ CI

• Default behavior of i18n-export is changed to export all keys to .lokalise.json and .ts files in src/livecodes/i18n/locales/en. The --overwrite option is removed and a new --save-temp option is added to save the generated files in src/livecodes/i18n/locales/tmp. This also removes src/livecodes/i18n/locales/tmp from git history and move all .lokasile.json files to src/livecodes/i18n/locales/en.
• Four actions and corresponding scripts are introduced to implement the i18n workflow. Further details are depicted in i18n.md mentioned above.
• Existing workflow has been improved. To be more specific:
  • External actions used (like actions/checkout) are updated to the latest version. Besides, matrix strategy is utilized in CI-e2e to avoid duplicated jobs.
  • Caching dependencies (Github Docs) is used in all actions to speed up the workflow. I simply used same cache for all workflows - if you want to separate CI test build and release build, different caching keys could be used.

I've created a demo repo and a demo project on Lokalise to show how the i18n workflow works. Feel free to check them out (like the PRs, actions) and let me know if you have any questions or suggestions.

P.S. The i18n-update-scheduled action might not work properly in the demo repo, as the demo repo is created earlier from my fork. I've already fixed the issue in the last couple of commits.

[zyf722]

Thanks @hatemhosny !

I'll look into your comments and try to fix issues found / give some feedbacks later this week - stay tuned :)

[zyf722]

Some updates and some points which need to be further discussed:

• Add npm test script for i18n-export
• Add npm fix script that runs i18n-export to "fix" i18n output
  • Further details are needed. Should we use prettier here or you mean something else? I previously did point out to let CI run i18n-export instead of developers, but soon realized that it would be more clear and reasonable to put these changes within the source PR - so basically it is not designed to be integrated with the i18n-update-push workflow.
• Add tmp to .gitignore
• Check process.env.CI and add --force for two CI workflows
• Add require.main === module check for i18n-exclude
  • Actually i18n-exclude is used in build only and directly, so this check may not work as expected.
• Add timeout for i18n-upload.mjs
• Add comments to mark auto-generated .ts files
• Merge i18n/types/utils.ts with i18n/utils.ts

Some other tasks planned:

• Add npm test script for i18n-exclude in CI to ensure // ts-nocheck won't be commited to git when unexpected break occurs during build

[hatemhosny]

Further details are needed. Should we use prettier here or you mean something else? I previously did point out to let CI run i18n-export instead of developers, but soon realized that it would be more clear and reasonable to put these changes within the source PR - so basically it is not designed to be integrated with the i18n-update-push workflow.

Sorry for not being clear.
I just meant adding npm script like this:

"fix:i18n": "npm run i18n-export",

so that when we run npm run fix it runs with format and lint fixes.

Actually i18n-exclude is used in build only and directly, so this check may not work as expected.

I see. So let's just keep it as it is.

[zyf722]

I believe as of now all the changes discussed here have been done. I also find that actions/setup-node has a built-in caching feature, so I switched from using actions/cache to that.

If you think it is time to move on, then we might get back to the previous to-do list and see what we can do next?

[hatemhosny]

Great.
Yes, let's proceed.

[zyf722]

So below is the trimmed list of tasks that haven't been done yet:

• Add checksum for translation keys to keep sync
  • I personally think it is not necessary to do so anymore since we have already had a check for i18n-export in test to ensure all keys in sync.
• Add hash for translations and put them in cache
• Add e2e tests for i18n
• Add related documentation about appLanguage
• Create a PR to live-codes/livecodes repo

The topic about hash and cache was postponed before and here I'd like to hear your thoughts on this and whether it's necessary to implement this feature. The rest of the tasks are relatively minor and could be done in a short time.

I'm looking forward to your ideas and suggestions on this.

[hatemhosny]

• Add checksum for translation keys to keep sync

  • I personally think it is not necessary to do so anymore since we have already had a check for i18n-export in test to ensure all keys in sync.

I want to elaborate about keeping keys in sync, as mentioned before.

i18n-export does a great job keeping English translation in sync with source files. However, I'm not sure we are enforcing to keep translation "values" in sync across different translations, if the text was changed but not the key.

For example, consider this scenario:

Let's assume that this code has been already translated to different languages.

<div class="description" data-i18n="deploy.create.desc" data-i18n-prop="innerHTML">
  A new <strong>public</strong> repo will be created. The result page will be pushed to <span class="code">gh-pages</span> branch.
</div>

and then a maintainer changes it to that

<div class="description" data-i18n="deploy.create.desc" data-i18n-prop="innerHTML">
  Deploy to <a href="...">Netlify</a>. 
</div>

The English translation will indeed be changed after running i18n-export. However, now the other translations will be out of sync.

Do we need to change the key every time we change the text? This is a bit cumbersome and error-prone. What if we forget?
How can we invalidate translations when the original text changes?

[zyf722]

Technically possible - there is nothing different with adding only one checksum for the whole file and making it hidden on Lokalise. Then, in translate and translateString we fetch the checksum of the given key in current language and English, and compare them. If they don't match, the value should fallback to English.

But that might double the size of the locale files as we need to store the checksum for each key. If you think this is acceptable, I will implement it as you suggested.

[hatemhosny]

The check should ideally not be in run-time. May be not even checksum.
When pulling data from lokalise, we can keep a map between each translation entry and its English value.
We then keep this map in a separate file for each language.
Then when running i18n-export we check for consistency.
If there is a mismatch, we may just delete the key from the translation file. It would then fallback to English.

[zyf722]

That makes sense and this could be integrated in i18n-import.mjs. We consider the downloaded en locale directory as the source on Lokalise, and compare it with the source in the app by running npm run i18n-export -- --save-tmp. All mismatched keys will be recorded and be deleted later in all other locales.

[hatemhosny]

Good idea. That's even better than maintaining separate maps.

[zyf722]

Implemented at zyf722/livecodes@2c4b49a. Please kindly share your feedback on it!

[hatemhosny]

The topic about hash and cache was postponed before and here I'd like to hear your thoughts on this and whether it's necessary to implement this feature.

After production build, file hashes are added to all files in build/livecodes/ directory. The hash is the checksum of the file content. So if the file content does not change, it will get the same hash across builds.

This assumes that all files to be hashed are in that directory (without nesting), and that they are referenced in code like that {{hash:file-name.js}}:

const mod = await import(baseUrl + '{{hash:file-name.js}}');

Then, these files are aggressively cached (for 1 year).

So, any file that has not changed will continue to be served from cach even for later releases.

Based on this, if we do not apply hashing for translation files, the old files will continue to be served from cache and updates will not be visible even across releases.

I can think of 3 options:

• Do the same with file hashing and get the same benefit.
• Put files in a separate directory with app version or git commit sha (e.g. build/livecodes/i18n-v32). So with every release, all translation files are invalidated and re-fetched. That's no problem because every user will download 1 or 2 files.
• Put files in a separate directory and exclude it from the long cache, like we do with build/livecodes/assets/.

I'm fine with the 3 options. You choose what you think works best and is more maintainable.

[hatemhosny]

I believe it fully works now. Please check commits here to see the changes.

That's very nice. It works very well now. Thank you.

If everything is fine, shall we now focus on e2e tests and related docs that needs to be done (e.g. appLanguage in query params, how to add new language, explanation of what we've done in recent commits)? Please kindly share your idea on these topics.

For e2e tests, we are using playwright. There are some helpers that allow access to the app and result page, etc, so that the test code would not need to struggle with working around iframes.
This is a simple example.
You can run e2e tests using npm run e2e.
You may want to temporarily use test.only for the test you want to run, so that you do not have to wait for the whole test suit. Just remember to take it off before committing!

  test.only('Title', async ({ page, getTestUrl }) => {
    await page.goto(getTestUrl());
    const name = await page.title();
    expect(name).toBe('LiveCodes');
  });

By default, e2e tests run in headless mode. You may want to temporarily change the config while debugging tests.

For docs, we have 2 aspects:

• user-facing features: The availability of i18n feature and the appLanguage configuration option.
  These need to be documented in the docs website.
• contributor guides: these include how to add a new language, the use of lokalise, considerations while developing or building the app, etc.
  These would stay in the contribution guide you authored here

For the docs website, I can think of 2 places that need attention:

• Configuration object (under "User Settings"): to document the appLanguage option. You can find it here. Docs for TS types are automatically generated. You can link to this option type like this: ../api/interfaces/Config.md#appLanguage
  (I just noticed the welcome config is not documented)

• I think we need to add a new page under Features, giving an idea for the end user about the feature and how to use/configure it, including screenshots. You can put that as a new markdown file here, and then add it to the sidebar

Later on we can add a hint about it in the Features overview and the screenshots slider on the homepage

The docs website is a docusaurus app. You can run it using: npm run docs

Please let me know if you need any guidance.

P.S. I think we need to use this discussion thread to improve the contribution guide 😂

[zyf722]

Thanks @hatemhosny for the detailed guide. This is really helpful.

As for tests, I think it would be better to write them after finishing at least the first version of translation in another language outer than English (e.g. Simplified Chinese). So I'd like to focus on translation first, if you don't mind?

For docs, I have some experience in Docusaurus before and I think I can handle this.

In fact, the initial reason drives me to this project is exactly that I want to use an online web playground with several programming languages support for my personal Docusaurus notes site 😉 ! (And of course I've already added an "Run in LiveCodes" button for code blocks)

P.S. I think we need to use this discussion thread to improve the contribution guide 😂

To what extent do you think we should improve the guide? Besides, is it necessary to put them on the docs site, or just continue to keep them only in the repository?

[hatemhosny]

As for tests, I think it would be better to write them after finishing at least the first version of translation in another language outer than English (e.g. Simplified Chinese). So I'd like to focus on translation first, if you don't mind?

Yes, sure. This is reasonable.

To what extent do you think we should improve the guide?

I mean in this long discussion thread we touched different parts of the project, like different builds, hashing, lazy-loading, UI screens, tests, docs, etc. It might be useful later after we finish to use this as a guide to improve the contribution guide. I think we need to make this project a bit more friendly for contributors.

Besides, is it necessary to put them on the docs site, or just continue to keep them only in the repository?

I think keeping it in GitHub is fine. This is where contributors are. And there is already a reference to that document in the docs site.

In fact, the initial reason drives me to this project is exactly that I want to use an online web playground with several programming languages support for my personal Docusaurus notes site 😉 ! (And of course I've already added an "Run in LiveCodes" button for code blocks)

Oh, wow! That's very nice 👍
I'm very lucky indeed to have such a great contributor like you.
Thank you.

[hatemhosny]

And of course I've already added an "Run in LiveCodes" button for code blocks

I like it 👍

You may want to use the SDK function getPlaygroundUrl, which allows you to use the embed options (including all configuration options) and get a direct URL to playground instead of manually composing the URL.

[zyf722]

That's great. I'll check it out soon and try to adopt this. Thanks for the tip 👍

[zyf722]

By the way, I come up with a workaround for the Lokalise badge this afternoon. Basically it just uses Github Actions to fetch the project status from Lokalise and generate a .json file for Shields.io to render the badge. If you're interested, you can check it out and create a fork in the live-codes organization.

P.S. To perform this you might want to register a special bot account on Lokalise. Its API token might also be used for CI actions in the i18n workflow.

[hatemhosny]

Nice 👍

[hatemhosny]

Another small suggestion:

I think it would be more clear if we change the auto-generation notice to something like

// ATTENTION: This file is auto-generated from source code. Do not edit manually!

Someone might understand the current message that the ts file is generated from the json file, since it doesn't have this notice.

// ATTENTION: This is an auto-generated file. Do not edit this file manually.

Also, I'm thinking about adding an entry at the top of the json file like this:

{
  "// ____ATTENTION____": "This file is auto-generated from source code. Do not edit manually!",
  ...
}

But obviously, we do not want this entry to be propagated to other translations.
Not very important issue, but still.

what do you think?

[hatemhosny]

Or may be we can check if lokalise can just accept comments in json, something like json5, or $comment keys.
Again not super important, but would just be nice.

[hatemhosny]

do we even need the json files at all?
can we just use the ts files (either directly or with stripping of types)?

[zyf722]

do we even need the json files at all?
can we just use the ts files (either directly or with stripping of types)?

Unfortunately, although .ts and .js files are supported by Lokalise, they are not supported in the same way as .json files. Lokalise treats .ts and .js files as plain source text files - this means we cannot use the same features like adding notes or context to keys, which is crucial for translators when it comes to innerHTML and abstractified HTML tags. So I guess we still need the .json files to have a better experience.

For comments, I just found that entries do not follow the specific format of structured JSON will be stripped out, so I think adding comments in the .json files directly is totally fine.

{
  "$comment": "ATTENTION: This file is auto-generated from source code. Do not edit manually!",
  "artTemplate.desc": {
    "notes": "",
    "translation": "High performance JavaScript templating engine."
  },
  ...
}

[hatemhosny]

Unfortunately, although .ts and .js files are supported by Lokalise, they are not supported in the same way as .json files. Lokalise treats .ts and .js files as plain source text files - this means we cannot use the same features like adding notes or context to keys, which is crucial for translators when it comes to innerHTML and abstractified HTML tags. So I guess we still need the .json files to have a better experience.

That's fine. I was just thinking.

For comments, I just found that entries do not follow the specific format of structured JSON will be stripped out, so I think adding comments in the .json files directly is totally fine.

Great 👍

[zyf722]

After a silent period of three weeks, I'm glad to announce my return with the news that the Simplified Chinese translation is now ready!

Check Lokalise project page to have an overview and here to see all commits and changes since last update.

Here are some screenshots:

• About screen
  
• Lang-info dialog
  
• Main menu
  
• Share dialog
  
• New screen
  
• Welcome screen
  

Generally speaking, most of source texts should be correctly marked out and translated. However, there are still some exceptions (and basically they are all related to hard-coded texts):

• Toolspane tab titles. The titles are constrained by their tab type defined in sdk/models.ts, which are hard-coded.
  
• Templates' name. Hard-coded in their corresponding .ts files.

Other similar issues might also exist. Because I didn't know what impact it would have, I just left them as they are. Any suggestions on how to deal with them?

[zyf722]

Bundlewatch file size limit for .json translation files is now added with a maximum size of 10kB after brotli compression.

As for appLanguage as a configurable option, I have made a preliminary prototype here. Might be a bit rough and tricky. Feel free to check it out and leave your comments there on specific code details.

Another minor refactoring is that i18n .json files are renamed from translation-{locale}-{ns}.json to i18n-{locale}-{ns}.json. As the default namespace named translation, the previous naming way might be confusing.

[hatemhosny]

That looks good. Thank you.

I have 2 comments:

• Why do we need to change this?

if (isEmbed || !userLang || userLang.toLowerCase().startsWith('en')) return;

to that

if (isEmbed || !userLang) return;

Ideally users with English language should not need to download the module or use it at all. The mock translation should just use the strings in source code, as discussed here.

• When changing language, the loading text (in splash screen) uses the previous language. Can we set it to the new language before reload or just fall back to English?

[zyf722]

The change is due to my previous attempt to implement i18n hot-reloading without refreshing the page, which needs to call i18n.changeLanguage with the i18n instance. As the way we implement the string-level translation is, to some extent, static, the idea was later deprecated. I believe this was an oversight on my part and I'll fix it soon.

When changing language, the loading text (in splash screen) uses the previous language.

Yes, this is exactly a problem that needs to be solved.

To force it fall back to English is quite easy to implement as we only need yo clear the localStorage.

If we want to show it in the new language, we might need to load the target language with the i18n instance. Although it won't hurt much performance for loading the json twice (before and after reloading) thanks to caching, we still need to consider the issue I mentioned above - there is no i18n instance at all when user's previous language is English! I guess we have to reach a compromise here.

[hatemhosny]

If we want to show it in the new language, we might need to load the target language with the i18n instance. Although it won't hurt much performance for loading the json twice (before and after reloading) thanks to caching, we still need to consider the issue I mentioned above - there is no i18n instance at all when user's previous language is English! I guess we have to reach a compromise here.

I don't mind both options.
A middle ground would be to check if the instance is available, then load the translation and set the text in local storage otherwise clear it.

[zyf722]

A middle ground would be to check if the instance is available, then load the translation and set the text in local storage otherwise clear it.

Makes sense. Let's do it this way.

[hatemhosny]

After some thinking regarding enabling i18n in embeds:

We do not want to increase bundle size for embedded playgrounds by loading the i18n module, specially that most users will use the English language and most of translations affect modal screens while most of them are not enabled in embeds anyways.

However, still some screen are available (e.g. external resources). Also tools pane.

It might be nice if we allow site owners to explicitly enable i18n in embedded playgrounds by setting appLanguage to a specific language code or to 'auto' for auto-detection. Otherwise, by default embeds will not use i18n and just fall back to English.

I have opened a PR with the suggested changes.

[zyf722]

Thanks for the idea.

Just to confirm, are you suggesting that i18n could be explicitly enabled in embedded playgrounds when the appLanguage is set?

I will check the PR and will get back to you with further updates soon.

By the way, it appears there are issues with the e2e tests in the GitLab specs. Any insights on this?

[hatemhosny]

Just to confirm, are you suggesting that i18n could be explicitly enabled in embedded playgrounds when the appLanguage is set?

Yes, that's right.

By the way, it appears there are issues with the e2e tests in the GitLab specs. Any insights on this?

I think this was a temporary network issue. The introduced changes do not affect Gitlab imports. May be just try to re-run the failing tests.

[hatemhosny]

Hi @zyf722

There is a new PR that makes some major changes to UI.

Would you please check if this will cause significant merge conflicts on your side?

Just to check what is the best way to merge both and in what order.

Thank you.

[hatemhosny]

Sure, no worries. Take your time.

I just wanted to let you know about this because I thought it might impact your work.

Yes, it would be reasonable to focus on docs for now.

Best wishes for your internship ❤

[zyf722]

Back with good news: it seems that there are much fewer merge conflicts than I expected - only 8 files, with less than 350 lines of valid conflict codes. So I think it should be fine to merge this PR first if it can be finished before the completion of i18n feature.

Considering the current progress of i18n, I think it won't take too long to be finished. It is also acceptable to merge it first and then carry out some i18n-related supplementary changes on the new GUI part merged later.

So generally it is both OK for me, just depends on this PR's progress and your preference :)

[hatemhosny]

That's great news!
So let's see how both features progress and decide which one to merge first.
Thank you :)

[hatemhosny]

@zyf722
So we think merging the i18n feature would not cause major conflicts in UI updates.
I believe the work on the i18n has more progress for now.
So you do not need to wait. Whenever suits you (obviously, at your convenience), you may merge your work and then we can work to resolve conflicts introduced there.
Thank you.

[zyf722]

Hi @hatemhosny,

I've just finished working on the documentation and made some fixes related to i18n. The only remaining task is the e2e tests. I believe it's now ready for a PR to the main repository, and I'll be doing that shortly.

Additionally, I noticed and corrected some typos in the documentation and source code. You might want to consider using typos as a pre-commit hook or integrating it into the CI process to prevent such errors in the future. It also has a handy LSP integration including VSCode extension.

Thank you!

[zyf722]

Added some missing translations and updated on Lokalise (commits here). I18n now should be consistent with recent changes in upstream repo.

Besides, I found that monaco-editor has not been localized (like the context menu and hover tooltips). It seems that there is only an official example to do so with AMD module style and currently it lacks enough support for ESM as described in microsoft/monaco-editor#1514. I've noticed that livecodes uses a custom build of monaco. Is there anything we can do over there?

---

Another minor display issue (not related to i18n) found during my local test is that when hovering on the Click for info button on the bottom of the Change Language dropdown, the tooltip seems not being displayed correctly. Here is a screenshot:

I believe it has something to do with the multi-column layout and the overflow style of the dropdown (with more information at chinchang/hint.css#78). I'm not sure if it's necessary to fix this, but I'd like to hear your thoughts on this.

[hatemhosny]

Besides, I found that monaco-editor has not been localized (like the context menu and hover tooltips). It seems that there is only an official example to do so with AMD module style and currently it lacks enough support for ESM as described in microsoft/monaco-editor#1514.

The AMD version of monaco-editor caused many conflicts with other libraries. I use a custom build so as to use the same TypeScript version as the TypeScript compiler, otherwise the compiler might support some new features while the editor shows them as invalid syntax.

This repo seems useful: https://github.com/wang12124468/monaco-editor-nls
May need to do some work to try adapt that to our build. I think we should keep that for later.

Another minor display issue (not related to i18n) found during my local test is that when hovering on the Click for info button on the bottom of the Change Language dropdown, the tooltip seems not being displayed correctly.

I did notice this. Let's wait after the UI changes get merged. May be the style changes would help fix this.

[zyf722]

This repo seems useful: https://github.com/wang12124468/monaco-editor-nls

Thanks for the reference. I'll take a look at it and see what we can do.

I did notice this. Let's wait after the UI changes get merged. May be the style changes would help fix this.

Sure. I just accidentally found this and thought it might be worth mentioning.