refactor: Restructure ViewMediaActivity and related fragments (#489)

Highlights:

- Implement fragment transitions for video to improve the UX, video
won't start playing until the transition completes
- Remove rxJava
- Move duplicate code in to base classes

Details:

`MediaActionsListener`:

- Move to `ViewMediaFragment` as it's used by both subclasses
- Remove need for separate `VideoActionsListener`
- Rename methods to better reflect their purpose and improve readability

`ViewMediaFragment`:

- Move duplicated code from `ViewImageFragment` and `ViewVideoFragment`
- Rewrite code that handles fragment transitions to use a
`CompleteableDeferred` instead of `BehaviorSubject` (removes rxJava).
- Rename methods and properties to better reflect their purpose and
improve readability
- Add extra comments

`ViewImageFragment`:
- Rewrite code that handles fragment transitions to use a
`CompleteableDeferred` instead of `BehaviorSubject` (removes rxJava).

`ViewVideoFragment`:

- Implement fragment transitions for video to improve the UX, video
won't start playing until the transition completes
- Manage toolbar visibility with a coroutine instead of a handler
- Add extra comments

`ViewMediaActivity`:
- Rename properties to better reflect their purpose and improve
readability
- Add extra comments

`ImagePagerAdapter`:
- Rename properties to better reflect their purpose and improve
readability
- Add extra comments

app/src/main/java/app/pachli/ViewMediaActivity.kt

@@ -51,8 +51,7 @@ import app.pachli.core.navigation.AttachmentViewData
 import app.pachli.core.navigation.ViewMediaActivityIntent
 import app.pachli.core.navigation.ViewThreadActivityIntent
 import app.pachli.databinding.ActivityViewMediaBinding
-import app.pachli.fragment.ViewImageFragment
+import app.pachli.fragment.MediaActionsListener
-import app.pachli.fragment.ViewVideoFragment
 import app.pachli.pager.ImagePagerAdapter
 import app.pachli.pager.SingleImagePagerAdapter
 import app.pachli.util.getTemporaryMediaFilename
@@ -76,7 +75,7 @@ typealias ToolbarVisibilityListener = (isVisible: Boolean) -> Unit
  * Show one or more media items (pictures, video, audio, etc).
  */
 @AndroidEntryPoint
-class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener, ViewVideoFragment.VideoActionsListener {
+class ViewMediaActivity : BaseActivity(), MediaActionsListener {
     @Inject
     lateinit var okHttpClient: OkHttpClient
 
@@ -88,15 +87,21 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
     var isToolbarVisible = true
         private set
 
-    private var attachments: List<AttachmentViewData>? = null
+    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> {
-        this.toolbarVisibilityListeners.add(listener)
+        toolbarVisibilityListeners.add(listener)
         listener(isToolbarVisible)
         return { toolbarVisibilityListeners.remove(listener) }
     }
@@ -108,16 +113,16 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
         supportPostponeEnterTransition()
 
         // Gather the parameters.
-        attachments = ViewMediaActivityIntent.getAttachments(intent)
+        attachmentViewData = ViewMediaActivityIntent.getAttachments(intent)
         val initialPosition = ViewMediaActivityIntent.getAttachmentIndex(intent)
 
         // Adapter is actually of existential type PageAdapter & SharedElementsTransitionListener
         // but it cannot be expressed and if I don't specify type explicitly compilation fails
         // (probably a bug in compiler)
-        val adapter: ViewMediaAdapter = if (attachments != null) {
+        val adapter: ViewMediaAdapter = if (attachmentViewData != null) {
-            val realAttachs = attachments!!.map(AttachmentViewData::attachment)
+            val attachments = attachmentViewData!!.map(AttachmentViewData::attachment)
             // Setup the view pager.
-            ImagePagerAdapter(this, realAttachs, initialPosition)
+            ImagePagerAdapter(this, attachments, initialPosition)
         } else {
             imageUrl = ViewMediaActivityIntent.getImageUrl(intent)
                 ?: throw IllegalArgumentException("attachment list or image url has to be set")
@@ -137,12 +142,12 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
 
         // Setup the toolbar.
         setSupportActionBar(binding.toolbar)
-        val actionBar = supportActionBar
+        supportActionBar?.apply {
-        if (actionBar != null) {
+            setDisplayHomeAsUpEnabled(true)
-            actionBar.setDisplayHomeAsUpEnabled(true)
+            setDisplayShowHomeEnabled(true)
-            actionBar.setDisplayShowHomeEnabled(true)
+            title = getPageTitle(initialPosition)
-            actionBar.title = getPageTitle(initialPosition)
         }
+
         binding.toolbar.setNavigationOnClickListener { supportFinishAfterTransition() }
         binding.toolbar.setOnMenuItemClickListener { item: MenuItem ->
             when (item.itemId) {
@@ -170,7 +175,7 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
     override fun onCreateOptionsMenu(menu: Menu): Boolean {
         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 = (attachments != null)
+        menu.findItem(R.id.action_open_status)?.isVisible = (attachmentViewData != null)
         return true
     }
 
@@ -179,15 +184,15 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
         return true
     }
 
-    override fun onBringUp() {
+    override fun onMediaReady() {
         supportStartPostponedEnterTransition()
     }
 
-    override fun onDismiss() {
+    override fun onMediaDismiss() {
         supportFinishAfterTransition()
     }
 
-    override fun onPhotoTap() {
+    override fun onMediaTap() {
         isToolbarVisible = !isToolbarVisible
         for (listener in toolbarVisibilityListeners) {
             listener(isToolbarVisible)
@@ -214,14 +219,12 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
     }
 
     private fun getPageTitle(position: Int): CharSequence {
-        if (attachments == null) {
+        attachmentViewData ?: return ""
-            return ""
+        return String.format(Locale.getDefault(), "%d/%d", position + 1, attachmentViewData?.size)
-        }
-        return String.format(Locale.getDefault(), "%d/%d", position + 1, attachments?.size)
     }
 
     private fun downloadMedia() {
-        val url = imageUrl ?: attachments!![binding.viewPager.currentItem].attachment.url
+        val url = imageUrl ?: attachmentViewData!![binding.viewPager.currentItem].attachment.url
         val filename = Uri.parse(url).lastPathSegment
         Toast.makeText(applicationContext, resources.getString(R.string.download_image, filename), Toast.LENGTH_SHORT).show()
 
@@ -248,12 +251,12 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
     }
 
     private fun onOpenStatus() {
-        val attach = attachments!![binding.viewPager.currentItem]
+        val attach = attachmentViewData!![binding.viewPager.currentItem]
         startActivityWithSlideInAnimation(ViewThreadActivityIntent(this, attach.statusId, attach.statusUrl))
     }
 
     private fun copyLink() {
-        val url = imageUrl ?: attachments!![binding.viewPager.currentItem].attachment.url
+        val url = imageUrl ?: attachmentViewData!![binding.viewPager.currentItem].attachment.url
         val clipboard = getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager
         clipboard.setPrimaryClip(ClipData.newPlainText(null, url))
     }
@@ -268,7 +271,7 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
         if (imageUrl != null) {
             shareMediaFile(directory, imageUrl!!)
         } else {
-            val attachment = attachments!![binding.viewPager.currentItem].attachment
+            val attachment = attachmentViewData!![binding.viewPager.currentItem].attachment
             shareMediaFile(directory, attachment.url)
         }
     }

app/src/main/java/app/pachli/fragment/ViewImageFragment.kt

@@ -19,7 +19,6 @@ package app.pachli.fragment
 import android.animation.Animator
 import android.animation.AnimatorListenerAdapter
 import android.annotation.SuppressLint
-import android.content.Context
 import android.graphics.PointF
 import android.graphics.drawable.Drawable
 import android.os.Bundle
@@ -30,6 +29,7 @@ import android.view.View
 import android.view.ViewGroup
 import android.widget.ImageView
 import androidx.core.view.GestureDetectorCompat
+import androidx.lifecycle.lifecycleScope
 import app.pachli.R
 import app.pachli.ViewMediaActivity
 import app.pachli.core.common.extensions.hide
@@ -43,35 +43,21 @@ import com.bumptech.glide.request.RequestListener
 import com.bumptech.glide.request.target.Target
 import com.ortiz.touchview.OnTouchCoordinatesListener
 import com.ortiz.touchview.TouchImageView
-import io.reactivex.rxjava3.subjects.BehaviorSubject
 import kotlin.math.abs
+import kotlinx.coroutines.launch
 
 class ViewImageFragment : ViewMediaFragment() {
-    interface PhotoActionsListener {
-        fun onBringUp()
-        fun onDismiss()
-        fun onPhotoTap()
-    }
 
     private val binding by viewBinding(FragmentViewImageBinding::bind)
 
-    private lateinit var photoActionsListener: PhotoActionsListener
     private lateinit var toolbar: View
-    private var transition = BehaviorSubject.create<Unit>()
 
     // Volatile: Image requests happen on background thread and we want to see updates to it
     // immediately on another thread. Atomic is an overkill for such thing.
     @Volatile
     private var startedTransition = false
 
-    override fun onAttach(context: Context) {
+    override fun setupMediaView(showingDescription: Boolean) {
-        super.onAttach(context)
-        photoActionsListener = context as PhotoActionsListener
-    }
-
-    override fun setupMediaView(
-        showingDescription: Boolean,
-    ) {
         binding.photoView.transitionName = attachment.url
         binding.mediaDescription.text = attachment.description
         binding.captionSheet.visible(showingDescription)
@@ -82,7 +68,6 @@ class ViewImageFragment : ViewMediaFragment() {
 
     override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
         toolbar = (requireActivity() as ViewMediaActivity).toolbar
-        this.transition = BehaviorSubject.create()
         return inflater.inflate(R.layout.fragment_view_image, container, false)
     }
 
@@ -95,7 +80,7 @@ class ViewImageFragment : ViewMediaFragment() {
             object : GestureDetector.SimpleOnGestureListener() {
                 override fun onDown(e: MotionEvent) = true
                 override fun onSingleTapConfirmed(e: MotionEvent): Boolean {
-                    photoActionsListener.onPhotoTap()
+                    mediaActionsListener.onMediaTap()
                     return false
                 }
             },
@@ -191,7 +176,7 @@ class ViewImageFragment : ViewMediaFragment() {
                  */
                 private fun onGestureEnd(view: View) {
                     if (abs(view.translationY) > 180) {
-                        photoActionsListener.onDismiss()
+                        mediaActionsListener.onMediaDismiss()
                     } else {
                         view.animate().translationY(0f).scaleX(1f).scaleY(1f).start()
                     }
@@ -200,11 +185,6 @@ class ViewImageFragment : ViewMediaFragment() {
         )
     }
 
-    override fun onResume() {
-        super.onResume()
-        finalizeViewSetup()
-    }
-
     override fun onToolbarVisibilityChange(visible: Boolean) {
         if (!userVisibleHint) return
 
@@ -228,11 +208,7 @@ class ViewImageFragment : ViewMediaFragment() {
         Glide.with(this).clear(binding.photoView)
     }
 
-    override fun onDestroyView() {
+    @SuppressLint("CheckResult")
-        transition.onComplete()
-        super.onDestroyView()
-    }
-
     private fun loadImageFromNetwork(url: String, previewUrl: String?, photoView: ImageView) {
         val glide = Glide.with(this)
         // Request image from the any cache
@@ -240,18 +216,16 @@ class ViewImageFragment : ViewMediaFragment() {
             .load(url)
             .dontAnimate()
             .onlyRetrieveFromCache(true)
-            .let {
+            .apply {
-                if (previewUrl != null) {
+                previewUrl?.let {
-                    it.thumbnail(
+                    thumbnail(
                         glide
-                            .load(previewUrl)
+                            .load(it)
                             .dontAnimate()
                             .onlyRetrieveFromCache(true)
                             .centerInside()
                             .addListener(ImageRequestListener(true, isThumbnailRequest = true)),
                     )
-                } else {
-                    it
                 }
             }
             // Request image from the network on fail load image from cache
@@ -297,10 +271,10 @@ class ViewImageFragment : ViewMediaFragment() {
             isFirstResource: Boolean,
         ): Boolean {
             // If cache for full image failed complete transition
-            if (isCacheRequest && !isThumbnailRequest && shouldStartTransition &&
+            if (isCacheRequest && !isThumbnailRequest && shouldCallMediaReady &&
                 !startedTransition
             ) {
-                photoActionsListener.onBringUp()
+                mediaActionsListener.onMediaReady()
             }
             // Hide progress bar only on fail request from internet
             if (!isCacheRequest) binding.progressBar.hide()
@@ -318,29 +292,26 @@ class ViewImageFragment : ViewMediaFragment() {
         ): Boolean {
             binding.progressBar.hide() // Always hide the progress bar on success
 
-            if (!startedTransition || !shouldStartTransition) {
+            if (!startedTransition || !shouldCallMediaReady) {
                 // Set this right away so that we don't have to concurrent post() requests
                 startedTransition = true
                 // post() because load() replaces image with null. Sometimes after we set
                 // the thumbnail.
                 binding.photoView.post {
                     target.onResourceReady(resource, null)
-                    if (shouldStartTransition) photoActionsListener.onBringUp()
+                    if (shouldCallMediaReady) mediaActionsListener.onMediaReady()
                 }
             } else {
-                // This wait for transition. If there's no transition then we should hit
+                // Wait for the fragment transition to complete before signalling to
-                // another branch. take() will unsubscribe after we have it to not leak memory
+                // Glide that the resource is ready.
-                transition
+                transitionComplete?.let {
-                    .take(1)
+                    viewLifecycleOwner.lifecycleScope.launch {
-                    .subscribe {
+                        it.await()
                         target.onResourceReady(resource, null)
                     }
+                }
             }
             return true
         }
     }
-
-    override fun onTransitionEnd() {
-        this.transition.onNext(Unit)
-    }
 }

app/src/main/java/app/pachli/fragment/ViewMediaFragment.kt

@@ -16,6 +16,7 @@
 
 package app.pachli.fragment
 
+import android.content.Context
 import android.os.Bundle
 import android.text.TextUtils
 import android.view.View
@@ -24,23 +25,65 @@ import androidx.fragment.app.Fragment
 import androidx.media3.common.util.UnstableApi
 import app.pachli.ViewMediaActivity
 import app.pachli.core.network.model.Attachment
+import kotlinx.coroutines.CompletableDeferred
+
+/** Interface for actions that may happen while media is being displayed */
+interface MediaActionsListener {
+    /**
+     * The media fragment is ready for the hosting activity to complete the
+     * fragment transition; typically because any media to be displayed has
+     * been loaded.
+     */
+    fun onMediaReady()
+
+    /** The user is dismissing the media (e.g., by flinging up) */
+    fun onMediaDismiss()
+
+    /** The user has tapped on the media (typically to show/hide UI controls) */
+    fun onMediaTap()
+}
 
 abstract class ViewMediaFragment : Fragment() {
-    private var toolbarVisibilityDisposable: Function0<Boolean>? = null
+    /** Function to remove the toolbar listener */
+    private var removeToolbarListener: Function0<Boolean>? = null
 
-    abstract fun setupMediaView(
+    /**
-        showingDescription: Boolean,
+     * Called after [onResume], subclasses should override this and update
-    )
+     * the contents of views (including loading any media).
+     *
+     * @param showingDescription True if the media's description should be shown
+     */
+    abstract fun setupMediaView(showingDescription: Boolean)
 
+    /**
+     * Called when the visibility of the toolbar changes.
+     *
+     * @param visible True if the toolbar is visible
+     */
     abstract fun onToolbarVisibilityChange(visible: Boolean)
 
     protected var showingDescription = false
     protected var isDescriptionVisible = false
 
-    /** The attachment to show. Set in [onViewCreated] */
+    /** The attachment to show */
     protected lateinit var attachment: Attachment
 
-    protected var shouldStartTransition = false
+    /** Listener to call as media is loaded or on user interaction */
+    protected lateinit var mediaActionsListener: MediaActionsListener
+
+    /**
+     * True if the fragment should call [MediaActionsListener.onMediaReady]
+     * when the media is loaded.
+     */
+    protected var shouldCallMediaReady = false
+
+    /** Awaitable signal that the transition has completed */
+    var transitionComplete: CompletableDeferred<Unit>? = CompletableDeferred()
+
+    override fun onAttach(context: Context) {
+        super.onAttach(context)
+        mediaActionsListener = context as MediaActionsListener
+    }
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
@@ -48,23 +91,60 @@ abstract class ViewMediaFragment : Fragment() {
         attachment = arguments?.getParcelable<Attachment>(ARG_ATTACHMENT)
             ?: throw IllegalArgumentException("ARG_ATTACHMENT has to be set")
 
-        shouldStartTransition = arguments?.getBoolean(ARG_START_POSTPONED_TRANSITION)
+        shouldCallMediaReady = arguments?.getBoolean(ARG_SHOULD_CALL_MEDIA_READY)
             ?: throw IllegalArgumentException("ARG_START_POSTPONED_TRANSITION has to be set")
     }
 
+    /**
+     * Called by the fragment adapter to notify the fragment that the shared
+     * element transition has been completed.
+     */
+    open fun onTransitionEnd() {
+        this.transitionComplete?.complete(Unit)
+    }
+
+    override fun onResume() {
+        super.onResume()
+        finalizeViewSetup()
+    }
+
+    private fun finalizeViewSetup() {
+        val mediaActivity = activity as ViewMediaActivity
+
+        showingDescription = !TextUtils.isEmpty(attachment.description)
+        isDescriptionVisible = showingDescription
+        setupMediaView(showingDescription && mediaActivity.isToolbarVisible)
+
+        removeToolbarListener = (activity as ViewMediaActivity)
+            .addToolbarVisibilityListener { isVisible ->
+                onToolbarVisibilityChange(isVisible)
+            }
+    }
+
+    override fun onDestroyView() {
+        removeToolbarListener?.invoke()
+        transitionComplete = null
+        super.onDestroyView()
+    }
+
     companion object {
-        @JvmStatic
+        protected const val ARG_SHOULD_CALL_MEDIA_READY = "shouldCallMediaReady"
-        protected val ARG_START_POSTPONED_TRANSITION = "startPostponedTransition"
 
-        @JvmStatic
+        protected const val ARG_ATTACHMENT = "attach"
-        protected val ARG_ATTACHMENT = "attach"
 
+        /**
+         * @param attachment The media attachment to display in the fragment
+         * @param shouldCallMediaReady If true this fragment should call
+         *  [MediaActionsListener.onMediaReady] when it has finished loading
+         *  media, so the calling activity can perform any final actions.
+         * @return A fragment that shows [attachment]
+         */
         @JvmStatic
         @OptIn(UnstableApi::class)
-        fun newInstance(attachment: Attachment, shouldStartPostponedTransition: Boolean): ViewMediaFragment {
+        fun newInstance(attachment: Attachment, shouldCallMediaReady: Boolean): ViewMediaFragment {
             val arguments = Bundle(2)
             arguments.putParcelable(ARG_ATTACHMENT, attachment)
-            arguments.putBoolean(ARG_START_POSTPONED_TRANSITION, shouldStartPostponedTransition)
+            arguments.putBoolean(ARG_SHOULD_CALL_MEDIA_READY, shouldCallMediaReady)
 
             val fragment = when (attachment.type) {
                 Attachment.Type.IMAGE -> ViewImageFragment()
@@ -94,30 +174,10 @@ abstract class ViewMediaFragment : Fragment() {
                     blurhash = null,
                 ),
             )
-            arguments.putBoolean(ARG_START_POSTPONED_TRANSITION, true)
+            arguments.putBoolean(ARG_SHOULD_CALL_MEDIA_READY, true)
 
             fragment.arguments = arguments
             return fragment
         }
     }
-
-    abstract fun onTransitionEnd()
-
-    protected fun finalizeViewSetup() {
-        val mediaActivity = activity as ViewMediaActivity
-
-        showingDescription = !TextUtils.isEmpty(attachment.description)
-        isDescriptionVisible = showingDescription
-        setupMediaView(showingDescription && mediaActivity.isToolbarVisible)
-
-        toolbarVisibilityDisposable = (activity as ViewMediaActivity)
-            .addToolbarVisibilityListener { isVisible ->
-                onToolbarVisibilityChange(isVisible)
-            }
-    }
-
-    override fun onDestroyView() {
-        toolbarVisibilityDisposable?.invoke()
-        super.onDestroyView()
-    }
 }

app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt

@@ -23,8 +23,6 @@ import android.content.Context
 import android.graphics.drawable.Drawable
 import android.os.Build
 import android.os.Bundle
-import android.os.Handler
-import android.os.Looper
 import android.text.method.ScrollingMovementMethod
 import android.view.GestureDetector
 import android.view.Gravity
@@ -36,6 +34,7 @@ import android.widget.FrameLayout
 import android.widget.LinearLayout
 import androidx.annotation.OptIn
 import androidx.core.view.GestureDetectorCompat
+import androidx.lifecycle.lifecycleScope
 import androidx.media3.common.MediaItem
 import androidx.media3.common.PlaybackException
 import androidx.media3.common.Player
@@ -50,10 +49,12 @@ import app.pachli.BuildConfig
 import app.pachli.R
 import app.pachli.ViewMediaActivity
 import app.pachli.core.common.extensions.hide
+import app.pachli.core.common.extensions.show
 import app.pachli.core.common.extensions.viewBinding
 import app.pachli.core.common.extensions.visible
 import app.pachli.core.network.model.Attachment
 import app.pachli.databinding.FragmentViewVideoBinding
+import app.pachli.fragment.ViewVideoFragment.Companion.CONTROLS_TIMEOUT
 import com.bumptech.glide.Glide
 import com.bumptech.glide.request.target.CustomTarget
 import com.bumptech.glide.request.transition.Transition
@@ -61,6 +62,10 @@ import com.google.android.material.snackbar.Snackbar
 import dagger.hilt.android.AndroidEntryPoint
 import javax.inject.Inject
 import kotlin.math.abs
+import kotlin.time.Duration.Companion.seconds
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import okhttp3.OkHttpClient
 
 /**
@@ -69,31 +74,24 @@ import okhttp3.OkHttpClient
  * UI behaviour:
  *
  * - Fragment starts, media description is visible at top of screen, video starts playing
- * - Media description + toolbar disappears after CONTROLS_TIMEOUT_MS
+ * - Media description + toolbar disappears after [CONTROLS_TIMEOUT]
- * - Tapping shows controls + media description + toolbar, which fade after CONTROLS_TIMEOUT_MS
+ * - Tapping shows controls + media description + toolbar, which fade after [CONTROLS_TIMEOUT]
  * - Tapping pause, or the media description, pauses the video and the controls + media description
  *   remain visible
  */
 @UnstableApi
 @AndroidEntryPoint
 class ViewVideoFragment : ViewMediaFragment() {
-    interface VideoActionsListener {
-        fun onDismiss()
-    }
-
     @Inject
     lateinit var okHttpClient: OkHttpClient
 
     private val binding by viewBinding(FragmentViewVideoBinding::bind)
 
-    private lateinit var videoActionsListener: VideoActionsListener
     private lateinit var toolbar: View
-    private val handler = Handler(Looper.getMainLooper())
+
-    private val hideToolbar = Runnable {
+    /** Hoist toolbar hiding to activity so it can track state across different fragments */
-        // Hoist toolbar hiding to activity so it can track state across different fragments
+    private var hideToolbarJob: Job? = null
-        // This is explicitly stored as runnable so that we pass it to the handler later for cancellation
+
-        mediaActivity.onPhotoTap()
-    }
     private lateinit var mediaActivity: ViewMediaActivity
     private lateinit var mediaPlayerListener: Player.Listener
     private var isAudio = false
@@ -107,13 +105,13 @@ class ViewVideoFragment : ViewMediaFragment() {
 
     private lateinit var mediaSourceFactory: DefaultMediaSourceFactory
 
+    private var startedTransition = false
+
     override fun onAttach(context: Context) {
         super.onAttach(context)
 
         mediaSourceFactory = DefaultMediaSourceFactory(context)
             .setDataSourceFactory(DefaultDataSource.Factory(context, OkHttpDataSource.Factory(okHttpClient)))
-
-        videoActionsListener = context as VideoActionsListener
     }
 
     @SuppressLint("PrivateResource", "MissingInflatedId")
@@ -137,13 +135,12 @@ class ViewVideoFragment : ViewMediaFragment() {
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
 
-        binding.videoView.controllerShowTimeoutMs = CONTROLS_TIMEOUT_MS
+        binding.videoView.controllerShowTimeoutMs = CONTROLS_TIMEOUT.inWholeMilliseconds.toInt()
 
         isAudio = attachment.type == Attachment.Type.AUDIO
+        binding.progressBar.show()
 
-        /**
+        /** Handle single taps, flings, and dragging */
-         * Handle single taps, flings, and dragging
-         */
         val touchListener = object : View.OnTouchListener {
             var lastY = 0f
 
@@ -160,7 +157,7 @@ class ViewVideoFragment : ViewMediaFragment() {
 
                     /** A single tap should show/hide the media description */
                     override fun onSingleTapUp(e: MotionEvent): Boolean {
-                        mediaActivity.onPhotoTap()
+                        mediaActivity.onMediaTap()
                         return false
                     }
 
@@ -172,7 +169,7 @@ class ViewVideoFragment : ViewMediaFragment() {
                         velocityY: Float,
                     ): Boolean {
                         if (abs(velocityY) > abs(velocityX)) {
-                            videoActionsListener.onDismiss()
+                            mediaActionsListener.onMediaDismiss()
                             return true
                         }
                         return false
@@ -196,7 +193,7 @@ class ViewVideoFragment : ViewMediaFragment() {
                     }
                 } else if (event.action == MotionEvent.ACTION_UP || event.action == MotionEvent.ACTION_CANCEL) {
                     if (abs(contentFrame.translationY) > 180) {
-                        videoActionsListener.onDismiss()
+                        mediaActionsListener.onMediaDismiss()
                     } else {
                         contentFrame.animate().translationY(0f).scaleX(1f).scaleY(1f).start()
                     }
@@ -222,6 +219,18 @@ class ViewVideoFragment : ViewMediaFragment() {
 
                         binding.progressBar.hide()
                         binding.videoView.useController = true
+
+                        if (shouldCallMediaReady && !startedTransition) {
+                            startedTransition = true
+                            mediaActivity.onMediaReady()
+                        }
+
+                        transitionComplete?.let {
+                            viewLifecycleOwner.lifecycleScope.launch {
+                                it.await()
+                                player?.play()
+                            }
+                        }
                     }
                     else -> { /* do nothing */ }
                 }
@@ -234,7 +243,7 @@ class ViewVideoFragment : ViewMediaFragment() {
                 if (isPlaying) {
                     hideToolbarAfterDelay()
                 } else {
-                    handler.removeCallbacks(hideToolbar)
+                    hideToolbarJob?.cancel()
                 }
             }
 
@@ -266,8 +275,6 @@ class ViewVideoFragment : ViewMediaFragment() {
     override fun onResume() {
         super.onResume()
 
-        finalizeViewSetup()
-
         if (Build.VERSION.SDK_INT <= 23 || player == null) {
             initializePlayer()
             if (mediaActivity.isToolbarVisible && !isAudio) {
@@ -294,7 +301,7 @@ class ViewVideoFragment : ViewMediaFragment() {
         if (Build.VERSION.SDK_INT <= 23) {
             binding.videoView.onPause()
             releasePlayer()
-            handler.removeCallbacks(hideToolbar)
+            hideToolbarJob?.cancel()
         }
     }
 
@@ -306,7 +313,7 @@ class ViewVideoFragment : ViewMediaFragment() {
         if (Build.VERSION.SDK_INT > 23) {
             binding.videoView.onPause()
             releasePlayer()
-            handler.removeCallbacks(hideToolbar)
+            hideToolbarJob?.cancel()
         }
     }
 
@@ -323,7 +330,10 @@ class ViewVideoFragment : ViewMediaFragment() {
                 setMediaItem(MediaItem.fromUri(mediaAttachment.url))
                 addListener(mediaPlayerListener)
                 repeatMode = Player.REPEAT_MODE_ONE
-                playWhenReady = true
+
+                // Playback is automatically started in onPlaybackStateChanged when
+                // any transitions have completed.
+                playWhenReady = false
                 seekTo(savedSeekPosition)
                 prepare()
                 player = this
@@ -355,9 +365,9 @@ class ViewVideoFragment : ViewMediaFragment() {
     }
 
     @SuppressLint("ClickableViewAccessibility")
-    override fun setupMediaView(
+    override fun setupMediaView(showingDescription: Boolean) {
-        showingDescription: Boolean,
+        startedTransition = false
-    ) {
+
         binding.mediaDescription.text = attachment.description
         binding.mediaDescription.visible(showingDescription)
         binding.mediaDescription.movementMethod = ScrollingMovementMethod()
@@ -377,21 +387,18 @@ class ViewVideoFragment : ViewMediaFragment() {
         }
 
         binding.videoView.requestFocus()
-
-        if (requireArguments().getBoolean(ARG_START_POSTPONED_TRANSITION)) {
-            mediaActivity.onBringUp()
-        }
     }
 
     private fun hideToolbarAfterDelay() {
-        handler.postDelayed(hideToolbar, CONTROLS_TIMEOUT_MS.toLong())
+        hideToolbarJob?.cancel()
+        hideToolbarJob = lifecycleScope.launch {
+            delay(CONTROLS_TIMEOUT)
+            mediaActivity.onMediaTap()
+        }
     }
 
     override fun onToolbarVisibilityChange(visible: Boolean) {
-        if (!userVisibleHint) {
+        if (!userVisibleHint) return
-            return
-        }
-
         view ?: return
 
         isDescriptionVisible = showingDescription && visible
@@ -417,14 +424,12 @@ class ViewVideoFragment : ViewMediaFragment() {
         if (visible && (binding.videoView.player?.isPlaying == true) && !isAudio) {
             hideToolbarAfterDelay()
         } else {
-            handler.removeCallbacks(hideToolbar)
+            hideToolbarJob?.cancel()
         }
     }
 
-    override fun onTransitionEnd() { }
-
     companion object {
-        private const val CONTROLS_TIMEOUT_MS = 2000 // Consistent with YouTube player
+        private val CONTROLS_TIMEOUT = 2.seconds // Consistent with YouTube player
         private const val SEEK_POSITION = "seekPosition"
     }
 }

app/src/main/java/app/pachli/pager/ImagePagerAdapter.kt

@@ -13,7 +13,8 @@ class ImagePagerAdapter(
     private val initialPosition: Int,
 ) : ViewMediaAdapter(activity) {
 
-    private var didTransition = false
+    /** True if the animated transition to the fragment has completed */
+    private var transitionComplete = false
     private val fragments = MutableList<WeakReference<ViewMediaFragment>?>(attachments.size) { null }
 
     override fun getItemCount() = attachments.size
@@ -26,7 +27,7 @@ class ImagePagerAdapter(
             // transition and wait until it's over and it will never take place.
             val fragment = ViewMediaFragment.newInstance(
                 attachment = attachments[position],
-                shouldStartPostponedTransition = !didTransition && position == initialPosition,
+                shouldCallMediaReady = !transitionComplete && position == initialPosition,
             )
             fragments[position] = WeakReference(fragment)
             return fragment
@@ -35,8 +36,14 @@ class ImagePagerAdapter(
         }
     }
 
+    /**
+     * Called by the hosting activity to notify the adapter that the shared element
+     * transition to the first displayed item in the adapter has completed.
+     *
+     * Forward the notification to the fragment.
+     */
     override fun onTransitionEnd(position: Int) {
-        this.didTransition = true
+        this.transitionComplete = true
         fragments[position]?.get()?.onTransitionEnd()
     }
 }