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 language selector to change the app language #2921

Merged
merged 24 commits into from
Feb 1, 2020
Merged

Conversation

B0pol
Copy link
Member

@B0pol B0pol commented Jan 4, 2020

Fixes #2794, fixes #2162, fixes #2887, fixes #1215, fixes #1130, fixes #2848, fixes #578, fix r/NewPipe : Any way to change the app language? & r/NewPipe : how to change app language ?

Explaination :
Before, NewPipe's language was phone's language. Now, it's the language selected in Settings -> Content -> App language, and by default, it stays system default language.
Thus, you can force an app language (eg. English for developing / debugging : take a screenshot in English) or use non-available system language (Esperanto isn't translated for Android) or simply just set the language you want without changing phone / system language.

APK: 2020-01-27_language-selector#2921_APK.zip

1) now, on « content language » change, it will also change the app language
2) added Esperanto to the list of language in content language
@TobiGr TobiGr added feature request Issue is related to a feature in the app localisation / translation Everything that has to do with translations or Weblate labels Jan 5, 2020
@B0pol
Copy link
Member Author

B0pol commented Jan 6, 2020

I found out what does content language, it's used by NewPipe Extractor.

Then think we should use a third selector that would only change the language of NewPipe.

I think that would be implemented like that :

where we would set the language already translated in NewPipe, + at the top a button that would be : system language.

I'll work on it this week

@B0pol
Copy link
Member Author

B0pol commented Jan 9, 2020

I found an exception (#2940), but it's not related to my PR, it's related to the formatter and %1$s not in the good order and arabic alphabet.

@B0pol
Copy link
Member Author

B0pol commented Jan 9, 2020

I changed the way time is displayed, it now use NewPipe's language (the new selector), previously it was only systems's language.
This fixes #1215 . In the limit of the languages of the library Pretty time we use : https://www.ocpsoft.org/prettytime/#section-12

@B0pol B0pol changed the title Update localizations settings [WIP] Update localizations settings Jan 9, 2020
@B0pol B0pol changed the title [WIP] Update localizations settings Add language selector to change NewPipe's language Jan 10, 2020
@B0pol
Copy link
Member Author

B0pol commented Jan 10, 2020

Here are some screenshots :
System language was Spanish
US trendings displayed in English Settings in English

US trendings displayed in German Settings in German

@B0pol B0pol mentioned this pull request Jan 10, 2020
3 tasks
@TobiGr TobiGr added this to the 0.18.2 milestone Jan 13, 2020
@B0pol
Copy link
Member Author

B0pol commented Jan 14, 2020

I just fixed Indonesian that wasn't possible to select by renaming values-id to values-in, but also wasn't possible to use with device language. Indonesian was 99% translated but unusable.
Briefly, android use outdated version of an iso norm, then it doesn't recognize id, but in.
See https://stackoverflow.com/questions/13291578/how-to-localize-an-android-app-in-indonesian-language.
I also added Occitan to the selector.
So the selector is working for every language now.
Pirate will show English as values-pr/strings.xml is empty, and for some languages you will see a lot of english, as it's not translated much (I'm thinking to Interlingua, Occitan…)

This is a hardly reproduceable bug that I hopefully fixed. After a long time of watching videos, you could have your system language shown in playback parameters dialog.
Calling changeAppLanguage(getAppLocale(…),…) onCreate will most certainly fix this bug
@B0pol
Copy link
Member Author

B0pol commented Jan 24, 2020

@abusarimhindi APK: apk-debug
In settings, you can change for Urdu in settings / content settings / NewPipe's language.

For anyone else wondering, this is a TEST version: there might be some problems, because I was based on dev branch in extractor, and I switched to master branch of extractor (the only one that includes the fix for decryption bug). That's also why I didn't commit these changes.

I manually updated extractor version so that YouTube works again, and remove the urdu string, took the one from weblate.

@B0pol
Copy link
Member Author

B0pol commented Jan 24, 2020

This is 0.18.1 because I'm based on debug build, which is still on 0.18.1.
But as I said, I manually updated extractor version, to prevent the decryption crash, and I manually updated urdu strings from weblate, just for you, and I didn't commit these changes because this should be fixed by updating dev branch, not by fixing manually

But I think you have the wrong version, you have to take the app-debug that I gave in my last comment, or just use this link https://github.com/TeamNewPipe/NewPipe/files/4108063/app-debug.zip

@abusarimhindi
Copy link
Contributor

This is 0.18.1 because I'm based on debug build, which is still on 0.18.1.
But as I said, I manually updated extractor version, to prevent the decryption crash, and I manually updated urdu strings from weblate, just for you, and I didn't commit these changes because this should be fixed by updating dev branch, not by fixing manually

Sorry, It is Ok. Firstly I downloaded the old file, now updated. Thanks again.

see TeamNewPipe#2921 (comment)
It will add Esperanto, add bengali, update vietnamese
pretty time is the library used in the home page: … days ago
Copy link
Member

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Code looks good so far. Just some minor comments.

Is it necessary to call changeAppLanguage() in all onCreate and onResume methods or can we just do it in App.java in? If not, it might be good to add a new method and call it instead of changeAppLanguage() with the same parameters again and again to increase maintainability.

public static void assureCorrectAppLanguage() {
    changeAppLanguage(getAppLocale(getApplicationContext()), getResources());
}

