Interface language #329

tymmesyde · 2022-12-16T15:05:06Z

No description provided.

…into interface-language

elpiel

it seems that when you open the Settings it does not load the user chosen language in the dropdown. After changing the language, I do not experience this issue anymore. Was the value somehow wrong and the dropdown couldn't select the given language?! We haven't been able to reproduce this issue, if it happens again then a bug should be raised.

It would be nice feature to scroll the dropdown to the chosen language if this could be easily achieved with the current dropdown impl Multiselect search #350

Another thing that I noticed is that Settings and other parts of the application are still not translated but that is part of the stremio-translations strings rather than this PR.

sleeyax

I commented some minor suggestions.

sleeyax · 2023-03-17T22:26:03Z

src/App/App.js

+                    if (args && args.settings && typeof args.settings.interfaceLanguage === 'string') {
+                        i18n.changeLanguage(args.settings.interfaceLanguage);
+                    }


You can use optional chaining to simplify this. All latest browsers except IE already support it and it's included in babel's @babel/preset-env preset.

Suggested change

if (args && args.settings && typeof args.settings.interfaceLanguage === 'string') {

i18n.changeLanguage(args.settings.interfaceLanguage);

}

const language = args?.settings?.interfaceLanguage;

if (typeof language === 'string') {

i18n.changeLanguage(language);

}

The additional const variable assignment in my suggestion is optional, but makes it even more readable imo.

I did not used optional chaining because i wanted to follow the current way everything is checked in the app
Also i think in js it might be clearer to spot mistakes with this syntax since there are no linting

let's indeed keep it consistent and update it all at once if the time arises

There are other places where optional chaining is used. So an inconsistency already exists. I agree this is out of scope for this PR though, we can pick one style and create a refactoring PR later.

sleeyax · 2023-03-17T22:29:02Z

src/App/App.js

+            }
+        };
+        const onCtxState = (state) => {
+            if (state && state.profile && state.profile.settings && typeof state.profile.settings.interfaceLanguage === 'string') {


Suggested change

if (state && state.profile && state.profile.settings && typeof state.profile.settings.interfaceLanguage === 'string') {

if (typeof state?.profile?.settings?.interfaceLanguage === 'string') {

sleeyax · 2023-03-17T22:30:17Z

src/App/App.js

+            services.core.transport
+                .getState('ctx')
+                .then(onCtxState)
+                .catch((e) => console.error(e));


Nitpicking but this can be even shorter :)

Suggested change

.catch((e) => console.error(e));

.catch(console.error);

sleeyax · 2023-03-17T22:33:53Z

src/common/LibItem/LibItem.js

+                    case 'play':
+                        return props.deepLinks && typeof props.deepLinks.player === 'string';
+                    case 'details':
+                        return props.deepLinks && (typeof props.deepLinks.metaDetailsVideos === 'string' || typeof props.deepLinks.metaDetailsStreams === 'string');


Suggested change

case 'play':

return props.deepLinks && typeof props.deepLinks.player === 'string';

case 'details':

return props.deepLinks && (typeof props.deepLinks.metaDetailsVideos === 'string' || typeof props.deepLinks.metaDetailsStreams === 'string');

case 'play':

return typeof props.deepLinks?.player === 'string';

case 'details':

return (typeof props.deepLinks?.metaDetailsVideos === 'string' || typeof props.deepLinks?.metaDetailsStreams === 'string');

sleeyax · 2023-03-17T22:52:17Z

src/index.js

+    translation: value
+}]));
+
+i18n


Consider adding i18next-browser-languagedetector so the webapp tries to select whatever language the browser is set to before falling back to en-US.

sleeyax · 2023-03-17T23:08:55Z

src/common/translateOption.js

+const translateOption = (option, translateKeyPrefix = '') => {
+    const translateKey = `${translateKeyPrefix}${option}`;
+    const translateKeyUppercase = translateKey.toUpperCase();
+    const translateValue = t(translateKey);
+    const translateValueUppercase = t(translateKeyUppercase);
+    if (translateKey !== translateValue) {
+        return translateValue;
+    } else if (translateKeyUppercase !== translateValueUppercase) {
+        return translateValueUppercase;
+    }
+    return option.charAt(0).toUpperCase() + option.slice(1);
+};


This function is relatively hard to follow. If I understand what it's doing correctly, I believe it can be heavily simplified as follows:

Suggested change

const translateOption = (option, translateKeyPrefix = '') => {

const translateKey = `${translateKeyPrefix}${option}`;

const translateKeyUppercase = translateKey.toUpperCase();

const translateValue = t(translateKey);

const translateValueUppercase = t(translateKeyUppercase);

if (translateKey !== translateValue) {

return translateValue;

} else if (translateKeyUppercase !== translateValueUppercase) {

return translateValueUppercase;

}

return option.charAt(0).toUpperCase() + option.slice(1);

};

const translateOption = (option, translateKeyPrefix = '') => {

const translateKey = `${translateKeyPrefix}${option}`;

const translateValue = t(translateKey, {defaultValue: t(translateKey.toUpperCase(), {defaultValue: null})});

return translateValue ?? option.charAt(0).toUpperCase() + option.slice(1);

};

Much better thank you !

sleeyax · 2023-03-17T23:28:34Z

It would also be nice if the user could type the language they are looking for as they open the language selector. Just like elpiel mentioned, I'm not sure how hard this would be to add to the current dropdown implementation, but this is definitely something builtin into most browsers native select element and could work the same way here.

