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.

Sign up for GitHub

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

Remove experimental per app feature flag #21521

Merged
merged 10 commits into from
Dec 13, 2024
+68 −149
Merged

Remove experimental per app feature flag #21521

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
05ef74d
Removed experimental per app language feature flag
nbradbury Dec 12, 2024
d296608
Moved language change to PerAppLocaleManager
nbradbury Dec 12, 2024
e9110cb
Updated comment
nbradbury Dec 12, 2024
a0d8a81
Removed string res
nbradbury Dec 12, 2024
7d1b932
Updated comments
nbradbury Dec 12, 2024
ab18a7c
Fixed detekt error
nbradbury Dec 12, 2024
51f7470
Removed mLocaleProvider
nbradbury Dec 13, 2024
8b5b827
Use binary comparison
nbradbury Dec 13, 2024
b9e5c5d
Removed extra space in comment
nbradbury Dec 13, 2024
c62e604
Make fun private
nbradbury Dec 13, 2024
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
9 changes: 2 additions & 7 deletions WordPress/src/main/java/org/wordpress/android/AppInitializer.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,13 +950,8 @@ class AppInitializer @Inject constructor(
*/
private inner class MemoryAndConfigChangeMonitor : ComponentCallbacks2 {
override fun onConfigurationChanged(newConfig: Configuration) {
// If per-app locale is enabled make sure the in-app locale is correct,
// otherwise reapply in-app locale on configuration change
if (perAppLocaleManager.isPerAppLanguagePrefsEnabled()) {
perAppLocaleManager.checkAndUpdateOldLanguagePrefKey()
} else {
LocaleManager.setLocale(context)
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
}
// Make sure the in-app locale is correct
perAppLocaleManager.checkAndUpdateOldLanguagePrefKey()
}

override fun onLowMemory() {
Expand Down
39 changes: 5 additions & 34 deletions WordPress/src/main/java/org/wordpress/android/ui/LocaleAwareActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,53 +3,24 @@ package org.wordpress.android.ui
import android.content.Context
import android.content.res.Configuration
import androidx.appcompat.app.AppCompatActivity
import org.wordpress.android.ui.prefs.AppPrefs
import org.wordpress.android.util.LocaleManager
import org.wordpress.android.util.PerAppLocaleManager

/**
* Newer versions of the AppCompat library no longer support locale changes at application level,
* so this activity is used to help handle those changes at activity level.
* Reference: https://issuetracker.google.com/issues/141869006#comment9
*
* All the actual logic is inside the LocaleManager class, which should be used directly in cases where
* extending from this class is not possible/preferable.
*
* Note: please be mindful of the principle of favoring composition over inheritance and refrain from
* building upon this class unless it's absolutely necessary.
*
* Update Dec 2024: We've added experimental support for per-app language preferences which
* will eventually negate the need for this class. Instead of extending from this class, we
* should extend from AppCompatActivity once this feature is out of the experimental phase.
* Update Dec 2024: We've added support for per-app language preferences which negate
* the need for this class. Instead of extending from this class, we should extend
* from AppCompatActivity.
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
*/
abstract class LocaleAwareActivity : AppCompatActivity() {
/**
* Used to update locales on API 21 to API 25.
*/
override fun attachBaseContext(newBase: Context?) {
if (isPerAppLocaleEnabled()) {
super.attachBaseContext(newBase)
} else {
super.attachBaseContext(LocaleManager.setLocale(newBase))
}
super.attachBaseContext(newBase)
}

/**
* Used to update locales on API 26 and beyond.
*/
override fun applyOverrideConfiguration(overrideConfiguration: Configuration?) {
if (isPerAppLocaleEnabled()) {
super.applyOverrideConfiguration(overrideConfiguration)
} else {
super.applyOverrideConfiguration(LocaleManager.updatedConfigLocale(baseContext, overrideConfiguration))
}
}

/**
* Ideally we would use [PerAppLocaleManager.isPerAppLanguagePrefsEnabled] here, but we
* can't inject [PerAppLocaleManager] into an abstract class
*/
private fun isPerAppLocaleEnabled(): Boolean {
return AppPrefs.getManualFeatureConfig(PerAppLocaleManager.EXPERIMENTAL_PER_APP_LANGUAGE_PREF_KEY)
super.applyOverrideConfiguration(overrideConfiguration)
}
}
66 changes: 6 additions & 60 deletions WordPress/src/main/java/org/wordpress/android/ui/prefs/AppSettingsFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import android.preference.PreferenceFragment;
import android.preference.PreferenceScreen;
import android.preference.SwitchPreference;
import android.text.TextUtils;
import android.view.LayoutInflater;
import android.view.MenuItem;
import android.view.View;
Expand Down Expand Up @@ -49,31 +48,26 @@
import org.wordpress.android.ui.deeplinks.DeepLinkOpenWebLinksWithJetpackHelper;
import org.wordpress.android.ui.jetpackoverlay.JetpackFeatureRemovalPhaseHelper;
import org.wordpress.android.ui.mysite.jetpackbadge.JetpackPoweredBottomSheetFragment;
import org.wordpress.android.util.PerAppLocaleManager;
import org.wordpress.android.ui.prefs.language.LocalePickerBottomSheet;
import org.wordpress.android.ui.prefs.language.LocalePickerBottomSheet.LocalePickerCallback;
import org.wordpress.android.ui.reader.services.update.ReaderUpdateLogic;
import org.wordpress.android.ui.reader.services.update.ReaderUpdateServiceStarter;
import org.wordpress.android.ui.utils.UiHelpers;
import org.wordpress.android.ui.whatsnew.FeatureAnnouncementDialogFragment;
import org.wordpress.android.ui.whatsnew.FeatureAnnouncementProvider;
import org.wordpress.android.util.AppLog;
import org.wordpress.android.util.AppThemeUtils;
import org.wordpress.android.util.BuildConfigWrapper;
import org.wordpress.android.util.JetpackBrandingUtils;
import org.wordpress.android.util.LocaleManager;
import org.wordpress.android.util.LocaleProvider;
import org.wordpress.android.util.NetworkUtils;
import org.wordpress.android.util.PerAppLocaleManager;
import org.wordpress.android.util.ToastUtils;
import org.wordpress.android.util.WPActivityUtils;
import org.wordpress.android.util.WPPrefUtils;
import org.wordpress.android.util.analytics.AnalyticsUtils;
import org.wordpress.android.viewmodel.ContextProvider;

import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

import javax.inject.Inject;
Expand Down Expand Up @@ -101,7 +95,6 @@ public class AppSettingsFragment extends PreferenceFragment
private WPSwitchPreference mOpenWebLinksWithJetpack;

private Preference mWhatsNew;
private Boolean mIsPerAppLanguagePrefsEnabled;

@Inject SiteStore mSiteStore;
@Inject AccountStore mAccountStore;
Expand All @@ -125,8 +118,6 @@ public void onCreate(@Nullable Bundle savedInstanceState) {
((WordPress) getActivity().getApplication()).component().inject(this);
mDispatcher.register(this);

mIsPerAppLanguagePrefsEnabled = mPerAppLocaleManager.isPerAppLanguagePrefsEnabled();

addPreferencesFromResource(R.xml.app_settings);

findPreference(getString(R.string.pref_key_send_usage)).setOnPreferenceChangeListener(
Expand Down Expand Up @@ -259,11 +250,7 @@ public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container,
mLanguagePreference = (WPPreference) findPreference(getString(R.string.pref_key_language));
mLanguagePreference.setOnPreferenceChangeListener(this);
mLanguagePreference.setOnPreferenceClickListener(this);
if (mIsPerAppLanguagePrefsEnabled) {
mLanguagePreference.setSummary(mPerAppLocaleManager.getCurrentLocaleDisplayName());
} else {
mLanguagePreference.setSummary(mLocaleProvider.getAppLanguageDisplayString());
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
}
mLanguagePreference.setSummary(mPerAppLocaleManager.getCurrentLocaleDisplayName());

return view;
}
Expand Down Expand Up @@ -436,10 +423,7 @@ public boolean onPreferenceChange(Preference preference, Object newValue) {
return false;
}

if (preference == mLanguagePreference) {
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
changeLanguage(newValue.toString());
return false;
} else if (preference == mOptimizedImage) {
if (preference == mOptimizedImage) {
AppPrefs.setImageOptimize((Boolean) newValue);
mImageMaxSizePref.setEnabled((Boolean) newValue);
Map<String, Object> properties = new HashMap<>();
Expand Down Expand Up @@ -503,39 +487,6 @@ public boolean onOptionsItemSelected(@NonNull MenuItem item) {
return super.onOptionsItemSelected(item);
}

private void changeLanguage(String languageCode) {
if (mLanguagePreference == null || TextUtils.isEmpty(languageCode)) {
return;
}

if (LocaleManager.isSameLanguage(languageCode)) {
return;
}

LocaleManager.setNewLocale(WordPress.getContext(), languageCode);
WordPress.updateContextLocale();
mContextProvider.refreshContext();

// Track language change on Analytics because we have both the device language and app selected language
// data in Tracks metadata.
Map<String, Object> properties = new HashMap<>();
properties.put("app_locale", Locale.getDefault());
AnalyticsTracker.track(Stat.ACCOUNT_SETTINGS_LANGUAGE_CHANGED, properties);

// Language is now part of metadata, so we need to refresh them
AnalyticsUtils.refreshMetadata(mAccountStore, mSiteStore);

// Refresh the app
Intent refresh = new Intent(getActivity(), getActivity().getClass());
startActivity(refresh);
getActivity().setResult(LANGUAGE_CHANGED);
getActivity().overridePendingTransition(android.R.anim.fade_in, android.R.anim.fade_out);
getActivity().finish();

// update Reader tags as they need be localized
ReaderUpdateServiceStarter.startService(WordPress.getContext(), EnumSet.of(ReaderUpdateLogic.UpdateTask.TAGS));
}

private boolean handleDevicePreferenceClick() {
try {
// open specific app info screen
Expand Down Expand Up @@ -648,9 +599,8 @@ private boolean handleFeatureAnnouncementClick() {
}

private boolean handleAppLocalePickerClick() {
// if per-app language preferences are enabled and the device is on API 33+, take the user to the
// system app settings to change the language
if (mIsPerAppLanguagePrefsEnabled && Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
// if the device is on API 33+, take the user to the system app settings to change the language
nbradbury marked this conversation as resolved.
Show resolved Hide resolved
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
mPerAppLocaleManager.openAppLanguageSettings(getContext());
return true;
} else if (getActivity() instanceof AppCompatActivity) {
Expand Down Expand Up @@ -678,11 +628,7 @@ private void reattachLocalePickerCallback() {

@Override
public void onLocaleSelected(@NonNull String languageCode) {
if (mIsPerAppLanguagePrefsEnabled) {
mPerAppLocaleManager.setCurrentLocaleByLanguageCode(languageCode);
} else {
onPreferenceChange(mLanguagePreference, languageCode);
}
mPerAppLocaleManager.onLanguageChanged(languageCode);
}

private void handleOpenLinksInJetpack(Boolean newValue) {
Expand Down
21 changes: 1 addition & 20 deletions WordPress/src/main/java/org/wordpress/android/ui/prefs/ExperimentalFeaturesActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,33 +32,26 @@ import androidx.compose.ui.tooling.preview.Preview
import androidx.lifecycle.ViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import dagger.hilt.android.AndroidEntryPoint
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.update
import org.wordpress.android.R
import org.wordpress.android.ui.compose.theme.AppThemeM3
import org.wordpress.android.ui.compose.unit.Margin
import org.wordpress.android.util.PerAppLocaleManager
import org.wordpress.android.util.extensions.setContent
import javax.inject.Inject

val experimentalFeatures = listOf(
Feature(key = "experimental_block_editor"),
Feature(key = "experimental_block_editor_theme_styles"),
Feature(key = PerAppLocaleManager.EXPERIMENTAL_PER_APP_LANGUAGE_PREF_KEY)
)

data class Feature(
val enabled: Boolean = false,
val key: String,
)

@HiltViewModel
class FeatureViewModel @Inject constructor(
private val perAppLocaleManager: PerAppLocaleManager
) : ViewModel() {
class FeatureViewModel : ViewModel() {
private val _switchStates = MutableStateFlow<Map<String, Feature>>(emptyMap())
val switchStates: StateFlow<Map<String, Feature>> = _switchStates.asStateFlow()

Expand All @@ -76,18 +69,6 @@ class FeatureViewModel @Inject constructor(
AppPrefs.setManualFeatureConfig(enabled, key)
}
}

featureToggled(key, enabled)
}

private fun featureToggled(key: String, enabled: Boolean) {
if (key == PerAppLocaleManager.EXPERIMENTAL_PER_APP_LANGUAGE_PREF_KEY) {
if (enabled) {
perAppLocaleManager.performMigrationIfNecessary()
} else {
perAppLocaleManager.resetApplicationLocale()
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

Expand Down
Loading
Loading