Description improvements (#1846)

* Increase character limit for media descriptions to 1500 It was increased in Mastodon 3.0.0 which was released in October 2019. * Improve image description view Since media descriptions can be longer now, we need to adjust the UI. It is a common problem that description takes up the whole screen, it's hard for readers and also discourages people from adding descriptions. This commit uses bottom sheet to hide most of the description. Since we know how much screen space it will cover, we can use darker background which makes reading text easier. * Adjust description handle * Fix unable to dismiss image caption
2020-08-01 21:48:51 +02:00 · 2020-08-01 21:48:51 +02:00 · ed2918da2e
commit ed2918da2e
parent be8fc9f15a
8 changed files with 140 additions and 81 deletions
--- a/app/src/main/java/com/keylesspalace/tusky/adapter/StatusBaseViewHolder.java
+++ b/app/src/main/java/com/keylesspalace/tusky/adapter/StatusBaseViewHolder.java
@ -753,7 +753,7 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder {
            itemView.setAccessibilityDelegate(null);
        } else {
            if (payloads instanceof List)
-                for (Object item : (List) payloads) {
+                for (Object item : (List<?>) payloads) {
                    if (Key.KEY_CREATED.equals(item)) {
                        setCreatedAt(status.getCreatedAt(), statusDisplayOptions);
                    }
--- a/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt
@ -37,9 +37,8 @@ import com.github.chrisbanes.photoview.PhotoView
 import com.keylesspalace.tusky.R
 import com.keylesspalace.tusky.util.withLifecycleContext

-// https://github.com/tootsuite/mastodon/blob/1656663/app/models/media_attachment.rb#L94
-private const val MEDIA_DESCRIPTION_CHARACTER_LIMIT = 420
-
+// https://github.com/tootsuite/mastodon/blob/c6904c0d3766a2ea8a81ab025c127169ecb51373/app/models/media_attachment.rb#L32
+private const val MEDIA_DESCRIPTION_CHARACTER_LIMIT = 1500

 fun <T> T.makeCaptionDialog(existingDescription: String?,
                            previewUri: Uri,
--- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt
@ -21,9 +21,11 @@ import android.annotation.SuppressLint
 import android.content.Context
 import android.graphics.drawable.Drawable
 import android.os.Bundle
-import android.view.*
+import android.view.LayoutInflater
+import android.view.MotionEvent
+import android.view.View
+import android.view.ViewGroup
 import android.widget.ImageView
-import android.widget.TextView
 import com.bumptech.glide.Glide
 import com.bumptech.glide.load.DataSource
 import com.bumptech.glide.load.engine.GlideException
@ -57,16 +59,52 @@ class ViewImageFragment : ViewMediaFragment() {
    @Volatile
    private var startedTransition = false

-    override lateinit var descriptionView: TextView
    override fun onAttach(context: Context) {
        super.onAttach(context)
        photoActionsListener = context as PhotoActionsListener
    }

-    @SuppressLint("ClickableViewAccessibility")
-    override fun setupMediaView(url: String, previewUrl: String?) {
-        descriptionView = mediaDescription
+
+    override fun setupMediaView(
+            url: String,
+            previewUrl: String?,
+            description: String?,
+            showingDescription: Boolean
+    ) {
        photoView.transitionName = url
+        mediaDescription.text = description
+        captionSheet.visible(showingDescription)
+
+        startedTransition = false
+        loadImageFromNetwork(url, previewUrl, photoView)
+    }
+
+    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
+        toolbar = requireActivity().toolbar
+        this.transition = BehaviorSubject.create()
+        return inflater.inflate(R.layout.fragment_view_image, container, false)
+    }
+
+    @SuppressLint("ClickableViewAccessibility")
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
+        super.onViewCreated(view, savedInstanceState)
+
+        val arguments = this.requireArguments()
+        val attachment = arguments.getParcelable<Attachment>(ARG_ATTACHMENT)
+        this.shouldStartTransition = arguments.getBoolean(ARG_START_POSTPONED_TRANSITION)
+        val url: String?
+        var description: String? = null
+
+        if (attachment != null) {
+            url = attachment.url
+            description = attachment.description
+        } else {
+            url = arguments.getString(ARG_AVATAR_URL)
+            if (url == null) {
+                throw IllegalArgumentException("attachment or avatar url has to be set")
+            }
+        }
+
        attacher = PhotoViewAttacher(photoView).apply {
            // This prevents conflicts with ViewPager
            setAllowParentInterceptOnEdge(true)
@ -87,21 +125,12 @@ class ViewImageFragment : ViewMediaFragment() {
            }
        }

-        val gestureDetector = GestureDetector(requireContext(), object : GestureDetector.SimpleOnGestureListener() {
-            override fun onSingleTapConfirmed(e: MotionEvent?): Boolean {
-                onMediaTap()
-                return true
-            }
-        })
-
        var lastY = 0f

        photoView.setOnTouchListener { v, event ->
            // This part is for scaling/translating on vertical move.
            // We use raw coordinates to get the correct ones during scaling

-            gestureDetector.onTouchEvent(event)
-
            if (event.action == MotionEvent.ACTION_DOWN) {
                lastY = event.rawY
            } else if (event.pointerCount == 1
@ -125,36 +154,6 @@ class ViewImageFragment : ViewMediaFragment() {
            attacher.onTouch(v, event)
        }

-        startedTransition = false
-        loadImageFromNetwork(url, previewUrl, photoView)
-    }
-
-    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
-        toolbar = requireActivity().toolbar
-        this.transition = BehaviorSubject.create()
-        return inflater.inflate(R.layout.fragment_view_image, container, false)
-    }
-
-    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-        super.onViewCreated(view, savedInstanceState)
-
-
-        val arguments = this.requireArguments()
-        val attachment = arguments.getParcelable<Attachment>(ARG_ATTACHMENT)
-        this.shouldStartTransition = arguments.getBoolean(ARG_START_POSTPONED_TRANSITION)
-        val url: String?
-        var description: String? = null
-
-        if (attachment != null) {
-            url = attachment.url
-            description = attachment.description
-        } else {
-            url = arguments.getString(ARG_AVATAR_URL)
-            if (url == null) {
-                throw IllegalArgumentException("attachment or avatar url has to be set")
-            }
-        }
-
        finalizeViewSetup(url, attachment?.previewUrl, description)
    }

@ -176,10 +175,10 @@ class ViewImageFragment : ViewMediaFragment() {
        }
        isDescriptionVisible = showingDescription && visible
        val alpha = if (isDescriptionVisible) 1.0f else 0.0f
-        descriptionView.animate().alpha(alpha)
+        captionSheet.animate().alpha(alpha)
                .setListener(object : AnimatorListenerAdapter() {
                    override fun onAnimationEnd(animation: Animator) {
-                        descriptionView.visible(isDescriptionVisible)
+                        captionSheet.visible(isDescriptionVisible)
                        animation.removeListener(this)
                    }
                })
--- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewMediaFragment.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewMediaFragment.kt
@ -17,18 +17,20 @@ 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, previewUrl: String?)
+    abstract fun setupMediaView(
+            url: String,
+            previewUrl: String?,
+            description: String?,
+            showingDescription: Boolean
+    )
+
    abstract fun onToolbarVisibilityChange(visible: Boolean)
-    abstract val descriptionView: TextView

    protected var showingDescription = false
    protected var isDescriptionVisible = false
@ -36,6 +38,7 @@ abstract class ViewMediaFragment : BaseFragment() {
    companion object {
        @JvmStatic
        protected val ARG_START_POSTPONED_TRANSITION = "startPostponedTransition"
+
        @JvmStatic
        protected val ARG_ATTACHMENT = "attach"
        @JvmStatic
@ -74,13 +77,10 @@ abstract class ViewMediaFragment : BaseFragment() {

    protected fun finalizeViewSetup(url: String, previewUrl: String?, description: String?) {
        val mediaActivity = activity as ViewMediaActivity
-        setupMediaView(url, previewUrl)

-        descriptionView.text = description ?: ""
        showingDescription = !TextUtils.isEmpty(description)
        isDescriptionVisible = showingDescription
-
-        descriptionView.visible(showingDescription && mediaActivity.isToolbarVisible)
+        setupMediaView(url, previewUrl, description, showingDescription && mediaActivity.isToolbarVisible)

        toolbarVisibiltyDisposable = (activity as ViewMediaActivity)
                .addToolbarVisibilityListener { isVisible ->
--- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt
@ -26,7 +26,6 @@ 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
@ -47,7 +46,6 @@ class ViewVideoFragment : ViewMediaFragment() {
    }
    private lateinit var mediaActivity: ViewMediaActivity
    private val TOOLBAR_HIDE_DELAY_MS = 3000L
-    override lateinit var descriptionView : TextView
    private lateinit var mediaController : MediaController
    private var isAudio = false

@ -71,8 +69,14 @@ class ViewVideoFragment : ViewMediaFragment() {
    }

    @SuppressLint("ClickableViewAccessibility")
-    override fun setupMediaView(url: String, previewUrl: String?) {
-        descriptionView = mediaDescription
+    override fun setupMediaView(
+            url: String,
+            previewUrl: String?,
+            description: String?,
+            showingDescription: Boolean
+    ) {
+        mediaDescription.text = description
+        mediaDescription.visible(showingDescription)

        videoView.transitionName = url
        videoView.setVideoPath(url)
@ -178,14 +182,14 @@ class ViewVideoFragment : ViewMediaFragment() {
        val alpha = if (isDescriptionVisible) 1.0f else 0.0f
        if (isDescriptionVisible) {
            // If to be visible, need to make visible immediately and animate alpha
-            descriptionView.alpha = 0.0f
-            descriptionView.visible(isDescriptionVisible)
+            mediaDescription.alpha = 0.0f
+            mediaDescription.visible(isDescriptionVisible)
        }

-        descriptionView.animate().alpha(alpha)
+        mediaDescription.animate().alpha(alpha)
                .setListener(object : AnimatorListenerAdapter() {
                    override fun onAnimationEnd(animation: Animator) {
-                        descriptionView.visible(isDescriptionVisible)
+                        mediaDescription.visible(isDescriptionVisible)
                        animation.removeListener(this)
                    }
                })
--- a/app/src/main/res/drawable/description_bg_expanded.xml
+++ b/app/src/main/res/drawable/description_bg_expanded.xml
@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8"?>
+<shape xmlns:android="http://schemas.android.com/apk/res/android">
+    <corners
+        android:topLeftRadius="6dp"
+        android:topRightRadius="6dp" />
+    <stroke
+        android:color="?attr/colorBackgroundAccent"
+        android:width="1dp" />
+    <solid android:color="#B3000000" />
+</shape>
--- a/app/src/main/res/drawable/ic_drag_indicator_horiz_24dp.xml
+++ b/app/src/main/res/drawable/ic_drag_indicator_horiz_24dp.xml
@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<shape android:shape="rectangle"
+    xmlns:android="http://schemas.android.com/apk/res/android">
+    <size
+        android:width="8dp"
+        android:height="1dp" />
+    <solid android:color="?attr/colorBackgroundAccent" />
+    <corners android:radius="1dp" />
+</shape>
--- a/app/src/main/res/layout/fragment_view_image.xml
+++ b/app/src/main/res/layout/fragment_view_image.xml
@ -24,18 +24,56 @@
        app:layout_constraintRight_toRightOf="parent"
        app:layout_constraintTop_toTopOf="parent" />

+    <!--    This should be inside CoordinatorLayout for two reasons:
+
+            1. TouchImageView really wants some constraints ans has no size otherwise
+            2. We don't want sheet to overlap with appbar but the only way to do it with autosizing
+            is to gibe parent some margin. -->
+    <androidx.coordinatorlayout.widget.CoordinatorLayout
+        android:layout_width="match_parent"
+        android:layout_height="0dp"
+        android:layout_marginTop="70dp"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintLeft_toLeftOf="parent"
+        app:layout_constraintRight_toRightOf="parent"
+        app:layout_constraintTop_toTopOf="parent">
+
+        <LinearLayout
+            android:id="@+id/captionSheet"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:background="@drawable/description_bg_expanded"
+            android:orientation="vertical"
+            app:behavior_peekHeight="90dp"
+            app:layout_behavior="com.google.android.material.bottomsheet.BottomSheetBehavior">
+
+            <View
+                android:layout_width="24dp"
+                android:layout_height="3dp"
+                android:layout_marginTop="10dp"
+                android:layout_marginBottom="10dp"
+                android:layout_gravity="center_horizontal"
+                android:importantForAccessibility="no"
+                android:background="@drawable/ic_drag_indicator_horiz_24dp" />
+
+            <androidx.core.widget.NestedScrollView
+                android:layout_width="match_parent"
+                android:layout_height="wrap_content">
+
                <TextView
                    android:id="@+id/mediaDescription"
                    android:layout_width="match_parent"
                    android:layout_height="wrap_content"
-        android:background="#60000000"
                    android:hyphenationFrequency="full"
                    android:lineSpacingMultiplier="1.1"
-        android:padding="8dp"
+                    android:paddingLeft="8dp"
+                    android:paddingRight="8dp"
+                    android:paddingBottom="8dp"
                    android:textAlignment="center"
                    android:textColor="#eee"
                    android:textSize="?attr/status_text_medium"
-        app:layout_constraintBottom_toBottomOf="parent"
-        tools:text="Some media description" />
-
+                    tools:text="Some media description which might get quite long so that it won't easily fit in one line" />
+            </androidx.core.widget.NestedScrollView>
+        </LinearLayout>
+    </androidx.coordinatorlayout.widget.CoordinatorLayout>
 </androidx.constraintlayout.widget.ConstraintLayout>