-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Send all translations to gutenberg-mobile #9249
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
Changes from all commits
26806d3
882ef91
154d11d
af7cb42
ba2e6f0
c575a52
4d5096c
4a94466
c1bb23a
013186d
644579d
ac433d6
444bc1e
90c591e
dd1f1da
fdf04b6
3afa68e
41ddfaa
90fcc93
5508ee5
4f4c1d1
63498b1
fd308ea
2407b09
2b10286
dc5bbce
d53a9f4
067e721
15a9701
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| import android.content.DialogInterface; | ||
| import android.content.DialogInterface.OnClickListener; | ||
| import android.content.res.Configuration; | ||
| import android.content.res.Resources; | ||
| import android.os.Bundle; | ||
| import android.os.Handler; | ||
| import android.support.annotation.NonNull; | ||
|
|
@@ -16,6 +17,7 @@ | |
| import android.support.v7.app.AppCompatActivity; | ||
| import android.text.Editable; | ||
| import android.text.Spanned; | ||
| import android.util.DisplayMetrics; | ||
| import android.view.ContextThemeWrapper; | ||
| import android.view.LayoutInflater; | ||
| import android.view.Menu; | ||
|
|
@@ -41,6 +43,10 @@ | |
| import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnMediaLibraryButtonListener; | ||
| import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnReattachQueryListener; | ||
|
|
||
| import java.lang.reflect.Field; | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Locale; | ||
| import java.util.HashSet; | ||
| import java.util.Set; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
|
|
@@ -50,6 +56,7 @@ public class GutenbergEditorFragment extends EditorFragmentAbstract implements | |
| IHistoryListener { | ||
| private static final String KEY_HTML_MODE_ENABLED = "KEY_HTML_MODE_ENABLED"; | ||
| private static final String ARG_IS_NEW_POST = "param_is_new_post"; | ||
| private static final String ARG_LOCALE_SLUG = "param_locale_slug"; | ||
|
|
||
| private static final int CAPTURE_PHOTO_PERMISSION_REQUEST_CODE = 101; | ||
|
|
||
|
|
@@ -72,12 +79,14 @@ public class GutenbergEditorFragment extends EditorFragmentAbstract implements | |
|
|
||
| public static GutenbergEditorFragment newInstance(String title, | ||
| String content, | ||
| boolean isNewPost) { | ||
| boolean isNewPost, | ||
| String localeSlug) { | ||
| GutenbergEditorFragment fragment = new GutenbergEditorFragment(); | ||
| Bundle args = new Bundle(); | ||
| args.putString(ARG_PARAM_TITLE, title); | ||
| args.putString(ARG_PARAM_CONTENT, content); | ||
| args.putBoolean(ARG_IS_NEW_POST, isNewPost); | ||
| args.putString(ARG_LOCALE_SLUG, localeSlug); | ||
| fragment.setArguments(args); | ||
| return fragment; | ||
| } | ||
|
|
@@ -94,17 +103,86 @@ private GutenbergContainerFragment getGutenbergContainerFragment() { | |
| return mRetainedGutenbergContainerFragment; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the gutenberg-mobile specific translations | ||
| * | ||
| * @return Bundle a map of "english string" => [ "current locale string" ] | ||
| */ | ||
| public Bundle getTranslations() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a local method so we should change method access signature probably to private.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method name doesn't tell us enough information about what is the purpose of this method. |
||
| Bundle translations = new Bundle(); | ||
| Locale defaultLocale = new Locale("en"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code will always return English translation, is that idea?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The english locale yes, the idea is for getTranslations to return a mapping of |
||
| Resources currentResources = getActivity().getApplicationContext().getResources(); | ||
| Configuration currentConfiguration = currentResources.getConfiguration(); | ||
| // if the current locale of the app is english stop here and return an empty map | ||
| if (currentConfiguration.locale.equals(defaultLocale)) { | ||
| return translations; | ||
| } | ||
|
|
||
| // Let's create a Resources object for the default locale (english) to get the original values for our strings | ||
| DisplayMetrics metrics = new DisplayMetrics(); | ||
| Configuration defaultLocaleConfiguration = new Configuration(currentConfiguration); | ||
| defaultLocaleConfiguration.setLocale(defaultLocale); | ||
| getActivity().getWindowManager().getDefaultDisplay().getMetrics(metrics); | ||
| Resources defaultResources = new Resources(getActivity().getAssets(), metrics, defaultLocaleConfiguration); | ||
|
|
||
| // Strings are only being translated in the WordPress package | ||
| // thus we need to get a reference of the R class for this package | ||
| // Here we assume the Application class is at the same level as the R class | ||
| // It will not work if this lib is used outside of WordPress-Android, | ||
| // in this case let's just return an empty map | ||
| Class<?> rString; | ||
| Package mainPackage = getActivity().getApplication().getClass().getPackage(); | ||
|
|
||
| if (mainPackage == null) { | ||
| return translations; | ||
| } | ||
|
|
||
| try { | ||
| rString = getActivity().getApplication().getClassLoader().loadClass(mainPackage.getName() + ".R$string"); | ||
| } catch (ClassNotFoundException ex) { | ||
| return translations; | ||
| } | ||
|
|
||
| for (Field stringField : rString.getDeclaredFields()) { | ||
| int resourceId; | ||
| try { | ||
| resourceId = stringField.getInt(rString); | ||
| } catch (IllegalArgumentException | IllegalAccessException iae) { | ||
| AppLog.e(T.EDITOR, iae); | ||
| continue; | ||
| } | ||
|
|
||
| String fieldName = stringField.getName(); | ||
| // Filter out all strings that are not prefixed with `gutenberg_mobile_` | ||
| if (!fieldName.startsWith("gutenberg_mobile_")) { | ||
| continue; | ||
| } | ||
|
|
||
| // Add the mapping english => [ translated ] to the bundle if both string are not empty | ||
| String currentResourceString = currentResources.getString(resourceId); | ||
| String defaultResourceString = defaultResources.getString(resourceId); | ||
| if (currentResourceString.length() > 0 && defaultResourceString.length() > 0) { | ||
| translations.putStringArrayList( | ||
| defaultResourceString, | ||
| new ArrayList<>(Arrays.asList(currentResourceString)) | ||
| ); | ||
| } | ||
| } | ||
| return translations; | ||
| } | ||
|
|
||
| @Override | ||
| public void onCreate(Bundle savedInstanceState) { | ||
| super.onCreate(savedInstanceState); | ||
|
|
||
| if (getGutenbergContainerFragment() == null) { | ||
| boolean isNewPost = getArguments().getBoolean(ARG_IS_NEW_POST); | ||
| String localeSlug = getArguments().getString(ARG_LOCALE_SLUG); | ||
|
|
||
| FragmentManager fragmentManager = getChildFragmentManager(); | ||
| FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction(); | ||
| GutenbergContainerFragment gutenbergContainerFragment = | ||
| GutenbergContainerFragment.newInstance(isNewPost); | ||
| GutenbergContainerFragment.newInstance(isNewPost, localeSlug, this.getTranslations()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need for |
||
| gutenbergContainerFragment.setRetainInstance(true); | ||
| fragmentTransaction.add(gutenbergContainerFragment, GutenbergContainerFragment.TAG); | ||
| fragmentTransaction.commitNow(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's see, is there some specific reason we're using
LocaleManager.getLanguage()here (which gets the device language) but we useConfiguration.localelater in getTranslations() (which get's the current app locale)?Unfortunately, the device and app locales can be out of sync unless the app is restarted so, this ends up having, for example, English UI (see top bar) but Spanish content strings (see the title placeholder):

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note:
Locale.getLanguage(context)will only return the device language if no custom app-level language is set inSharedPreferences.WordPress-Android/WordPress/src/main/java/org/wordpress/android/util/LocaleManager.java
Line 82 in 699d215