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 translation support using i18next #681

Open
wants to merge 26 commits into
base: dev
Choose a base branch
from
Open

Conversation

Airyzz
Copy link

@Airyzz Airyzz commented Jul 14, 2022

Description

Adds support for new languages as per #164

Adds i18next, i18next-browser-languagedetector, i18next-http-backend, react-i18next as dependencies

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Preview: https://62fa1243e46824314616a3e3--pr-cinny.netlify.app
⚠️ Exercise caution. Use test accounts. ⚠️

Dylan Van Nielen and others added 11 commits June 14, 2022 18:15
Only implemented to a few pages as an example, looking to get feedback.

I do not know japanese, I just used google translate to translate a few words as an example - is most likely incorrect
Implemented the change to SpaceSettings as an example.
There was a 'jp' translation that I used to test
@Airyzz
Copy link
Author

Airyzz commented Jul 16, 2022

It seems the netlify preview 404s when trying to retrieve the translation.json file. Is this a problem with the netlify set up or something that I need to fix?

@menturion
Copy link

Did you check whether the path given in the 404 error corresponds with the path you expected to be set within the code?

@Airyzz
Copy link
Author

Airyzz commented Jul 16, 2022

Its the same path that gets requested when running locally, but netlify 404s while running locally works fine. If somebody could test running on their machine that might help to narrow to down?

@Airyzz
Copy link
Author

Airyzz commented Jul 17, 2022

I think it might actually be an issue with the webpack, looking in to it now

@kfiven
Copy link
Collaborator

kfiven commented Jul 17, 2022

You need to copy them to dist directory during build, see

new CopyPlugin({

@Airyzz
Copy link
Author

Airyzz commented Jul 17, 2022

At the moment I have it working using i18next-loader, would it be preferable to copy?

test: /locales/,

@kfiven
Copy link
Collaborator

kfiven commented Jul 17, 2022

No, you are doing it right with i18next-loader.

@Chatner-Dev
Copy link

That is such a needed feature, cant wait for merge

@ajbura ajbura self-requested a review August 3, 2022 14:31
Copy link
Member

@ajbura ajbura left a comment

Choose a reason for hiding this comment

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

Thanks. BTW please don't such big PR in future. 🙏

src/app/molecules/image-pack/ImagePack.jsx Outdated Show resolved Hide resolved
src/app/molecules/message/Message.jsx Outdated Show resolved Hide resolved
src/app/molecules/room-members/RoomMembers.jsx Outdated Show resolved Hide resolved
src/app/organisms/invite-user/InviteUser.jsx Outdated Show resolved Hide resolved
Comment on lines -22 to -68
const failedDialog = () => {
const renderFailure = (requestClose) => (
<div className="cross-signing__failure">
<Text variant="h1">{twemojify('❌')}</Text>
<Text weight="medium">Failed to setup cross signing. Please try again.</Text>
<Button onClick={requestClose}>Close</Button>
</div>
);
import '../../i18n';

openReusableDialog(
<Text variant="s1" weight="medium">Setup cross signing</Text>,
renderFailure,
);
};
function CrossSigningSetup() {
const { t } = useTranslation();

const securityKeyDialog = (key) => {
const downloadKey = () => {
const blob = new Blob([key.encodedPrivateKey], {
type: 'text/plain;charset=us-ascii',
});
FileSaver.saveAs(blob, 'security-key.txt');
};
const copyKey = () => {
copyToClipboard(key.encodedPrivateKey);
};
const initialValues = { phrase: '', confirmPhrase: '' };
const [genWithPhrase, setGenWithPhrase] = useState(undefined);

const renderSecurityKey = () => (
<div className="cross-signing__key">
<Text weight="medium">Please save this security key somewhere safe.</Text>
<Text className="cross-signing__key-text">
{key.encodedPrivateKey}
</Text>
<div className="cross-signing__key-btn">
<Button variant="primary" onClick={() => copyKey(key)}>Copy</Button>
<Button onClick={() => downloadKey(key)}>Download</Button>
const failedDialog = () => {
const renderFailure = (requestClose) => (
<div className="cross-signing__failure">
<Text variant="h1">{twemojify('❌')}</Text>
<Text weight="medium">{t('Organisms.CrossSigning.setup_failed')}</Text>
<Button onClick={requestClose}>{t('common.close')}</Button>
</div>
</div>
);
);

// Download automatically.
downloadKey();
openReusableDialog(
<Text variant="s1" weight="medium">{t('Organisms.CrossSigning.setup')}</Text>,
renderFailure,
);
};

openReusableDialog(
<Text variant="s1" weight="medium">Security Key</Text>,
() => renderSecurityKey(),
);
};
Copy link
Member

Choose a reason for hiding this comment

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

Can't we do translation without moving these inside component?

Copy link
Author

Choose a reason for hiding this comment

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

I think it needs to be inside component for the t() hook to function properly. Is there a particular reason to not have these inside component?

Copy link
Contributor

Choose a reason for hiding this comment

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

you can use t(), but <Trans i18nKey="" /> seems to work

src/app/organisms/settings/DeviceManage.jsx Outdated Show resolved Hide resolved
@Airyzz Airyzz requested a review from ajbura August 19, 2022 00:44
@2-www
Copy link

2-www commented Oct 3, 2022

hi! sorry, i am new to web dev, what program would i use to translate this format?

@Airyzz
Copy link
Author

Airyzz commented Oct 4, 2022

hi! sorry, i am new to web dev, what program would i use to translate this format?

At the moment we dont have a translation platform set up. It will be easier if you wait for that to be finalised, but if you want to work on it right now, just fork this repo and the language files are just .json so you can edit with any text editor

@notramo
Copy link

notramo commented Oct 6, 2022

@aybura Could you please use the open-source Weblate platform for translating this? Using closed platforms like Transifex or Crowdin prevents some potential translators to contribute (including myself as I have a Weblate account, but not Transifex or Crowdin).

@Airyzz
Copy link
Author

Airyzz commented Oct 7, 2022

@notramo discussion involving translation platforms has been taking place in #164

@Maders
Copy link

Maders commented Oct 16, 2022

Hi,
@ajbura According to your last review comments, which were from 2 months ago, have you any plans to merge this PR?

@aceArt-GmbH
Copy link
Contributor

@Airyzz could you perhaps rebase this PR to a more recent commit (doesn't have to be latest), or at least squash your commits?
Thanks for your work on this

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.

9 participants