From c05eaae317cc1cf133d6d5a44b2a1883a41dc967 Mon Sep 17 00:00:00 2001 From: Levi Bard Date: Thu, 1 Nov 2018 14:52:22 +0100 Subject: [PATCH] Fix media transitions, reduce code duplication between media fragments (#894) * Fix media transitions, reduce code duplication between media fragments * Remove redundant helper * Fix occasional crash when swiping between mixed media * Hide controls when swiping between media --- .../tusky/fragment/ViewImageFragment.kt | 44 ++++----------- .../tusky/fragment/ViewMediaFragment.kt | 18 ++++++- .../tusky/fragment/ViewVideoFragment.kt | 54 ++++++------------- .../main/res/layout/fragment_view_image.xml | 2 +- 4 files changed, 45 insertions(+), 73 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt index 467b684c..8873f67a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt @@ -20,18 +20,17 @@ import android.animation.AnimatorListenerAdapter import android.content.Context import android.os.Bundle import android.support.v4.view.ViewCompat -import android.text.TextUtils import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.ImageView +import android.widget.TextView import com.github.chrisbanes.photoview.PhotoViewAttacher import com.keylesspalace.tusky.R -import com.keylesspalace.tusky.ViewMediaActivity import com.keylesspalace.tusky.entity.Attachment import com.keylesspalace.tusky.util.hide -import com.keylesspalace.tusky.util.show +import com.keylesspalace.tusky.util.visible import com.squareup.picasso.Callback import com.squareup.picasso.NetworkPolicy import com.squareup.picasso.Picasso @@ -48,9 +47,7 @@ class ViewImageFragment : ViewMediaFragment() { private lateinit var attacher: PhotoViewAttacher private lateinit var photoActionsListener: PhotoActionsListener private lateinit var toolbar: View - - private var showingDescription = false - private var isDescriptionVisible = false + override lateinit var descriptionView : TextView companion object { private const val TAG = "ViewImageFragment" @@ -62,6 +59,8 @@ class ViewImageFragment : ViewMediaFragment() { } override fun setupMediaView(url: String) { + descriptionView = mediaDescription + ViewCompat.setTransitionName(photoView, url) attacher = PhotoViewAttacher(photoView) // Clicking outside the photo closes the viewer. @@ -132,38 +131,21 @@ class ViewImageFragment : ViewMediaFragment() { val arguments = this.arguments!! val attachment = arguments.getParcelable(ARG_ATTACHMENT) val url: String? + var description : String? = null if (attachment != null) { url = attachment.url - - val description = attachment.description - - descriptionView.text = description - showingDescription = !TextUtils.isEmpty(description) - isDescriptionVisible = showingDescription + description = attachment.description } else { url = arguments.getString(ARG_AVATAR_URL) if (url == null) { throw IllegalArgumentException("attachment or avatar url has to be set") } - - showingDescription = false - isDescriptionVisible = false - } - - // Setting visibility without animations so it looks nice when you scroll images - if (showingDescription && (activity as ViewMediaActivity).isToolbarVisible()) { - descriptionView.show() - } else { - descriptionView.hide() } - setupMediaView(url) - - setupToolbarVisibilityListener() + finalizeViewSetup(url, description) } - private fun onMediaTap() { photoActionsListener.onPhotoTap() } @@ -177,11 +159,7 @@ class ViewImageFragment : ViewMediaFragment() { descriptionView.animate().alpha(alpha) .setListener(object : AnimatorListenerAdapter() { override fun onAnimationEnd(animation: Animator) { - if (isDescriptionVisible) { - descriptionView.show() - } else { - descriptionView.hide() - } + descriptionView.visible(isDescriptionVisible) animation.removeListener(this) } }) @@ -204,13 +182,13 @@ class ViewImageFragment : ViewMediaFragment() { } override fun onError() { - progressBar.hide() + progressBar?.hide() } }) } private fun finishLoadingSuccessfully() { - progressBar.hide() + progressBar?.hide() attacher.update() photoActionsListener.onBringUp() } diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewMediaFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewMediaFragment.kt index 628ca568..47b480ba 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewMediaFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewMediaFragment.kt @@ -16,15 +16,22 @@ package com.keylesspalace.tusky.fragment import android.os.Bundle +import android.text.TextUtils +import android.widget.TextView import com.keylesspalace.tusky.ViewMediaActivity import com.keylesspalace.tusky.entity.Attachment +import com.keylesspalace.tusky.util.visible abstract class ViewMediaFragment : BaseFragment() { private var toolbarVisibiltyDisposable: Function0? = null abstract fun setupMediaView(url: String) abstract fun onToolbarVisibilityChange(visible: Boolean) + abstract val descriptionView : TextView + + protected var showingDescription = false + protected var isDescriptionVisible = false companion object { @JvmStatic protected val ARG_START_POSTPONED_TRANSITION = "startPostponedTransition" @@ -60,7 +67,16 @@ abstract class ViewMediaFragment : BaseFragment() { } } - protected fun setupToolbarVisibilityListener() { + protected fun finalizeViewSetup(url: String, description: String?) { + val mediaActivity = activity as ViewMediaActivity + setupMediaView(url) + + descriptionView.text = description ?: "" + showingDescription = !TextUtils.isEmpty(description) + isDescriptionVisible = showingDescription + + descriptionView.visible(showingDescription && mediaActivity.isToolbarVisible()) + toolbarVisibiltyDisposable = (activity as ViewMediaActivity).addToolbarVisibilityListener(object: ViewMediaActivity.ToolbarVisibilityListener { override fun onToolbarVisiblityChanged(isVisible: Boolean) { onToolbarVisibilityChange(isVisible) diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt index 924b428f..4ca0e0c6 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt @@ -22,17 +22,17 @@ import android.os.Bundle import android.os.Handler import android.os.Looper import android.support.v4.view.ViewCompat -import android.text.TextUtils import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.MediaController +import android.widget.TextView import com.keylesspalace.tusky.R import com.keylesspalace.tusky.ViewMediaActivity import com.keylesspalace.tusky.entity.Attachment import com.keylesspalace.tusky.util.hide -import com.keylesspalace.tusky.util.show +import com.keylesspalace.tusky.util.visible import kotlinx.android.synthetic.main.activity_view_media.* import kotlinx.android.synthetic.main.fragment_view_video.* @@ -46,9 +46,8 @@ class ViewVideoFragment : ViewMediaFragment() { } private lateinit var mediaActivity: ViewMediaActivity private val TOOLBAR_HIDE_DELAY_MS = 3000L - - private var showingDescription = false - private var isDescriptionVisible = false + override lateinit var descriptionView : TextView + private lateinit var mediaController : MediaController companion object { private const val TAG = "ViewVideoFragment" @@ -65,20 +64,23 @@ class ViewVideoFragment : ViewMediaFragment() { if (mediaActivity.isToolbarVisible()) { handler.postDelayed(hideToolbar, TOOLBAR_HIDE_DELAY_MS) } - videoPlayer?.start() + videoPlayer.start() } else { handler.removeCallbacks(hideToolbar) - videoPlayer?.pause() + videoPlayer.pause() + mediaController.hide() } } @SuppressLint("ClickableViewAccessibility") override fun setupMediaView(url: String) { + descriptionView = mediaDescription val videoView = videoPlayer + ViewCompat.setTransitionName(videoView, url) videoView.setVideoPath(url) - val controller = MediaController(mediaActivity) - controller.setMediaPlayer(videoView) - videoView.setMediaController(controller) + mediaController = MediaController(mediaActivity) + mediaController.setMediaPlayer(videoPlayer) + videoPlayer.setMediaController(mediaController) videoView.requestFocus() videoView.setOnTouchListener { _, _ -> mediaActivity.onPhotoTap() @@ -110,34 +112,14 @@ class ViewVideoFragment : ViewMediaFragment() { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - - val arguments = this.arguments!! - val attachment = arguments.getParcelable(ViewMediaFragment.ARG_ATTACHMENT) + val attachment = arguments?.getParcelable(ViewMediaFragment.ARG_ATTACHMENT) val url: String if (attachment == null) { throw IllegalArgumentException("attachment has to be set") } url = attachment.url - val description = attachment.description - mediaDescription.text = description - showingDescription = !TextUtils.isEmpty(description) - isDescriptionVisible = showingDescription - - // Setting visibility without animations so it looks nice when you scroll media - //noinspection ConstantConditions - if (showingDescription && mediaActivity.isToolbarVisible()) { - mediaDescription.show() - } else { - mediaDescription.hide() - - } - - ViewCompat.setTransitionName(videoPlayer!!, url) - - setupMediaView(url) - - setupToolbarVisibilityListener() + finalizeViewSetup(url, attachment.description) } override fun onToolbarVisibilityChange(visible: Boolean) { @@ -147,14 +129,10 @@ class ViewVideoFragment : ViewMediaFragment() { isDescriptionVisible = showingDescription && visible val alpha = if (isDescriptionVisible) 1.0f else 0.0f - mediaDescription.animate().alpha(alpha) + descriptionView.animate().alpha(alpha) .setListener(object : AnimatorListenerAdapter() { override fun onAnimationEnd(animation: Animator) { - if (isDescriptionVisible) { - mediaDescription.show() - } else { - mediaDescription.hide() - } + descriptionView.visible(isDescriptionVisible) animation.removeListener(this) } }) diff --git a/app/src/main/res/layout/fragment_view_image.xml b/app/src/main/res/layout/fragment_view_image.xml index 0ff03b2a..f2798d3b 100644 --- a/app/src/main/res/layout/fragment_view_image.xml +++ b/app/src/main/res/layout/fragment_view_image.xml @@ -26,7 +26,7 @@ android:layout_gravity="center" />