LiveCodes
Replies: 28 comments 50 replies
-
|
Thank you, @zyf722 for the suggestion. This is also related to providing a RTL layout, since a significant percentage of LiveCodes users have Arabic as their first language. |
Beta Was this translation helpful? Give feedback.
-
|
I have opened an issue for this here: |
Beta Was this translation helpful? Give feedback.
-
|
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:
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. |
Beta Was this translation helpful? Give feedback.
-
|
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 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 @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! |
Beta Was this translation helpful? Give feedback.
-
|
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.
exactly as you say, most UI strings can be found in those 2 folders. html files in 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. Thanks a lot for willing to help. |
Beta Was this translation helpful? Give feedback.
-
|
Here's some progress about infrastructure and HTML-related work. All changes could be checked here in my fork.
Infrastructure
HTML
As for static HTML, I checked Current workaround is to add Then, these elements will be processed in their container's corresponding handler in To preview, you may want to change 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 |
Beta Was this translation helpful? Give feedback.
-
|
@zyf722 I have some comments:
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. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for such a detailed explanation and guidance. I'll move the current logic to 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! |
Beta Was this translation helpful? Give feedback.
-
|
I will open a PR in your repo with the proposed changes when I have some time. |
Beta Was this translation helpful? Give feedback.
-
|
After checking bundle size, I found that the app minified bundle size increased from 313kb to 385kb after adding the i18n dependencies. The variable A helper function The type I have opened a PR in your repo with these changes |
Beta Was this translation helpful? Give feedback.
-
|
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 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. |
Beta Was this translation helpful? Give feedback.
-
|
I agree on removing language detection plugin. Other ways for language detection are either not needed or already provided by the app. Something else we need to consider. Currently the 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? |
Beta Was this translation helpful? Give feedback.
-
|
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. |
Beta Was this translation helpful? Give feedback.
-
Nice 👌
I think this is reasonable. We probably need to do this once here (we may need to turn it to |
Beta Was this translation helpful? Give feedback.
-
|
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 Are we going to use |
Beta Was this translation helpful? Give feedback.
-
|
In this case, I believe setting |
Beta Was this translation helpful? Give feedback.
-
|
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. |
Beta Was this translation helpful? Give feedback.
-
|
We may use Weblate to allow contributions for translation to different languages. 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. |
Beta Was this translation helpful? Give feedback.
-
|
I only have some experience with Crowdin and another similar platform, but Weblate seems to be a good choice as well 👍 |
Beta Was this translation helpful? Give feedback.
-
|
Crowdin also looks awesome. I have no experience with either. You choose the platform to use. |
Beta Was this translation helpful? Give feedback.
-
|
OK, then I might try starting with Crowdin first. |
Beta Was this translation helpful? Give feedback.
-
|
New progress here. Features discussed before have now been implemented, and some new features have been added. Changes
CommentsType-safe translationsThis 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 tagsAs 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 Abstract tags proposalBesides, containing a large <a> element like For example, we use 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. |
Beta Was this translation helpful? Give feedback.
-
|
As for pre-replaced values, we could utilize interpolation to replace the values in the string. Codes might behave similarly to the |
Beta Was this translation helpful? Give feedback.
-
|
@hatemhosny Could you please take a look here at your convenience? |
Beta Was this translation helpful? Give feedback.
-
|
Thank you. |
Beta Was this translation helpful? Give feedback.
-
|
Great work @zyf722 . Thank you. I like the type-safe solution. I have started another PR with the following changes:
const translation = {
welcome: 'Welcome',
hello: { textContent: 'Hello', title: 'Hello' }
}
Regarding your question, I like your proposal ( |
Beta Was this translation helpful? Give feedback.
-
|
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 |
Beta Was this translation helpful? Give feedback.
-
|
Done with abstract tags and intellisense enhancement. Please check commits for details.
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. |
Beta Was this translation helpful? Give feedback.
-
|
wow! this is impressive. some minor comments:
Nice 👍
This is fine. You may want to update the comment here to use the new type
Beautiful 😍 You may actually add I think the regex used here is too specific and can easily break with minor code change (e.g. if we later use
I like this indeed 👍 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 ( 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.
Very nice 💯 In my local setup, I have prettier configured to run on-save. I noticed some formatting changes (mostly whitspace formatting). Apart from these minor comments, I think we are ready. Please go ahead. |
Beta Was this translation helpful? Give feedback.
-
|
Got it. Thanks for suggestions on style and readability and I'll fix that soon :) |
Beta Was this translation helpful? Give feedback.
-
|
TODO: we need later on to add some e2e tests for significant cases. e.g.:
etc. Also, whenever you see suitable, you may start a [WIP] PR, so that we can run all CI checks early and fix anything that fails. And then you can keep pushing more changes. |
Beta Was this translation helpful? Give feedback.
-
|
We will also need to add something in the docs. A point for thought: May be add another screen for global app settings. |
Beta Was this translation helpful? Give feedback.
-
|
Or just change "Editor Settings" to "App Settings" and add them there? |
Beta Was this translation helpful? Give feedback.
-
|
Personally I'd prefer to have an independent App Settings screen with these appearance customization, including languages, about the app itself, while the Editor Settings continues to serve as options for the external editor only. This also reminds me that we need to take editors into consideration when it comes to i18n. Currently the context menu of monaco remains in English. Maybe some changes need to be done about it. |
Beta Was this translation helpful? Give feedback.
-
|
Needs some thinking. |
Beta Was this translation helpful? Give feedback.
-
|
I think some changes here are worth discussion. Further type-safety and nested keysType-safety and intellisense of
Translation processDuring add keys and translations for I also take collaboration and online translation platform into consideration. The main problem here is that I think it'd be much better if translators can see the actual HTML elements for abstract tags in the form of context, description or etc. (as shown in the screenshot)
However, i18next's key-translation JSON schema doesn't support this kind of comment information, which then drives me to come up with the following localization process:
Even myself must admit that I seem to have made this too complicated. However, there seems to be no better way to add context information except manually maintaining it as we use a custom format for translation files to have better type-safety and developing experience - we either have to give up providing context information for translators or make the process more complicated. Any ideas or suggestions shall be greatly appreciated. PS: After doing some research, I believe Crowdin has been a little bit too strict for open-source projects, so I then turn to Lokalise.
|
Beta Was this translation helpful? Give feedback.
-
|
Please check full commits here to get more details. |
Beta Was this translation helpful? Give feedback.
-
|
Some significant changes here :) |
Beta Was this translation helpful? Give feedback.
-
|
@zyf722 So I have to admit, this is impressive. However, it is getting quite complex. I like that contributors to translations can have much simpler workflow using Lokalise while having context, without having to mess with code. I also like the proposed GitHub actions workflow to automate incorporating the translations back into the code base. If there is no simpler way, we may just accept that, and clearly document the workflow in contribution docs here, and refer to it under "Specific topics" here. In this documentation we need to explain the full workflow (including when running scripts are required), with guides for:
I also have some comments for specific topics:
This is great in principle. However, I see we are not making use of this since we are just looping through the keys in
This is nice. const type = namespace === 'translation' ? 'I18nTranslationTemplate' : 'I18nTranslationTemplate';Also I think this is error-prone: const definition = JSON.stringify(trans[namespace], null, 2)
.replace(/"([^"]+)":/g, '$1:')
// .replace(/"(.*)\/\* COMMENTS \*\/(.*)"(,?)/g, "'$1'$3 // $2")
.replace(/"/g, "'");These are examples for how this can break: https://livecodes.io/?x=id/rxmym34u9va This type does not allow a tranlation file to be missing top-level keys while the translation is still in progress: type UnAsConst<T> = {
readonly [K in keyof T]: T[K] extends I18nAttributes | I18nTranslationTemplate
? RequireAtLeastOne<UnAsConst<T[K]>>
: string;
};consider wrapping it with This is indeed becoming a big project. However, I'm really grateful for you making this effort. |
Beta Was this translation helpful? Give feedback.
-
|
Oh, I could be sure that the You're right about using parsing instead. I indeed didn't notice that babel is a dependency of prettier. Such being the case, I will try to use prettier and babel instead. Thanks for pointing out this. I'll also fix the definition for
I don't think that In summary, thank you for your detailed reply. PS: I noticed that v27 was just released yesterday. Congrats! |
Beta Was this translation helpful? Give feedback.
-
|
Thank you. Yes, that was a relatively big release 😀
It might be more efficient to run the |
Beta Was this translation helpful? Give feedback.
-
|
We need to think of a way to keep the translations in sync. Consider the case when some text in HTML is changed. We need a check that detects that this key in all translations is no longer valid, so it should not be used and defaults to the text in HTML till the translations are updated. May be add a checksum for the source string with translation key. This check can only run with production build. The build should fail if this is the case till we mark that this key should be ignored, so that we are aware of this situation. Specially that maintainers may not be able to read different translations or update them on their own. |
Beta Was this translation helpful? Give feedback.
-
|
FYI |
Beta Was this translation helpful? Give feedback.
-
|
Looks great! Actually I've discovered a similar plugin (lokalise/i18n-ally) for VSCode before. Glad to see that there's a similar one for WebStorm. Thanks for sharing! 👍 |
Beta Was this translation helpful? Give feedback.
-
|
These days I fixed few typo alongside small mistakes, and more importantly, briefly reviewed the entire thread and now have a to-do list below (ones with higher priority are listed first):
Here are some comments on part of the tasks and need your help to clarify them: Solution for
|
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the explanation and I'll try looking into these two opitons. The list will be updated once relavant changes are finished and commited :) |
Beta Was this translation helpful? Give feedback.
-
|
I'm actually more in favour of the custom events approach. You may even add a helper function (similar to the |
Beta Was this translation helpful? Give feedback.
-
|
Here is my event-driven translation solution. I add a custom event type How do you think about this solution? Any suggestions or improvements? |
Beta Was this translation helpful? Give feedback.
-
|
This is nice. Thank you. |
Beta Was this translation helpful? Give feedback.
-
|
Thank you for your suggestions. I have fixed these issues in the latest commit. |
Beta Was this translation helpful? Give feedback.
-
|
Got a response from Lokalise support team. There are a couple of questions that need to be clarified before they can approve the open-source plan:
They also points out that it will be appreciated if we could spread the word about Lokalise on X and other media. Original message: |
Beta Was this translation helpful? Give feedback.
-
No, LiveCodes is fully open-source under MIT license.
LiveCodes has a sponsorship program to support its development: https://livecodes.io/docs/sponsor
I'm happy to add a logo with a link to Lokalise in the credits section in GitHub readme and the docs website. |
Beta Was this translation helpful? Give feedback.
-
|
The Lokalise support has granted the open source plan for us, and I will start integration related work soon. I suggest you register a Lokalise account (maybe using the organization e-mail) so that I can add you to the project and set you up as the team owner. I've also asked the support team about the renewal and whether the plan will expire after a certain period of time:
|
Beta Was this translation helpful? Give feedback.
-
|
That's great. Thank you. |
Beta Was this translation helpful? Give feedback.
-
|
Added 🆗
|
Beta Was this translation helpful? Give feedback.
-
|
Hmm. Found something tricky to deal with. Some values of attributes in those dynamically-added elements are only determined at runtime (like here), which means that there is no simple way to extract their default value through a static analysis. This makes it hard to integrate with current Moreover, our current i18n mechanism is element-level (using PS. The work on Lokalise is progressing smoothly. I turned abstract tags in
|
Beta Was this translation helpful? Give feedback.
-
My suggestion for both of these problems is to add a This can then be added to this object which is globally available in the window object: livecodes/src/livecodes/core.ts Line 4514 in b66c2f8 const initApp = async (config: Partial<Config>, baseUrl: string) => {
window.deps = {
showMode,
translateString, // pass here the helper function (only in the app build)
};
// rest of code
};
const initEmbed = async (config: Partial<Config>, baseUrl: string) => {
window.deps = {
showMode,
translateString: (str: string) => str, // use a mock in other builds
};
// rest of code
};
// also in initLite and initHeadlessthis allows using it elsewhere without having to pass reference to the logOutLink.dataset.hint = window.deps.translateString('Logged in as ') + displayName;
That's very nice indeed. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for pointing out this solution and this is a great way to handle the translation. In my opinon, instead of using concatenation: window.deps.translateString('Logged in as ') + displayName;I think it could be better to use both key and value while utilizing interpolation: window.deps.translateString('login.loggedIn.hint', 'Logged in as {{name}}', {name: displayName})With export const translateString = (i18n: typeof import('./i18n').default, key: string, value: string, interpolation?: any) => {
// ...
return i18n.t(key, value, interpolation) as string;
}Using a fixed key can prevent unexpected access to a string with same value but different context. Also, it's easier to maintain and update the translation file using the Besides, it seems that it is still necessary to pass i18n instance to the function so that it can call window.deps = {
showMode,
translateString: (key: string, value: string, interpolation?: any) => translateString(i18n, key, value, interpolation)
}; |
Beta Was this translation helpful? Give feedback.
-
|
I agree. const translateString = (
_key: string,
value: string,
interpolation: { [s: string]: string },
) => {
for (const [k, v] of Object.entries(interpolation)) {
value = value.replaceAll(`{{${k}}}`, v);
}
return value;
}; |
Beta Was this translation helpful? Give feedback.
-
|
New progress in these days. Check here to see all changes. String-level translationAdded the string-level translation and enhance its intellisense and static checking with type-safety (Reused some code from Now we can use window.deps.translateString('namespace:file.key1.subkey1', 'default <strong>value</strong>, {{interpol}}', {
isHTML: true,
interpol: "abc"
})The function is completely type-safe:
To provide better readability and maintainability, only string-level translation will be used in Marked all strings in
|
Beta Was this translation helpful? Give feedback.
-
|
PS. Don't know if keys (here to take a glance) are elegant and neat enough. Please feel free to suggest any improvements. |
Beta Was this translation helpful? Give feedback.
-
|
@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! |
Beta Was this translation helpful? Give feedback.
-
|
hi @zyf722 |
Beta Was this translation helpful? Give feedback.
-
|
wow! I like the added type-safety and extracting data from ts files (using babel!). Impressive. I reviewed the changes and have some comments:
I have no problem with the namings of the keys. Actually, I find them descriptive. 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 want to know your thoughts. |
Beta Was this translation helpful? Give feedback.
-
Firstly, it's my pleasure to contribute to this project. I'm glad you like the changes and the new features :)
I'll check and fix these issues as soon as possible. Thanks for pointing them out.
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, 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
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. |
Beta Was this translation helpful? Give feedback.
-
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.
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.
Thank you very much. I really appreciate the effort. |
Beta Was this translation helpful? Give feedback.
-
|
New updates. All commits are listed here.
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:
|
Beta Was this translation helpful? Give feedback.
-
|
Please note that the loading screen runs in the main page, while the app runs in an iframe. In core.ts, after you load the i18n module you can do something like: if (!isEmbed) {
parent.postMessage({ args: 'i18n', payload: { /* pass translation data here */ } }, location.origin);
}then in main.ts, you can add: if (e.data?.args === 'i18n') {
// save payload to localStorage
}and in index.ts if (!isEmbed) {
// check for values in localStorage and manipulate the element `loadingText`
} |
Beta Was this translation helpful? Give feedback.
-
|
Done with loading screen translation (zyf722/livecodes@1ff478d). I noticed that in |
Beta Was this translation helpful? Give feedback.
-
|
Thank you. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for your prompt comments. Necessary changes has now been made in the last three commits. |
Beta Was this translation helpful? Give feedback.
-
|
even more :) |
Beta Was this translation helpful? Give feedback.
-
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?
Beta Was this translation helpful? Give feedback.
All reactions