feat: Improve Image Viewer toolbar auto-hide (#521)

Cancel the auto-hide behavior in case the toolbar menu items are interacted with. Improves #505, #507 --------- Co-authored-by: Nik Clayton <[email protected]>
pachli · Mar 13, 2024 · 5be93ac · 5be93ac
1 parent d9fc664
commit 5be93ac
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 50 deletions.
diff --git a/app/src/main/java/app/pachli/ViewMediaActivity.kt b/app/src/main/java/app/pachli/ViewMediaActivity.kt
@@ -32,10 +32,12 @@ import android.os.Bundle
 import android.os.Environment
 import android.transition.Transition
 import android.view.Menu
+import android.view.MenuInflater
 import android.view.MenuItem
 import android.view.View
 import android.webkit.MimeTypeMap
 import android.widget.Toast
+import androidx.activity.viewModels
 import androidx.core.app.ShareCompat
 import androidx.core.content.FileProvider
 import androidx.fragment.app.FragmentActivity
@@ -69,8 +71,6 @@ import okio.buffer
 import okio.sink
 import timber.log.Timber
 
-typealias ToolbarVisibilityListener = (isVisible: Boolean) -> Unit
-
 /**
  * Show one or more media items (pictures, video, audio, etc).
  */
@@ -79,36 +79,26 @@ class ViewMediaActivity : BaseActivity(), MediaActionsListener {
     @Inject
     lateinit var okHttpClient: OkHttpClient
 
+    private val viewModel: ViewMediaViewModel by viewModels()
+
     private val binding by viewBinding(ActivityViewMediaBinding::inflate)
 
     val toolbar: View
         get() = binding.toolbar
 
-    var isToolbarVisible = true
-        private set
-
     private var attachmentViewData: List<AttachmentViewData>? = null
-    private val toolbarVisibilityListeners = mutableListOf<ToolbarVisibilityListener>()
     private var imageUrl: String? = null
 
     /** True if a download to share media is in progress */
     private var isDownloading: Boolean = false
 
-    /**
-     * Adds [listener] to the list of toolbar listeners and immediately calls
-     * it with the current toolbar visibility.
-     *
-     * @return A function that must be called to remove the listener.
-     */
-    fun addToolbarVisibilityListener(listener: ToolbarVisibilityListener): Function0<Boolean> {
-        toolbarVisibilityListeners.add(listener)
-        listener(isToolbarVisible)
-        return { toolbarVisibilityListeners.remove(listener) }
-    }
+    /** True if a call to [onPrepareMenu] represents a user-initiated action */
+    private var respondToPrepareMenu = false
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         setContentView(binding.root)
+        addMenuProvider(this)
 
         supportPostponeEnterTransition()
 
@@ -156,6 +146,7 @@ class ViewMediaActivity : BaseActivity(), MediaActionsListener {
                 R.id.action_share_media -> shareMedia()
                 R.id.action_copy_media_link -> copyLink()
             }
+            viewModel.onToolbarMenuInteraction()
             true
         }
 
@@ -172,16 +163,26 @@ class ViewMediaActivity : BaseActivity(), MediaActionsListener {
         )
     }
 
-    override fun onCreateOptionsMenu(menu: Menu): Boolean {
+    override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) {
+        super.onCreateMenu(menu, menuInflater)
+
         menuInflater.inflate(R.menu.view_media_toolbar, menu)
         // We don't support 'open status' from single image views
         menu.findItem(R.id.action_open_status)?.isVisible = (attachmentViewData != null)
-        return true
     }
 
