Improve i18n support and add missing translations #6070
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The localization is very incomplete. For instance, look at these two screens when Japanese is selected as a language: 
Only one part of the screen is actually translated in the two main screens. All of the English should be converted into the appropriate language when a different language is selected. |
neubig
commented
Jan 6, 2025
- Add missing translations for all supported languages - Add i18n test utilities and translation tests - Fix duplicate key detection in tests - Update UI components to use translations
neubig
changed the title
- Add missing translation keys for browser, user avatar, and suggestions - Add translations for all supported languages - Create test to catch hardcoded English strings - Fix components to use translation system properly
- Remove CHAT$WHAT_DO_YOU_WANT_TO_BUILD and LANDING$BUILD_PROMPT in favor of SUGGESTIONS$WHAT_TO_BUILD - Remove unused keys: - STATUS$CONNECTED_TO_SERVER - BROWSER$SCREENSHOT and BROWSER$SCREENSHOT_ALT - SUGGESTIONS$AUTO_MERGE and SUGGESTIONS$AUTO_MERGE_PRS - USER$AVATAR_PLACEHOLDER (duplicate)
- Remove WORKSPACE$LABEL (unused) - Remove CONNECT_TO_GITHUB_BY_TOKEN_MODAL$TERMS_OF_SERVICE (unused)
…penHands into add-japanese-translations
|
@amanape , apologies that this is a huge PR but hopefully the logic is relatively straightforward. |
amanape
reviewed
Jan 7, 2025
| expect(uniqueDuplicates).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should have consistent translations for each key', () => { |
Comment on lines
+22
to
+23
| describe('Translations', () => { | ||
| describe('Translation Coverage', () => { |
There was a problem hiding this comment.
Nested describe, we can unpack to have under a single describe since there are no other describe suites present.
Comment on lines
+15
to
+33
| const { container } = render( | ||
| <InteractiveChatBox | ||
| onSubmit={() => {}} | ||
| onStop={() => {}} | ||
| /> | ||
| ); | ||
|
|
||
| // Get all text content | ||
| const text = container.textContent; | ||
|
|
||
| // List of English strings that should be translated | ||
| const hardcodedStrings = [ | ||
| "What do you want to build?", | ||
| ]; | ||
|
|
||
| // Check each string | ||
| hardcodedStrings.forEach(str => { | ||
| expect(text).not.toContain(str); | ||
| }); |
There was a problem hiding this comment.
Suggested change
| const { container } = render( | |
| <InteractiveChatBox | |
| onSubmit={() => {}} | |
| onStop={() => {}} | |
| /> | |
| ); | |
| // Get all text content | |
| const text = container.textContent; | |
| // List of English strings that should be translated | |
| const hardcodedStrings = [ | |
| "What do you want to build?", | |
| ]; | |
| // Check each string | |
| hardcodedStrings.forEach(str => { | |
| expect(text).not.toContain(str); | |
| }); | |
| render(<InteractiveChatBox onSubmit={() => {}} onStop={() => {}} />); | |
| expect(screen.queryByText("What do you want to build?")).not.toBeInTheDocument(); |
It is not necessary to introduce an array since it contains a single value only, and I doubt this test will be extended in the foreseeable future.
| @@ -0,0 +1,47 @@ | |||
| import { render } from "@testing-library/react"; | |||
There was a problem hiding this comment.
Suggested change
| import { render } from "@testing-library/react"; | |
| import { render, screen } from "@testing-library/react"; |
Comment on lines
+37
to
+45
| const { container } = render( | ||
| <ChatInput | ||
| onSubmit={() => {}} | ||
| /> | ||
| ); | ||
|
|
||
| // The placeholder should be a translation key, not English text | ||
| const textarea = container.querySelector("textarea"); | ||
| expect(textarea?.placeholder).toBe("SUGGESTIONS$WHAT_TO_BUILD"); |
There was a problem hiding this comment.
Suggested change
| const { container } = render( | |
| <ChatInput | |
| onSubmit={() => {}} | |
| /> | |
| ); | |
| // The placeholder should be a translation key, not English text | |
| const textarea = container.querySelector("textarea"); | |
| expect(textarea?.placeholder).toBe("SUGGESTIONS$WHAT_TO_BUILD"); | |
| render(<ChatInput onSubmit={() => {}} />); | |
| screen.getByPlaceholderText("SUGGESTIONS$WHAT_TO_BUILD"); |
There was a problem hiding this comment.
Are these changes necessary? They seem repetitive and it previously worked without them
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves the internationalization (i18n) support in the frontend by:
Adding missing translations for all supported languages (zh-CN, zh-TW, ko-KR, de, no, it, pt, es, ar, fr, tr) for:
Adding i18n test utilities and translation tests:
Updating UI components to use translations consistently
Fixes #6066
Fixes #4280
Here are examples of the localized screens. I have confirmed that all settings menus, etc. are localized.
To run this PR locally, use the following command: