-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat: add German language support #241
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for putting this together.
Now that #234 has been merged there were significant changes to the locales, a handful of keys were removed and several more were added.
Here's the specific list of changes:
https://github.com/fmaclen/hollama/pull/234/files#diff-dfa2062fc0f5cfa7c4ef6179d7480621e7f0ab7a8a0e51fe25df994871186146
tests/settings.test.ts
Outdated
test.describe('German', () => { | ||
test.use({ locale: 'de-DE' }); | ||
test('default language is german', async ({ page }) => { | ||
await page.goto('/settings'); | ||
expect(await page.evaluate(() => navigator.language)).toBe('de-DE'); | ||
|
||
await page.evaluate(() => window.localStorage.clear()); | ||
await page.reload(); | ||
await expect(page.getByText('Current version')).not.toBeVisible(); | ||
await expect(page.getByText('Aktuelle Version')).toBeVisible(); | ||
expect(await page.evaluate(() => window.localStorage.getItem('hollama-settings'))).toContain( | ||
'"userLanguage":"de"' | ||
); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localization tests have been moved to locales.test.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved now - still need to adapt the messages though.
src/routes/settings/Interface.svelte
Outdated
{ value: 'en', label: 'English' }, | ||
{ value: 'zh-cn', label: '中文 (简体)' }, | ||
{ value: 'es', label: 'Español' }, | ||
{ value: 'pt-br', label: 'Português (Brasil)' }, | ||
{ value: 'de', label: 'Deutsch' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move Deutsch
under English
since Germany has the 2nd largest amount of users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed now.
By number of worldwide speakers
eba4639
to
b887435
Compare
3b99156
to
1376902
Compare
Did all the changes - can you please check again @fmaclen? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks @jhoermann!
🎉 This PR is included in version 0.23.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Closes #240