app/src/main/res/values/settings_keys.xml Outdated Show resolved Hide resolved
app/src/main/res/values/settings_keys.xml Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
@B0pol
Copy link
Member Author

B0pol commented Jan 28, 2020

public static void assureCorrectAppLanguage() {
    changeAppLanguage(getAppLocale(getApplicationContext()), getResources());
}

I can't do that, I can't have context and resources from a static context.
The most optimized thing I can have is

public static void assureCorrectAppLanguage(Context c) {
    changeAppLanguage(getAppLocale(c), c.getResources());
}

and then, in mainactivity and other activites needing it:
assureCorrectAppLanguage(this);

Is it necessary to call changeAppLanguage() in all onCreate and onResume methods or can we just do it in App.java in?

Yesn't sir! If you put in onCreate and onResume of mainActivity, it will work for about 95%. (And putting it in App.java does nothing) I'll say why I put for the other files:

  • AboutActivity: everything is fine because strings are loaded on runtime, except title which is set in manifest.xml, so I changed it on runtime
  • Player: kinda tough, it is in the language you want, except if you press a button, change orientation (landscape / portrait).
  • PlaybackParameterDialog: it was showing wrong language without it.
  • The other file honestly I don't remember, but at first I put in MainActivity, and while testing phase I added where it would show the wrong language.

It's kinda like the ThemeHelper.setTheme(this, ServiceHelper.getSelectedServiceId(this));: you don't need everywhere, but in activities who are making problems without it.

@B0pol B0pol changed the title Add language selector to change NewPipe's language Add language selector to change the app language Jan 28, 2020
@TobiGr
Copy link
Member

TobiGr commented Jan 29, 2020

public static void assureCorrectAppLanguage() {
    changeAppLanguage(getAppLocale(getApplicationContext()), getResources());
}

I can't do that, I can't have context and resources from a static context.
The most optimized thing I can have is

public static void assureCorrectAppLanguage(Context c) {
    changeAppLanguage(getAppLocale(c), c.getResources());
}

My bad. This happens when you write reviews during lectures :D

@TobiGr
Copy link
Member

TobiGr commented Jan 29, 2020

Code looks go so far. I'll test it as soon as possible

renamed NewPipe's language into App language, and same for all the
concerning thing (keys, comments…)

we now call assureCorrectAppLanguage(CONTEXT) in activities needing it
instead of changeAppLanguage(getAppLocale(CONTEXT), RESOURCES)
changeAppLanguage becomes private.
@B0pol
Copy link
Member Author

B0pol commented Jan 29, 2020

Just one thing you should know
I made changes to values-ur/strings.xml, because of formatter crash (#2940)
They were fixed by weblate in the meantime, so either you update dev with weblate soon and I solve conficts before you merge the pr, or for every future conflicts existing with Urdu strings, you should pick the Weblate version.

My bad. This happens when you write reviews during lectures :D

No problem

@B0pol B0pol mentioned this pull request Jan 30, 2020
1 task
Changed android.R.string.ok, which is "OK", into R.string.finish, which is also OK, but from our strings
Then for a small amount of languages that don't have Android translation, it will show the good string.
@TobiGr
Copy link
Member

TobiGr commented Feb 1, 2020

I made changes to values-ur/strings.xml, because of formatter crash (#2940)
They were fixed by weblate in the meantime, so either you update dev with weblate soon and I solve conficts before you merge the pr, or for every future conflicts existing with Urdu strings, you should pick the Weblate version.

I merged weblate into dev. Could you take a look at the conflicts? I am short on time until I finished my exams. Thanks :)

@B0pol
Copy link
Member Author

B0pol commented Feb 1, 2020

I merged weblate into dev. Could you take a look at the conflicts? I am short on time until I finished my exams. Thanks :)

Done. Actually, the only conflict shown on Github was the indonesian file which have been renamed, but while merging with git I had no problem.

I think the PR is finally fully ready.

@TobiGr
Copy link
Member

TobiGr commented Feb 1, 2020

Thanks. One small thing. Can you please integrate this setting into the mechanism which displays a toast that language changes require an app restart?

if (!selectedLocalization.equals(initialSelectedLocalization)
|| !selectedContentCountry.equals(initialSelectedContentCountry) || !selectedLanguage.equals(initialLanguage)) {
Toast.makeText(requireContext(), R.string.localization_changes_requires_app_restart, Toast.LENGTH_LONG).show();
NewPipe.setupLocalization(selectedLocalization, selectedContentCountry);
}

I forgot to change the key here when I renamed it
@B0pol
Copy link
Member Author

B0pol commented Feb 1, 2020

Thanks. One small thing. Can you please integrate this setting into the mechanism which displays a toast that language changes require an app restart?

if (!selectedLocalization.equals(initialSelectedLocalization)
|| !selectedContentCountry.equals(initialSelectedContentCountry) || !selectedLanguage.equals(initialLanguage)) {
Toast.makeText(requireContext(), R.string.localization_changes_requires_app_restart, Toast.LENGTH_LONG).show();
NewPipe.setupLocalization(selectedLocalization, selectedContentCountry);
}

done

Copy link
Member

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app localisation / translation Everything that has to do with translations or Weblate
Projects
None yet
3 participants