From 259df21d090c8276faa466f82041b41a8bfee32c Mon Sep 17 00:00:00 2001 From: Brayan Oliveira <69634269+brayandso@users.noreply.github.com> Date: Sat, 23 Nov 2024 16:18:57 -0300 Subject: [PATCH] refactor: move most of the Preferences activity into a fragment The activity wasn't fully removed because currently SearchConfiguration demands an activity that implements SearchPreferenceResultListener. That probably can be changed in the future in the upstream library --- AnkiDroid/src/main/AndroidManifest.xml | 2 +- .../java/com/ichi2/anki/CollectionHelper.kt | 3 +- .../main/java/com/ichi2/anki/DeckPicker.kt | 3 +- .../ichi2/anki/NavigationDrawerActivity.kt | 5 +- .../preferences/AdvancedSettingsFragment.kt | 7 - .../CustomButtonsSettingsFragment.kt | 8 - .../anki/preferences/DevOptionsFragment.kt | 8 +- .../com/ichi2/anki/preferences/Preferences.kt | 156 +++++++++++------- .../anki/preferences/SettingsFragment.kt | 10 -- AnkiDroid/src/main/res/layout/preferences.xml | 2 +- .../anki/ActivityStartupUnderBackupTest.kt | 4 +- .../anki/preferences/PreferenceTestUtils.kt | 6 +- .../ichi2/anki/preferences/PreferencesTest.kt | 6 +- .../anki/preferences/SettingsSearchBarTest.kt | 5 +- .../java/com/ichi2/testutils/ActivityList.kt | 4 +- 15 files changed, 119 insertions(+), 110 deletions(-) diff --git a/AnkiDroid/src/main/AndroidManifest.xml b/AnkiDroid/src/main/AndroidManifest.xml index 93b3e6f37cd1..ba8d6f6cc241 100644 --- a/AnkiDroid/src/main/AndroidManifest.xml +++ b/AnkiDroid/src/main/AndroidManifest.xml @@ -346,7 +346,7 @@ android:configChanges="orientation|screenSize" android:exported="false" /> diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CollectionHelper.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CollectionHelper.kt index 4a5c4452ae2d..508e4e5d2248 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CollectionHelper.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CollectionHelper.kt @@ -25,7 +25,6 @@ import androidx.core.content.edit import com.ichi2.anki.AnkiDroidFolder.AppPrivateFolder import com.ichi2.anki.exception.StorageAccessException import com.ichi2.anki.exception.UnknownDatabaseVersionException -import com.ichi2.anki.preferences.Preferences import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.libanki.Collection import com.ichi2.libanki.DB @@ -47,7 +46,7 @@ object CollectionHelper { * This directory contains all AnkiDroid data and media for a given collection * Except the Android preferences, cached files and [MetaDB] * - * This can be changed by the [Preferences] screen + * This can be changed by the Preferences screen * to allow a user to access a second collection via the same AnkiDroid app instance. * * The path also defines the collection that the AnkiDroid API accesses diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 1b3ec8821bc6..df20b6cedc60 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -141,6 +141,7 @@ import com.ichi2.anki.pages.AnkiPackageImporterFragment import com.ichi2.anki.pages.CongratsPage import com.ichi2.anki.pages.CongratsPage.Companion.onDeckCompleted import com.ichi2.anki.preferences.AdvancedSettingsFragment +import com.ichi2.anki.preferences.PreferencesActivity import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.anki.receiver.SdCardReceiver import com.ichi2.anki.servicelayer.ScopedStorageService @@ -788,7 +789,7 @@ open class DeckPicker : convertDpToPixel(32F, this@DeckPicker).toInt() ) positiveButton(R.string.open_settings) { - val settingsIntent = AdvancedSettingsFragment.getSubscreenIntent(this@DeckPicker) + val settingsIntent = PreferencesActivity.getIntent(this@DeckPicker, AdvancedSettingsFragment::class) requestPathUpdateLauncher.launch(settingsIntent) } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/NavigationDrawerActivity.kt b/AnkiDroid/src/main/java/com/ichi2/anki/NavigationDrawerActivity.kt index 61c24b05301c..9369d771be28 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/NavigationDrawerActivity.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/NavigationDrawerActivity.kt @@ -41,7 +41,7 @@ import androidx.drawerlayout.widget.DrawerLayout import com.google.android.material.color.MaterialColors import com.google.android.material.navigation.NavigationView import com.ichi2.anki.dialogs.help.HelpDialog -import com.ichi2.anki.preferences.Preferences +import com.ichi2.anki.preferences.PreferencesActivity import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.anki.workarounds.FullDraggableContainerFix import com.ichi2.compat.CompatHelper @@ -357,10 +357,9 @@ abstract class NavigationDrawerActivity : /** * Opens AnkiDroid's Settings Screen. - * @see Preferences */ protected fun openSettings() { - val intent = Intent(this, Preferences::class.java) + val intent = PreferencesActivity.getIntent(this) preferencesLauncher.launch(intent) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/AdvancedSettingsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/AdvancedSettingsFragment.kt index 85415c62b9e4..ea3f2e7fefb0 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/AdvancedSettingsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/AdvancedSettingsFragment.kt @@ -16,7 +16,6 @@ package com.ichi2.anki.preferences import android.content.ComponentName -import android.content.Context import android.content.Intent import android.content.pm.PackageManager import androidx.appcompat.app.AlertDialog @@ -142,10 +141,4 @@ class AdvancedSettingsFragment : SettingsFragment() { } } } - - companion object { - fun getSubscreenIntent(context: Context): Intent { - return getSubscreenIntent(context, AdvancedSettingsFragment::class) - } - } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/CustomButtonsSettingsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/CustomButtonsSettingsFragment.kt index 879e56ebac8f..ed7d8002906c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/CustomButtonsSettingsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/CustomButtonsSettingsFragment.kt @@ -15,8 +15,6 @@ */ package com.ichi2.anki.preferences -import android.content.Context -import android.content.Intent import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.core.content.edit @@ -59,10 +57,4 @@ class CustomButtonsSettingsFragment : SettingsFragment() { fun allKeys(): HashSet { return allPreferences().mapTo(hashSetOf()) { it.key } } - - companion object { - fun getSubscreenIntent(context: Context): Intent { - return getSubscreenIntent(context, CustomButtonsSettingsFragment::class) - } - } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/DevOptionsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/DevOptionsFragment.kt index fc5e7f880823..4427685a590e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/DevOptionsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/DevOptionsFragment.kt @@ -55,8 +55,12 @@ class DevOptionsFragment : SettingsFragment() { * Ensure that the preference is searchable or not * based on the same condition at [HeaderFragment.configureSearchBar] */ + enableDevOptionsPref.setOnPreferenceChangeListener { _, _ -> + showDisableDevOptionsDialog() + false + } if (BuildConfig.DEBUG) { - enableDevOptionsPref.isVisible = false + enableDevOptionsPref.isVisible = true } else { enableDevOptionsPref.setOnPreferenceChangeListener { _, _ -> showDisableDevOptionsDialog() @@ -165,7 +169,7 @@ class DevOptionsFragment : SettingsFragment() { * or if the user has enabled it with the secret on [com.ichi2.anki.preferences.AboutFragment] */ fun isEnabled(context: Context): Boolean { - return BuildConfig.DEBUG || context.sharedPrefs() + return context.sharedPrefs() .getBoolean(context.getString(R.string.dev_options_enabled_by_user_key), false) } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/Preferences.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/Preferences.kt index fcc98fb2c877..9674591593cc 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/Preferences.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/Preferences.kt @@ -19,83 +19,77 @@ ****************************************************************************************/ package com.ichi2.anki.preferences +import android.content.Context +import android.content.Intent import android.os.Bundle -import android.view.MenuItem +import android.view.View import androidx.annotation.XmlRes +import androidx.core.os.bundleOf import androidx.fragment.app.Fragment +import androidx.fragment.app.FragmentManager +import androidx.fragment.app.FragmentOnAttachListener import androidx.fragment.app.commit import androidx.preference.Preference import androidx.preference.PreferenceFragmentCompat +import com.bytehamster.lib.preferencesearch.SearchConfiguration import com.bytehamster.lib.preferencesearch.SearchPreferenceResult import com.bytehamster.lib.preferencesearch.SearchPreferenceResultListener import com.google.android.material.appbar.AppBarLayout import com.google.android.material.appbar.CollapsingToolbarLayout -import com.ichi2.anki.AnkiActivity +import com.google.android.material.appbar.MaterialToolbar import com.ichi2.anki.R +import com.ichi2.anki.SingleFragmentActivity import com.ichi2.anki.utils.isSw600dp -import com.ichi2.themes.setTransparentStatusBar import com.ichi2.utils.getInstanceFromClassName import timber.log.Timber +import kotlin.reflect.KClass import kotlin.reflect.jvm.jvmName -class Preferences : - AnkiActivity(), - PreferenceFragmentCompat.OnPreferenceStartFragmentCallback, - SearchPreferenceResultListener { - - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) - setContentView(R.layout.preferences) - setTransparentStatusBar() - - enableToolbar().setDisplayHomeAsUpEnabled(true) +class PreferencesFragment : + Fragment(R.layout.preferences), + PreferenceFragmentCompat.OnPreferenceStartFragmentCallback { + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + view.findViewById(R.id.toolbar).apply { + setNavigationOnClickListener { + if (resources.isSw600dp()) { + requireActivity().finish() + } else { + requireActivity().onBackPressedDispatcher.onBackPressed() + } + } + } - // Load initial fragment if activity is being first created + // Load initial subscreen if activity is being first created if (savedInstanceState == null) { - loadInitialFragment() + loadInitialSubscreen() + } else { + parentFragmentManager.findFragmentById(R.id.settings_container)?.let { + setFragmentTitleOnToolbar(it) + } } - supportFragmentManager.addOnBackStackChangedListener { + + parentFragmentManager.addOnBackStackChangedListener { + val fragment = parentFragmentManager.findFragmentById(R.id.settings_container) + ?: return@addOnBackStackChangedListener + + setFragmentTitleOnToolbar(fragment) + // Expand bar in new fragments if scrolled to top - val fragment = supportFragmentManager.findFragmentById(R.id.settings_container) - as? PreferenceFragmentCompat ?: return@addOnBackStackChangedListener - fragment.listView.post { + (fragment as? PreferenceFragmentCompat)?.listView?.post { val viewHolder = fragment.listView?.findViewHolderForAdapterPosition(0) val isAtTop = viewHolder != null && viewHolder.itemView.top >= 0 - findViewById(R.id.appbar).setExpanded(isAtTop, false) + view.findViewById(R.id.appbar).setExpanded(isAtTop, false) } - - val title = if (fragment is TitleProvider) fragment.title else "" - findViewById(R.id.collapsingToolbarLayout)?.title = title - supportActionBar?.title = title } } - /** - * Starts the first fragment for the [Preferences] activity, - * which by default is [HeaderFragment]. - * The initial fragment may be overridden by putting the java class name - * of the fragment on an intent extra with the key [INITIAL_FRAGMENT_EXTRA] - */ - private fun loadInitialFragment() { - val fragmentClassName = intent?.getStringExtra(INITIAL_FRAGMENT_EXTRA) - val initialFragment = if (fragmentClassName == null) { - if (resources.isSw600dp()) GeneralSettingsFragment() else HeaderFragment() - } else { - try { - getInstanceFromClassName(fragmentClassName) - } catch (e: Exception) { - throw RuntimeException("Failed to load $fragmentClassName", e) - } - } - supportFragmentManager.commit { - // In tablets, show the headers fragment at the lateral navigation container - if (resources.isSw600dp()) { - replace(R.id.lateral_nav_container, HeaderFragment()) - replace(R.id.settings_container, initialFragment, initialFragment::class.java.name) - } else { - replace(R.id.settings_container, initialFragment, initialFragment::class.java.name) - } + private fun setFragmentTitleOnToolbar(fragment: Fragment) { + val title = when (fragment) { + is TitleProvider -> fragment.title + else -> getString(R.string.settings) } + view?.findViewById(R.id.collapsingToolbarLayout)?.title = title } override fun onPreferenceStartFragment( @@ -103,37 +97,63 @@ class Preferences : pref: Preference ): Boolean { // avoid reopening the same fragment if already active - val currentFragment = supportFragmentManager.findFragmentById(R.id.settings_container) + val currentFragment = parentFragmentManager.findFragmentById(R.id.settings_container) ?: return true if (pref.fragment == currentFragment::class.jvmName) return true - val fragment = supportFragmentManager.fragmentFactory.instantiate( - classLoader, + val fragment = parentFragmentManager.fragmentFactory.instantiate( + requireContext().classLoader, pref.fragment ?: return true ) fragment.arguments = pref.extras - supportFragmentManager.commit { + parentFragmentManager.commit { replace(R.id.settings_container, fragment, fragment::class.jvmName) addToBackStack(null) } return true } - override fun onOptionsItemSelected(item: MenuItem): Boolean { - if (item.itemId == android.R.id.home) { + /** + * Starts the first settings fragment, which by default is [HeaderFragment]. + * The initial fragment may be overridden by putting the java class name + * of the fragment on an intent extra with the key [INITIAL_FRAGMENT_EXTRA] + */ + private fun loadInitialSubscreen() { + val fragmentClassName = arguments?.getString(INITIAL_FRAGMENT_EXTRA) + val initialFragment = if (fragmentClassName == null) { + if (resources.isSw600dp()) GeneralSettingsFragment() else HeaderFragment() + } else { + try { + getInstanceFromClassName(fragmentClassName) + } catch (e: Exception) { + throw RuntimeException("Failed to load $fragmentClassName", e) + } + } + parentFragmentManager.addFragmentOnAttachListener(object : FragmentOnAttachListener { + override fun onAttachFragment(fragmentManager: FragmentManager, fragment: Fragment) { + setFragmentTitleOnToolbar(fragment) + fragmentManager.removeFragmentOnAttachListener(this) + } + }) + parentFragmentManager.commit { + // In big screens, show the headers fragment at the lateral navigation container if (resources.isSw600dp()) { - finish() + replace(R.id.lateral_nav_container, HeaderFragment()) + replace(R.id.settings_container, initialFragment, initialFragment::class.java.name) } else { - onBackPressedDispatcher.onBackPressed() + replace(R.id.settings_container, initialFragment, initialFragment::class.java.name) } } - return true } +} - // ---------------------------------------------------------------------------- - // Class methods - // ---------------------------------------------------------------------------- - +/** + * Host activity for [PreferencesFragment]. + * + * Only necessary because [SearchConfiguration] demands an activity that implements + * [SearchPreferenceResultListener]. + */ +class PreferencesActivity : SingleFragmentActivity(), SearchPreferenceResultListener { override fun onSearchResultClicked(result: SearchPreferenceResult) { val fragment = getFragmentFromXmlRes(result.resourceFile) ?: return @@ -146,6 +166,16 @@ class Preferences : Timber.i("Highlighting key '%s' on %s", result.key, fragment) result.highlight(fragment as PreferenceFragmentCompat) } + + companion object { + fun getIntent(context: Context, initialFragment: KClass? = null): Intent { + val arguments = bundleOf(INITIAL_FRAGMENT_EXTRA to initialFragment?.jvmName) + return Intent(context, PreferencesActivity::class.java).apply { + putExtra(FRAGMENT_NAME_EXTRA, PreferencesFragment::class.jvmName) + putExtra(FRAGMENT_ARGS_EXTRA, arguments) + } + } + } } interface TitleProvider { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/SettingsFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/SettingsFragment.kt index a517e639dcaf..565a3b9a6b47 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/preferences/SettingsFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/preferences/SettingsFragment.kt @@ -15,8 +15,6 @@ */ package com.ichi2.anki.preferences -import android.content.Context -import android.content.Intent import android.content.SharedPreferences import android.os.Bundle import androidx.annotation.VisibleForTesting @@ -31,8 +29,6 @@ import com.ichi2.anki.analytics.UsageAnalytics import com.ichi2.preferences.DialogFragmentProvider import timber.log.Timber import java.lang.NumberFormatException -import kotlin.reflect.KClass -import kotlin.reflect.jvm.jvmName abstract class SettingsFragment : PreferenceFragmentCompat(), @@ -132,12 +128,6 @@ abstract class SettingsFragment : } companion object { - @JvmStatic // Using protected members which are not @JvmStatic in the superclass companion is unsupported yet - protected fun getSubscreenIntent(context: Context, fragmentClass: KClass): Intent { - return Intent(context, Preferences::class.java) - .putExtra(INITIAL_FRAGMENT_EXTRA, fragmentClass.jvmName) - } - /** * Converts a preference value to a numeric number that * can be reported to analytics, since analytics events only accept diff --git a/AnkiDroid/src/main/res/layout/preferences.xml b/AnkiDroid/src/main/res/layout/preferences.xml index b443e5a86168..809a5ba4e4cf 100644 --- a/AnkiDroid/src/main/res/layout/preferences.xml +++ b/AnkiDroid/src/main/res/layout/preferences.xml @@ -22,7 +22,7 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:fitsSystemWindows="true" - tools:context=".preferences.Preferences"> + tools:context=".preferences.PreferencesFragment"> { val ret = AtomicReference>() - val i = CustomButtonsSettingsFragment.getSubscreenIntent(context) - ActivityScenario.launch(i).use { scenario -> + val i = PreferencesActivity.getIntent(context, CustomButtonsSettingsFragment::class) + ActivityScenario.launch(i).use { scenario -> scenario.moveToState(Lifecycle.State.STARTED) - scenario.onActivity { a: Preferences -> + scenario.onActivity { a: PreferencesActivity -> val customButtonsFragment = a.supportFragmentManager .findFragmentByTag(CustomButtonsSettingsFragment::class.java.name) as CustomButtonsSettingsFragment ret.set(customButtonsFragment.allKeys()) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/PreferencesTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/PreferencesTest.kt index 38c04f212dc3..f146a6a4662a 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/PreferencesTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/PreferencesTest.kt @@ -34,12 +34,12 @@ import org.robolectric.annotation.Config @RunWith(AndroidJUnit4::class) class PreferencesTest : RobolectricTest() { - private lateinit var preferences: Preferences + private lateinit var preferences: PreferencesActivity @Before override fun setUp() { super.setUp() - preferences = Preferences() + preferences = PreferencesActivity() val attachBaseContext = getJavaMethodAsAccessible( AppCompatActivity::class.java, "attachBaseContext", @@ -72,7 +72,7 @@ class PreferencesTest : RobolectricTest() { /** checks if any of the Preferences fragments throws while being created */ @Test fun fragmentsDoNotThrowOnCreation() { - val activityScenario = ActivityScenario.launch(Preferences::class.java) + val activityScenario = ActivityScenario.launch(PreferencesActivity.getIntent(targetContext)) activityScenario.onActivity { activity -> PreferenceTestUtils.getAllPreferencesFragments(activity).forEach { diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/SettingsSearchBarTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/SettingsSearchBarTest.kt index 31f38a9cf6be..cbe54c7efed7 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/preferences/SettingsSearchBarTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/preferences/SettingsSearchBarTest.kt @@ -73,8 +73,9 @@ class SettingsSearchBarTest : RobolectricTest() { } } - private fun getPreferencesActivity(): Preferences { - return Robolectric.buildActivity(Preferences::class.java) + private fun getPreferencesActivity(): PreferencesActivity { + val intent = PreferencesActivity.getIntent(targetContext) + return Robolectric.buildActivity(PreferencesActivity::class.java, intent) .create().start().resume().get() } } diff --git a/AnkiDroid/src/test/java/com/ichi2/testutils/ActivityList.kt b/AnkiDroid/src/test/java/com/ichi2/testutils/ActivityList.kt index 061daa079bb9..48da2b582864 100644 --- a/AnkiDroid/src/test/java/com/ichi2/testutils/ActivityList.kt +++ b/AnkiDroid/src/test/java/com/ichi2/testutils/ActivityList.kt @@ -41,7 +41,7 @@ import com.ichi2.anki.StudyOptionsActivity import com.ichi2.anki.instantnoteeditor.InstantNoteEditorActivity import com.ichi2.anki.multimedia.MultimediaActivity import com.ichi2.anki.notetype.ManageNotetypes -import com.ichi2.anki.preferences.Preferences +import com.ichi2.anki.preferences.PreferencesActivity import com.ichi2.anki.previewer.CardViewerActivity import com.ichi2.anki.services.ReminderService.Companion.getReviewDeckIntent import com.ichi2.anki.ui.windows.managespace.ManageSpaceActivity @@ -74,7 +74,7 @@ object ActivityList { // Likely has unhandled intents get(Reviewer::class.java), get(MyAccount::class.java), - get(Preferences::class.java), + get(PreferencesActivity::class.java), get(FilteredDeckOptions::class.java), get(DrawingActivity::class.java), // Info has unhandled intents