-    override fun onPrepareOptionsMenu(menu: Menu?): Boolean {
-        menu?.findItem(R.id.action_share_media)?.isEnabled = !isDownloading
-        return true
+    override fun onPrepareMenu(menu: Menu) {
+        menu.findItem(R.id.action_share_media)?.isEnabled = !isDownloading
+
+        // onPrepareMenu is called immediately after onCreateMenu when the activity
+        // is created (https://issuetracker.google.com/issues/329322653), and this is
+        // not in response to user action. Ignore the first call, respond to
+        // subsequent calls.
+        if (respondToPrepareMenu) {
+            viewModel.onToolbarMenuInteraction()
+        } else {
+            respondToPrepareMenu = true
+        }
     }
 
     override fun onMediaReady() {
@@ -193,10 +194,7 @@ class ViewMediaActivity : BaseActivity(), MediaActionsListener {
     }
 
     override fun onMediaTap() {
-        isToolbarVisible = !isToolbarVisible
-        for (listener in toolbarVisibilityListeners) {
-            listener(isToolbarVisible)
-        }
+        val isToolbarVisible = viewModel.toggleToolbarVisibility()
 
         val visibility = if (isToolbarVisible) View.VISIBLE else View.INVISIBLE
         val alpha = if (isToolbarVisible) 1.0f else 0.0f

diff --git a/app/src/main/java/app/pachli/ViewMediaViewModel.kt b/app/src/main/java/app/pachli/ViewMediaViewModel.kt
@@ -0,0 +1,44 @@
+package app.pachli
+
+import androidx.lifecycle.ViewModel
+import kotlinx.coroutines.channels.BufferOverflow
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.SharedFlow
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asSharedFlow
+import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.updateAndGet
+
+class ViewMediaViewModel : ViewModel() {
+    private val _toolbarVisibility = MutableStateFlow(true)
+
+    /** Emits Toolbar visibility changes */
+    val toolbarVisibility: StateFlow<Boolean> get() = _toolbarVisibility.asStateFlow()
+
+    private val _toolbarMenuInteraction = MutableSharedFlow<Unit>(
+        extraBufferCapacity = 1,
+        onBufferOverflow = BufferOverflow.DROP_OLDEST,
+    )
+
+    /**
+     * Emits whenever a Toolbar menu interaction happens (ex: open overflow menu, item action)
+     * Fragments use this to determine whether the toolbar can be hidden after a delay.
+     */
+    val toolbarMenuInteraction: SharedFlow<Unit> get() = _toolbarMenuInteraction.asSharedFlow()
+
+    /** Convenience getter for the current Toolbar visibility */
+    val isToolbarVisible: Boolean
+        get() = toolbarVisibility.value
+
+    /**
+     * Toggle the current state of the toolbar's visibility.
+     *
+     * @return The new visibility
+     */
+    fun toggleToolbarVisibility() = _toolbarVisibility.updateAndGet { !it }
+
+    fun onToolbarMenuInteraction() {
+        _toolbarMenuInteraction.tryEmit(Unit)
+    }
+}
diff --git a/app/src/main/java/app/pachli/fragment/ViewImageFragment.kt b/app/src/main/java/app/pachli/fragment/ViewImageFragment.kt
@@ -30,7 +30,9 @@ import android.view.ViewGroup
 import android.widget.ImageView
 import androidx.coordinatorlayout.widget.CoordinatorLayout
 import androidx.core.view.GestureDetectorCompat
+import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
+import androidx.lifecycle.repeatOnLifecycle
 import app.pachli.R
 import app.pachli.core.common.extensions.hide
 import app.pachli.core.common.extensions.viewBinding
@@ -59,10 +61,7 @@ class ViewImageFragment : ViewMediaFragment() {
 
     private var scheduleToolbarHide = false
 
-    override fun setupMediaView(
-        isToolbarVisible: Boolean,
-        showingDescription: Boolean,
-    ) {
+    override fun setupMediaView(showingDescription: Boolean) {
         binding.photoView.transitionName = attachment.url
         binding.mediaDescription.text = attachment.description
         binding.captionSheet.visible(showingDescription)
@@ -71,7 +70,7 @@ class ViewImageFragment : ViewMediaFragment() {
         loadImageFromNetwork(attachment.url, attachment.previewUrl, binding.photoView)
 
         // Only schedule hiding the toolbar once
-        scheduleToolbarHide = isToolbarVisible
+        scheduleToolbarHide = viewModel.isToolbarVisible
     }
 
     override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
@@ -191,13 +190,23 @@ class ViewImageFragment : ViewMediaFragment() {
             },
         )
 
+        // Cancel hiding the toolbar whenever interacting with the captionSheet
         val captionSheetParams = (binding.captionSheet.layoutParams as CoordinatorLayout.LayoutParams)
         (captionSheetParams.behavior as BottomSheetBehavior).addBottomSheetCallback(
             object : BottomSheetCallback() {
                 override fun onStateChanged(bottomSheet: View, newState: Int) = cancelToolbarHide()
                 override fun onSlide(bottomSheet: View, slideOffset: Float) = cancelToolbarHide()
             },
         )
+
+        // Cancel hiding the toolbar whenever interacting with the toolbar (items and overflow menu)
+        viewLifecycleOwner.lifecycleScope.launch {
+            viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
+                viewModel.toolbarMenuInteraction.collect {
+                    cancelToolbarHide()
+                }
+            }
+        }
     }
 
     override fun onToolbarVisibilityChange(visible: Boolean) {

diff --git a/app/src/main/java/app/pachli/fragment/ViewMediaFragment.kt b/app/src/main/java/app/pachli/fragment/ViewMediaFragment.kt
@@ -24,9 +24,13 @@ import android.view.View
 import androidx.annotation.CallSuper
 import androidx.annotation.OptIn
 import androidx.fragment.app.Fragment
+import androidx.fragment.app.activityViewModels
+import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
+import androidx.lifecycle.repeatOnLifecycle
 import androidx.media3.common.util.UnstableApi
 import app.pachli.ViewMediaActivity
+import app.pachli.ViewMediaViewModel
 import app.pachli.core.network.model.Attachment
 import kotlin.time.Duration.Companion.seconds
 import kotlinx.coroutines.CompletableDeferred
@@ -51,20 +55,16 @@ interface MediaActionsListener {
 }
 
 abstract class ViewMediaFragment : Fragment() {
-    /** Function to remove the toolbar listener */
-    private var removeToolbarListener: Function0<Boolean>? = null
+
+    protected val viewModel: ViewMediaViewModel by activityViewModels()
 
     /**
      * Called after [onResume], subclasses should override this and update
      * the contents of views (including loading any media).
      *
-     * @param isToolbarVisible True if the toolbar is visible
      * @param showingDescription True if the media's description should be shown
      */
-    abstract fun setupMediaView(
-        isToolbarVisible: Boolean,
-        showingDescription: Boolean,
-    )
+    abstract fun setupMediaView(showingDescription: Boolean)
 
     /**
      * Called when the visibility of the toolbar changes.
@@ -141,6 +141,12 @@ abstract class ViewMediaFragment : Fragment() {
 
         shouldCallMediaReady = arguments?.getBoolean(ARG_SHOULD_CALL_MEDIA_READY)
             ?: throw IllegalArgumentException("ARG_START_POSTPONED_TRANSITION has to be set")
+
+        viewLifecycleOwner.lifecycleScope.launch {
+            viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
+                viewModel.toolbarVisibility.collect(::onToolbarVisibilityChange)
+            }
+        }
     }
 
     /**
@@ -159,12 +165,7 @@ abstract class ViewMediaFragment : Fragment() {
     private fun finalizeViewSetup() {
         showingDescription = !TextUtils.isEmpty(attachment.description)
         isDescriptionVisible = showingDescription
-        setupMediaView(mediaActivity.isToolbarVisible, showingDescription && mediaActivity.isToolbarVisible)
-
-        removeToolbarListener = mediaActivity
-            .addToolbarVisibilityListener { isVisible ->
-                onToolbarVisibilityChange(isVisible)
-            }
+        setupMediaView(showingDescription && viewModel.isToolbarVisible)
     }
 
     override fun onPause() {
@@ -188,7 +189,6 @@ abstract class ViewMediaFragment : Fragment() {
     }
 
     override fun onDestroyView() {
-        removeToolbarListener?.invoke()
         transitionComplete = null
         super.onDestroyView()
     }

diff --git a/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt b/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt
@@ -267,7 +267,7 @@ class ViewVideoFragment : ViewMediaFragment() {
 
         if (Build.VERSION.SDK_INT <= 23 || player == null) {
             initializePlayer()
-            if (mediaActivity.isToolbarVisible && !isAudio) {
+            if (viewModel.isToolbarVisible && !isAudio) {
                 hideToolbarAfterDelay()
             }
             binding.videoView.onResume()
@@ -353,10 +353,7 @@ class ViewVideoFragment : ViewMediaFragment() {
     }
 
     @SuppressLint("ClickableViewAccessibility")
-    override fun setupMediaView(
-        isToolbarVisible: Boolean,
-        showingDescription: Boolean,
-    ) {
+    override fun setupMediaView(showingDescription: Boolean) {
         startedTransition = false
 
         binding.mediaDescription.text = attachment.description