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

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<Attachment>(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()
     }

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<Boolean>? = 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)

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<Attachment>(ViewMediaFragment.ARG_ATTACHMENT)
+        val attachment = arguments?.getParcelable<Attachment>(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)
                     }
                 })

app/src/main/res/layout/fragment_view_image.xml

@@ -26,7 +26,7 @@
         android:layout_gravity="center" />
 
     <TextView
-        android:id="@+id/descriptionView"
+        android:id="@+id/mediaDescription"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
         app:layout_constraintBottom_toBottomOf="parent"