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

add localization support #523

Merged
merged 20 commits into from Jun 22, 2023
Merged

add localization support #523

merged 20 commits into from Jun 22, 2023

Conversation

L1cardo
Copy link
Contributor

@L1cardo L1cardo commented Jun 16, 2023

#463
Works well on macOS, but need to be tested on Win and Linux.
image
image

@justinlampley
Copy link
Collaborator

Overall it looks very good!

I'll look at doing a more comprehensive review later, but there are a few things I noticed after a quick look through the changes.

  • There are a few missing entries from the translations file, like ConfiguratorView.SelectFirmwareVersionFirst and SerialDeviceSelect.ManualSelection. There may be more, but those are the ones I immediately noticed.

  • For more complex text that includes react components, look at using the Trans component provided by react-i18next (https://react.i18next.com/latest/trans-component). An example of where that component would be useful is the error message in the BuildResponse component. Instead of breaking out the text into multiple translation entries and interweaving those between the link components, you can embed the components in the text which gives translators the flexibility of where the link components appear within the text for a particular language. As an example, here is the build response error alert using the Trans component.

  <Alert sx={styles.errorMessage} severity="error">
	<AlertTitle>
	  {toTitle(
		response?.errorType ?? BuildFirmwareErrorType.GenericError
	  )}
	</AlertTitle>
	<p>
	  <Trans
		i18nKey="BuildResponse.ErrorMessage"
		defaults="An error has occured, see the above log for the exact error message. If you have not already done so, visit <ExpressLRSLink>Expresslrs.org</ExpressLRSLink> and read the <FlashingGuideLink>Flashing Guide</FlashingGuideLink> for your particular device as well as the <TroubleshootingGuideLink>Troubleshooting Guide</TroubleshootingGuideLink>.  If you are still having issues after reviewing the documentation, please copy the build logs above to an online paste site and post in the #help-and-support channel on the <DiscordLink>ExpressLRS Discord</DiscordLink> with a link to the logs and other relevant information like your device, which flashing method you were using, and what steps you have already taken to resolve the issue."
		components={{
		  ExpressLRSLink: (
			<DocumentationLink url="https://www.expresslrs.org/" />
		  ),
		  FlashingGuideLink: (
			<DocumentationLink url="https://www.expresslrs.org/quick-start/getting-started/" />
		  ),
		  TroubleshootingGuideLink: (
			<DocumentationLink url="https://www.expresslrs.org/quick-start/troubleshooting/#flashingupdating" />
		  ),
		  DiscordLink: (
			<DocumentationLink url="https://discord.gg/dS6ReFY" />
		  ),
		}}
	  />
	</p>
  </Alert>
  • yarn.lock was changed a lot and the registry was changed from yarnpkg.com to npmjs.org, the only changes that should have occurred in this file is the addition of the new packages to support react-i18next. It is quite strange that running yarn install to add the new packages would have changed the lock file that much.

src/app/updater.ts Outdated Show resolved Hide resolved
src/app/updater.ts Outdated Show resolved Hide resolved
src/ui/components/BuildResponse/index.tsx Outdated Show resolved Hide resolved
src/ui/components/BuildResponse/index.tsx Outdated Show resolved Hide resolved
src/ui/components/BuildResponse/index.tsx Outdated Show resolved Hide resolved
src/ui/components/FirmwareVersionForm/index.tsx Outdated Show resolved Hide resolved
src/ui/components/UserDefinesAdvisor/index.tsx Outdated Show resolved Hide resolved
src/ui/components/WifiDeviceSelect/index.tsx Outdated Show resolved Hide resolved
src/ui/views/ConfiguratorView/index.tsx Outdated Show resolved Hide resolved
src/ui/components/Header/index.tsx Outdated Show resolved Hide resolved
2. fix typos
3. complex text that includes react components
4.
@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 17, 2023

  1. added missing localization entries
  2. fixed typos
  3. improved complex text that includes react components

src/i18n/en/translation.json Outdated Show resolved Hide resolved
src/ui/components/BuildResponse/index.tsx Outdated Show resolved Hide resolved
src/ui/components/UserDefineDescription/index.tsx Outdated Show resolved Hide resolved
2. Fixed a typo in translation file en.json
3. Improved one zh-CN localization.
@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 21, 2023

  1. Removed default text value for <Trans> component
  2. Fixed a typo in translation file en.json
  3. Improved one zh-CN localization.

src/i18n/i18n.ts Outdated Show resolved Hide resolved
src/i18n/i18n.ts Outdated Show resolved Hide resolved
src/i18n/i18n.ts Outdated Show resolved Hide resolved
src/index.tsx Outdated Show resolved Hide resolved
src/ui/components/BuildResponse/index.tsx Outdated Show resolved Hide resolved
src/ui/views/SettingsView/index.tsx Show resolved Hide resolved
@jurgelenas
Copy link
Member

@L1cardo can you please look into making translation type-safe according to https://www.i18next.com/overview/typescript ?

@jurgelenas
Copy link
Member

I have created a project on crowdin.com: https://crowdin.com/project/expresslrs-configurator

Will work on this: https://support.crowdin.com/github-integration/

@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 21, 2023

  1. renamed i18n.ts to index.ts
  2. removed TODO: translations
  3. i18next debug true when only 'debug'
  4. made translation type-safe
  5. added settings view and a simple language switcher(I am really unfamiliar with ts coding, so this was very difficult for me, I wanted to add a drop down menu but I failed, so maybe really "programmers" can work on this)
image image

@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 21, 2023

I have created a project on crowdin.com: crowdin.com/project/expresslrs-configurator

Will work on this: support.crowdin.com/github-integration

For github and crowding integration, crowdin official integration only sync at the first time and we should manually sync every time after. https://support.crowdin.com/github-integration/#uploading-translations-from-repo.
For auto sync, you can take a look at how BetaFlight does.
https://github.com/betaflight/betaflight-configurator/blob/master/.github/workflows/translations-pr.yml
https://github.com/betaflight/betaflight-configurator/blob/master/.github/workflows/translations-upload.yml

@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 21, 2023

make ELRS Configurator title localizable
image

@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 21, 2023

Fixed icon and title are not aligned center.

Before:
image

After:
image

@jurgelenas
Copy link
Member

@L1cardo can you resolve merge conflicts?

src/ui/views/SettingsView/index.tsx Outdated Show resolved Hide resolved
src/@types/i18next.d.ts Show resolved Hide resolved
@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 22, 2023

  1. resolved conflicts and update to latest code base
  2. added mising localizations

@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 22, 2023

  1. added language switcher and system info
  2. added src/@types and node_modules/@types
image image

@jurgelenas jurgelenas merged commit f6ccf9a into ExpressLRS:master Jun 22, 2023
@L1cardo
Copy link
Contributor Author

L1cardo commented Jun 22, 2023

I forgot to add i18n to resources or distribution app won't have i18n files, really sorry for that.
Here is the solution:

  1. loadPath: './i18n/{{lng}}/translation.json',

    This should be loadPath: process.env.NODE_ENV === 'production' ? '../src/i18n/{{lng}}/translation.json' : './i18n/{{lng}}/translation.json',
image
  1. "extraResources": [

    Should add "./src/i18n/**" to extraResources
image

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.

None yet

4 participants