tymmesyde · 2023-03-18T08:14:30Z

it seems that when you open the Settings it does not load the user chosen language in the dropdown. After changing the language, I do not experience this issue anymore. Was the value somehow wrong and the dropdown couldn't select the given language?!

I cannot reproduce this issue neither on chrome or firefox as guest or logged in, what steps did you take ?

elpiel · 2023-03-23T12:25:06Z

@tymmesyde I'm not sure how to reproducing this. I tried clearing cache and to login again but I don't see this afr value anymore.
Is it possible that my User data was corrupted with a bad value for the language in the first place?

@sleeyax I didn't mean search but for the select to scroll down to the picked choice when opened.
But a search functionality might be a good idea in the long run, though, we don't have that many languages and that's why I think this is low priority.

sleeyax · 2023-03-23T12:54:19Z

@tymmesyde I'm not sure how to reproducing this. I tried clearing cache and to login again but I don't see this afr value anymore.
Is it possible that my User data was corrupted with a bad value for the language in the first place?

@sleeyax I didn't mean search but for the select to scroll down to the picked choice when opened.
But a search functionality might be a good idea in the long run, though, we don't have that many languages and that's why I think this is low priority.

Yes, I know, I just added my own suggestion on top :)

elpiel

LGTM! @tymmesyde Decide if you'd like to tackle PR comments from @sleeyax in this PR or can be done in subsequent PR. At this stage I consider the PR ready.

elpiel

LGTM - Tested locally and it works.

package.json

tymmesyde added 13 commits December 14, 2022 21:36

feat: implement interface language

fc0dff4

refactor: translate more strings

f06f21c

refactor: add more translations

9938e30

refactor(Discover): translate selectable options

7ed6081

refactor(Library): translate selectable options

2061f65

refactor(SubtitlesMenu): translate more strings

e9ff812

chore: update stremio-translations

329812f

refactor: add translations

87fb0c5

refactor(Addons): translate selectables options

ec338b1

refactor: move translateOption to a common file

fd08dcc

refactor(MetaLinks): translate buttons

445b093

chore: update stremio-translations

4008a62

refactor: translate remaining strings

fed28bf

elpiel requested review from nklhtv and elpiel and removed request for nklhtv March 10, 2023 12:45

elpiel mentioned this pull request Mar 10, 2023

Translations and interface language #347

Closed

elpiel linked an issue Mar 10, 2023 that may be closed by this pull request

Translations and interface language #347

Closed

tymmesyde added 2 commits March 10, 2023 14:32

Merge branch 'development' of https://github.com/Stremio/stremio-web …

2275f40

…into interface-language

refactor(MetaDetails): add missing translations

567af0b

elpiel requested changes Mar 15, 2023

View reviewed changes

elpiel requested a review from sleeyax March 17, 2023 14:47

sleeyax previously approved these changes Mar 17, 2023

View reviewed changes

tymmesyde mentioned this pull request Mar 20, 2023

Multiselect search #350

Open

elpiel previously approved these changes Apr 12, 2023

View reviewed changes

refactor(translateOption): use Sleeyax suggestion

32a7117

tymmesyde added 2 commits April 12, 2023 13:43

refactor(Discover): add translation on extra selectable

b41cc41

refactor(Library): add translation to sort selectable

02fceea

tymmesyde dismissed stale reviews from elpiel and sleeyax via 02fceea April 12, 2023 12:00

elpiel requested a review from sleeyax April 12, 2023 14:36

elpiel approved these changes Apr 12, 2023

View reviewed changes

package.json Show resolved Hide resolved

sleeyax approved these changes Apr 12, 2023

View reviewed changes

tymmesyde merged commit 5705233 into development Apr 13, 2023

nklhtv deleted the interface-language branch May 25, 2023 11:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interface language #329

Interface language #329

tymmesyde commented Dec 16, 2022

elpiel left a comment •

edited

Loading

sleeyax left a comment

sleeyax Mar 17, 2023

tymmesyde Mar 18, 2023

elpiel Apr 12, 2023

sleeyax Apr 12, 2023

sleeyax Mar 17, 2023

sleeyax Mar 17, 2023

sleeyax Mar 17, 2023

sleeyax Mar 17, 2023

sleeyax Mar 17, 2023

tymmesyde Apr 12, 2023

sleeyax commented Mar 17, 2023

tymmesyde commented Mar 18, 2023

elpiel commented Mar 23, 2023

sleeyax commented Mar 23, 2023

elpiel left a comment

elpiel left a comment

-                    if (args && args.settings && typeof args.settings.interfaceLanguage === 'string') {
-                        i18n.changeLanguage(args.settings.interfaceLanguage);
-                    }
+                   const language = args?.settings?.interfaceLanguage;
+                    if (typeof language === 'string') {
+                        i18n.changeLanguage(language);
+                    }

	if (state && state.profile && state.profile.settings && typeof state.profile.settings.interfaceLanguage === 'string') {
	if (typeof state?.profile?.settings?.interfaceLanguage === 'string') {

Interface language #329

Interface language #329

Conversation

tymmesyde commented Dec 16, 2022

elpiel left a comment • edited Loading

Choose a reason for hiding this comment

sleeyax 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

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

sleeyax commented Mar 17, 2023

tymmesyde commented Mar 18, 2023

elpiel commented Mar 23, 2023

sleeyax commented Mar 23, 2023

elpiel left a comment

Choose a reason for hiding this comment

elpiel left a comment

Choose a reason for hiding this comment

elpiel left a comment •

edited

Loading