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

Interface language #329

Merged
merged 18 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 130 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"eventemitter3": "4.0.7",
"filter-invalid-dom-props": "2.1.0",
"hat": "0.0.3",
"i18next": "^22.4.3",
"langs": "^2.0.0",
"lodash.debounce": "4.0.8",
"lodash.intersection": "4.4.0",
Expand All @@ -35,8 +36,10 @@
"react": "18.2.0",
"react-dom": "18.2.0",
"react-focus-lock": "2.9.1",
"react-i18next": "^12.1.1",
"react-is": "18.2.0",
"spatial-navigation-polyfill": "git+https://git@github.com/Stremio/spatial-navigation.git#64871b1422466f5f45d24ebc8bbd315b2ebab6a6",
"stremio-translations": "git+https://git@github.com/Stremio/stremio-translations.git#ef047329f5bcb0a8f96008fca05c68b449b34cb1",
elpiel marked this conversation as resolved.
Show resolved Hide resolved
"url": "0.11.0"
},
"devDependencies": {
Expand Down
27 changes: 26 additions & 1 deletion src/App/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require('spatial-navigation-polyfill');
const React = require('react');
const { useTranslation } = require('react-i18next');
const { Router } = require('stremio-router');
const { Core, Shell, Chromecast, DragAndDrop, KeyboardShortcuts, ServicesProvider } = require('stremio/services');
const { NotFound } = require('stremio/routes');
Expand All @@ -13,6 +14,7 @@ const routerViewsConfig = require('./routerViewsConfig');
const styles = require('./styles');

const App = () => {
const { i18n } = useTranslation();
const onPathNotMatch = React.useCallback(() => {
return NotFound;
}, []);
Expand Down Expand Up @@ -90,6 +92,21 @@ const App = () => {
};
}, []);
React.useEffect(() => {
const onCoreEvent = ({ event, args }) => {
switch (event) {
case 'SettingsUpdated': {
if (args && args.settings && typeof args.settings.interfaceLanguage === 'string') {
i18n.changeLanguage(args.settings.interfaceLanguage);
}
Comment on lines +98 to +100
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

break;
}
}
};
const onCtxState = (state) => {
if (state && state.profile && state.profile.settings && typeof state.profile.settings.interfaceLanguage === 'string') {
Copy link
Member

Choose a reason for hiding this comment

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

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

i18n.changeLanguage(state.profile.settings.interfaceLanguage);
}
};
const onWindowFocus = () => {
services.core.transport.dispatch({
action: 'Ctx',
Expand All @@ -113,8 +130,16 @@ const App = () => {
if (services.core.active) {
onWindowFocus();
window.addEventListener('focus', onWindowFocus);
services.core.transport.on('CoreEvent', onCoreEvent);
services.core.transport
.getState('ctx')
.then(onCtxState)
.catch((e) => console.error(e));
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicking but this can be even shorter :)

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

}
return () => window.removeEventListener('focus', onWindowFocus);
return () => {
window.removeEventListener('focus', onWindowFocus);
services.core.transport.off('CoreEvent', onCoreEvent);
};
}, [initialized]);
return (
<React.StrictMode>
Expand Down
6 changes: 4 additions & 2 deletions src/common/ColorInput/ColorInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const React = require('react');
const PropTypes = require('prop-types');
const classnames = require('classnames');
const AColorPicker = require('a-color-picker');
const { useTranslation } = require('react-i18next');
const Button = require('stremio/common/Button');
const ModalDialog = require('stremio/common/ModalDialog');
const useBinaryState = require('stremio/common/useBinaryState');
Expand All @@ -16,6 +17,7 @@ const parseColor = (value) => {
};

const ColorInput = ({ className, value, dataset, onChange, ...props }) => {
const { t } = useTranslation();
const [modalOpen, openModal, closeModal] = useBinaryState(false);
const [tempValue, setTempValue] = React.useState(() => {
return parseColor(value);
Expand Down Expand Up @@ -69,11 +71,11 @@ const ColorInput = ({ className, value, dataset, onChange, ...props }) => {
setTempValue(parseColor(value));
}, [value, modalOpen]);
return (
<Button title={isTransparent ? 'Transparent' : value} {...props} style={labelButtonStyle} className={classnames(className, styles['color-input-container'])} onClick={labelButtonOnClick}>
<Button title={isTransparent ? t('BUTTON_COLOR_TRANSPARENT') : value} {...props} style={labelButtonStyle} className={classnames(className, styles['color-input-container'])} onClick={labelButtonOnClick}>
{
isTransparent ?
<div className={styles['transparent-label-container']}>
<div className={styles['transparent-label']}>Transparent</div>
<div className={styles['transparent-label']}>{ t('BUTTON_COLOR_TRANSPARENT') }</div>
</div>
:
null
Expand Down
38 changes: 22 additions & 16 deletions src/common/LibItem/LibItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,35 @@ const React = require('react');
const { useServices } = require('stremio/services');
const PropTypes = require('prop-types');
const MetaItem = require('stremio/common/MetaItem');
const { t } = require('i18next');

const OPTIONS = [
{ label: 'Play', value: 'play' },
{ label: 'Details', value: 'details' },
{ label: 'Dismiss', value: 'dismiss' },
{ label: 'Remove', value: 'remove' },
{ label: 'LIBRARY_PLAY', value: 'play' },
{ label: 'LIBRARY_DETAILS', value: 'details' },
{ label: 'LIBRARY_RESUME_DISMISS', value: 'dismiss' },
{ label: 'LIBRARY_REMOVE', value: 'remove' },
];

const LibItem = ({ _id, removable, ...props }) => {
const { core } = useServices();
const options = React.useMemo(() => {
return OPTIONS.filter(({ value }) => {
switch (value) {
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 'dismiss':
return typeof _id === 'string' && props.progress !== null && !isNaN(props.progress);
case 'remove':
return typeof _id === 'string' && removable;
}
});
return OPTIONS
.filter(({ value }) => {
switch (value) {
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');
Comment on lines +22 to +25
Copy link
Member

Choose a reason for hiding this comment

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

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');

case 'dismiss':
return typeof _id === 'string' && props.progress !== null && !isNaN(props.progress);
case 'remove':
return typeof _id === 'string' && removable;
}
})
.map((option) => ({
...option,
label: t(option.label)
}));
}, [_id, removable, props.progress, props.deepLinks]);
const optionOnSelect = React.useCallback((event) => {
if (typeof props.optionOnSelect === 'function') {
Expand Down
Loading