Apply Element language to Jitsi Widget #23412

Fox32 · 2022-10-04T11:56:21Z

This PR is ready, but I will keep it as a Draft as it requires matrix-org/matrix-react-sdk#9346.

This PR translates the start screen of the Jitsi Widget to the current language. It also supplies the current Element language to Jitsi, so that the UI is in the same language as Element.

Previously the start screen of the Jitsi Widget was hard-coded to English only. Jitsi itself auto-detected the language from the browser, so that the Element language had no influence.

Could be best tested with Cypress, but that isn't available here, right?

Checklist

Tests written for new code (and old code if feasible)
Linter and other CI checks pass
Sign-off given on the changes (see CONTRIBUTING.md)

Type: enhancement

Here's what your changelog entry will look like:

✨ Features

Apply Element language to Jitsi Widget (#23412). Contributed by @Fox32.

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>

Fox32 · 2022-10-05T19:17:12Z

src/i18n/strings/de_DE.json

@@ -17,6 +17,9 @@
    "Failed to start": "Start fehlgeschlagen",
    "%(brand)s Desktop (%(platformName)s)": "%(brand)s Desktop (%(platformName)s)",
    "%(appName)s (%(browserName)s, %(osName)s)": "%(appName)s (%(browserName)s, %(osName)s)",
+    "Failed to load Jitsi widget": "Das Jitsi Widget konnte nicht geladen werden",
+    "Jitsi Video Conference": "Videokonferenz",
+    "Join Conference": "Videokonferenz beitreten",
    "Please install <chromeLink>Chrome</chromeLink>, <firefoxLink>Firefox</firefoxLink>, or <safariLink>Safari</safariLink> for the best experience.": "Bitte installiere <chromeLink>Chrome</chromeLink>, <firefoxLink>Firefox</firefoxLink> oder <safariLink>Safari</safariLink> für das beste Erlebnis.",


As far as I understand, these changes are actually not allowed manually. I would like to keep them during the review phase as it is otherwise hard to test. I can remove them before a merge and submit them via the translation portal.

Yes, that is why https://github.com/vector-im/element-web/actions/runs/3183049256/jobs/5209163177 fails.

You should just run yarn run i18n after modifying translatable strings. For tests you may modify them locally.

turt2live

generally this is looking good - will check the webpack bundle size this week.

turt2live · 2022-10-05T20:32:50Z

src/vector/jitsi/index.html

@@ -10,7 +10,6 @@
    <div class="joinConferenceFloating">
        <div class="joinConferencePrompt">
            <span class="icon"><!-- managed by CSS --></span>
-            <!-- TODO: i18n -->


The h2 still isn't translated though?

It is:

https://github.com/vector-im/element-web/blob/d9431e0d5956be978aafdfb632d4896a3ae16a42/src/vector/jitsi/index.ts#L235

I can't do it inline, but I'm applying it later as soon as a I know the current language.

a comment to indicate that would be appreciated :)

turt2live · 2022-10-05T20:33:46Z

src/vector/jitsi/index.ts

@@ -27,6 +27,7 @@ import {
 import { ElementWidgetActions } from "matrix-react-sdk/src/stores/widgets/ElementWidgetActions";
 import { logger } from "matrix-js-sdk/src/logger";
 import { IConfigOptions } from "matrix-react-sdk/src/IConfigOptions";
+import { _t, setLanguage } from "matrix-react-sdk/src/languageHandler";


we'll have to double check that this doesn't cause the bundle to rapidly expand, as it was a major concern last time we tried to add translations to the widget.

Good point, sadly I would say the bundle sizes explodes: From 400k to 6mb…

It pulls in the whole matrix-react-sdk, which is huge.

Related: #12839

Tree Shaking would be great, but that is not an easy change. Not sure if can refactor the language handler to be better useable. Building a custom translation solution for this widget doesn't feel right. I guess we have to postpone this PR.

If we can better isolate the language handling code, I think that would be good. From inspecting the bundle it appears to be pulling in the js-sdk and a bunch of its dependencies, which would explain the size (the obvious indicator was the EventType enum).

Looks like this will all be because of the SettingsStore call in the language functions

It's not just SettingsStore, ModuleRunner also pulls everything in. If I remove both, the bundle size is back to the previous size.

The good thing is that we don't need to read/write the settings for the Jitsi widget. It would be good if the SettingsStore was just a wrapper interface around an actual implementation, either as a NoopSettingsStore for the Jitsi Widget, or the full store for the rest? The actual implementation could be inserted on startup. What do you think?

For ModuleRunner, I guess the problem comes from ProxiedModuleApi, but I haven't discovered the exact problem yet.

weeman1337 · 2023-02-02T10:45:19Z

@Fox32 what is the status of this pull request? If you solved all issues and you think this PR is ready to be reviewed, please request a re-review.

Fox32 · 2023-02-02T11:01:19Z

I plan to pull out at least the changes that apply the language to Jitsi, I think the rest of the PR is unlikely to be accepted due to the bundle size issues?

Fox32 · 2023-02-22T08:19:08Z

I pulled out passing the language into #24609

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>

Fox32 · 2023-02-22T19:04:06Z

I update this PR to the latest state and resolved some of the comments above. In the meantime I did #24609 which moves passing the language to Jitsi out of this PR. This PR is now only about applying the translations to the widget itself (error messages and welcome page).

As discussed above, including the translation system of Element increases the bundle size significantly. I'm probably not the right one to refactor the matrix-react-sdk in a way that avoid this.

We also started to use the skip_built_in_welcome_screen option for Element which actually removes the need for having the translations at all, so my desire to resolve this issue isn't that high anymore.

Therefore, if we don't plan to merge it as-is, I would suggest to just close the PR 😁.
In case we want to merge it, we have to fix the German translation file first, that still contains my test data.

robintown

I would also suggest to close this PR, since the increase in bundle size is a bit larger than I'm comfortable with, and the fact that Jitsi is planned for deprecation makes me hesitant to suggest investing much more time in this.

t3chguy · 2023-02-24T14:47:01Z

Closing - though it really is a shame that our languageHandler isn't more standalone and just given the parameters it needs rather than fetching them from SettingStore, should really be a class with a singleton or DI.

@Fox32 thanks for splitting out the non-contentious part of the PR and getting it landed!!

Fox32 · 2023-02-24T17:07:25Z

Thanks for the responses 😊

github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Oct 4, 2022

Fox32 mentioned this pull request Oct 4, 2022

Pass language to Jitsi Widget matrix-org/matrix-react-sdk#9346

Merged

3 tasks

Fox32 force-pushed the nic/feat/jitsi-translate branch 3 times, most recently from bd1a41e to b54324c Compare October 4, 2022 14:34

Fox32 changed the title ~~Translate jitsi widget~~ Apply Element language to Jitsi Widget Oct 4, 2022

Translate jitsi widget

d9431e0

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>

Fox32 force-pushed the nic/feat/jitsi-translate branch from b54324c to d9431e0 Compare October 4, 2022 14:42

github-actions bot added the T-Enhancement label Oct 4, 2022

Fox32 marked this pull request as ready for review October 5, 2022 19:15

Fox32 requested a review from a team as a code owner October 5, 2022 19:15

Fox32 requested review from germain-gg, turt2live and weeman1337 October 5, 2022 19:15

Fox32 commented Oct 5, 2022

View reviewed changes

turt2live reviewed Oct 5, 2022

View reviewed changes

turt2live enabled auto-merge (squash) October 5, 2022 20:48

turt2live disabled auto-merge October 6, 2022 19:18

germain-gg removed their request for review December 5, 2022 08:56

Fox32 mentioned this pull request Feb 22, 2023

Pass current language into Jitsi (#24609 #24609

Merged

3 tasks

Fox32 added 3 commits February 22, 2023 19:54

Merge branch 'develop' into nic/feat/jitsi-translate

5c3dcd1

Fix translation file

670df1c

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>

Fix missing return types

1fa3e28

Signed-off-by: Oliver Sand <oliver.sand@nordeck.net>

Fox32 requested review from turt2live and removed request for weeman1337 February 22, 2023 19:04

turt2live removed their request for review February 22, 2023 19:24

turt2live requested review from a team, robintown and artcodespace and removed request for a team February 22, 2023 19:24

robintown requested changes Feb 24, 2023

View reviewed changes

t3chguy closed this Feb 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply Element language to Jitsi Widget #23412

Apply Element language to Jitsi Widget #23412

Fox32 commented Oct 4, 2022 •

edited by github-actions bot

Loading

Fox32 Oct 5, 2022

weeman1337 Oct 6, 2022

turt2live left a comment

turt2live Oct 5, 2022

Fox32 Oct 6, 2022

turt2live Oct 6, 2022

turt2live Oct 5, 2022

Fox32 Oct 6, 2022

Fox32 Oct 6, 2022

Fox32 Oct 6, 2022 •

edited

Loading

turt2live Oct 6, 2022

turt2live Oct 6, 2022

Fox32 Oct 8, 2022

weeman1337 commented Feb 2, 2023

Fox32 commented Feb 2, 2023

Fox32 commented Feb 22, 2023

Fox32 commented Feb 22, 2023

robintown left a comment

t3chguy commented Feb 24, 2023 •

edited

Loading

Fox32 commented Feb 24, 2023

Apply Element language to Jitsi Widget #23412

Apply Element language to Jitsi Widget #23412

Conversation

Fox32 commented Oct 4, 2022 • edited by github-actions bot Loading

Checklist

✨ Features

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fox32 Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weeman1337 commented Feb 2, 2023

Fox32 commented Feb 2, 2023

Fox32 commented Feb 22, 2023

Fox32 commented Feb 22, 2023

robintown left a comment

Choose a reason for hiding this comment

t3chguy commented Feb 24, 2023 • edited Loading

Fox32 commented Feb 24, 2023

Fox32 commented Oct 4, 2022 •

edited by github-actions bot

Loading

Fox32 Oct 6, 2022 •

edited

Loading

t3chguy commented Feb 24, 2023 •

edited

Loading