Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Internationalization (i18n) support to Appwrite console. #451

Closed
wants to merge 4 commits into from

Conversation

goswamianshuman
Copy link

What does this PR do?

  • adding i18n support to the console.

Test Plan

  • no code structure was changed.

Related PRs and Issues

@vercel
Copy link

vercel bot commented May 22, 2023

@goswamianshuman is attempting to deploy a commit to the appwrite Team on Vercel.

A member of the Team first needs to authorize it.

@goswamianshuman
Copy link
Author

@TorstenDittmann @TGlide please have a look over this PR, the provided i18n is available in German and English.

the main dashboard is not having any changes but you can have a look over the other files.

Copy link
Contributor

@TGlide TGlide left a comment

Choose a reason for hiding this comment

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

Really nice start! Not sure if I missed it, is there any way to change the language within the console?

@@ -4,7 +4,8 @@
"node": ">=16"
},
"scripts": {
"dev": "vite dev",
"dev": "vite",
"vite": "vite dev",
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed

Copy link
Author

Choose a reason for hiding this comment

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

sure, I was testing different libraries so I did the following changes. I'll make it back to the original.

Comment on lines 115 to 119
{#if !$isLoading}
<slot />
{:else if $loading}
<Loading />
{/if}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{#if !$isLoading}
<slot />
{:else if $loading}
<Loading />
{/if}
{#if $isLoading || $loading}
<Loading />
{/if}

package.json Outdated
@@ -30,6 +31,7 @@
"logrocket": "^3.0.1",
"pretty-bytes": "^6.1.0",
"prismjs": "^1.29.0",
"svelte-i18n": "^3.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should discuss if this is the best library for the console. @TorstenDittmann has some features he'd like, and it also seems unmaintained.

Copy link
Member

Choose a reason for hiding this comment

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

I agree I would use i18next directly without relying on the svelte version

@@ -40,28 +41,29 @@
</script>

<svelte:head>
<title>Accept invite - Appwrite</title>
<title>{$_('accept_invite.title')} - Appwrite</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, does the $_ store have autocomplete?

Copy link
Author

Choose a reason for hiding this comment

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

Out of curiosity, does the $_ store have autocomplete?

No, Here _ is a function that takes a string input. and this string is the path of data in the JSON.

@goswamianshuman
Copy link
Author

goswamianshuman commented May 23, 2023

Really nice start! Not sure if I missed it, is there any way to change the language within the console?

Hey Thomas, I will be working on it today. 💪

package.json Outdated
"pretty-bytes": "^6.1.0",
"prismjs": "^1.29.0",
"svelte-i18next": "^2.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Remember to remove this one 👍

(I think we can remove npm-run-all)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I used it before while testing other libraries. All the unwanted libraries will be removed from package.json 🙂

Comment on lines +111 to +114
// $: {
// const getUserLanguage = window.navigator.language;
// console.log(getUserLanguage);
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, how is language initialized and changed in this implementation?

@@ -40,28 +41,29 @@
</script>

<svelte:head>
<title>Accept invite - Appwrite</title>
<title>{$_.t('accept_invite.title')} - Appwrite</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot, is this typesafe? E.g. if I pass in a string that doesn't exist, will the IDE (not the server) return an error?

Copy link
Author

@goswamianshuman goswamianshuman Jul 10, 2023

Choose a reason for hiding this comment

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

@TGlide I think now we are working with pull request #459, so we have stopped working on this PR we can close this PR, but firstly we need to discuss it with @TorstenDittmann.

@goswamianshuman
Copy link
Author

Hello, @TorstenDittmann I think we don't need this PR anymore already we have continued with #459 so I'm closing it for now we can open it again if we require to make any new changes to it.😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants