From 1026fccc4047edac3e7060b4fd13fa13c87c500d Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Sun, 3 Mar 2024 16:07:39 +0100 Subject: [PATCH] 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 --- .../main/java/app/pachli/ViewMediaActivity.kt | 55 ++++---- .../app/pachli/fragment/ViewImageFragment.kt | 69 +++------- .../app/pachli/fragment/ViewMediaFragment.kt | 128 +++++++++++++----- .../app/pachli/fragment/ViewVideoFragment.kt | 97 ++++++------- .../app/pachli/pager/ImagePagerAdapter.kt | 13 +- 5 files changed, 204 insertions(+), 158 deletions(-) diff --git a/app/src/main/java/app/pachli/ViewMediaActivity.kt b/app/src/main/java/app/pachli/ViewMediaActivity.kt index b7e713501..5c08151c7 100644 --- a/app/src/main/java/app/pachli/ViewMediaActivity.kt +++ b/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.ViewVideoFragment +import app.pachli.fragment.MediaActionsListener 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? = null + 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 { - 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 realAttachs = attachments!!.map(AttachmentViewData::attachment) + val adapter: ViewMediaAdapter = if (attachmentViewData != null) { + 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 - if (actionBar != null) { - actionBar.setDisplayHomeAsUpEnabled(true) - actionBar.setDisplayShowHomeEnabled(true) - actionBar.title = getPageTitle(initialPosition) + supportActionBar?.apply { + setDisplayHomeAsUpEnabled(true) + setDisplayShowHomeEnabled(true) + 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) { - return "" - } - return String.format(Locale.getDefault(), "%d/%d", position + 1, attachments?.size) + attachmentViewData ?: return "" + return String.format(Locale.getDefault(), "%d/%d", position + 1, attachmentViewData?.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) } } diff --git a/app/src/main/java/app/pachli/fragment/ViewImageFragment.kt b/app/src/main/java/app/pachli/fragment/ViewImageFragment.kt index e0e59d9bc..3b19e35de 100644 --- a/app/src/main/java/app/pachli/fragment/ViewImageFragment.kt +++ b/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() // 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) { - super.onAttach(context) - photoActionsListener = context as PhotoActionsListener - } - - override fun setupMediaView( - showingDescription: Boolean, - ) { + 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() { - transition.onComplete() - super.onDestroyView() - } - + @SuppressLint("CheckResult") 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 { - if (previewUrl != null) { - it.thumbnail( + .apply { + previewUrl?.let { + 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 - // another branch. take() will unsubscribe after we have it to not leak memory - transition - .take(1) - .subscribe { + // Wait for the fragment transition to complete before signalling to + // Glide that the resource is ready. + transitionComplete?.let { + viewLifecycleOwner.lifecycleScope.launch { + it.await() target.onResourceReady(resource, null) } + } } return true } } - - override fun onTransitionEnd() { - this.transition.onNext(Unit) - } } diff --git a/app/src/main/java/app/pachli/fragment/ViewMediaFragment.kt b/app/src/main/java/app/pachli/fragment/ViewMediaFragment.kt index 7a228a06c..2c94f9c09 100644 --- a/app/src/main/java/app/pachli/fragment/ViewMediaFragment.kt +++ b/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? = null + /** Function to remove the toolbar listener */ + private var removeToolbarListener: Function0? = 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? = 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(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 val ARG_START_POSTPONED_TRANSITION = "startPostponedTransition" + protected const val ARG_SHOULD_CALL_MEDIA_READY = "shouldCallMediaReady" - @JvmStatic - protected val ARG_ATTACHMENT = "attach" + protected const 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() - } } diff --git a/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt b/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt index 7e1c9b2fb..0a98cf5a1 100644 --- a/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt +++ b/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 - * - Tapping shows controls + media description + toolbar, which fade after CONTROLS_TIMEOUT_MS + * - Media description + toolbar disappears after [CONTROLS_TIMEOUT] + * - 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 - // This is explicitly stored as runnable so that we pass it to the handler later for cancellation - mediaActivity.onPhotoTap() - } + + /** Hoist toolbar hiding to activity so it can track state across different fragments */ + private var hideToolbarJob: Job? = null + 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( - showingDescription: Boolean, - ) { + override fun setupMediaView(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) { - return - } - + if (!userVisibleHint) 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" } } diff --git a/app/src/main/java/app/pachli/pager/ImagePagerAdapter.kt b/app/src/main/java/app/pachli/pager/ImagePagerAdapter.kt index 8cdb8baa9..193cd9b5d 100644 --- a/app/src/main/java/app/pachli/pager/ImagePagerAdapter.kt +++ b/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?>(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() } }