From 5be93acf6b51c183678d656b6a08273a25bd72e1 Mon Sep 17 00:00:00 2001 From: Angelo Suzuki <1063155+tinsukE@users.noreply.github.com> Date: Wed, 13 Mar 2024 20:19:46 +0100 Subject: [PATCH] feat: Improve Image Viewer toolbar auto-hide (#521) Cancel the auto-hide behavior in case the toolbar menu items are interacted with. Improves https://github.com/pachli/pachli-android/issues/505, https://github.com/pachli/pachli-android/issues/507 --------- Co-authored-by: Nik Clayton --- .../main/java/app/pachli/ViewMediaActivity.kt | 50 +++++++++---------- .../java/app/pachli/ViewMediaViewModel.kt | 44 ++++++++++++++++ .../app/pachli/fragment/ViewImageFragment.kt | 19 +++++-- .../app/pachli/fragment/ViewMediaFragment.kt | 28 +++++------ .../app/pachli/fragment/ViewVideoFragment.kt | 7 +-- 5 files changed, 98 insertions(+), 50 deletions(-) create mode 100644 app/src/main/java/app/pachli/ViewMediaViewModel.kt diff --git a/app/src/main/java/app/pachli/ViewMediaActivity.kt b/app/src/main/java/app/pachli/ViewMediaActivity.kt index 5c08151c71..c9bb69646f 100644 --- 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? = null - private val toolbarVisibilityListeners = mutableListOf() 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 { - 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 new file mode 100644 index 0000000000..98bc283c3e --- /dev/null +++ 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 get() = _toolbarVisibility.asStateFlow() + + private val _toolbarMenuInteraction = MutableSharedFlow( + 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 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 index 8974836a79..4a229acbac 100644 --- 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,6 +190,7 @@ 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() { @@ -198,6 +198,15 @@ class ViewImageFragment : ViewMediaFragment() { 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 index 31d355cb70..c5eb8a6281 100644 --- 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? = 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 index e46784ad01..92d1f07e37 100644 --- 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