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

Introduce BackPressInterceptor #1172

Merged
merged 1 commit into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 39 additions & 9 deletions app/src/main/java/org/jellyfin/mobile/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -95,6 +130,9 @@ class MainActivity : AppCompatActivity() {
}
}

// Handle back presses
onBackPressedDispatcher.addCallback(this, onBackPressed = onBackPressedCallback)

// Setup Chromecast
chromecast.initializePlugin(this)
}
Expand Down Expand Up @@ -140,7 +178,7 @@ class MainActivity : AppCompatActivity() {
}

override fun onSupportNavigateUp(): Boolean {
onBackPressed()
onBackPressedDispatcher.onBackPressed()
return true
}

Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import de.Maxr1998.modernpreferences.preferences.choice.SelectionItem
import org.jellyfin.mobile.R
import org.jellyfin.mobile.app.AppPreferences
import org.jellyfin.mobile.databinding.FragmentSettingsBinding
import org.jellyfin.mobile.utils.BackPressInterceptor
import org.jellyfin.mobile.utils.Constants
import org.jellyfin.mobile.utils.applyWindowInsetsAsMargins
import org.jellyfin.mobile.utils.extensions.requireMainActivity
Expand All @@ -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()) }
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.jellyfin.mobile.utils

import androidx.activity.OnBackPressedDispatcher
import androidx.fragment.app.Fragment
import org.jellyfin.mobile.MainActivity
import org.jellyfin.mobile.utils.extensions.addFragment

/**
* 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
}
15 changes: 6 additions & 9 deletions app/src/main/java/org/jellyfin/mobile/webapp/WebViewFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,6 +30,7 @@ import org.jellyfin.mobile.data.entity.ServerEntity
import org.jellyfin.mobile.databinding.FragmentWebviewBinding
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.Constants.FRAGMENT_WEB_VIEW_EXTRA_SERVER
import org.jellyfin.mobile.utils.applyDefault
Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -167,6 +160,10 @@ class WebViewFragment : Fragment() {
}
}

override fun onInterceptBackPressed(): Boolean {
return connected && webappFunctionChannel.goBack()
}

override fun onDestroyView() {
super.onDestroyView()
webViewBinding = null
Expand Down