From 140ce80ad7a89477909f6b005fa494961584abac Mon Sep 17 00:00:00 2001 From: timm0e <11885527+timm0e@users.noreply.github.com> Date: Fri, 25 Aug 2023 00:17:40 +0200 Subject: [PATCH] Introduce BackPressInterceptor This removes usages of the deprecated onBackPress method and instead relies on the new onBackPressedDispatcher while providing compatibility with the way we stack Fragments on top of each other (see BackPressInterceptor.kt for a more detailed explanation). --- .../java/org/jellyfin/mobile/MainActivity.kt | 48 +++++++++++++++---- .../mobile/settings/SettingsFragment.kt | 7 ++- .../mobile/utils/BackPressInterceptor.kt | 41 ++++++++++++++++ .../jellyfin/mobile/webapp/WebViewFragment.kt | 15 +++--- 4 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 app/src/main/java/org/jellyfin/mobile/utils/BackPressInterceptor.kt diff --git a/app/src/main/java/org/jellyfin/mobile/MainActivity.kt b/app/src/main/java/org/jellyfin/mobile/MainActivity.kt index 512f090a88..1f8b40dbf3 100644 --- a/app/src/main/java/org/jellyfin/mobile/MainActivity.kt +++ b/app/src/main/java/org/jellyfin/mobile/MainActivity.kt @@ -9,9 +9,12 @@ import android.os.IBinder import android.provider.Settings import android.view.OrientationEventListener import android.widget.Toast +import androidx.activity.OnBackPressedCallback +import androidx.activity.addCallback import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AppCompatActivity import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen +import androidx.fragment.app.Fragment import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.lifecycle.repeatOnLifecycle @@ -22,6 +25,7 @@ import org.jellyfin.mobile.player.cast.IChromecast import org.jellyfin.mobile.player.ui.PlayerFragment import org.jellyfin.mobile.setup.ConnectFragment import org.jellyfin.mobile.utils.AndroidVersion +import org.jellyfin.mobile.utils.BackPressInterceptor import org.jellyfin.mobile.utils.Constants import org.jellyfin.mobile.utils.PermissionRequestHelper import org.jellyfin.mobile.utils.SmartOrientationListener @@ -54,6 +58,37 @@ class MainActivity : AppCompatActivity() { private val orientationListener: OrientationEventListener by lazy { SmartOrientationListener(this) } + /** + * Passes back press events onto the currently visible [Fragment] if it implements the [BackPressInterceptor] interface. + * + * If the current fragment does not implement [BackPressInterceptor] or has decided not to intercept the event + * (see result of [BackPressInterceptor.onInterceptBackPressed]), the topmost backstack entry will be popped. + * + * If there is no topmost backstack entry, the event will be passed onto the dispatcher's fallback handler. + */ + private val onBackPressedCallback : OnBackPressedCallback.() -> Unit = callback@{ + val currentFragment = supportFragmentManager.findFragmentById(R.id.fragment_container) + if (currentFragment is BackPressInterceptor && currentFragment.onInterceptBackPressed()) { + // Top fragment handled back press + return@callback + } + + // This is the same default action as in Activity.onBackPressed + if (!supportFragmentManager.isStateSaved && supportFragmentManager.popBackStackImmediate()) { + // Removed fragment from back stack + return@callback + } + + // Let the system handle the back press + isEnabled = false + // Make sure that we *really* call the fallback handler + assert(!onBackPressedDispatcher.hasEnabledCallbacks()) { + "MainActivity should be the lowest onBackPressCallback" + } + onBackPressedDispatcher.onBackPressed() + isEnabled = true // re-enable callback in case activity isn't finished + } + override fun onCreate(savedInstanceState: Bundle?) { installSplashScreen() setupKoinFragmentFactory() @@ -95,6 +130,9 @@ class MainActivity : AppCompatActivity() { } } + // Handle back presses + onBackPressedDispatcher.addCallback(this, onBackPressed = onBackPressedCallback) + // Setup Chromecast chromecast.initializePlugin(this) } @@ -140,7 +178,7 @@ class MainActivity : AppCompatActivity() { } override fun onSupportNavigateUp(): Boolean { - onBackPressed() + onBackPressedDispatcher.onBackPressed() return true } @@ -157,14 +195,6 @@ class MainActivity : AppCompatActivity() { orientationListener.disable() } - override fun onBackPressed() { - if (supportFragmentManager.backStackEntryCount > 0) { - supportFragmentManager.popBackStack() - } else { - super.onBackPressed() - } - } - override fun onDestroy() { unbindService(serviceConnection) chromecast.destroy() diff --git a/app/src/main/java/org/jellyfin/mobile/settings/SettingsFragment.kt b/app/src/main/java/org/jellyfin/mobile/settings/SettingsFragment.kt index a85013c007..471205ebb5 100644 --- a/app/src/main/java/org/jellyfin/mobile/settings/SettingsFragment.kt +++ b/app/src/main/java/org/jellyfin/mobile/settings/SettingsFragment.kt @@ -22,6 +22,7 @@ import de.Maxr1998.modernpreferences.preferences.CheckBoxPreference import de.Maxr1998.modernpreferences.preferences.choice.SelectionItem import org.jellyfin.mobile.R import org.jellyfin.mobile.app.AppPreferences +import org.jellyfin.mobile.utils.BackPressInterceptor import org.jellyfin.mobile.databinding.FragmentSettingsBinding import org.jellyfin.mobile.utils.Constants import org.jellyfin.mobile.utils.applyWindowInsetsAsMargins @@ -31,7 +32,7 @@ import org.jellyfin.mobile.utils.isPackageInstalled import org.jellyfin.mobile.utils.withThemedContext import org.koin.android.ext.android.inject -class SettingsFragment : Fragment() { +class SettingsFragment : Fragment(), BackPressInterceptor { private val appPreferences: AppPreferences by inject() private val settingsAdapter: PreferencesAdapter by lazy { PreferencesAdapter(buildSettingsScreen()) } @@ -59,6 +60,10 @@ class SettingsFragment : Fragment() { return binding.root } + override fun onInterceptBackPressed(): Boolean { + return settingsAdapter.goBack() + } + override fun onDestroyView() { super.onDestroyView() requireMainActivity().setSupportActionBar(null) diff --git a/app/src/main/java/org/jellyfin/mobile/utils/BackPressInterceptor.kt b/app/src/main/java/org/jellyfin/mobile/utils/BackPressInterceptor.kt new file mode 100644 index 0000000000..bcb5d0e3f1 --- /dev/null +++ b/app/src/main/java/org/jellyfin/mobile/utils/BackPressInterceptor.kt @@ -0,0 +1,41 @@ +package org.jellyfin.mobile.utils + +import androidx.activity.OnBackPressedDispatcher +import androidx.fragment.app.Fragment +import org.jellyfin.mobile.utils.extensions.addFragment +import org.jellyfin.mobile.MainActivity + +/** + * Additional hook for handling back presses in [Fragments][Fragment] (see [onInterceptBackPressed]). + * + * This hook is introduced since the AndroidX onBackPressedDispatcher system does not play well with the way we handle fragments: + * The WebViewFragment always needs to be active, since it contains the state of the web interface. + * To achieve this, we only add fragments (see [addFragment]) instead of doing the more common way + * and replacing the current fragment. + * + * This keeps the WebViewFragment alive, but unless the new fragment registers its own onBackPressedCallback, + * this also means that the WebViewFragment's onBackPressedCallbacks would still be the topmost dispatcher and therefore + * would be called (see [OnBackPressedDispatcher.onBackPressed]). + * + * This wouldn't be a problem if there was some way for the WebViewFragment (or any other fragment that's active) to + * know if it is the currently displayed fragment, since then it could deactivate its own onBackPressedCallback + * and the next callback would be called instead. + * The [MainActivity's][MainActivity] callback would then default to popping the backstack. + * + * There might be a way to implement this by using the backstack to determine if the current fragment is the topmost fragment, + * but sadly it seems that this isn't possible in a non-hacky way (as in hardcoding names of backstack entries). + * + * Instead, the MainActivity determines the currently visible fragment, + * and passes the back press event to it via the [onInterceptBackPressed] method. + */ +interface BackPressInterceptor { + /** + * Called when a back press is performed while this fragment is currently visible. + * + * @return `true` if the event was intercepted by the fragment, + * `false` if the back press was not handled by the fragment. + * The latter will result in a default action that closes the fragment + * @see MainActivity.onBackPressedCallback + */ + fun onInterceptBackPressed(): Boolean = false +} diff --git a/app/src/main/java/org/jellyfin/mobile/webapp/WebViewFragment.kt b/app/src/main/java/org/jellyfin/mobile/webapp/WebViewFragment.kt index 6b80851ffc..00e608393a 100644 --- a/app/src/main/java/org/jellyfin/mobile/webapp/WebViewFragment.kt +++ b/app/src/main/java/org/jellyfin/mobile/webapp/WebViewFragment.kt @@ -10,7 +10,6 @@ import android.view.View import android.view.ViewGroup import android.webkit.WebView import android.widget.Toast -import androidx.activity.addCallback import androidx.appcompat.app.AlertDialog import androidx.core.view.ViewCompat import androidx.core.view.doOnNextLayout @@ -24,6 +23,7 @@ import kotlinx.coroutines.launch import org.jellyfin.mobile.R import org.jellyfin.mobile.app.ApiClientController import org.jellyfin.mobile.app.AppPreferences +import org.jellyfin.mobile.utils.BackPressInterceptor import org.jellyfin.mobile.bridge.ExternalPlayer import org.jellyfin.mobile.bridge.NativeInterface import org.jellyfin.mobile.bridge.NativePlayer @@ -45,7 +45,7 @@ import org.jellyfin.mobile.utils.requestNoBatteryOptimizations import org.jellyfin.mobile.utils.runOnUiThread import org.koin.android.ext.android.inject -class WebViewFragment : Fragment() { +class WebViewFragment : Fragment(), BackPressInterceptor { val appPreferences: AppPreferences by inject() private val apiClientController: ApiClientController by inject() private val webappFunctionChannel: WebappFunctionChannel by inject() @@ -102,13 +102,6 @@ class WebViewFragment : Fragment() { } } externalPlayer = ExternalPlayer(requireContext(), this, requireActivity().activityResultRegistry) - - requireActivity().onBackPressedDispatcher.addCallback(this) { - if (!connected || !webappFunctionChannel.goBack()) { - isEnabled = false - activity?.onBackPressed() - } - } } override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View { @@ -167,6 +160,10 @@ class WebViewFragment : Fragment() { } } + override fun onInterceptBackPressed(): Boolean { + return connected && webappFunctionChannel.goBack() + } + override fun onDestroyView() { super.onDestroyView() webViewBinding = null